>From Ian Maxon <[email protected]>: Attention is currently required from: Peeyush Gupta.
Ian Maxon has posted comments on this change by Peeyush Gupta. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20771?usp=email ) Change subject: [ASTERIXDB-3687][API] Incorrect metrics and missing fields in status API ...................................................................... Patch Set 5: (3 comments) File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryStatusApiServlet.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20771/comment/f68637f5_08ddf973?usp=email : PS5, Line 83: default -> null; might kind of be paranoia but to me it's better to return a dummy/invalid response metric here instead of null, because if somehow this gets reused without the guards from the caller it can make metricsprinter eat the null File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryStatusApiServlet.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20771/comment/3a3222b8_63818302?usp=email : PS5, Line 207: long currentTime = System.currentTimeMillis(); : long requestCreateTime = ((ClientRequest) clientRequest.get()).getCreationSystemTime(); : long elapsedTime = TimeUnit.MILLISECONDS.toNanos(currentTime - requestCreateTime); : long compileTime = TimeUnit.MILLISECONDS.toNanos(run.getCreateTime() - requestCreateTime); : ResponseMetrics metrics = switch (status) { : case QUEUED -> ResponseMetrics.of( : elapsedTime, 0, 0, : 0, 0, 0, 0, compileTime, : run.getQueueWaitTimeInMillis(), 0, 0, 0, 0, 0); : case RUNNING -> ResponseMetrics.of( : elapsedTime, : TimeUnit.MILLISECONDS.toNanos(currentTime - run.getStartTime()), 0, 0, 0, 0, 0, : compileTime, : run.getQueueWaitTimeInMillis(), 0, 0, 0, 0, 0); : default -> null; : }; : printer.addFooterPrinter(new MetricsPrinter(metrics, HttpUtil.getPreferredCharset(request), : Set.of(MetricsPrinter.Metrics.ELAPSED_TIME, MetricsPrinter.Metrics.EXECUTION_TIME, : MetricsPrinter.Metrics.QUEUE_WAIT_TIME, MetricsPrinter.Metrics.COMPILE_TIME))); : printer.addFooterPrinter(new CreatedAtPrinter(requestCreateTime)); this fragment is almost the same as the one in NCQueryServiceServlet isn't it? is there no easy way to reuse it? File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20771/comment/2e454310_b27c9a82?usp=email : PS5, Line 5577: resultMetadata.setCreateTime(System.currentTimeMillis()); this changes the meaning of the field to now be slightly after compile time rather than when the job was started on the CC side. since for running requests it seems like we can just access the jobRun directly instead of waiting on the result to be set, why not keep it as it was? -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20771?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: master Gerrit-Change-Id: Id5fd38b4273d1a3c98936cea63b8c879e474d619 Gerrit-Change-Number: 20771 Gerrit-PatchSet: 5 Gerrit-Owner: Peeyush Gupta <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Attention: Peeyush Gupta <[email protected]> Gerrit-Comment-Date: Tue, 13 Jan 2026 01:05:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
