[PR] HDDS-15269. Avoid 30s shutdown wait in ReconTaskController, speeding up its test [ozone]

["chihsuan (via GitHub)" <gi...@apache.org>]

chihsuan opened a new pull request, #10402:
URL: https://github.com/apache/ozone/pull/10402

   ## What changes were proposed in this pull request?
   
   `TestReconTaskControllerImpl` took ~145s to run 17 tests. Profiling each method
   showed the time was not spread out: four tests each blocked for almost exactly
   **30 seconds** inside `ReconTaskController.stop()`.
   
   ### Root cause
   
   `ReconTaskControllerImpl` runs a single background thread
   (`processBufferedEventsAsync`) that loops on an interruptible
   `eventBuffer.poll(...)` and only exits when the thread is **interrupted**:
   
   ```java
   while (!Thread.currentThread().isInterrupted()) {
       ReconEvent event = eventBuffer.poll(1000);
       ...
   }
   ```
   
   When this loop was introduced (HDDS-13576), `stop()` used `shutdownNow()`, which
   interrupts the thread, so it exited immediately. A later reliability change
   (HDDS-13956) replaced that with a graceful shutdown:
   
   ```java
   executor.shutdown();                       // does NOT interrupt running tasks
   executor.awaitTermination(30, SECONDS);    // waits the full timeout
   executor.shutdownNow();                     // only now interrupts
   ```
   
   `shutdown()` stops accepting new work but never interrupts the running thread,
   and the loop has no non-interrupt exit path. So the graceful phase can never
   drain the loop: `awaitTermination` always burns its full 30s timeout before the
   fallback `shutdownNow()` finally stops it. Every `stop()` therefore costs ~30s.
   This is not test-only: in production it delays Recon shutdown by ~30s and logs a
   misleading "did not terminate within 30 seconds" warning on every shutdown.
   
   ### Fix (production)
   
   Give the loop a cooperative exit path: a `volatile boolean running` flag set in
   `start()` and cleared in `stop()`. The loop checks it each iteration, so after
   `stop()` it exits on the next poll cycle and the existing graceful shutdown
   completes promptly.
   
   We chose the cooperative flag over simply reverting to `shutdownNow()` because it
   **preserves the intent of HDDS-13956**: an in-flight event is still allowed to
   finish instead of being interrupted mid-processing. The 30s `awaitTermination`
   stays as a genuine safety net rather than the normal path.
   
   ### Fix (test)
   
   Two tests (`testProcessReInitializationEventWith*`) call `stop()` on a Mockito
   `spy(controller)` to quiesce the background loop. A spy is a shallow copy, so it
   flips the `running` flag on the copy while the live thread (started on the
   original controller in `setUp`) keeps running, still hitting the 30s timeout.
   They now stop the original controller. This does not change what the tests
   assert; `stop()` there is only setup to avoid a race with the background loop.
   
   ### Scope
   
   Kept intentionally small to fix only the critical issue (the 30s production
   shutdown hang). Two unrelated, lower-impact test-only slowdowns remain and are
   left for a follow-up to avoid scope creep:
   - `testNewRetryLogicWithMaxRetriesExceeded` and `testFailedTaskRetryLogic` use
     real-clock `Thread.sleep` to wait out `RETRY_DELAY_MS`; speeding these up needs
     the retry delay to be injectable.
   
   ### Result
   
   `TestReconTaskControllerImpl`: **~145s → ~26s**, all tests pass, checkstyle clean.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-15269
   
   ## How was this patch tested?
   
   - Added `testStopCompletesPromptly`, which fails (~31s) before the fix and passes
     (~1s) after, guarding against regression.
   - Ran the full `TestReconTaskControllerImpl` class: 18 tests, 0 failures,
     ~26s (down from ~145s).
   - `mvn -pl hadoop-ozone/recon checkstyle:check`: 0 violations.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

Re: [PR] HDDS-15269. Avoid 30s shutdown wait in ReconTaskController [ozone]

["adoroszlai (via GitHub)" <gi...@apache.org>]

adoroszlai merged PR #10402:
URL: https://github.com/apache/ozone/pull/10402


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org