errose28 commented on code in PR #9015:
URL: https://github.com/apache/ozone/pull/9015#discussion_r2383620325
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java:
##########
@@ -348,6 +307,33 @@ public ContainerProtos.ContainerChecksumInfo
read(ContainerData data) throws IOE
}
}
+ /**
+ * Called by the container scanner and reconciliation to update the merkle
tree persisted to disk.
+ * For live (non-deleted) blocks, only those in the incoming treeWriter
parameter are used.
+ * For deleted blocks, those in the incoming treeWriter are merged with
those on disk.
+ */
+ public ContainerProtos.ContainerChecksumInfo updateTree(ContainerData data,
ContainerMerkleTreeWriter treeWriter)
+ throws IOException {
+ return write(data, treeWriter::update);
+ }
+
+ /**
+ * Called by block deletion to update the merkle tree persisted to disk with
more deleted blocks.
+ * If a block with the same ID already exists in the tree, it is overwritten
as deleted with the checksum computed
+ * from the chunk checksums in the BlockData.
+ *
+ * The top level container data checksum is only updated if the existing
tree on disk already has this value present.
+ * This lets the block deleting service add blocks to the tree before the
scanner has reached the container, and that
+ * list of deleted blocks will not be mistaken for the list of all blocks
seen in the container.
+ * See {@link #hasDataChecksum(ContainerProtos.ContainerChecksumInfo)}.
+ */
+ public void addDeletedBlocks(ContainerData data, Collection<BlockData>
blocks) throws IOException {
Review Comment:
The longer name seems redundant. This class only has two modifier methods:
`updateTree` and `addDeletedBlocks`, both of which write to the file. There is
no modifier in this class that does not persist to the file which we might want
to distinguish from. We could standardize on `writeTree` and
`writeDeletedBlocks`, but IMO that loses the semantics of the methods. For
deleted blocks we are appending to a list, and for the tree we are doing an
operation that sets live blocks and merges the deleted ones.
Basically I think it's easier to read if each method has one verb that
defines the semantics of the modification it is doing, and all modifiers in
this class are assumed (and documented) to persist their results.
--
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]
