Dart: Serve reports for running and recently-finished queries. #18886

gianm · 2026-01-06T17:57:54Z

This patch adds query reports, and a mapping from SQL query ID to Dart query ID, to DartControllerRegistry. It is made available through a new getQueryReport method on SqlEngine, and through a new /druid/v2/sql/queries/<sqlQueryId>/reports endpoint on SqlResource.

The format of the response is a JSON object with two keys, "query" and "report". The "query" key is the same info that is available from the existing /druid/v2/sql/queries endpoint. The "report" key is a report map including an MSQ report.

New Broker configurations control retention of reports:

druid.msq.dart.controller.maxRetainedReportCount (default 0, meaning no reports are retained)
druid.msq.dart.controller.maxRetainedReportDuration (default PT0S, meaning time-based expiration is disabled)

This patch adds query reports, and a mapping from SQL query ID to Dart query ID, to DartControllerRegistry. It is made available through a new getQueryReport method on SqlEngine, and through a new "/druid/v2/sql/queries/<sqlQueryId>/report" endpoint on SqlResource. The format of the response is a JSON object with two keys, "query" and "report". The "query" key is the same info that is available from the existing "/druid/v2/sql/queries" endpoint. The "report" key is an MSQ query report. New Broker configurations control retention of reports: - druid.msq.dart.controller.maxRetainedReportCount (default 0, meaning no reports are retained) - druid.msq.dart.controller.maxRetainedReportDuration (default PT0S, meaning time-based expiration is disabled)

gianm · 2026-01-06T20:20:52Z

Added jacoco:skip because most of the files that are being complained about, are being tested with embedded tests.

…rts. Change API URL.

clintropolis · 2026-01-08T02:14:36Z

multi-stage-query/src/main/java/org/apache/druid/msq/counters/CounterSnapshotsTree.java

