abdullah alamoudi has posted comments on this change.

Change subject: Add a REST endpoint for query cancellation.
......................................................................


Patch Set 13:

(9 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1564/13/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/RestApiServlet.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/RestApiServlet.java:

PS13, Line 206:  null, null)
these seem to be null most of the time. maybe consider overloading?


https://asterix-gerrit.ics.uci.edu/#/c/1564/13/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/servlet/ServletConstants.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/servlet/ServletConstants.java:

PS13, Line 25: EXECUTOR_SERVICE
this seems to be named differently from other attr variables. make them 
consistent please.


https://asterix-gerrit.ics.uci.edu/#/c/1564/13/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/java/AsterixJavaClient.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/api/java/AsterixJavaClient.java:

PS13, Line 108: null, null
yet another place where we pass nulls.


https://asterix-gerrit.ics.uci.edu/#/c/1564/13/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/ResultExtractor.java
File 
asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/ResultExtractor.java:

PS13, Line 62: "clientContextID"
make those constants? so if needed, they only need to change in a single 
location


https://asterix-gerrit.ics.uci.edu/#/c/1564/13/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
File 
asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java:

PS13, Line 1228: if (result == null) {
               :                 return;
               :             }
how can result be null for select dataverses?


https://asterix-gerrit.ics.uci.edu/#/c/1564/13/asterixdb/asterix-app/src/test/resources/runtimets/queries/types/any-object/any-object.2.query.aql
File 
asterixdb/asterix-app/src/test/resources/runtimets/queries/types/any-object/any-object.2.query.aql:

PS13, Line 21: where $x.DataverseName = "test"
why the additional condition?


https://asterix-gerrit.ics.uci.edu/#/c/1564/2/asterixdb/asterix-app/src/test/resources/runtimets/results/types/any-object/any-object.2.adm
File 
asterixdb/asterix-app/src/test/resources/runtimets/results/types/any-object/any-object.2.adm:

the old result is ordered correctly but the new one is not?
why is that? even though the query had order by?

I know this is patchset 2 but this worries me.


https://asterix-gerrit.ics.uci.edu/#/c/1564/13/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/utils/Servlets.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/utils/Servlets.java:

PS13, Line 37: QUERY_CANCEL = "/query/admin/active_requests
should this be renamed?
Query Cancel -> Active requests?


https://asterix-gerrit.ics.uci.edu/#/c/1564/13/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMPrimaryUpsertOperatorNodePushable.java:

PS13, Line 319:    indexHelper.close();
if index is not assigned in open(), then indexHelper wasn't opened. add a check?


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1564
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2936ac83f71bbef533e2695ed0a2b220c23fc483
Gerrit-PatchSet: 13
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Yingyi Bu <buyin...@gmail.com>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-Reviewer: Steven Jacobs <sjaco...@ucr.edu>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to