errose28 commented on code in PR #7083:
URL: https://github.com/apache/ozone/pull/7083#discussion_r1735209299
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java:
##########
@@ -121,10 +149,16 @@ public ContainerDiff diff(KeyValueContainerData
thisContainer, ContainerProtos.C
/**
* Returns the container checksum tree file for the specified container
without deserializing it.
*/
+ @VisibleForTesting
public static File getContainerChecksumFile(ContainerData data) {
return new File(data.getMetadataPath(), data.getContainerID() + ".tree");
}
+ @VisibleForTesting
+ public static File getTmpContainerChecksumFile(ContainerData data) {
+ return new File(data.getMetadataPath(), data.getContainerID() +
".tree.tmp");
Review Comment:
Cleanup of semi-random file names is more complicated, requiring either a
regex match or dedicated directory that gets wiped. By re-using the same file
name any leftovers from previous runs due to a failed move are automatically
overwritten by protobuf's `writeTo`. Spurious errors would cause failures in
either implementation since ultimately the file write must succeed to move
forwards.
I wanted to add a test for this specific case where the tmp write succeeds
and the move fails, but could not get the permissions to work correctly.
Removing write permissions on the destination file still allowed the move to
happen.
--
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]