+    synchronized (snapshotsMap) {
+      synchronized (that.snapshotsMap) {


is this deadlocky if multiple things are calling equals? i'm nervous at least

Hmm, I think in theory yes. equals is only going to be used in unit tests though so it probably doesn't matter (this was added to make a test case easier to write). But, still not great, so I'll change it to copyMap().equals(that.copyMap()).

clintropolis · 2026-01-08T02:18:23Z

...e-query/src/main/java/org/apache/druid/msq/dart/controller/sql/DartSqlClientFactoryImpl.java

    return new DartSqlClientImpl(client, jsonMapper);
  }
+
+  private static String removeTrailingSlash(String path)


we also have StringUtils.maybeRemoveTrailingSlash

Ha, I thought there was something but couldn't find it. Updated.

clintropolis · 2026-01-08T02:23:08Z

multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/sql/DartQueryMaker.java

+        this.complete = true;
+        this.report = report;


do the order these are set in matter? I can't see any overlap, but .. didn't follow all around everything that is using complete so just wanting to make sure nothing will try to get the report after checking completed

I don't believe it matters, but I switched the order anyway because that's easier than trying to prove it doesn't matter.

kfaraz

Minor nitpicks, otherwise LGTM 👍🏻

kfaraz · 2026-01-08T06:49:18Z

sql/src/main/java/org/apache/druid/sql/http/GetReportResponse.java

+/**
+ * Class returned by {@link SqlResource#doGetQueryReport}, the "get query report" API.
+ */
+public class GetReportResponse


Nit: rename to GetQueryReportResponse instead?

kfaraz · 2026-01-08T06:53:11Z

embedded-tests/src/test/java/org/apache/druid/testing/embedded/msq/EmbeddedMSQApis.java

+  /**
+   * Creates a {@link BrokerClient} that targets a specific broker using a {@link FixedServiceLocator}.
+   */
+  private static BrokerClient createBrokerClientForBroker(EmbeddedBroker targetBroker)


Nit: I think it may be useful to move this method to EmbeddedServiceClient as a variation of onAnyBroker(). The new method could be onTargetBroker() which accepts the target EmbeddedBroker and the API lambda to be called on the underlying client.

kfaraz · 2026-01-08T06:59:22Z

...ded-tests/src/test/java/org/apache/druid/testing/embedded/msq/EmbeddedDartReportApiTest.java

+  {
+    final long timeout = 30_000;
+    final long deadline = System.currentTimeMillis() + timeout;
+    while (System.currentTimeMillis() < deadline) {


Is there any "query has started running" metric that we could wait for instead of sleeping?
Nit: You could also use Stopwatch.hasMillisElapsed() instead of System.currentTimeMillis().

Not really, the metrics are emitted when a query finishes. This could likely be refactored to use the waitForResult utility in #18766 (comment) once that PR is merged.

kfaraz · 2026-01-08T07:04:34Z

multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/QueryInfoAndReport.java

+/**
+ * Object returned by {@link DartControllerRegistry#getQueryInfoAndReport(String)}.
+ */
+public class QueryInfoAndReport


This class seems very similar to GetReportResponse. Do we need the two to be distinct?

I didn't want to have an unnecessary timestamp field polluting the object that represents what the API returns. This class has the timestamp so it can be used for expiration of reports.

kfaraz · 2026-01-08T07:13:08Z

multi-stage-query/src/main/java/org/apache/druid/msq/dart/guice/DartControllerConfig.java

  private double heapFraction = DEFAULT_HEAP_FRACTION;

+  @JsonProperty("maxRetainedReportCount")
+  private int maxRetainedReportCount = 0;


Should we default to a small positive number, say 10? This seems like a useful feature to have out of the box.

I was being conservative here with the new functionality, and also thinking that a small number might not be that useful. A cluster that gets real usage will probably go through 10 queries pretty quickly. In a production situation it probably makes sense to set it much higher and to have a time-based expiration of at least a few minutes.

kfaraz · 2026-01-08T07:21:47Z

...i-stage-query/src/main/java/org/apache/druid/msq/dart/controller/DartControllerRegistry.java

+    // Schedule periodic cleanup of expired reports.
+    if (!config.getMaxRetainedReportDuration().equals(Period.ZERO)) {
+      final String threadNameFormat = StringUtils.format("%s-ReportCleanupExec-%%s", getClass().getSimpleName());
+      final long cleanupPeriodMs = Math.max(


Just curious, is it necessary to do periodic cleanup?
Isn't it sufficient to just have a limit on the number of reports?

I was thinking that in a production scenario, an admin probably wants to think about things in terms of time-based expiration (i.e. I'm going to have reports available for 5 minutes). I was thinking the count-based retention is more just to protect against the broker going OOM if there is a burst of queries. It would be better for the limit to be footprint-based, but the code for that is more complex :|

That makes sense. thanks!

kfaraz · 2026-01-08T07:33:36Z

...i-stage-query/src/main/java/org/apache/druid/msq/dart/controller/DartControllerRegistry.java

+        int reportsToRemove = completeReports.size() - config.getMaxRetainedReportCount() + 1;
+        if (reportsToRemove > 0) {


We might be able to get rid of this logic by overriding the removeEldestEntry method on the LinkedHashMap completeReports.

True but I always find that approach ugly.

kfaraz · 2026-01-08T07:34:38Z

...i-stage-query/src/main/java/org/apache/druid/msq/dart/controller/DartControllerRegistry.java

+    if (didRemove && completeReport != null && config.getMaxRetainedReportCount() > 0) {
+      synchronized (completeReports) {
+        // Remove reports if size is greater than maxRetainedReportCount - 1.
+        int reportsToRemove = completeReports.size() - config.getMaxRetainedReportCount() + 1;


Since we are acquiring a lock before this, I suppose this will always be <= 1.

I think so too, just being defensive.

github-actions bot added Area - Batch Ingestion Area - Querying Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Jan 6, 2026

gianm added 2 commits January 6, 2026 10:04

Fixes.

6613ae8

More fixes.

02cb6e6

gianm added the jacoco:skip label Jan 6, 2026

gianm closed this Jan 6, 2026

gianm reopened this Jan 6, 2026

Use entire report map, not just MSQ report. Add live counters to repo…

0f18d1b

…rts. Change API URL.

gianm added the Needs web console change Backend API changes that would benefit from frontend support in the web console label Jan 7, 2026

Fix test.

27a5cbb

clintropolis reviewed Jan 8, 2026

View reviewed changes

Updates from review.

6d9eb6d

clintropolis approved these changes Jan 8, 2026

View reviewed changes

Fix equals verification.

c7f8923

kfaraz approved these changes Jan 8, 2026

View reviewed changes

Comments from review.

e411435

gianm merged commit 93735f5 into apache:master Jan 8, 2026
125 of 130 checks passed

gianm deleted the sql-engines-get-reports branch January 8, 2026 19:04

		synchronized (snapshotsMap) {
		synchronized (that.snapshotsMap) {

		int reportsToRemove = completeReports.size() - config.getMaxRetainedReportCount() + 1;
		if (reportsToRemove > 0) {

Dart: Serve reports for running and recently-finished queries. #18886

Dart: Serve reports for running and recently-finished queries. #18886

Uh oh!

Conversation

gianm commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gianm commented Jan 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gianm Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kfaraz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kfaraz Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gianm Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kfaraz Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gianm commented Jan 6, 2026 •

edited

Loading

gianm Jan 8, 2026 •

edited

Loading

kfaraz Jan 8, 2026 •

edited

Loading

gianm Jan 8, 2026 •

edited

Loading

kfaraz Jan 8, 2026 •

edited

Loading