Fix flaky NativeBridgeLocalSessionTests spill-dir cleanup race with native runtime

[LantaoJin]

Description

Root cause: createRuntime() passed a Lucene-managed createTempDir("datafusion-spill") to the native runtime. createGlobalRuntime renames the dir's entries to *.stale and deletes them on a detached background thread that closeGlobalRuntime never joins. At suite teardown, Lucene's TestRuleTemporaryFilesCleanup does a strict recursive rm of the same tree, racing the native deleter → NoSuchFileException: extra0.stale.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

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.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 62ae8a1)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 62ae8a1

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Catch unchecked IO errors during walk Files.walk can throw UncheckedIOException (wrapping e.g. NoSuchFileException) from the stream's terminal operation when the native thread deletes an entry mid-iteration. The current catch (IOException) will not catch this, re-introducing the race this PR aims to fix. Also catch UncheckedIOException (and ideally RuntimeException) around the stream consumption. sandbox/plugins/analytics-backend-datafusion/src/test/java/org/opensearch/be/datafusion/NativeBridgeLocalSessionTests.java [104-114] try (Stream<Path> walk = Files.walk(root)) { walk.sorted(Comparator.reverseOrder()).forEach(p -> { try { Files.deleteIfExists(p); } catch (IOException ignored) { // Concurrent native cleanup may have removed it, or a child reappeared mid-walk. } }); -} catch (IOException ignored) { +} catch (IOException | UncheckedIOException ignored) { // Directory vanished under us (native thread finished the wipe) — nothing left to do. } Suggestion importance[1-10]: 8 __ Why: Accurate observation: Files.walk can indeed throw UncheckedIOException during terminal stream operations when directory entries disappear mid-iteration, which is exactly the race condition this PR is trying to handle. Catching UncheckedIOException would make the cleanup more robust against the native cleanup thread race.	Medium

---

Previous suggestions

Suggestions up to commit e04b1d7

Category	Suggestion	Impact
General	Ensure superclass teardown always executes If deleteBestEffort throws an unexpected runtime exception, super.tearDown() is skipped, leaving test framework state uncleaned. Wrap the loop in a try/finally to guarantee super.tearDown() always runs. sandbox/plugins/analytics-backend-datafusion/src/test/java/org/opensearch/be/datafusion/NativeBridgeLocalSessionTests.java [83-95] @Override public void tearDown() throws Exception { - // Best-effort, race-tolerant delete. The runtime is already closed by each test, so the native - // cleanup thread has been triggered but may still be unlinking "*.stale" entries concurrently. - // We must NOT use the strict IOUtils.rm here (it throws NoSuchFileException on a file the native - // thread deleted first — the very bug this test hit). Swallow per-entry failures; the OS reaps - // the temp root either way. - for (Path dir : spillDirs) { - deleteBestEffort(dir); + try { + for (Path dir : spillDirs) { + deleteBestEffort(dir); + } + spillDirs.clear(); + } finally { + super.tearDown(); } - spillDirs.clear(); - super.tearDown(); } Suggestion importance[1-10]: 5 __ Why: Valid defensive suggestion — deleteBestEffort already swallows IOException, so unexpected exceptions are unlikely, but wrapping in try/finally is a reasonable robustness improvement for test teardown.	Low

Suggestions up to commit f8c9c2b

Category	Suggestion	Impact
Possible issue	Also swallow UncheckedIOException during walk Files.walk can throw UncheckedIOException mid-stream (e.g., when a directory disappears due to concurrent native cleanup), which is not caught by the current IOException catch and would fail tearDown. Wrap the forEach to also swallow UncheckedIOException (or catch RuntimeException) to preserve the race-tolerant contract described in the comments. sandbox/plugins/analytics-backend-datafusion/src/test/java/org/opensearch/be/datafusion/NativeBridgeLocalSessionTests.java [101-111] try (Stream<Path> walk = Files.walk(root)) { walk.sorted(Comparator.reverseOrder()).forEach(p -> { try { Files.deleteIfExists(p); } catch (IOException ignored) { // Concurrent native cleanup may have removed it, or a child reappeared mid-walk. } }); -} catch (IOException ignored) { +} catch (IOException | UncheckedIOException ignored) { // Directory vanished under us (native thread finished the wipe) — nothing left to do. } Suggestion importance[1-10]: 7 __ Why: Valid point: Files.walk can throw UncheckedIOException during stream traversal when directories disappear concurrently, which the current catch (IOException) won't handle. This directly addresses the race-tolerance goal stated in the comments.	Medium

[github-actions]

❌ Gradle check result for f8c9c2b: 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]

Persistent review updated to latest commit e04b1d7

[github-actions]

✅ Gradle check result for e04b1d7: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.41%. Comparing base (8d2b041) to head (e04b1d7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #22367      +/-   ##
============================================
+ Coverage     73.37%   73.41%   +0.03%     
- Complexity    76081    76140      +59     
============================================
  Files          6076     6076              
  Lines        345525   345525              
  Branches      49739    49739              
============================================
+ Hits         253541   253663     +122     
+ Misses        71760    71575     -185     
- Partials      20224    20287      +63     

☔ View full report in Codecov by Harness.
📢 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.

[github-actions]

Persistent review updated to latest commit 62ae8a1

[github-actions]

❌ Gradle check result for 62ae8a1: null

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?