shoemoney commented on PR #8705:
URL: https://github.com/apache/hadoop/pull/8705#issuecomment-5467605416
Thanks @slfan1989 - good catch, that was exactly the gap. Added in 040fa443.
`testAggregatorDeletesUploadedLogsOfEveryContainerInACycle` starts two
containers with distinct log files in a single aggregation cycle and asserts
the uploaded files of both are scheduled for deletion.
Two notes on how it is written:
- The existing `createDeletionServiceWithExpectedFile2Delete` helper only
inspects the **first** `delete()` invocation. That is not sufficient here: the
buggy implementation still issued one `delete()` call, just with the wrong and
incomplete set of paths. So the test uses a separate recording
`DeletionService` that accumulates `FileDeletionTask#getBaseDirs()` across
every invocation in the cycle.
- It asserts containment rather than set equality, because once the
application finishes the aggregator also schedules the application log
directory itself for cleanup, which shows up as an extra path alongside the
per-container tasks.
Verified RED -> GREEN locally. Against the single-variable implementation:
```
Expected the uploaded log file .../container_..._000001/logfile2 to be
scheduled for deletion, but the scheduled paths were
[.../application_..., .../container_..._000002/logfile0,
.../container_..._000002/logfile1, .../container_..._000002/logfile2]
```
Only the last container's files, which is the YARN-11963 cardinality. With
the patch the full class passes, 5/5. Checkstyle on the module reports no new
violations from the added code.
--
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]
