Add ability for MSQ tasks to query realtime tasks #15024

adarshsanjeev · 2023-09-22T05:21:09Z

Currently, MSQ is unable to include realtime results from realtime queries.

This problem is present both in the controller, which only fetches metadata from the metadata store so that only published segments would be read and the worker, which reads segments from deep storage and lacks the ability to communicate with data servers.

This PR aims to add the capabilities to:

Fetch the realtime segment metadata from the coordinator server view,
Adds the ability for workers to query indexers, similar to how brokers do the same for native queries.

Release Notes

Adds a context parameter includeSegmentSource which can be either NONE(default) or REALTIME. If this parameter is set to REALTIME, MSQ will include real time segments in the query results.

This PR has:

server/src/main/java/org/apache/druid/discovery/DataServerClient.java

...-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/TaskDataSegmentProvider.java

server/src/main/java/org/apache/druid/rpc/ServiceLocation.java

server/src/main/java/org/apache/druid/rpc/FixedSetServiceLocator.java

.../multi-stage-query/src/main/java/org/apache/druid/msq/input/table/RichSegmentDescriptor.java

...ery/src/main/java/org/apache/druid/msq/querykit/groupby/GroupByPreShuffleFrameProcessor.java

...ti-stage-query/src/main/java/org/apache/druid/msq/querykit/scan/ScanQueryFrameProcessor.java

cryptoe

Reviewed the overall design. Left some comments.
Please add UT's for this.

...s-core/multi-stage-query/src/main/java/org/apache/druid/msq/util/MultiStageQueryContext.java

extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java

...ore/multi-stage-query/src/main/java/org/apache/druid/msq/exec/LoadedSegmentDataProvider.java

...multi-stage-query/src/main/java/org/apache/druid/msq/exec/LoadedSegmentDataProviderImpl.java

...ulti-stage-query/src/main/java/org/apache/druid/msq/input/table/DataSegmentWithLocation.java

...re/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/BaseLeafFrameProcessor.java

.../multi-stage-query/src/main/java/org/apache/druid/msq/input/table/SegmentWithDescriptor.java

LakshSingla

Since numRetriesOnMissingSegments is not added in the documentation, perhaps we should leave it out of the release notes as well.

...ulti-stage-query/src/main/java/org/apache/druid/msq/input/table/DataSegmentWithLocation.java

cryptoe

Classes left are
BaseLeafFrameProcesor
DataServerClient
LoadedSegmentDataProvider
GroupingEngine

...ore/multi-stage-query/src/main/java/org/apache/druid/msq/exec/LoadedSegmentDataProvider.java

docs/multi-stage-query/reference.md

extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/SegmentSource.java

extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java

.../multi-stage-query/src/main/java/org/apache/druid/msq/input/table/RichSegmentDescriptor.java

.../multi-stage-query/src/main/java/org/apache/druid/msq/input/table/SegmentWithDescriptor.java

...-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/IndexerFrameContext.java

...ore/multi-stage-query/src/main/java/org/apache/druid/msq/exec/LoadedSegmentDataProvider.java

server/src/main/java/org/apache/druid/discovery/DataServerClient.java

cryptoe · 2023-10-06T11:02:09Z

server/src/main/java/org/apache/druid/discovery/DataServerResponseHandler.java

+  @Override
+  public ClientResponse<AppendableByteArrayInputStream> handleResponse(HttpResponse response, TrafficCop trafficCop)
+  {
+    log.debug("Received response status[%s] for queryId[%s]", response.getStatus(), queryId);


Should we check for timeout?
Shoudnt traffic cop be used for backpressure?
We can do these things in a follow up PR as well.

What about metrics like bytes scanned and stuff ?

server/src/main/java/org/apache/druid/discovery/DataServerResponseHandler.java

server/src/test/java/org/apache/druid/rpc/ServiceLocationTest.java

server/src/main/java/org/apache/druid/rpc/ServiceLocation.java

cryptoe · 2023-10-06T11:18:31Z

server/src/main/java/org/apache/druid/discovery/DataServerClient.java

+        StandardRetryPolicy.noRetries()
+    );
+    this.objectMapper = objectMapper;
+    this.queryCancellationExecutor = Execs.scheduledSingleThreaded("query-cancellation-executor");


