joseluisll commented on PR #8717:
URL: https://github.com/apache/hadoop/pull/8717#issuecomment-5566244310
> Pre-existing, but the leak fix now relies on it:
`TimelineReaderServer.serviceStop()` calls `readerWebServer.stop()` before
`super.serviceStop()` with no try/finally. `HttpServer2.stop()` rethrows its
`MultiException`, and the service is already STOPPED by then so a retry is a
no-op, leaving the storage monitor running. `try { readerWebServer.stop(); }
finally { super.serviceStop(); }` closes it; fine as a follow-up if this PR
stays test-only.
>
> TestStandbyCheckpoints.java L844: `testPutFsimagePartFailed` has the same
shape: `snnCheckpointTime2` is read right after `waitForCheckpoint(cluster, 0,
[23])` and asserted `> snnCheckpointTime1`, but `nns[1]` stamps
`lastCheckpointTime` only after `doCheckpoint()` returns, which here is after
the upload to the stopped `nns[2]` fails. The same wait fixes it.
TestStandbyCheckpoints.testPutFsimagePartFailed — good catch, it's the same
race. Fixed here rather than deferred: nns[2] is shut down in that test, so
nns[1] is the uploader and the anchor is the same one used in
testLastCheckpointTime — waitForCheckpoint(cluster, 1, ImmutableList.of(23))
followed by a wait on getStandbyLastCheckpointTime() moving past the baseline.
TimelineReaderServer.serviceStop() — fixed here too, since the leak fix
depends on it: try { readerWebServer.stop(); } finally { super.serviceStop();
}, so a web server that fails to stop can't leave the storage monitor's polling
executor behind.
--
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]
