You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "kerneltime (via GitHub)" <gi...@apache.org> on 2024/02/29 22:55:38 UTC
[PR] HDDS-10239: Initial proto file for Merkle Tree [ozone]
kerneltime opened a new pull request, #6302:
URL: https://github.com/apache/ozone/pull/6302
## What changes were proposed in this pull request?
Proto definitions for Merkle Tree
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10239
## How was this patch tested?
Basic build for now.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10239. [Reconciliation] Initial proto file for Merkle Tree [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #6302:
URL: https://github.com/apache/ozone/pull/6302#discussion_r1522803227
##########
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto:
##########
@@ -525,3 +525,37 @@ service IntraDatanodeProtocolService {
rpc download (CopyContainerRequestProto) returns (stream CopyContainerResponseProto);
rpc upload (stream SendContainerRequest) returns (SendContainerResponse);
}
+
+/*
+BlockMerkle tree stores the checksums of the chunks in a block.
+The Block checksum is derived from the checksums of the chunks in case of replicated blocks and derived from the
+metadata of the chunks in case of erasure coding.
+Two Blocks across container instances on two nodes have the same checksum if they have the same set of chunks.
+A Block upon deletion will be marked as deleted but will preserve the rest of the metadata.
+*/
+message BlockMerkleTree {
+ optional BlockData blockData = 1; // The chunks in this should be sorted by the order of chunks written.
+ optional ChecksumData checksumData = 2; // Checksum of the checksums of the chunks.
+ optional bool deleted = 3; // If the block is deleted.
+}
+
+/*
+ContainerMerkleTree stores the checksums of the blocks in a container.
+The Container checksum is derived from the checksums of the blocks.
+Two containers across container instances on two nodes have the same checksum if they have the same set of blocks.
+If a block is deleted within the container, the checksum of the container will remain unchanged.
+ */
+message ContainerMerkleTree {
+ enum FailureCause {
+ NO_HEALTHY_CHUNK_FOUND_WITH_PEERS = 1; // No healthy chunk found with peers.
+ NO_PEER_FOUND = 2; // No peer found.
+ }
+ optional int64 containerID = 1; // The container ID.
+ repeated BlockMerkleTree blockMerkleTrees = 2; // The blocks in this should be sorted by the order of blocks written.
+ optional ChecksumData dataChecksumData = 3; // Checksum of the checksums of the blocks.
+ optional ChecksumData deletedChecksumData = 4; // Checksum of the checksums of the deleted blocks.
+ optional int64 length = 5; // Length of the container.
+ optional int64 blockCount = 6; // Number of blocks in the container.
+ optional FailureCause failureCause = 7; // The cause of the failure.
+ optional int64 reconciliationCount = 8; // The reconciliation count.
Review Comment:
Thank you for clarifying @errose28!
It should be a good idea to add some of these details to the comments, thanks!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10239. [Reconciliation] Initial proto file for Merkle Tree [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime closed pull request #6302: HDDS-10239. [Reconciliation] Initial proto file for Merkle Tree
URL: https://github.com/apache/ozone/pull/6302
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10239. [Reconciliation] Initial proto file for Merkle Tree [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #6302:
URL: https://github.com/apache/ozone/pull/6302#discussion_r1520737624
##########
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto:
##########
@@ -525,3 +525,37 @@ service IntraDatanodeProtocolService {
rpc download (CopyContainerRequestProto) returns (stream CopyContainerResponseProto);
rpc upload (stream SendContainerRequest) returns (SendContainerResponse);
}
+
+/*
+BlockMerkle tree stores the checksums of the chunks in a block.
+The Block checksum is derived from the checksums of the chunks in case of replicated blocks and derived from the
+metadata of the chunks in case of erasure coding.
+Two Blocks across container instances on two nodes have the same checksum if they have the same set of chunks.
+A Block upon deletion will be marked as deleted but will preserve the rest of the metadata.
+*/
+message BlockMerkleTree {
+ optional BlockData blockData = 1; // The chunks in this should be sorted by the order of chunks written.
+ optional ChecksumData checksumData = 2; // Checksum of the checksums of the chunks.
+ optional bool deleted = 3; // If the block is deleted.
+}
+
+/*
+ContainerMerkleTree stores the checksums of the blocks in a container.
+The Container checksum is derived from the checksums of the blocks.
+Two containers across container instances on two nodes have the same checksum if they have the same set of blocks.
+If a block is deleted within the container, the checksum of the container will remain unchanged.
+ */
+message ContainerMerkleTree {
+ enum FailureCause {
+ NO_HEALTHY_CHUNK_FOUND_WITH_PEERS = 1; // No healthy chunk found with peers.
+ NO_PEER_FOUND = 2; // No peer found.
+ }
+ optional int64 containerID = 1; // The container ID.
+ repeated BlockMerkleTree blockMerkleTrees = 2; // The blocks in this should be sorted by the order of blocks written.
+ optional ChecksumData dataChecksumData = 3; // Checksum of the checksums of the blocks.
+ optional ChecksumData deletedChecksumData = 4; // Checksum of the checksums of the deleted blocks.
+ optional int64 length = 5; // Length of the container.
+ optional int64 blockCount = 6; // Number of blocks in the container.
+ optional FailureCause failureCause = 7; // The cause of the failure.
+ optional int64 reconciliationCount = 8; // The reconciliation count.
Review Comment:
This will be the number of times reconciliation has been triggered for this container. In the case where we cannot get all containers to have a matching checksum, this number can be used for exponential backoff or other mechanism to limit retries.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10239. [Reconciliation] Initial proto file for Merkle Tree [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #6302:
URL: https://github.com/apache/ozone/pull/6302#discussion_r1520739954
##########
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto:
##########
@@ -525,3 +525,37 @@ service IntraDatanodeProtocolService {
rpc download (CopyContainerRequestProto) returns (stream CopyContainerResponseProto);
rpc upload (stream SendContainerRequest) returns (SendContainerResponse);
}
+
+/*
+BlockMerkle tree stores the checksums of the chunks in a block.
+The Block checksum is derived from the checksums of the chunks in case of replicated blocks and derived from the
+metadata of the chunks in case of erasure coding.
+Two Blocks across container instances on two nodes have the same checksum if they have the same set of chunks.
+A Block upon deletion will be marked as deleted but will preserve the rest of the metadata.
+*/
+message BlockMerkleTree {
+ optional BlockData blockData = 1; // The chunks in this should be sorted by the order of chunks written.
+ optional ChecksumData checksumData = 2; // Checksum of the checksums of the chunks.
+ optional bool deleted = 3; // If the block is deleted.
+}
+
+/*
+ContainerMerkleTree stores the checksums of the blocks in a container.
+The Container checksum is derived from the checksums of the blocks.
+Two containers across container instances on two nodes have the same checksum if they have the same set of blocks.
+If a block is deleted within the container, the checksum of the container will remain unchanged.
+ */
+message ContainerMerkleTree {
+ enum FailureCause {
+ NO_HEALTHY_CHUNK_FOUND_WITH_PEERS = 1; // No healthy chunk found with peers.
+ NO_PEER_FOUND = 2; // No peer found.
+ }
+ optional int64 containerID = 1; // The container ID.
+ repeated BlockMerkleTree blockMerkleTrees = 2; // The blocks in this should be sorted by the order of blocks written.
+ optional ChecksumData dataChecksumData = 3; // Checksum of the checksums of the blocks.
+ optional ChecksumData deletedChecksumData = 4; // Checksum of the checksums of the deleted blocks.
+ optional int64 length = 5; // Length of the container.
Review Comment:
These look like AI generated comments 😄 Let's either add more details to what the fields mean or omit the comment if it is self explanatory. For example, Tanvi's question below could be answered by a comment and I don't know what container length is.
##########
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto:
##########
@@ -525,3 +525,37 @@ service IntraDatanodeProtocolService {
rpc download (CopyContainerRequestProto) returns (stream CopyContainerResponseProto);
rpc upload (stream SendContainerRequest) returns (SendContainerResponse);
}
+
+/*
+BlockMerkle tree stores the checksums of the chunks in a block.
+The Block checksum is derived from the checksums of the chunks in case of replicated blocks and derived from the
+metadata of the chunks in case of erasure coding.
+Two Blocks across container instances on two nodes have the same checksum if they have the same set of chunks.
+A Block upon deletion will be marked as deleted but will preserve the rest of the metadata.
+*/
+message BlockMerkleTree {
+ optional BlockData blockData = 1; // The chunks in this should be sorted by the order of chunks written.
+ optional ChecksumData checksumData = 2; // Checksum of the checksums of the chunks.
+ optional bool deleted = 3; // If the block is deleted.
+}
+
+/*
+ContainerMerkleTree stores the checksums of the blocks in a container.
+The Container checksum is derived from the checksums of the blocks.
+Two containers across container instances on two nodes have the same checksum if they have the same set of blocks.
+If a block is deleted within the container, the checksum of the container will remain unchanged.
+ */
+message ContainerMerkleTree {
+ enum FailureCause {
+ NO_HEALTHY_CHUNK_FOUND_WITH_PEERS = 1; // No healthy chunk found with peers.
+ NO_PEER_FOUND = 2; // No peer found.
+ }
+ optional int64 containerID = 1; // The container ID.
+ repeated BlockMerkleTree blockMerkleTrees = 2; // The blocks in this should be sorted by the order of blocks written.
Review Comment:
We don't persist the order of blocks written. Probably sorting by blockID/`localID` is easier.
##########
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto:
##########
@@ -525,3 +525,37 @@ service IntraDatanodeProtocolService {
rpc download (CopyContainerRequestProto) returns (stream CopyContainerResponseProto);
rpc upload (stream SendContainerRequest) returns (SendContainerResponse);
}
+
+/*
+BlockMerkle tree stores the checksums of the chunks in a block.
+The Block checksum is derived from the checksums of the chunks in case of replicated blocks and derived from the
+metadata of the chunks in case of erasure coding.
+Two Blocks across container instances on two nodes have the same checksum if they have the same set of chunks.
+A Block upon deletion will be marked as deleted but will preserve the rest of the metadata.
+*/
+message BlockMerkleTree {
+ optional BlockData blockData = 1; // The chunks in this should be sorted by the order of chunks written.
Review Comment:
We don't really persist the order of chunks written, but we do persist the offset. Is sorting by offset any better than sorting by `chunkName` which seems more intuitive?
##########
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto:
##########
@@ -525,3 +525,37 @@ service IntraDatanodeProtocolService {
rpc download (CopyContainerRequestProto) returns (stream CopyContainerResponseProto);
rpc upload (stream SendContainerRequest) returns (SendContainerResponse);
}
+
+/*
+BlockMerkle tree stores the checksums of the chunks in a block.
+The Block checksum is derived from the checksums of the chunks in case of replicated blocks and derived from the
+metadata of the chunks in case of erasure coding.
+Two Blocks across container instances on two nodes have the same checksum if they have the same set of chunks.
+A Block upon deletion will be marked as deleted but will preserve the rest of the metadata.
+*/
+message BlockMerkleTree {
+ optional BlockData blockData = 1; // The chunks in this should be sorted by the order of chunks written.
+ optional ChecksumData checksumData = 2; // Checksum of the checksums of the chunks.
+ optional bool deleted = 3; // If the block is deleted.
+}
+
+/*
+ContainerMerkleTree stores the checksums of the blocks in a container.
+The Container checksum is derived from the checksums of the blocks.
+Two containers across container instances on two nodes have the same checksum if they have the same set of blocks.
+If a block is deleted within the container, the checksum of the container will remain unchanged.
+ */
+message ContainerMerkleTree {
+ enum FailureCause {
+ NO_HEALTHY_CHUNK_FOUND_WITH_PEERS = 1; // No healthy chunk found with peers.
+ NO_PEER_FOUND = 2; // No peer found.
+ }
+ optional int64 containerID = 1; // The container ID.
+ repeated BlockMerkleTree blockMerkleTrees = 2; // The blocks in this should be sorted by the order of blocks written.
+ optional ChecksumData dataChecksumData = 3; // Checksum of the checksums of the blocks.
+ optional ChecksumData deletedChecksumData = 4; // Checksum of the checksums of the deleted blocks.
+ optional int64 length = 5; // Length of the container.
+ optional int64 blockCount = 6; // Number of blocks in the container.
Review Comment:
We already have this field in the main container info proto. Is it useful for reconciliation too? Regardless of whether block counts match or not we still need to walk all the blocks to see which ones to repair.
##########
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto:
##########
@@ -525,3 +525,37 @@ service IntraDatanodeProtocolService {
rpc download (CopyContainerRequestProto) returns (stream CopyContainerResponseProto);
rpc upload (stream SendContainerRequest) returns (SendContainerResponse);
}
+
+/*
+BlockMerkle tree stores the checksums of the chunks in a block.
+The Block checksum is derived from the checksums of the chunks in case of replicated blocks and derived from the
+metadata of the chunks in case of erasure coding.
+Two Blocks across container instances on two nodes have the same checksum if they have the same set of chunks.
+A Block upon deletion will be marked as deleted but will preserve the rest of the metadata.
+*/
+message BlockMerkleTree {
+ optional BlockData blockData = 1; // The chunks in this should be sorted by the order of chunks written.
+ optional ChecksumData checksumData = 2; // Checksum of the checksums of the chunks.
+ optional bool deleted = 3; // If the block is deleted.
+}
+
+/*
+ContainerMerkleTree stores the checksums of the blocks in a container.
+The Container checksum is derived from the checksums of the blocks.
+Two containers across container instances on two nodes have the same checksum if they have the same set of blocks.
+If a block is deleted within the container, the checksum of the container will remain unchanged.
+ */
+message ContainerMerkleTree {
+ enum FailureCause {
+ NO_HEALTHY_CHUNK_FOUND_WITH_PEERS = 1; // No healthy chunk found with peers.
+ NO_PEER_FOUND = 2; // No peer found.
+ }
+ optional int64 containerID = 1; // The container ID.
+ repeated BlockMerkleTree blockMerkleTrees = 2; // The blocks in this should be sorted by the order of blocks written.
+ optional ChecksumData dataChecksumData = 3; // Checksum of the checksums of the blocks.
+ optional ChecksumData deletedChecksumData = 4; // Checksum of the checksums of the deleted blocks.
Review Comment:
```suggestion
optional ChecksumData blockChecksumData = 3; // Checksum of the checksums of the blocks.
optional ChecksumData deletedBlockChecksumData = 4; // Checksum of the checksums of the deleted blocks.
```
Suggesting this because `dataChecksumData` is kind of a confusing name without context.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10239. [Reconciliation] Initial proto file for Merkle Tree [ozone]
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on PR #6302:
URL: https://github.com/apache/ozone/pull/6302#issuecomment-1977082026
@errose28 @tanvipenumudy @xichen01 could you please take a look?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10239. [Reconciliation] Initial proto file for Merkle Tree [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #6302:
URL: https://github.com/apache/ozone/pull/6302#issuecomment-1989891646
Also, let's file a new Jira subtask to link this PR to instead of using the parent Jira HDDS-10239.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10239. [Reconciliation] Initial proto file for Merkle Tree [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #6302:
URL: https://github.com/apache/ozone/pull/6302#discussion_r1512215898
##########
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto:
##########
@@ -525,3 +525,37 @@ service IntraDatanodeProtocolService {
rpc download (CopyContainerRequestProto) returns (stream CopyContainerResponseProto);
rpc upload (stream SendContainerRequest) returns (SendContainerResponse);
}
+
+/*
+BlockMerkle tree stores the checksums of the chunks in a block.
+The Block checksum is derived from the checksums of the chunks in case of replicated blocks and derived from the
+metadata of the chunks in case of erasure coding.
+Two Blocks across container instances on two nodes have the same checksum if they have the same set of chunks.
+A Block upon deletion will be marked as deleted but will preserve the rest of the metadata.
+*/
+message BlockMerkleTree {
+ optional BlockData blockData = 1; // The chunks in this should be sorted by the order of chunks written.
+ optional ChecksumData checksumData = 2; // Checksum of the checksums of the chunks.
+ optional bool deleted = 3; // If the block is deleted.
+}
+
+/*
+ContainerMerkleTree stores the checksums of the blocks in a container.
+The Container checksum is derived from the checksums of the blocks.
+Two containers across container instances on two nodes have the same checksum if they have the same set of blocks.
+If a block is deleted within the container, the checksum of the container will remain unchanged.
+ */
+message ContainerMerkleTree {
+ enum FailureCause {
+ NO_HEALTHY_CHUNK_FOUND_WITH_PEERS = 1; // No healthy chunk found with peers.
+ NO_PEER_FOUND = 2; // No peer found.
+ }
+ optional int64 containerID = 1; // The container ID.
+ repeated BlockMerkleTree blockMerkleTrees = 2; // The blocks in this should be sorted by the order of blocks written.
+ optional ChecksumData dataChecksumData = 3; // Checksum of the checksums of the blocks.
+ optional ChecksumData deletedChecksumData = 4; // Checksum of the checksums of the deleted blocks.
+ optional int64 length = 5; // Length of the container.
+ optional int64 blockCount = 6; // Number of blocks in the container.
+ optional FailureCause failureCause = 7; // The cause of the failure.
+ optional int64 reconciliationCount = 8; // The reconciliation count.
Review Comment:
Thank you for the patch, what does the field `reconciliationCount` indicate here?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10239. [Reconciliation] Initial proto file for Merkle Tree [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #6302:
URL: https://github.com/apache/ozone/pull/6302#issuecomment-2127609298
The proto changes are being merged in https://github.com/apache/ozone/pull/6708
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org