Can we make a pool for this. For each segment you are creating a new executor service which seems wasteful.

Who closes this executor service.

Moved this to a common threadpool and added a wait to close it.

cryptoe · 2023-10-06T11:37:27Z

...ore/multi-stage-query/src/main/java/org/apache/druid/msq/exec/LoadedSegmentDataProvider.java

+    try {
+      statusSequencePair = RetryUtils.retry(
+          () -> {
+            Sequence<QueryType> sequence = dataServerClient.run(preparedQuery, responseContext, queryResultType, closer)


What happen's if sequence is interrupted or closed?

It would be thrown outside this class, which would cause the query to fail

...ore/multi-stage-query/src/main/java/org/apache/druid/msq/exec/LoadedSegmentDataProvider.java

cryptoe · 2023-10-06T11:42:20Z

...ore/multi-stage-query/src/main/java/org/apache/druid/msq/exec/LoadedSegmentDataProvider.java

+            } else {
+              Boolean wasHandedOff = checkSegmentHandoff(coordinatorClient, dataSource, segmentDescriptor);
+              if (Boolean.TRUE.equals(wasHandedOff)) {
+                log.debug("Segment[%s] was handed off.", segmentDescriptor);


can we log the counts in the stage somewhere ?

Will add this as part of the counters

cryptoe

Left some comments.
LGTM overall.

docs/multi-stage-query/reference.md

cryptoe · 2023-10-09T05:10:46Z

...ore/multi-stage-query/src/main/java/org/apache/druid/msq/exec/LoadedSegmentDataProvider.java

      statusSequencePair = RetryUtils.retry(
          () -> {
+            ServiceLocation serviceLocation = Iterables.getOnlyElement(fixedSetServiceLocator.locate().get().getLocations());


Nit: Use collectionUtils.getOnlyElement() so that a nicer error message is thrown.

...ore/multi-stage-query/src/main/java/org/apache/druid/msq/exec/LoadedSegmentDataProvider.java

cryptoe · 2023-10-09T05:17:01Z

extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/SegmentSource.java

+ */
+public enum SegmentSource
+{
+  NONE(ImmutableSet.of()),


NIt: Lets add that None also includes deep storage segments.

Added comments here

...ti-stage-query/src/main/java/org/apache/druid/msq/querykit/scan/ScanQueryFrameProcessor.java

...multi-stage-query/src/test/java/org/apache/druid/msq/exec/LoadedSegmentDataProviderTest.java

...ns-core/multi-stage-query/src/test/java/org/apache/druid/msq/test/CalciteMSQTestsHelper.java

server/src/main/java/org/apache/druid/discovery/DataServerClient.java

cryptoe

Change LGTM.

This PR aims to add the capabilities to: 1. Fetch the realtime segment metadata from the coordinator server view, 2. Adds the ability for workers to query indexers, similar to how brokers do the same for native queries.

adarshsanjeev added 3 commits September 22, 2023 10:15

Add metadata fetch

4dc7d68

Add ability to query data servers

c2289d1

Cleanup

5296d1d

github-actions bot added Area - Batch Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Sep 22, 2023

github-advanced-security bot found potential problems Sep 22, 2023

View reviewed changes

adarshsanjeev added 8 commits September 24, 2023 15:35

Add smile format

80da7b2

Clean

e2fe883

Refactor code

faeec15

Cover handoff cases

0afb4d3

Resolve bug

46f84c8

Change handoff from exception to return

3183f37

Add smile check

2570840

Change realtime check

8d7d6a9

adarshsanjeev marked this pull request as ready for review September 27, 2023 16:11

github-advanced-security bot found potential problems Sep 27, 2023

View reviewed changes

.../multi-stage-query/src/main/java/org/apache/druid/msq/input/table/RichSegmentDescriptor.java Fixed Show fixed Hide fixed

adarshsanjeev added 3 commits September 28, 2023 16:31

Add logs

92c6d75

Fix checkstyles

6bfa3a6

Change return to yielder

065b7fd

github-advanced-security bot found potential problems Sep 28, 2023

View reviewed changes

...ery/src/main/java/org/apache/druid/msq/querykit/groupby/GroupByPreShuffleFrameProcessor.java Fixed Show fixed Hide fixed

...ti-stage-query/src/main/java/org/apache/druid/msq/querykit/scan/ScanQueryFrameProcessor.java Fixed Show fixed Hide fixed

cryptoe reviewed Sep 29, 2023

View reviewed changes

adarshsanjeev added 9 commits September 29, 2023 21:01

Fix NPE

f9a05d0

Add unit tests

3ea2b43

Address review comments

763fea2

Add unit tests

9c9e099

Merge remote-tracking branch 'origin/master' into realtime-msq

c4189de

Address review comments

d5199d7

Delete unwanted test

53bf349

Add unit tests

45bdfa4

Add unit tests

85c4923

github-actions bot added the Area - Querying label Oct 3, 2023

Fix tests

cc3db10

github-actions bot removed the Area - Querying label Oct 3, 2023

adarshsanjeev added 6 commits October 3, 2023 19:53

Add tests and remove interface

ed5b461

Add check for case where realtime servers cannot be contacted

7fedc60

Cancel query on interrupt

e4792f9

Merge remote-tracking branch 'origin/master' into realtime-msq

a10d5a7

Resolve test failures

fb27479

Merge remote-tracking branch 'origin/master' into realtime-msq

705e53b

LakshSingla added the Release Notes label Oct 5, 2023

LakshSingla reviewed Oct 5, 2023

View reviewed changes

...ulti-stage-query/src/main/java/org/apache/druid/msq/input/table/DataSegmentWithLocation.java Show resolved Hide resolved

adarshsanjeev added 3 commits October 5, 2023 23:22

Fix bug with certain groupby queries

190880a

Merge remote-tracking branch 'origin/master' into realtime-msq

14f0d5b

Resolve unclean merge

feec36f

cryptoe reviewed Oct 6, 2023

View reviewed changes

adarshsanjeev added 4 commits October 7, 2023 15:19

Address review comments

d295b36

Address review comments

69eb9c8

Address review comments

d235618

Resolve checkstyle failures

e573e31

cryptoe reviewed Oct 9, 2023

View reviewed changes

adarshsanjeev added 2 commits October 9, 2023 11:43

Address review comments

7ddf025

Add log statement

1cfe72a

cryptoe approved these changes Oct 9, 2023

View reviewed changes

cryptoe merged commit 7a35ce8 into apache:master Oct 9, 2023

LakshSingla mentioned this pull request Oct 9, 2023

Fix compilation failure in master #15111

Merged

LakshSingla added this to the 28.0 milestone Oct 12, 2023

LakshSingla mentioned this pull request Nov 4, 2023

[DRAFT] 28.0.0 release notes #15326

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability for MSQ tasks to query realtime tasks #15024

Add ability for MSQ tasks to query realtime tasks #15024

adarshsanjeev commented Sep 22, 2023 •

edited

Loading

cryptoe left a comment

LakshSingla left a comment

cryptoe left a comment •

edited

Loading

cryptoe Oct 6, 2023

cryptoe Oct 6, 2023

cryptoe Oct 6, 2023

cryptoe Oct 6, 2023

adarshsanjeev Oct 8, 2023

cryptoe Oct 6, 2023

adarshsanjeev Oct 6, 2023

cryptoe Oct 6, 2023

adarshsanjeev Oct 7, 2023

cryptoe left a comment

cryptoe Oct 9, 2023

adarshsanjeev Oct 9, 2023

cryptoe Oct 9, 2023

adarshsanjeev Oct 9, 2023

cryptoe left a comment

Add ability for MSQ tasks to query realtime tasks #15024

Add ability for MSQ tasks to query realtime tasks #15024

Conversation

adarshsanjeev commented Sep 22, 2023 • edited Loading

cryptoe left a comment

Choose a reason for hiding this comment

LakshSingla left a comment

Choose a reason for hiding this comment

cryptoe left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cryptoe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cryptoe left a comment

Choose a reason for hiding this comment

adarshsanjeev commented Sep 22, 2023 •

edited

Loading

cryptoe left a comment •

edited

Loading