Skip to content

fix(processing): finalize SuperSorter progress before completion - #20054

Open
FrankChen021 wants to merge 1 commit into
apache:masterfrom
FrankChen021:codex/fix-super-sorter-progress-race
Open

fix(processing): finalize SuperSorter progress before completion#20054
FrankChen021 wants to merge 1 commit into
apache:masterfrom
FrankChen021:codex/fix-super-sorter-progress-race

Conversation

@FrankChen021

@FrankChen021 FrankChen021 commented Aug 18, 2026

Copy link
Copy Markdown
Member

Description

SuperSorter could complete its allDone future before the asynchronous output-partitions listener initialized the progress tracker's ultimate merger count. A caller returning from run().get() could therefore briefly observe a progress digest of 0.0 instead of 1.0. This surfaced as repeated SuperSorterTest flakes in the JDK 25 S* CI shard.

This change routes every successful sorter completion through a helper that initializes the ultimate merger count before publishing allDone. Initialization is idempotent under the existing runWorkersLock, so the output-partitions listener can run either before or after worker completion without double-setting the tracker.

The regression test delays the output-partitions listener, waits for sorting to complete, and verifies that final progress was already initialized before releasing the listener.

CI evidence

The same SuperSorterTest progress assertion (expected: <1.0> but was: <0.0>) appeared independently in these JDK 25 S* jobs:

The unchanged #20045 head then passed on an exact job rerun, confirming that the failure is timing-sensitive.

The same SuperSorterTest failure also reproduced on PR #20078's initial JDK 25 S* job (https://github.com/apache/druid/actions/runs/32325623822/job/96296160599); its single permitted rerun (https://github.com/apache/druid/actions/runs/32325623822/job/96324754231) reproduced the flake, further confirming the timing-sensitive failure.

The same expected: <1.0> but was: <0.0> assertion also reproduced on PR #20085's failing JDK 25 S* job, specifically in SuperSorterTest$ParameterizedCasesTest.test_clusterByPlacementishDescRowNumberAsc_fourPartitions; the shard later timed out after 60 minutes.

Release note

None. This fixes internal progress-reporting ordering and test flakiness without changing configuration, APIs, persisted data, or query results.


Key changed/added classes in this PR
  • SuperSorter
  • SuperSorterTest

This PR has:

  • been self-reviewed.
  • added comments explaining the concurrency intent where it is not obvious.
  • added or updated unit tests to cover the affected ordering.

Validation:

JAVA_HOME=/Library/Java/JavaVirtualMachines/temurin-25.jdk/Contents/Home \
mvn test -pl processing -am \
  -Dtest='org.apache.druid.frame.processor.SuperSorterTest*' \
  -Dsurefire.failIfNoSpecifiedTests=false \
  -Pskip-static-checks -Dweb.console.skip=true -T1C

Result: 10,905 tests run, 0 failures, 0 errors, 0 skipped.

@FrankChen021
FrankChen021 marked this pull request as ready for review August 18, 2026 09:20
Copilot AI lite review requested due to automatic review settings August 18, 2026 09:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@FrankChen021 FrankChen021 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reviewed the code for correctness, edge cases, concurrency, and integration risks; no issues found.

Reviewed 2 of 2 changed files.

Validation: git diff --check cb26014728ab3239bc8d7391f9cf6466e63905b0..HEAD passed. Builds and tests were not run.


This is an automated review by Codex GPT-5.6-Luna(max)

@FrankChen021

Copy link
Copy Markdown
Member Author

@gianm please take a look at this change. Currently many PRs are experencing the same SuperSorterTest flake

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants