You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "jojochuang (via GitHub)" <gi...@apache.org> on 2023/11/22 20:07:53 UTC
[PR] HDDS-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
jojochuang opened a new pull request, #5662:
URL: https://github.com/apache/ozone/pull/5662
## What changes were proposed in this pull request?
Implement DataNode side change for HDDS-8047 incremental chunk list for PutBlock. See HDDS-8047 for the problem description and design doc.
Please describe your PR in detail:
* This PR depends on HDDS-9750 / #5661
* Test code and supporting mock classes. Tests are enabled if schema is v3.
* Implementation details
[PutBlock metadata optimization.pdf](https://github.com/apache/ozone/files/13443985/PutBlock.metadata.optimization.pdf)
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9751
## How was this patch tested?
* New unit tests in TestBlockManagerImpl
* Full cluster HBase ycsb workload test
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1454839540
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,168 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
+ // check block data table
+ BlockData blockData = getBlockDataTable().get(blockKey);
+
+ if (blockData == null) {
+ if (lastChunk == null) {
+ throw new StorageContainerException(
+ NO_SUCH_BLOCK_ERR_MSG + " BlockID : " + blockID, NO_SUCH_BLOCK);
+ } else {
+ LOG.debug("blockData=(null), lastChunk={}", lastChunk.getChunks());
+ return lastChunk;
+ }
+ } else {
+ if (lastChunk != null) {
+ reconcilePartialChunks(lastChunk, blockData);
+ } else {
+ LOG.debug("blockData={}, lastChunk=(null)", blockData.getChunks());
+ }
+ }
+
+ return blockData;
+ }
+
+ private void reconcilePartialChunks(
+ BlockData lastChunk, BlockData blockData) {
+ LOG.debug("blockData={}, lastChunk={}",
+ blockData.getChunks(), lastChunk.getChunks());
+ Preconditions.checkState(lastChunk.getChunks().size() == 1);
+ ContainerProtos.ChunkInfo lastChunkInBlockData =
+ blockData.getChunks().get(blockData.getChunks().size() - 1);
+ Preconditions.checkState(
+ lastChunkInBlockData.getOffset() + lastChunkInBlockData.getLen()
+ == lastChunk.getChunks().get(0).getOffset(),
+ "chunk offset does not match");
+
+ // append last partial chunk to the block data
+ List<ContainerProtos.ChunkInfo> chunkInfos =
+ new ArrayList<>(blockData.getChunks());
+ chunkInfos.add(lastChunk.getChunks().get(0));
+ blockData.setChunks(chunkInfos);
+
+ blockData.setBlockCommitSequenceId(
+ lastChunk.getBlockCommitSequenceId());
+ }
+
+ private static boolean isPartialChunkList(BlockData data) {
+ return data.getMetadata().containsKey(INCREMENTAL_CHUNK_LIST);
+ }
+
+ private static boolean isFullChunk(ContainerProtos.ChunkInfo chunkInfo) {
+ for (ContainerProtos.KeyValue kv: chunkInfo.getMetadataList()) {
+ if (kv.getKey().equals(FULL_CHUNK)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ // if eob or if the last chunk is full,
+ private static boolean shouldAppendLastChunk(boolean endOfBlock,
+ BlockData data) {
+ if (endOfBlock || data.getChunks().isEmpty()) {
Review Comment:
hsync right after creating a file might cause client to send empty chunk
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1452899232
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,168 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
+ // check block data table
+ BlockData blockData = getBlockDataTable().get(blockKey);
+
+ if (blockData == null) {
+ if (lastChunk == null) {
+ throw new StorageContainerException(
+ NO_SUCH_BLOCK_ERR_MSG + " BlockID : " + blockID, NO_SUCH_BLOCK);
+ } else {
+ LOG.debug("blockData=(null), lastChunk={}", lastChunk.getChunks());
+ return lastChunk;
+ }
+ } else {
+ if (lastChunk != null) {
+ reconcilePartialChunks(lastChunk, blockData);
+ } else {
+ LOG.debug("blockData={}, lastChunk=(null)", blockData.getChunks());
+ }
+ }
+
+ return blockData;
+ }
+
+ private void reconcilePartialChunks(
+ BlockData lastChunk, BlockData blockData) {
+ LOG.debug("blockData={}, lastChunk={}",
+ blockData.getChunks(), lastChunk.getChunks());
+ Preconditions.checkState(lastChunk.getChunks().size() == 1);
+ ContainerProtos.ChunkInfo lastChunkInBlockData =
+ blockData.getChunks().get(blockData.getChunks().size() - 1);
+ Preconditions.checkState(
+ lastChunkInBlockData.getOffset() + lastChunkInBlockData.getLen()
Review Comment:
Shall we remove this lastChunkInBlockData.getLen() in the offset comparison?
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1452897214
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,168 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
+ // check block data table
+ BlockData blockData = getBlockDataTable().get(blockKey);
+
+ if (blockData == null) {
+ if (lastChunk == null) {
+ throw new StorageContainerException(
+ NO_SUCH_BLOCK_ERR_MSG + " BlockID : " + blockID, NO_SUCH_BLOCK);
+ } else {
+ LOG.debug("blockData=(null), lastChunk={}", lastChunk.getChunks());
+ return lastChunk;
+ }
+ } else {
+ if (lastChunk != null) {
+ reconcilePartialChunks(lastChunk, blockData);
+ } else {
+ LOG.debug("blockData={}, lastChunk=(null)", blockData.getChunks());
Review Comment:
Could you add the LOG.isDebugEnabled check here, L254 and L270?
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #5662:
URL: https://github.com/apache/ozone/pull/5662#issuecomment-1893128884
@jojochuang , for other cases like block deletion, container deletion, and container replica, will you file other JIRAs to support the incremental list table handling?
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1452939594
##########
hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MockXceiverClientSpi.java:
##########
@@ -115,8 +115,9 @@ public XceiverClientReply sendCommandAsync(
private ReadChunkResponseProto readChunk(ReadChunkRequestProto readChunk) {
return ReadChunkResponseProto.newBuilder()
- .setChunkData(datanodeStorage
- .readChunkInfo(readChunk.getBlockID(), readChunk.getChunkData()))
+ //.setChunkData(datanodeStorage
+ // .readChunkInfo(readChunk.getBlockID(), readChunk.getChunkData()))
Review Comment:
Shall we remove this two commented out lines？
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1454844418
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestBlockManagerImpl.java:
##########
@@ -225,4 +230,173 @@ public void testListBlock() throws Exception {
assertNotNull(listBlockData);
assertTrue(listBlockData.size() == 10);
}
+
+ private BlockData createBlockData(long containerID, long blockNo,
+ int chunkID, long begining, long offset, long len, long bcsID)
+ throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ ChunkInfo info1 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), chunkID), offset, len);
+ chunkList1.add(info1.getProtoBufMessage());
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ private BlockData createBlockDataWithOneFullChunk(long containerID,
+ long blockNo, int chunkID, long begining, long offset, long len,
+ long bcsID)
+ throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ ChunkInfo info1 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), 1), 0, 4 * 1024 * 1024);
+ info1.addMetadata(FULL_CHUNK, "");
+
+ ChunkInfo info2 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), chunkID), offset, len);
+ chunkList1.add(info1.getProtoBufMessage());
+ chunkList1.add(info2.getProtoBufMessage());
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ private BlockData createBlockDataWithThreeFullChunks(long containerID,
+ long blockNo, long bcsID) throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ long chunkLimit = 4 * 1024 * 1024;
+ for (int i = 1; i < 4; i++) {
+ ChunkInfo info1 = new ChunkInfo(
+ String.format("%d_chunk_%d", blockID1.getLocalID(), i),
+ chunkLimit * i, chunkLimit);
+ info1.addMetadata(FULL_CHUNK, "");
+ chunkList1.add(info1.getProtoBufMessage());
+ }
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ @Test
+ public void testFlush1() throws Exception {
+ assumeTrue(isSameSchemaVersion(schemaVersion, OzoneConsts.SCHEMA_V3));
Review Comment:
do it in another way: skip if schema is v1.
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1452897214
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,168 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
+ // check block data table
+ BlockData blockData = getBlockDataTable().get(blockKey);
+
+ if (blockData == null) {
+ if (lastChunk == null) {
+ throw new StorageContainerException(
+ NO_SUCH_BLOCK_ERR_MSG + " BlockID : " + blockID, NO_SUCH_BLOCK);
+ } else {
+ LOG.debug("blockData=(null), lastChunk={}", lastChunk.getChunks());
+ return lastChunk;
+ }
+ } else {
+ if (lastChunk != null) {
+ reconcilePartialChunks(lastChunk, blockData);
+ } else {
+ LOG.debug("blockData={}, lastChunk=(null)", blockData.getChunks());
Review Comment:
Could you add the LOG.isDebugEnabled check here and L254?
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1452956665
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,168 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
+ // check block data table
+ BlockData blockData = getBlockDataTable().get(blockKey);
+
+ if (blockData == null) {
+ if (lastChunk == null) {
+ throw new StorageContainerException(
+ NO_SUCH_BLOCK_ERR_MSG + " BlockID : " + blockID, NO_SUCH_BLOCK);
+ } else {
+ LOG.debug("blockData=(null), lastChunk={}", lastChunk.getChunks());
+ return lastChunk;
+ }
+ } else {
+ if (lastChunk != null) {
+ reconcilePartialChunks(lastChunk, blockData);
+ } else {
+ LOG.debug("blockData={}, lastChunk=(null)", blockData.getChunks());
+ }
+ }
+
+ return blockData;
+ }
+
+ private void reconcilePartialChunks(
+ BlockData lastChunk, BlockData blockData) {
+ LOG.debug("blockData={}, lastChunk={}",
+ blockData.getChunks(), lastChunk.getChunks());
+ Preconditions.checkState(lastChunk.getChunks().size() == 1);
+ ContainerProtos.ChunkInfo lastChunkInBlockData =
+ blockData.getChunks().get(blockData.getChunks().size() - 1);
+ Preconditions.checkState(
+ lastChunkInBlockData.getOffset() + lastChunkInBlockData.getLen()
+ == lastChunk.getChunks().get(0).getOffset(),
+ "chunk offset does not match");
+
+ // append last partial chunk to the block data
+ List<ContainerProtos.ChunkInfo> chunkInfos =
+ new ArrayList<>(blockData.getChunks());
+ chunkInfos.add(lastChunk.getChunks().get(0));
+ blockData.setChunks(chunkInfos);
+
+ blockData.setBlockCommitSequenceId(
+ lastChunk.getBlockCommitSequenceId());
+ }
+
+ private static boolean isPartialChunkList(BlockData data) {
+ return data.getMetadata().containsKey(INCREMENTAL_CHUNK_LIST);
+ }
+
+ private static boolean isFullChunk(ContainerProtos.ChunkInfo chunkInfo) {
+ for (ContainerProtos.KeyValue kv: chunkInfo.getMetadataList()) {
+ if (kv.getKey().equals(FULL_CHUNK)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ // if eob or if the last chunk is full,
+ private static boolean shouldAppendLastChunk(boolean endOfBlock,
+ BlockData data) {
+ if (endOfBlock || data.getChunks().isEmpty()) {
Review Comment:
In what case that the data.getChunks will be empty?
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1446870325
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java:
##########
@@ -354,14 +367,6 @@ public void shutdown() {
private BlockData getBlockByID(DBHandle db, BlockID blockID,
KeyValueContainerData containerData) throws IOException {
- String blockKey = containerData.getBlockKey(blockID.getLocalID());
-
- BlockData blockData = db.getStore().getBlockDataTable().get(blockKey);
- if (blockData == null) {
- throw new StorageContainerException(NO_SUCH_BLOCK_ERR_MSG +
- " BlockID : " + blockID, NO_SUCH_BLOCK);
- }
-
- return blockData;
+ return db.getStore().getBlockByID(blockID, containerData);
Review Comment:
blockKey can be passed instead of containerData.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,185 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
+ // check block data table
+ BlockData blockData = getBlockDataTable().get(blockKey);
+
+ if (blockData == null) {
+ if (lastChunk == null) {
+ throw new StorageContainerException(
+ NO_SUCH_BLOCK_ERR_MSG + " BlockID : " + blockID, NO_SUCH_BLOCK);
+ } else {
+ LOG.debug("blockData=(null), lastChunk={}", lastChunk.getChunks());
+ return lastChunk;
+ }
+ } else {
+ if (lastChunk != null) {
+ reconcilePartialChunks(lastChunk, blockData);
+ } else {
+ LOG.debug("blockData={}, lastChunk=(null)", blockData.getChunks());
+ }
+ }
+
+ return blockData;
+ }
+
+ private static void reconcilePartialChunks(
Review Comment:
I think no need of static method here.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,185 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
+ // check block data table
+ BlockData blockData = getBlockDataTable().get(blockKey);
+
+ if (blockData == null) {
+ if (lastChunk == null) {
+ throw new StorageContainerException(
+ NO_SUCH_BLOCK_ERR_MSG + " BlockID : " + blockID, NO_SUCH_BLOCK);
+ } else {
+ LOG.debug("blockData=(null), lastChunk={}", lastChunk.getChunks());
+ return lastChunk;
+ }
+ } else {
+ if (lastChunk != null) {
+ reconcilePartialChunks(lastChunk, blockData);
+ } else {
+ LOG.debug("blockData={}, lastChunk=(null)", blockData.getChunks());
+ }
+ }
+
+ return blockData;
+ }
+
+ private static void reconcilePartialChunks(
+ BlockData lastChunk, BlockData blockData) {
+ LOG.debug("blockData={}, lastChunk={}",
+ blockData.getChunks(), lastChunk.getChunks());
+ Preconditions.checkState(lastChunk.getChunks().size() == 1);
+ ContainerProtos.ChunkInfo lastChunkInBlockData =
+ blockData.getChunks().get(blockData.getChunks().size() - 1);
+ Preconditions.checkState(
+ lastChunkInBlockData.getOffset() + lastChunkInBlockData.getLen()
+ == lastChunk.getChunks().get(0).getOffset(),
+ "chunk offset does not match");
+
+ // append last partial chunk to the block data
+ List<ContainerProtos.ChunkInfo> chunkInfos =
+ new ArrayList<>(blockData.getChunks());
+ chunkInfos.add(lastChunk.getChunks().get(0));
+ blockData.setChunks(chunkInfos);
+
+ blockData.setBlockCommitSequenceId(
+ lastChunk.getBlockCommitSequenceId());
+ }
+
+ private static boolean isPartialChunkList(BlockData data) {
+ return data.getMetadata().containsKey(INCREMENTAL_CHUNK_LIST);
+ }
+
+ private static boolean isFullChunk(ContainerProtos.ChunkInfo chunkInfo) {
+ for (ContainerProtos.KeyValue kv: chunkInfo.getMetadataList()) {
+ if (kv.getKey().equals(FULL_CHUNK)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ // if eob or if the last chunk is full,
+ private static boolean shouldAppendLastChunk(boolean endOfBlock,
+ BlockData data) {
+ if (endOfBlock || data.getChunks().isEmpty()) {
+ return true;
+ }
+ return isFullChunk(data.getChunks().get(data.getChunks().size() - 1));
+ }
+
+ public void putBlockByID(BatchOperation batch, boolean incremental,
+ long localID, BlockData data, KeyValueContainerData containerData,
+ boolean endOfBlock) throws IOException {
+ if (!incremental && !isPartialChunkList(data)) {
+ // Case (1) old client: override chunk list.
+ getBlockDataTable().putWithBatch(
+ batch, containerData.getBlockKey(localID), data);
+ } else if (shouldAppendLastChunk(endOfBlock, data)) {
+ moveLastChunkToBlockData(batch, localID, data, containerData);
+ } else {
+ // incremental chunk list,
+ // not end of block, has partial chunks
+ putBlockWithPartialChunks(batch, localID, data, containerData);
+ }
+ }
+
+ private void moveLastChunkToBlockData(BatchOperation batch, long localID,
+ BlockData data, KeyValueContainerData containerData) throws IOException {
+ // if eob or if the last chunk is full,
+ // the 'data' is full so append it to the block table's chunk info
+ // and then remove from lastChunkInfo
+ BlockData blockData = getBlockDataTable().get(
+ containerData.getBlockKey(localID));
+ if (blockData == null) {
+ // Case 2.1 if the block did not have full chunks before,
+ // the block's chunk is what received from client this time.
+ blockData = data;
+ } else {
+ // case 2.2 the block already has some full chunks
+ List<ContainerProtos.ChunkInfo> chunkInfoList = blockData.getChunks();
+ blockData.setChunks(new ArrayList<>(chunkInfoList));
+ for (ContainerProtos.ChunkInfo chunk : data.getChunks()) {
+ blockData.addChunk(chunk);
+ }
+ blockData.setBlockCommitSequenceId(data.getBlockCommitSequenceId());
+
+ /*int next = 0;
+ for (ContainerProtos.ChunkInfo info : chunkInfoList) {
+ assert info.getOffset() == next;
Review Comment:
Please remove commented code
##########
hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MockDatanodeStorage.java:
##########
@@ -77,31 +159,56 @@ public void writeChunk(
if (exception != null) {
throw exception;
}
- data.put(createKey(blockID, chunkInfo),
- ByteString.copyFrom(bytes.toByteArray()));
- chunks.put(createKey(blockID, chunkInfo), chunkInfo);
+ String blockKey = createKey(blockID);
+ ByteString block;
+ if (data.containsKey(blockKey)) {
+ block = data.get(blockKey);
+ assert block.size() == chunkInfo.getOffset();
+ data.put(blockKey, block.concat(bytes));
+ } else {
+ assert chunkInfo.getOffset() == 0;
+ data.put(blockKey, bytes);
+ }
+
fullBlockData
.put(new BlockID(blockID.getContainerID(), blockID.getLocalID()),
- fullBlockData.getOrDefault(blockID, "")
+ fullBlockData.getOrDefault(toBlockID(blockID), "")
.concat(bytes.toStringUtf8()));
}
public ChunkInfo readChunkInfo(
DatanodeBlockID blockID,
ChunkInfo chunkInfo) {
- return chunks.get(createKey(blockID, chunkInfo));
+ BlockData blockData = getBlock(blockID);
+ for (ChunkInfo info : blockData.getChunksList()) {
+ if (info.getLen() == chunkInfo.getLen() &&
+ info.getOffset() == chunkInfo.getOffset()) {
+ return info;
+ }
+ }
+ throw new AssertionError("chunk " + chunkInfo + " not found");
}
public ByteString readChunkData(
DatanodeBlockID blockID,
ChunkInfo chunkInfo) {
- return data.get(createKey(blockID, chunkInfo));
-
+ //return data.get(createKey(blockID, chunkInfo));
+ LOG.debug("readChunkData: blockID=" + createKey(blockID) +
+ " chunkInfo offset=" + chunkInfo.getOffset() +
+ " chunkInfo len=" + chunkInfo.getLen());
+ ByteString str = data.get(createKey(blockID)).substring(
+ (int)chunkInfo.getOffset(),
+ (int)chunkInfo.getOffset() + (int)chunkInfo.getLen());
+ return str;
}
- private String createKey(DatanodeBlockID blockId, ChunkInfo chunkInfo) {
+ /*private String createKey(DatanodeBlockID blockId, ChunkInfo chunkInfo) {
Review Comment:
Unused method can be removed.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,185 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
Review Comment:
If incremental is disabled we can avoid calling getLastChunkInfoTable().
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,185 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
+ // check block data table
+ BlockData blockData = getBlockDataTable().get(blockKey);
+
+ if (blockData == null) {
+ if (lastChunk == null) {
+ throw new StorageContainerException(
+ NO_SUCH_BLOCK_ERR_MSG + " BlockID : " + blockID, NO_SUCH_BLOCK);
+ } else {
+ LOG.debug("blockData=(null), lastChunk={}", lastChunk.getChunks());
+ return lastChunk;
+ }
+ } else {
+ if (lastChunk != null) {
+ reconcilePartialChunks(lastChunk, blockData);
+ } else {
+ LOG.debug("blockData={}, lastChunk=(null)", blockData.getChunks());
+ }
+ }
+
+ return blockData;
+ }
+
+ private static void reconcilePartialChunks(
+ BlockData lastChunk, BlockData blockData) {
+ LOG.debug("blockData={}, lastChunk={}",
+ blockData.getChunks(), lastChunk.getChunks());
+ Preconditions.checkState(lastChunk.getChunks().size() == 1);
+ ContainerProtos.ChunkInfo lastChunkInBlockData =
+ blockData.getChunks().get(blockData.getChunks().size() - 1);
+ Preconditions.checkState(
+ lastChunkInBlockData.getOffset() + lastChunkInBlockData.getLen()
+ == lastChunk.getChunks().get(0).getOffset(),
+ "chunk offset does not match");
+
+ // append last partial chunk to the block data
+ List<ContainerProtos.ChunkInfo> chunkInfos =
+ new ArrayList<>(blockData.getChunks());
+ chunkInfos.add(lastChunk.getChunks().get(0));
+ blockData.setChunks(chunkInfos);
+
+ blockData.setBlockCommitSequenceId(
+ lastChunk.getBlockCommitSequenceId());
+ }
+
+ private static boolean isPartialChunkList(BlockData data) {
+ return data.getMetadata().containsKey(INCREMENTAL_CHUNK_LIST);
+ }
+
+ private static boolean isFullChunk(ContainerProtos.ChunkInfo chunkInfo) {
+ for (ContainerProtos.KeyValue kv: chunkInfo.getMetadataList()) {
+ if (kv.getKey().equals(FULL_CHUNK)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ // if eob or if the last chunk is full,
+ private static boolean shouldAppendLastChunk(boolean endOfBlock,
+ BlockData data) {
+ if (endOfBlock || data.getChunks().isEmpty()) {
+ return true;
+ }
+ return isFullChunk(data.getChunks().get(data.getChunks().size() - 1));
+ }
+
+ public void putBlockByID(BatchOperation batch, boolean incremental,
+ long localID, BlockData data, KeyValueContainerData containerData,
+ boolean endOfBlock) throws IOException {
+ if (!incremental && !isPartialChunkList(data)) {
+ // Case (1) old client: override chunk list.
+ getBlockDataTable().putWithBatch(
+ batch, containerData.getBlockKey(localID), data);
+ } else if (shouldAppendLastChunk(endOfBlock, data)) {
+ moveLastChunkToBlockData(batch, localID, data, containerData);
+ } else {
+ // incremental chunk list,
+ // not end of block, has partial chunks
+ putBlockWithPartialChunks(batch, localID, data, containerData);
+ }
+ }
+
+ private void moveLastChunkToBlockData(BatchOperation batch, long localID,
+ BlockData data, KeyValueContainerData containerData) throws IOException {
+ // if eob or if the last chunk is full,
+ // the 'data' is full so append it to the block table's chunk info
+ // and then remove from lastChunkInfo
+ BlockData blockData = getBlockDataTable().get(
+ containerData.getBlockKey(localID));
+ if (blockData == null) {
+ // Case 2.1 if the block did not have full chunks before,
+ // the block's chunk is what received from client this time.
+ blockData = data;
+ } else {
+ // case 2.2 the block already has some full chunks
+ List<ContainerProtos.ChunkInfo> chunkInfoList = blockData.getChunks();
+ blockData.setChunks(new ArrayList<>(chunkInfoList));
+ for (ContainerProtos.ChunkInfo chunk : data.getChunks()) {
+ blockData.addChunk(chunk);
+ }
+ blockData.setBlockCommitSequenceId(data.getBlockCommitSequenceId());
+
+ /*int next = 0;
+ for (ContainerProtos.ChunkInfo info : chunkInfoList) {
+ assert info.getOffset() == next;
+ next += info.getLen();
+ }*/
+ }
+ // delete the entry from last chunk info table
+ getLastChunkInfoTable().deleteWithBatch(
+ batch, containerData.getBlockKey(localID));
+ // update block data table
+ getBlockDataTable().putWithBatch(batch,
+ containerData.getBlockKey(localID), blockData);
+ }
+
+ private void putBlockWithPartialChunks(BatchOperation batch, long localID,
+ BlockData data, KeyValueContainerData containerData) throws IOException {
+ if (data.getChunks().size() == 1) {
+ // Case (3.1) replace/update the last chunk info table
+ getLastChunkInfoTable().putWithBatch(
+ batch, containerData.getBlockKey(localID), data);
+ } else {
+ int lastChunkIndex = data.getChunks().size() - 1;
+ // received more than one chunk this time
+ List<ContainerProtos.ChunkInfo> lastChunkInfo =
+ Collections.singletonList(
+ data.getChunks().get(lastChunkIndex));
+ BlockData blockData = getBlockDataTable().get(
+ containerData.getBlockKey(localID));
+ if (blockData == null) {
+ // Case 3.2: if the block does not exist in the block data table
+ List<ContainerProtos.ChunkInfo> chunkInfos =
+ new ArrayList<>(data.getChunks());
+ chunkInfos.remove(lastChunkIndex);
+ data.setChunks(chunkInfos);
+ blockData = data;
+ LOG.debug("block {} does not have full chunks yet. Adding the " +
+ "chunks to it {}", localID, blockData);
+ } else {
+ // Case 3.3: if the block exists in the block data table,
+ // append chunks till except the last one (supposedly partial)
+ List<ContainerProtos.ChunkInfo> chunkInfos =
+ new ArrayList<>(blockData.getChunks());
+
+ LOG.debug("blockData.getChunks()={}", chunkInfos);
+ LOG.debug("data.getChunks()={}", data.getChunks());
+
+ for (int i = 0; i < lastChunkIndex; i++) {
+ chunkInfos.add(data.getChunks().get(i));
+ }
+ blockData.setChunks(chunkInfos);
+ blockData.setBlockCommitSequenceId(data.getBlockCommitSequenceId());
+
+ /*int next = 0;
+ for (ContainerProtos.ChunkInfo info : chunkInfos) {
+ if (info.getOffset() != next) {
+
+ LOG.error("blockData.getChunks()={}", chunkInfos);
+ LOG.error("data.getChunks()={}", data.getChunks());
+ }
+ assert info.getOffset() == next;
+ next += info.getLen();
+ }*/
+ }
+ getBlockDataTable().putWithBatch(batch,
+ containerData.getBlockKey(localID), blockData);
+ // update the last partial chunk
+ data.setChunks(lastChunkInfo);
+ getLastChunkInfoTable().putWithBatch(
+ batch, containerData.getBlockKey(localID), data);
Review Comment:
Can we just define and store the last chunk instead of blockData containing last chunk.
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1452962012
##########
hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MockDatanodeStorage.java:
##########
@@ -77,31 +159,51 @@ public void writeChunk(
if (exception != null) {
throw exception;
}
- data.put(createKey(blockID, chunkInfo),
- ByteString.copyFrom(bytes.toByteArray()));
- chunks.put(createKey(blockID, chunkInfo), chunkInfo);
+ String blockKey = createKey(blockID);
+ ByteString block;
+ if (data.containsKey(blockKey)) {
+ block = data.get(blockKey);
+ assert block.size() == chunkInfo.getOffset();
+ data.put(blockKey, block.concat(bytes));
+ } else {
+ assert chunkInfo.getOffset() == 0;
+ data.put(blockKey, bytes);
+ }
+
fullBlockData
.put(new BlockID(blockID.getContainerID(), blockID.getLocalID()),
- fullBlockData.getOrDefault(blockID, "")
+ fullBlockData.getOrDefault(toBlockID(blockID), "")
.concat(bytes.toStringUtf8()));
}
public ChunkInfo readChunkInfo(
DatanodeBlockID blockID,
ChunkInfo chunkInfo) {
- return chunks.get(createKey(blockID, chunkInfo));
+ BlockData blockData = getBlock(blockID);
+ for (ChunkInfo info : blockData.getChunksList()) {
+ if (info.getLen() == chunkInfo.getLen() &&
+ info.getOffset() == chunkInfo.getOffset()) {
+ return info;
+ }
+ }
+ throw new AssertionError("chunk " + chunkInfo + " not found");
}
public ByteString readChunkData(
DatanodeBlockID blockID,
ChunkInfo chunkInfo) {
- return data.get(createKey(blockID, chunkInfo));
-
+ //return data.get(createKey(blockID, chunkInfo));
Review Comment:
remove this line.
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1454912674
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,185 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
Review Comment:
good idea.
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1462937423
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestBlockManagerImpl.java:
##########
@@ -222,4 +227,175 @@ public void testListBlock(ContainerTestVersionInfo versionInfo)
assertNotNull(listBlockData);
assertEquals(10, listBlockData.size());
}
+
+ private BlockData createBlockData(long containerID, long blockNo,
+ int chunkID, long offset, long len, long bcsID)
+ throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ ChunkInfo info1 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), chunkID), offset, len);
+ chunkList1.add(info1.getProtoBufMessage());
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ private BlockData createBlockDataWithOneFullChunk(long containerID,
+ long blockNo, int chunkID, long offset, long len, long bcsID)
+ throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ ChunkInfo info1 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), 1), 0, 4 * 1024 * 1024);
+ info1.addMetadata(FULL_CHUNK, "");
+
+ ChunkInfo info2 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), chunkID), offset, len);
+ chunkList1.add(info1.getProtoBufMessage());
+ chunkList1.add(info2.getProtoBufMessage());
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ private BlockData createBlockDataWithThreeFullChunks(long containerID,
+ long blockNo, long bcsID) throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ long chunkLimit = 4 * 1024 * 1024;
+ for (int i = 1; i < 4; i++) {
+ ChunkInfo info1 = new ChunkInfo(
+ String.format("%d_chunk_%d", blockID1.getLocalID(), i),
+ chunkLimit * i, chunkLimit);
+ info1.addMetadata(FULL_CHUNK, "");
+ chunkList1.add(info1.getProtoBufMessage());
+ }
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ @ContainerTestVersionInfo.ContainerTest
Review Comment:
Great catch! And I actually realized I had to move the code to a shared class so that both v2 and v3 can support this feature.
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1452957068
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestBlockManagerImpl.java:
##########
@@ -225,4 +230,173 @@ public void testListBlock() throws Exception {
assertNotNull(listBlockData);
assertTrue(listBlockData.size() == 10);
}
+
+ private BlockData createBlockData(long containerID, long blockNo,
+ int chunkID, long begining, long offset, long len, long bcsID)
Review Comment:
this beginning is not used, same in next createBlockDataWithOneFullChunk function.
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #5662:
URL: https://github.com/apache/ozone/pull/5662#issuecomment-1895240765
Yes HDDS-9753 is the catch-all for handling incremental chunk lists in other operations.
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1452963266
##########
hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MockDatanodeStorage.java:
##########
@@ -30,17 +31,26 @@
import java.util.List;
import java.util.Map;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
/**
* State represents persisted data of one specific datanode.
*/
public class MockDatanodeStorage {
-
- private final Map<DatanodeBlockID, BlockData> blocks = new HashedMap();
+ public static final Logger LOG =
+ LoggerFactory.getLogger(MockDatanodeStorage.class);
+ public static final String INCREMENTAL_CHUNK_LIST = "incremental";
+ public static final String FULL_CHUNK = "full";
+ public static final ContainerProtos.KeyValue FULL_CHUNK_KV =
+ ContainerProtos.KeyValue.newBuilder().setKey(FULL_CHUNK).build();
+
+ private final Map<BlockID, BlockData> blocks = new HashedMap();
private final Map<Long, List<DatanodeBlockID>>
containerBlocks = new HashedMap();
private final Map<BlockID, String> fullBlockData = new HashMap<>();
- private final Map<String, ChunkInfo> chunks = new HashMap<>();
+ //private final Map<String, ChunkInfo> chunks = new HashMap<>();
Review Comment:
remove this line.
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1452952017
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestBlockManagerImpl.java:
##########
@@ -225,4 +230,173 @@ public void testListBlock() throws Exception {
assertNotNull(listBlockData);
assertTrue(listBlockData.size() == 10);
}
+
+ private BlockData createBlockData(long containerID, long blockNo,
+ int chunkID, long begining, long offset, long len, long bcsID)
+ throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ ChunkInfo info1 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), chunkID), offset, len);
+ chunkList1.add(info1.getProtoBufMessage());
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ private BlockData createBlockDataWithOneFullChunk(long containerID,
+ long blockNo, int chunkID, long begining, long offset, long len,
+ long bcsID)
+ throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ ChunkInfo info1 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), 1), 0, 4 * 1024 * 1024);
+ info1.addMetadata(FULL_CHUNK, "");
+
+ ChunkInfo info2 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), chunkID), offset, len);
+ chunkList1.add(info1.getProtoBufMessage());
+ chunkList1.add(info2.getProtoBufMessage());
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ private BlockData createBlockDataWithThreeFullChunks(long containerID,
+ long blockNo, long bcsID) throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ long chunkLimit = 4 * 1024 * 1024;
+ for (int i = 1; i < 4; i++) {
+ ChunkInfo info1 = new ChunkInfo(
+ String.format("%d_chunk_%d", blockID1.getLocalID(), i),
+ chunkLimit * i, chunkLimit);
+ info1.addMetadata(FULL_CHUNK, "");
+ chunkList1.add(info1.getProtoBufMessage());
+ }
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ @Test
+ public void testFlush1() throws Exception {
+ assumeTrue(isSameSchemaVersion(schemaVersion, OzoneConsts.SCHEMA_V3));
Review Comment:
Since SCHEMA_V2 is also supported. We can use parameterized test to test both SCHEMA_V3 and SCHEMA_V2.
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1452897214
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,168 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
+ // check block data table
+ BlockData blockData = getBlockDataTable().get(blockKey);
+
+ if (blockData == null) {
+ if (lastChunk == null) {
+ throw new StorageContainerException(
+ NO_SUCH_BLOCK_ERR_MSG + " BlockID : " + blockID, NO_SUCH_BLOCK);
+ } else {
+ LOG.debug("blockData=(null), lastChunk={}", lastChunk.getChunks());
+ return lastChunk;
+ }
+ } else {
+ if (lastChunk != null) {
+ reconcilePartialChunks(lastChunk, blockData);
+ } else {
+ LOG.debug("blockData={}, lastChunk=(null)", blockData.getChunks());
Review Comment:
Could you add the LOG.isDebugEnabled check here, L254， L270，L377？
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1454869654
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,185 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
+ // check block data table
+ BlockData blockData = getBlockDataTable().get(blockKey);
+
+ if (blockData == null) {
+ if (lastChunk == null) {
+ throw new StorageContainerException(
+ NO_SUCH_BLOCK_ERR_MSG + " BlockID : " + blockID, NO_SUCH_BLOCK);
+ } else {
+ LOG.debug("blockData=(null), lastChunk={}", lastChunk.getChunks());
+ return lastChunk;
+ }
+ } else {
+ if (lastChunk != null) {
+ reconcilePartialChunks(lastChunk, blockData);
+ } else {
+ LOG.debug("blockData={}, lastChunk=(null)", blockData.getChunks());
+ }
+ }
+
+ return blockData;
+ }
+
+ private static void reconcilePartialChunks(
+ BlockData lastChunk, BlockData blockData) {
+ LOG.debug("blockData={}, lastChunk={}",
+ blockData.getChunks(), lastChunk.getChunks());
+ Preconditions.checkState(lastChunk.getChunks().size() == 1);
+ ContainerProtos.ChunkInfo lastChunkInBlockData =
+ blockData.getChunks().get(blockData.getChunks().size() - 1);
+ Preconditions.checkState(
+ lastChunkInBlockData.getOffset() + lastChunkInBlockData.getLen()
+ == lastChunk.getChunks().get(0).getOffset(),
+ "chunk offset does not match");
+
+ // append last partial chunk to the block data
+ List<ContainerProtos.ChunkInfo> chunkInfos =
+ new ArrayList<>(blockData.getChunks());
+ chunkInfos.add(lastChunk.getChunks().get(0));
+ blockData.setChunks(chunkInfos);
+
+ blockData.setBlockCommitSequenceId(
+ lastChunk.getBlockCommitSequenceId());
+ }
+
+ private static boolean isPartialChunkList(BlockData data) {
+ return data.getMetadata().containsKey(INCREMENTAL_CHUNK_LIST);
+ }
+
+ private static boolean isFullChunk(ContainerProtos.ChunkInfo chunkInfo) {
+ for (ContainerProtos.KeyValue kv: chunkInfo.getMetadataList()) {
+ if (kv.getKey().equals(FULL_CHUNK)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ // if eob or if the last chunk is full,
+ private static boolean shouldAppendLastChunk(boolean endOfBlock,
+ BlockData data) {
+ if (endOfBlock || data.getChunks().isEmpty()) {
+ return true;
+ }
+ return isFullChunk(data.getChunks().get(data.getChunks().size() - 1));
+ }
+
+ public void putBlockByID(BatchOperation batch, boolean incremental,
+ long localID, BlockData data, KeyValueContainerData containerData,
+ boolean endOfBlock) throws IOException {
+ if (!incremental && !isPartialChunkList(data)) {
+ // Case (1) old client: override chunk list.
+ getBlockDataTable().putWithBatch(
+ batch, containerData.getBlockKey(localID), data);
+ } else if (shouldAppendLastChunk(endOfBlock, data)) {
+ moveLastChunkToBlockData(batch, localID, data, containerData);
+ } else {
+ // incremental chunk list,
+ // not end of block, has partial chunks
+ putBlockWithPartialChunks(batch, localID, data, containerData);
+ }
+ }
+
+ private void moveLastChunkToBlockData(BatchOperation batch, long localID,
+ BlockData data, KeyValueContainerData containerData) throws IOException {
+ // if eob or if the last chunk is full,
+ // the 'data' is full so append it to the block table's chunk info
+ // and then remove from lastChunkInfo
+ BlockData blockData = getBlockDataTable().get(
+ containerData.getBlockKey(localID));
+ if (blockData == null) {
+ // Case 2.1 if the block did not have full chunks before,
+ // the block's chunk is what received from client this time.
+ blockData = data;
+ } else {
+ // case 2.2 the block already has some full chunks
+ List<ContainerProtos.ChunkInfo> chunkInfoList = blockData.getChunks();
+ blockData.setChunks(new ArrayList<>(chunkInfoList));
+ for (ContainerProtos.ChunkInfo chunk : data.getChunks()) {
+ blockData.addChunk(chunk);
+ }
+ blockData.setBlockCommitSequenceId(data.getBlockCommitSequenceId());
+
+ /*int next = 0;
+ for (ContainerProtos.ChunkInfo info : chunkInfoList) {
+ assert info.getOffset() == next;
+ next += info.getLen();
+ }*/
+ }
+ // delete the entry from last chunk info table
+ getLastChunkInfoTable().deleteWithBatch(
+ batch, containerData.getBlockKey(localID));
+ // update block data table
+ getBlockDataTable().putWithBatch(batch,
+ containerData.getBlockKey(localID), blockData);
+ }
+
+ private void putBlockWithPartialChunks(BatchOperation batch, long localID,
+ BlockData data, KeyValueContainerData containerData) throws IOException {
+ if (data.getChunks().size() == 1) {
+ // Case (3.1) replace/update the last chunk info table
+ getLastChunkInfoTable().putWithBatch(
+ batch, containerData.getBlockKey(localID), data);
+ } else {
+ int lastChunkIndex = data.getChunks().size() - 1;
+ // received more than one chunk this time
+ List<ContainerProtos.ChunkInfo> lastChunkInfo =
+ Collections.singletonList(
+ data.getChunks().get(lastChunkIndex));
+ BlockData blockData = getBlockDataTable().get(
+ containerData.getBlockKey(localID));
+ if (blockData == null) {
+ // Case 3.2: if the block does not exist in the block data table
+ List<ContainerProtos.ChunkInfo> chunkInfos =
+ new ArrayList<>(data.getChunks());
+ chunkInfos.remove(lastChunkIndex);
+ data.setChunks(chunkInfos);
+ blockData = data;
+ LOG.debug("block {} does not have full chunks yet. Adding the " +
+ "chunks to it {}", localID, blockData);
+ } else {
+ // Case 3.3: if the block exists in the block data table,
+ // append chunks till except the last one (supposedly partial)
+ List<ContainerProtos.ChunkInfo> chunkInfos =
+ new ArrayList<>(blockData.getChunks());
+
+ LOG.debug("blockData.getChunks()={}", chunkInfos);
+ LOG.debug("data.getChunks()={}", data.getChunks());
+
+ for (int i = 0; i < lastChunkIndex; i++) {
+ chunkInfos.add(data.getChunks().get(i));
+ }
+ blockData.setChunks(chunkInfos);
+ blockData.setBlockCommitSequenceId(data.getBlockCommitSequenceId());
+
+ /*int next = 0;
+ for (ContainerProtos.ChunkInfo info : chunkInfos) {
+ if (info.getOffset() != next) {
+
+ LOG.error("blockData.getChunks()={}", chunkInfos);
+ LOG.error("data.getChunks()={}", data.getChunks());
+ }
+ assert info.getOffset() == next;
+ next += info.getLen();
+ }*/
+ }
+ getBlockDataTable().putWithBatch(batch,
+ containerData.getBlockKey(localID), blockData);
+ // update the last partial chunk
+ data.setChunks(lastChunkInfo);
+ getLastChunkInfoTable().putWithBatch(
+ batch, containerData.getBlockKey(localID), data);
Review Comment:
i think there could still be cases where the client's buffer has more than one chunk's full of data that gets flushed out at a time.
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1461967899
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,168 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
+ // check block data table
+ BlockData blockData = getBlockDataTable().get(blockKey);
+
+ if (blockData == null) {
+ if (lastChunk == null) {
+ throw new StorageContainerException(
+ NO_SUCH_BLOCK_ERR_MSG + " BlockID : " + blockID, NO_SUCH_BLOCK);
+ } else {
+ LOG.debug("blockData=(null), lastChunk={}", lastChunk.getChunks());
+ return lastChunk;
+ }
+ } else {
+ if (lastChunk != null) {
+ reconcilePartialChunks(lastChunk, blockData);
+ } else {
+ LOG.debug("blockData={}, lastChunk=(null)", blockData.getChunks());
+ }
+ }
+
+ return blockData;
+ }
+
+ private void reconcilePartialChunks(
+ BlockData lastChunk, BlockData blockData) {
+ LOG.debug("blockData={}, lastChunk={}",
+ blockData.getChunks(), lastChunk.getChunks());
+ Preconditions.checkState(lastChunk.getChunks().size() == 1);
+ ContainerProtos.ChunkInfo lastChunkInBlockData =
+ blockData.getChunks().get(blockData.getChunks().size() - 1);
+ Preconditions.checkState(
+ lastChunkInBlockData.getOffset() + lastChunkInBlockData.getLen()
+ == lastChunk.getChunks().get(0).getOffset(),
+ "chunk offset does not match");
+
+ // append last partial chunk to the block data
+ List<ContainerProtos.ChunkInfo> chunkInfos =
+ new ArrayList<>(blockData.getChunks());
+ chunkInfos.add(lastChunk.getChunks().get(0));
+ blockData.setChunks(chunkInfos);
+
+ blockData.setBlockCommitSequenceId(
+ lastChunk.getBlockCommitSequenceId());
+ }
+
+ private static boolean isPartialChunkList(BlockData data) {
+ return data.getMetadata().containsKey(INCREMENTAL_CHUNK_LIST);
+ }
+
+ private static boolean isFullChunk(ContainerProtos.ChunkInfo chunkInfo) {
+ for (ContainerProtos.KeyValue kv: chunkInfo.getMetadataList()) {
+ if (kv.getKey().equals(FULL_CHUNK)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ // if eob or if the last chunk is full,
+ private static boolean shouldAppendLastChunk(boolean endOfBlock,
+ BlockData data) {
+ if (endOfBlock || data.getChunks().isEmpty()) {
Review Comment:
I see.
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #5662:
URL: https://github.com/apache/ozone/pull/5662#issuecomment-1906654351
Thanks @ChenSammi for the review!
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang merged PR #5662:
URL: https://github.com/apache/ozone/pull/5662
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1461983458
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestBlockManagerImpl.java:
##########
@@ -222,4 +227,175 @@ public void testListBlock(ContainerTestVersionInfo versionInfo)
assertNotNull(listBlockData);
assertEquals(10, listBlockData.size());
}
+
+ private BlockData createBlockData(long containerID, long blockNo,
+ int chunkID, long offset, long len, long bcsID)
+ throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ ChunkInfo info1 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), chunkID), offset, len);
+ chunkList1.add(info1.getProtoBufMessage());
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ private BlockData createBlockDataWithOneFullChunk(long containerID,
+ long blockNo, int chunkID, long offset, long len, long bcsID)
+ throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ ChunkInfo info1 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), 1), 0, 4 * 1024 * 1024);
+ info1.addMetadata(FULL_CHUNK, "");
+
+ ChunkInfo info2 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), chunkID), offset, len);
+ chunkList1.add(info1.getProtoBufMessage());
+ chunkList1.add(info2.getProtoBufMessage());
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ private BlockData createBlockDataWithThreeFullChunks(long containerID,
+ long blockNo, long bcsID) throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ long chunkLimit = 4 * 1024 * 1024;
+ for (int i = 1; i < 4; i++) {
+ ChunkInfo info1 = new ChunkInfo(
+ String.format("%d_chunk_%d", blockID1.getLocalID(), i),
+ chunkLimit * i, chunkLimit);
+ info1.addMetadata(FULL_CHUNK, "");
+ chunkList1.add(info1.getProtoBufMessage());
+ }
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ @ContainerTestVersionInfo.ContainerTest
Review Comment:
When using @ContainerTestVersionInfo.ContainerTest, the test shall have a "ContainerTestVersionInfo versionInfo" parameter, otherwise, the parameterized test is not actually enabled.
` @ContainerTestVersionInfo.ContainerTest
public void testPendingDeleteBlockReset(ContainerTestVersionInfo versionInfo)
`
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1461983458
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestBlockManagerImpl.java:
##########
@@ -222,4 +227,175 @@ public void testListBlock(ContainerTestVersionInfo versionInfo)
assertNotNull(listBlockData);
assertEquals(10, listBlockData.size());
}
+
+ private BlockData createBlockData(long containerID, long blockNo,
+ int chunkID, long offset, long len, long bcsID)
+ throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ ChunkInfo info1 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), chunkID), offset, len);
+ chunkList1.add(info1.getProtoBufMessage());
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ private BlockData createBlockDataWithOneFullChunk(long containerID,
+ long blockNo, int chunkID, long offset, long len, long bcsID)
+ throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ ChunkInfo info1 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), 1), 0, 4 * 1024 * 1024);
+ info1.addMetadata(FULL_CHUNK, "");
+
+ ChunkInfo info2 = new ChunkInfo(String.format("%d_chunk_%d", blockID1
+ .getLocalID(), chunkID), offset, len);
+ chunkList1.add(info1.getProtoBufMessage());
+ chunkList1.add(info2.getProtoBufMessage());
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ private BlockData createBlockDataWithThreeFullChunks(long containerID,
+ long blockNo, long bcsID) throws IOException {
+ blockID1 = new BlockID(containerID, blockNo);
+ blockData = new BlockData(blockID1);
+ List<ContainerProtos.ChunkInfo> chunkList1 = new ArrayList<>();
+ long chunkLimit = 4 * 1024 * 1024;
+ for (int i = 1; i < 4; i++) {
+ ChunkInfo info1 = new ChunkInfo(
+ String.format("%d_chunk_%d", blockID1.getLocalID(), i),
+ chunkLimit * i, chunkLimit);
+ info1.addMetadata(FULL_CHUNK, "");
+ chunkList1.add(info1.getProtoBufMessage());
+ }
+ blockData.setChunks(chunkList1);
+ blockData.setBlockCommitSequenceId(bcsID);
+ blockData.addMetadata(INCREMENTAL_CHUNK_LIST, "");
+
+ return blockData;
+ }
+
+ @ContainerTestVersionInfo.ContainerTest
Review Comment:
When using @ContainerTestVersionInfo.ContainerTest, the test will have a "ContainerTestVersionInfo versionInfo" parameter, otherwise, the parameterized test is not actually enabled.
` @ContainerTestVersionInfo.ContainerTest
public void testPendingDeleteBlockReset(ContainerTestVersionInfo versionInfo)
`
--
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-9751. [hsync] Make Putblock performance acceptable - DataNode side [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #5662:
URL: https://github.com/apache/ozone/pull/5662#discussion_r1452899232
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java:
##########
@@ -224,4 +235,168 @@ public void compactionIfNeeded() throws Exception {
}
}
}
+
+ @Override
+ public BlockData getBlockByID(BlockID blockID,
+ KeyValueContainerData containerData) throws IOException {
+ String blockKey = containerData.getBlockKey(blockID.getLocalID());
+
+ // check last chunk table
+ BlockData lastChunk = getLastChunkInfoTable().get(blockKey);
+ // check block data table
+ BlockData blockData = getBlockDataTable().get(blockKey);
+
+ if (blockData == null) {
+ if (lastChunk == null) {
+ throw new StorageContainerException(
+ NO_SUCH_BLOCK_ERR_MSG + " BlockID : " + blockID, NO_SUCH_BLOCK);
+ } else {
+ LOG.debug("blockData=(null), lastChunk={}", lastChunk.getChunks());
+ return lastChunk;
+ }
+ } else {
+ if (lastChunk != null) {
+ reconcilePartialChunks(lastChunk, blockData);
+ } else {
+ LOG.debug("blockData={}, lastChunk=(null)", blockData.getChunks());
+ }
+ }
+
+ return blockData;
+ }
+
+ private void reconcilePartialChunks(
+ BlockData lastChunk, BlockData blockData) {
+ LOG.debug("blockData={}, lastChunk={}",
+ blockData.getChunks(), lastChunk.getChunks());
+ Preconditions.checkState(lastChunk.getChunks().size() == 1);
+ ContainerProtos.ChunkInfo lastChunkInBlockData =
+ blockData.getChunks().get(blockData.getChunks().size() - 1);
+ Preconditions.checkState(
+ lastChunkInBlockData.getOffset() + lastChunkInBlockData.getLen()
Review Comment:
Shall we remove this lastChunkInBlockData.getLen() in the offset comparison?
--
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