Skip to content

Fix worker gone test failure in sandbox - #21270

Merged
mch2 merged 3 commits into
opensearch-project:mainfrom
cwperks:worker-gone
Apr 22, 2026
Merged

Fix worker gone test failure in sandbox#21270
mch2 merged 3 commits into
opensearch-project:mainfrom
cwperks:worker-gone

Conversation

@cwperks

@cwperks cwperks commented Apr 17, 2026

Copy link
Copy Markdown
Member

Description

Reliably reproduce the test failure with ./gradlew :sandbox:plugins:analytics-backend-datafusion:test -Dtests.seed=218506E88D711883 -Dtests.jvms=1 -i

Fixes a flaky test failure in DatafusionSearchExecEngineTests where testEngineExecuteAggregation and testEngineExecuteSelectAll fail with RuntimeException: Execution error: Worker gone.

Root cause: DataFusionJniBridgeTests called NativeBridge.shutdownTokioRuntimeManager() at the end of both testRuntimeLifecycle() and testReaderLifecycle(). This permanently shuts down the DedicatedExecutor's CPU thread pool inside the Rust RuntimeManager. Because the native side stores the runtime manager in a OnceLock, subsequent calls to initTokioRuntimeManager() from other test classes are no-ops — the shut-down manager stays in place. When DatafusionSearchExecEngineTests or DataFusionQueryExecutionTests run later in the same JVM (Gradle does not set forkEvery, so all test classes share one JVM), CrossRtStream tries to spawn work on the dead executor and gets JobError::WorkerGone.

The failure is order-dependent: it only occurs when DataFusionJniBridgeTests runs before the other test classes. Since OpenSearch's randomized test runner controls class ordering via seed, the failure is flaky — it depends on the seed drawn for that CI run.

Fix: Remove the shutdownTokioRuntimeManager() calls from DataFusionJniBridgeTests. Per-test resources (DataFusionRuntime via createGlobalRuntime, readers) are still properly cleaned up. Only the process-level RuntimeManager is left alive for the duration of the JVM, which is the correct behavior for a shared singleton.

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

cwperks added 3 commits April 10, 2026 01:11
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@cwperks
cwperks requested a review from a team as a code owner April 17, 2026 14:21
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 Multiple PR themes

Sub-PR theme: Remove shutdownTokioRuntimeManager calls from DataFusionNativeBridgeTests

Relevant files:

  • sandbox/plugins/analytics-backend-datafusion/src/test/java/org/opensearch/be/datafusion/DataFusionNativeBridgeTests.java

Sub-PR theme: Remove AwaitsFix annotation from DatafusionSearchExecEngineTests

Relevant files:

  • sandbox/plugins/analytics-backend-datafusion/src/test/java/org/opensearch/be/datafusion/DatafusionSearchExecEngineTests.java

⚡ Recommended focus areas for review

Shared State Risk

The initTokioRuntimeManager(2) call in each test method is now a no-op if another test class already initialized the runtime with a different thread count. There is no assertion or guarantee that the runtime was initialized with the expected configuration (2 threads) when these tests run. This could silently mask misconfiguration issues.

NativeBridge.initTokioRuntimeManager(2);
AwaitsFix Removed

The @AwaitsFix annotation referencing issue #21195 has been removed. Verify that the linked issue is actually resolved and that these tests are now stable enough to run in CI without the suppression annotation.

public class DatafusionSearchExecEngineTests extends OpenSearchTestCase {

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Centralize one-time runtime initialization

Both testRuntimeLifecycle and testReaderLifecycle call initTokioRuntimeManager, but
since OnceLock only allows one initialization per JVM, the second call is a silent
no-op. If testReaderLifecycle runs first (test order is not guaranteed), it will
initialize the runtime with its own thread count, and the call in
testRuntimeLifecycle will be ignored. Consider extracting the initialization to a
@BeforeClass (or @Before) setup method to make the initialization explicit and
order-independent.

sandbox/plugins/analytics-backend-datafusion/src/test/java/org/opensearch/be/datafusion/DataFusionNativeBridgeTests.java [47-48]

+@BeforeClass
+public static void setUpRuntime() {
+    NativeBridge.initTokioRuntimeManager(2);
+}
+
 public void testReaderLifecycle() throws Exception {
-    NativeBridge.initTokioRuntimeManager(2);
+    // runtime already initialized in setUpRuntime()
Suggestion importance[1-10]: 5

__

Why: The suggestion is valid — calling initTokioRuntimeManager in multiple test methods is redundant due to OnceLock semantics, and centralizing it in a @BeforeClass method would make the intent clearer. However, the PR already adds a comment explaining the no-op behavior, and the fix is a minor code quality improvement rather than a critical bug fix.

Low

@github-actions

Copy link
Copy Markdown
Contributor

❌ Gradle check result for 64d145c: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for 64d145c: SUCCESS

@codecov

codecov Bot commented Apr 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.19%. Comparing base (c93eb90) to head (64d145c).
⚠️ Report is 27 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #21270      +/-   ##
============================================
- Coverage     73.22%   73.19%   -0.04%     
- Complexity    73325    73355      +30     
============================================
  Files          5910     5911       +1     
  Lines        334422   334825     +403     
  Branches      48207    48243      +36     
============================================
+ Hits         244880   245066     +186     
- Misses        69964    70139     +175     
- Partials      19578    19620      +42     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@andrross

Copy link
Copy Markdown
Member

@mch2 Can you look at this?

@mch2 mch2 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @cwperks for the fix!

@mch2
mch2 merged commit 3da681b into opensearch-project:main Apr 22, 2026
20 of 22 checks passed
abhishek00159 pushed a commit to abhishek00159/OpenSearch that referenced this pull request Apr 23, 2026
* Fix worker gone test failure in sandbox

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Fix flaky test

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Abhishek Som <abhissom@amazon.com>
divyaruhil pushed a commit to divyaruhil/OpenSearch that referenced this pull request Apr 23, 2026
* Fix worker gone test failure in sandbox

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Fix flaky test

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Divya <divyruhil999@gmail.com>
krishna-ggk pushed a commit to krishna-ggk/OpenSearch that referenced this pull request Apr 28, 2026
* Fix worker gone test failure in sandbox

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Fix flaky test

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
imRishN pushed a commit to imRishN/OpenSearch that referenced this pull request May 8, 2026
* Fix worker gone test failure in sandbox

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Fix flaky test

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
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