Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MSQ] Regression bug fix where ever LimitFrameProcessor's were used. #13941

Merged
merged 1 commit into from
Mar 16, 2023

Conversation

cryptoe
Copy link
Contributor

@cryptoe cryptoe commented Mar 16, 2023

An MSQ insert statement with LIMIT and PARTITIONED BY ALL TIME no longer works. It returns a Result partition information is not ready yet error. This is a regression introduced because of #13506 .

This is reproducible using any source datasource, for example, this 2-row, 1-column mytest

select * from mytest
__time                   c1
2022-01-01T00:00:00.000Z 1
2022-01-01T00:00:00.000Z 2

The insert statement that fails:

insert into new_ds
select __time, c1
from mytest
limit 1
partitioned by ALL TIME
clustered by c1
UnknownError: org.apache.druid.java.util.common.ISE: Result partition information is not ready yet (Stack trace)
Failed task ID: query-17a6a628-473d-4b54-a41a-64143b680e3a (on host: ip-10-201-4-98.ec2.internal:8100)
Debug: get query detail archive

The stack trace:

org.apache.druid.java.util.common.ISE: Result partition information is not ready yet
	at org.apache.druid.msq.kernel.controller.ControllerStageTracker.getResultPartitionBoundaries(ControllerStageTracker.java:224)
	at org.apache.druid.msq.kernel.controller.ControllerQueryKernel.getResultPartitionBoundariesForStage(ControllerQueryKernel.java:394)
	at org.apache.druid.msq.exec.ControllerImpl$RunQueryUntilDone.startStages(ControllerImpl.java:2385)
	at org.apache.druid.msq.exec.ControllerImpl$RunQueryUntilDone.run(ControllerImpl.java:2164)
	at org.apache.druid.msq.exec.ControllerImpl$RunQueryUntilDone.access$000(ControllerImpl.java:2121)
	at org.apache.druid.msq.exec.ControllerImpl.runTask(ControllerImpl.java:373)
	at org.apache.druid.msq.exec.ControllerImpl.run(ControllerImpl.java:317)
	at org.apache.druid.msq.indexing.MSQControllerTask.runTask(MSQControllerTask.java:179)
	at org.apache.druid.indexing.common.task.AbstractTask.run(AbstractTask.java:169)
	at org.apache.druid.indexing.overlord.SingleTaskBackgroundRunner$SingleTaskBackgroundRunnerCallable.call(SingleTaskBackgroundRunner.java:477)
	at org.apache.druid.indexing.overlord.SingleTaskBackgroundRunner$SingleTaskBackgroundRunnerCallable.call(SingleTaskBackgroundRunner.java:449)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

Thanks @weishiuntsai for catching this.

The fix

The limitprocessors are fed shuffled data already. Hence they need not add a shuffling step for the next stage.
Adjusted the ScanQuerKit, GroupByQueryKit for that.

Added a case in ControllerStageTracker#generateResultPartitionsAndBoundariesWithoutKeyStatistics for ShuffleKind.MIX

Fixed the bug ...

Release note

Release notes are not needed since #13506 did not go out.


Key changed/added classes in this PR
  • ScanQueryKit
  • GroupByQueryKit
  • ControllerStageTracker

This PR has:

  • been self-reviewed.
  • a release note entry in the PR description.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • been tested in a test Druid cluster.

@cryptoe cryptoe requested a review from gianm March 16, 2023 05:20
@cryptoe cryptoe added the Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 label Mar 16, 2023
@gianm
Copy link
Contributor

gianm commented Mar 16, 2023

Thanks for the fix! Looks good to me.

@gianm gianm merged commit bf13156 into apache:master Mar 16, 2023
@clintropolis clintropolis added this to the 26.0 milestone Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants