>From Murtadha Hubail <[email protected]>: Attention is currently required from: Peeyush Gupta.
Murtadha Hubail has posted comments on this change by Peeyush Gupta. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378?usp=email ) Change subject: [ASTERIXDB-3649][*DB] Improve async request API ...................................................................... Patch Set 9: (13 comments) File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/IStatementExecutor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/d6866358_7d766001?usp=email : PS9, Line 108: private static final long serialVersionUID = 2L; 1L File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/AbstractQueryApiServlet.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/3a332c1e_a174c27f?usp=email : PS9, Line 115: protected void handleExecuteStatementException(Throwable t, remove one File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryResultApiServlet.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/f0b890c9_ddd00386?usp=email : PS9, Line 60: messageFuture.get(DEFAULT_NC_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS); This can wait forever if CC sides after receiving the message. We can return 202 here and ensure the nodes clean up any result when they re-register with the CC. File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceRequestParameters.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/8cf134ca_3def9b0b?usp=email : PS9, Line 160: private boolean includeHostInHandle = true; Let's default this to false, and automatically set it to true on deferred for back compt with the JDBC driver File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/DiscardResultPartitionRequestMessage.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/c539e1b4_82412142?usp=email : PS9, Line 49: LOGGER.info("failed to get collection size", e); fix File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/result/fields/MetricsPrinter.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/de51efb8_4102e40a?usp=email : PS9, Line 86: private void printSelected(PrintWriter pw) { remove duplicated printing code File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/fa50560d_71feeee8?usp=email : PS9, Line 562: metadataProvider.setMaxResultReads(!sessionConfig.isIncludeHostInHandle() ? 0 : maxResultReads); refactor this as a constant called unlimited reads https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/45db00ed_09caaf72?usp=email : PS9, Line 562: metadataProvider.setMaxResultReads(!sessionConfig.isIncludeHostInHandle() ? 0 : maxResultReads); Don't base this logic on this parameter. You can add a new method related to backward compatibility File asterixdb/asterix-app/src/main/java/org/apache/asterix/utils/AsyncRequestsAPIUtil.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/a1d94d02_cfca8981?usp=email : PS9, Line 122: throw new RuntimeException(e); replace these if not needed https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/6f11c1b9_b6e0b1e6?usp=email : PS9, Line 142: (ClientInfoResponseMessage) messageFuture.get(DEFAULT_NC_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS); As discussed, this timeout is unlimited. You might want to use a smaller value to account for cases where the CC goes down after the message is sent. File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/RequestTracker.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/c11d7488_378762b5?usp=email : PS9, Line 98: public IClientRequest getAsyncOrDeferredRequest(String requestId) { Maybe use Optional? File hyracks-fullstack/hyracks/hyracks-client/src/main/java/org/apache/hyracks/client/result/ResultSetReader.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/d1eed940_4c9f75ce?usp=email : PS9, Line 196: throw new RuntimeException(e); replace this File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/result/ResultWriterOperatorDescriptor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378/comment/72b5796b_0ef45a9d?usp=email : PS9, Line 113: ((ResultPartitionWriter) resultPartitionWriter).incrementResultCount(); Just make sure this is a safe downcast -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20378?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: asterixdb Gerrit-Branch: phoenix Gerrit-Change-Id: I25c0651531a8a48271c901a7368d8668874a9d18 Gerrit-Change-Number: 20378 Gerrit-PatchSet: 9 Gerrit-Owner: Peeyush Gupta <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-CC: Murtadha Hubail <[email protected]> Gerrit-Attention: Peeyush Gupta <[email protected]> Gerrit-Comment-Date: Thu, 09 Oct 2025 19:39:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
