Repository navigation
YARN-11999. Fix race when checking application log directory deletion in TestLogAggregationService. - #8776
Merged
Conversation
|
💔 -1 overall
This message was automatically generated. |
… in TestLogAggregationService.
|
🎊 +1 overall
This message was automatically generated. |
joseluisll
suggested changes
Oct 3, 2026
joseluisll
left a comment
joseluisll
left a comment
Contributor
There was a problem hiding this comment.
I had the same fix in #8717 (HADOOP-19979), so happy to see it land here instead.
One suggestion: once we wait for app1LogDir, the per-file waits are redundant. The uploaded files and the app log dir are removed by two independent FileDeletionTasks (AppLogAggregatorImpl L377 and L599). Both run on the DeletionService pool, which has more than one thread and no ordering between tasks, so either can finish first. The dir deletion is recursive, so the dir being gone means the files are gone too. A single wait covers both. With the message overload, a timeout still reports which directory was left behind:
// The uploaded files and the app log dir are removed by two independent
// FileDeletionTasks on the DeletionService pool, with no ordering between
// them. The dir deletion is recursive, so waiting for app1LogDir covers
// the files inside it.
GenericTestUtils.waitFor(() -> !app1LogDir.exists(), 1000, 1000 * 50,
"Directory [" + app1LogDir + "] was not deleted");
This also drops the assertFalse calls after each waitFor. Those can never fail, because waitFor already throws TimeoutException when the condition isn't met.
… in TestLogAggregationService.
joseluisll
approved these changes
Oct 3, 2026
3 of 6 tasks
|
🎊 +1 overall
This message was automatically generated. |
Contributor
Author
|
@joseluisll @zhtttylz Thank you very much for helping to review the code! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
JIRA: https://issues.apache.org/jira/browse/YARN-11999
TestLogAggregationService.verifyLocalFileDeletion()waits for individual log files to disappear, then immediately asserts that the application log directory has also been deleted.However,
DeletionServiceperforms deletion asynchronously. The individual files can disappear before the application directory deletion completes, causingtestLocalFileDeletionAfterUploadto fail intermittently.Add a bounded wait for the application log directory to disappear before asserting its deletion, using the same polling interval and timeout as the existing file-deletion checks.
This changes only test synchronization. Production behavior remains unchanged, and a persistent deletion failure still fails the test.
How was this patch tested?
CI
For code changes:
declared according to the connector-specific documentation? Note: Automated CI
testing doesn't cover all cases so manual testing with cloud storage is still
required.
LICENSE,LICENSE-binary,NOTICE-binaryfiles?AI Tooling
If an AI tool was used:
where is the name of the AI tool used.
https://www.apache.org/legal/generative-tooling.html