Copilot commented on code in PR #11524:
URL: https://github.com/apache/ozone/pull/11524#discussion_r4266637325
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestLeaseRecovery.java:
##########
@@ -571,9 +573,14 @@ public void testRecoveryWithPartialFilledHsyncBlock()
throws Exception {
verifyData(data, (blockSize - 1) * 2, file, fs);
}
- private ContainerInfo closeLatestContainer() throws IOException,
TimeoutException, InterruptedException {
+ /** Closes the container (and its pipeline) holding the last block written
to {@code stream}. */
+ private ContainerInfo closeLatestContainer(FSDataOutputStream stream)
+ throws IOException, TimeoutException, InterruptedException {
StorageContainerManager scm = cluster.getStorageContainerManager();
- ContainerInfo container = new
LinkedList<>(scm.getContainerManager().getContainers()).getLast();
+ List<OmKeyLocationInfo> locations = ((OzoneFSOutputStream)
stream.getWrappedStream())
+ .getWrappedOutputStream().getKeyOutputStream().getLocationInfoList();
+ ContainerInfo container = scm.getContainerManager().getContainer(
+ ContainerID.valueOf(locations.get(locations.size() -
1).getContainerID()));
Review Comment:
This relies on a deep chain of concrete-type unwraps/casts into
`OzoneFSOutputStream` internals. That’s brittle if additional stream wrappers
are introduced (e.g., buffering/metrics/encryption) or implementation details
change. If possible, prefer retrieving the last written container ID through a
more stable test utility/API (e.g., querying OM/key metadata for the file, or
introducing a small helper on the Ozone output stream interface used in tests)
to reduce coupling to internal stream structure.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestLeaseRecovery.java:
##########
@@ -571,9 +573,14 @@ public void testRecoveryWithPartialFilledHsyncBlock()
throws Exception {
verifyData(data, (blockSize - 1) * 2, file, fs);
}
- private ContainerInfo closeLatestContainer() throws IOException,
TimeoutException, InterruptedException {
+ /** Closes the container (and its pipeline) holding the last block written
to {@code stream}. */
+ private ContainerInfo closeLatestContainer(FSDataOutputStream stream)
Review Comment:
The helper name `closeLatestContainer` no longer reflects its behavior (it
closes the container holding the last block written to the provided stream, not
the latest/highest-ID container). Consider renaming to something like
`closeLastWrittenContainer` / `closeContainerForLastBlock` to prevent future
misuse and make call sites self-documenting.
--
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]
