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

   ## What changes were proposed in this pull request?
   
   `TestKeyLifecycleService` intermittently reports failures in bursts: once 
one test leaves the shared lifecycle service in a bad state, dozens of later 
tests can time out. HDDS-16033 reports one such failure in 
`testKeyDeletedOrRenamed`, but repeated runs exposed several independent timing 
assumptions in the same class.
   
   The tests share one `OzoneManager`, one continuously running 
`KeyLifecycleService`, and a static list of fault injectors. This patch 
isolates that shared state between tests and replaces timing-dependent checks 
with synchronization on the events the tests actually need.
   
   ### Root causes and fixes
   
   | Root cause | Failure mode | Fix |
   | --- | --- | --- |
   | **Fault injectors and lifecycle policies leaked between tests.** 
Discarding or replacing an injector without releasing it can leave a scan task 
blocked in `FaultInjectorImpl.pause()`. `BackgroundService.PeriodicalTask` 
waits for the previous batch before scheduling another cycle, so one blocked 
task stops the lifecycle service and causes later tests to time out. A policy 
left behind also keeps its bucket active, changing OM-wide counters and 
consuming injectors installed by the next test. | Cascading timeouts and 
failures in otherwise unrelated methods. | Track every installed injector and 
created policy. `@AfterEach` releases all injectors, removes all policies, 
restores modified service settings, resumes the service, and waits for it to 
become idle. Restoration is in a `finally` block so policy cleanup failures 
cannot skip it. Both policy-creation helpers now use the same tracked write 
path. |
   | **Resume tests did not limit the service to one scan.** After a resumed 
scan completed, it marked its state finished. The next periodic scan could then 
start 300 ms later, overwrite the state under test, and delete objects that the 
test expected to remain. | Incorrect remaining-object counts, scan-state 
assertions reading the next scan, and exact-count waits timing out after the 
count had already advanced past the expected value. | Hold the resumed task at 
its start, remove the policy before releasing it, and wait for that single scan 
to finish. Where a test inspects an aborted scan, hold the follow-up task until 
the assertions are complete. |
   | **The in-flight bucket list was treated as proof that a task had 
started.** The bucket is added when the task is scheduled, before its `run()` 
method reaches the injector. Suspending the service at that point can make the 
task skip its run and never reach the injector. | 
`testAbortedScanDoesNotMarkScanComplete` could block until the 300-second class 
timeout. | Add a bounded `awaitPaused` helper and wait for the task to reach 
the injector before suspending or releasing it. The timeout also prevents a 
missed injector from blocking the test indefinitely. |
   | **Some assertions ran before asynchronous cache writes became stable.** 
Keys still in the key-table cache are read from an unordered map and skipped by 
the sorted table iterator without being counted. Multipart uploads are created 
through the request pipeline, whose later DB flush could overwrite the creation 
time written directly by the test. | Unstable `lastScannedKey` / 
`numKeyIterated` values, or an aged multipart upload becoming fresh again and 
never being aborted. | Wait for the relevant key or multipart-upload cache 
entries to drain before asserting iterator state or directly updating the 
stored creation time. |
   | **Several assertions depended on timing or global state rather than the 
intended outcome.** Date-based expiration compares the object's 
`modificationTime` with the rule date. A date only two seconds in the future 
could be overtaken by a rename or ACL update. Exact expiration-candidate log 
counts depended on whether an in-flight delete had landed. 
`testPrefixDirectoryNotExpired` counted every directory in the OM, including 
directories belonging to other tests. | The Jira failure in 
`testKeyDeletedOrRenamed`, the equivalent race in 
`testKeyUpdatedShouldNotGetDeleted`, and unrelated directory-count failures. | 
Use a date that remains in the future for the full test, assert the final key 
outcome instead of an intermediate candidate count, and verify that the 
directory created by the test still resolves instead of checking an OM-wide 
count. |
   
   Only test code is changed; there is no production behavior change.
   
   ## What is the link to the Apache JIRA?
   
   https://issues.apache.org/jira/browse/HDDS-16033
   
   ## How was this patch tested?
   
   The complete `TestKeyLifecycleService` class passes locally with both 
parameter sets:
   
   ```text
   Tests run: 368, Failures: 0, Errors: 0, Skipped: 18
   ```
   
   The individual races were also reproduced before applying their fixes:
   
   * The follow-up-scan race was reproduced deterministically by stalling a 
resume test long enough for the next periodic scan to start. With the fix, the 
test remains stable under the same stall.
   * The multipart-upload race reproduced twice in seven local runs before the 
fix and zero times in six runs after it. In the failing logs, the lifecycle 
scan iterated two multipart uploads but aborted none because the pending flush 
had restored their original creation times.
   * The fork's `flaky-test-check` workflow ran the whole class with 10 splits 
and 10 iterations. As the independent causes were fixed, the result improved 
from four failing splits with 44, 81, 82, and 84 cascading errors, to isolated 
single-test failures, and finally to all splits passing:
     https://github.com/chihsuan/ozone/actions/runs/33163031960
   
   Checkstyle and RAT report no errors.
   
   Generated-by: Claude Code (Opus 5)
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to