Skip to content

Conversation

@abhishekrb19
Copy link
Contributor

@abhishekrb19 abhishekrb19 commented Dec 5, 2025

  • The ForcePushDownNestedQueryTest is occasionally flaky on master because the MSQ query in loadWikipediaTable() can take longer than the default 10 second timeout to finish and for segments to be loaded and available. Other tests such as QueryVirtualStorageTest and EmbeddedDurableShuffleStorageTest use a 20 second timeout for similar loads, so increasing the timeout here to 20 seconds should address the flakiness. See the failed run: https://github.com/apache/druid/actions/runs/19955328339/job/57225328413
  • Include the timeout value in the error message so the timeout is apparent without needing to inspect the stack trace

The MSQ query appearst to take longer than 10s. Other tests like QueryVirtualStorageTest
and EmbeddedDurableShuffleStorageTest that use the same query use 20s timeout, so that should help.

Also, include the timeout in the error message so it's clear rather than having to look at the
stacktrace.
Copy link
Contributor

@jtuglu1 jtuglu1 left a comment

Choose a reason for hiding this comment

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

Thanks!

@abhishekrb19 abhishekrb19 merged commit 0b4bbbc into master Dec 6, 2025
54 checks passed
@abhishekrb19 abhishekrb19 deleted the bump_nested_query_timeout branch December 6, 2025 06:21
@kfaraz
Copy link
Contributor

kfaraz commented Dec 6, 2025

@abhishekrb19 , seems like it failed on this PR #18820 too.
https://github.com/apache/druid/actions/runs/19984107070/job/57315442102?pr=18820#step:6:8061

Should we try upping it to 30s?

@abhishekrb19
Copy link
Contributor Author

@kfaraz I think 18820 doesn’t include this patch. The log line in the run you linked doesn't include the timeout: org.apache.druid.java.util.common.ISE: Timed out waiting for event, which suggests it’s still using the 10s default. I think it may be worth retrying the failed test, if not, rebasing with master should help

@kfaraz
Copy link
Contributor

kfaraz commented Dec 6, 2025

Ah, thanks for the clarification @abhishekrb19 ! Let me double check.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants