Murtadha Hubail has posted comments on this change.

Change subject: [ASTERIXDB-2249][API] Add  Max Result Reads to API
......................................................................


Patch Set 1:

(2 comments)

https://asterix-gerrit.ics.uci.edu/#/c/2292/1/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java:

PS1, Line 29: Result
> Do we need this?
yes, to remind us of the xml days.


https://asterix-gerrit.ics.uci.edu/#/c/2292/1/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/dataset/DatasetPartitionReader.java
File 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/dataset/DatasetPartitionReader.java:

PS1, Line 89: LOGGER
> Should this/does this fail the request?
This whole thing is running on a thread from the node's executor service. Any 
uncaught exception here will shutdown the NC, which the original version was 
doing. This last catch is supposed to only catch clean up issues. I reorganized 
the code and added a test case to test the fail path to ensure that the job is 
aborted. Things could've been cleaner if close was idempotent in IFrameWriter, 
but let's not bring this up in front of Abdullah :)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I86f75c791f034142c5b046445870bd91378c5b3a
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Murtadha Hubail <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-HasComments: Yes

Reply via email to