Dmitry Lychagin has posted comments on this change.

Change subject: Enable HTTP API processing on NCs
......................................................................


Patch Set 8:

(4 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1709/8/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java:

PS8, Line 468:                     if (err instanceof Exception) {
             :                         throw (Exception) err;
             :                     } else if (err instanceof TokenMgrError) {
             :                         throw (TokenMgrError) err;
             :                     } else if (err instanceof 
org.apache.asterix.aqlplus.parser.TokenMgrError) {
             :                         throw 
(org.apache.asterix.aqlplus.parser.TokenMgrError) err;
             :                     } else {
             :                         throw new Exception(err.toString());
             :                     }
> why can't we just throw err here, and update catch below (line 493) to catc
Yes, looks like we can. This is a minor fix, I'll address this in a separate 
change.


https://asterix-gerrit.ics.uci.edu/#/c/1709/8/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java:

PS8, Line 147: e.getMessage()
> same as toString comment above
yes. I'll fix it in a separate change.


https://asterix-gerrit.ics.uci.edu/#/c/1709/8/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:

Line 823:                 Thread.sleep(8000);
> Why do we need to sleep 8s after every managix command now after this chang
I was getting frequent failures in 
asterix-lifecycle/backupRestore/backupRestore.9.query.aql
both on my machine and on jenkins. The actual result was empty, while the 
expected result was not. I'm not sure what's causing this, but this workaround 
seems to help. We need to look into fixing this properly, of course.


https://asterix-gerrit.ics.uci.edu/#/c/1709/8/asterixdb/asterix-installer/src/test/java/org/apache/asterix/installer/test/AsterixLifecycleIT.java
File 
asterixdb/asterix-installer/src/test/java/org/apache/asterix/installer/test/AsterixLifecycleIT.java:

Line 91:             Thread.sleep(4000);
> What problem are we solving with this?  Is stop returning before the cluste
yes, it seems. It looks like NCs were still not stopped at this point. 
state.getFailedNCs() was returning 0 leading to assertion failure at line 95. 
Thread.sleep() is just a temporary workaround. We should look into fixing this 
properly.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I19414a23e163fc4deef9805c8f9089609f1ebe07
Gerrit-PatchSet: 8
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Till Westmann <[email protected]>
Gerrit-Reviewer: Dmitry Lychagin
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Michael Blow <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to