You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/12/16 13:36:08 UTC
[GitHub] [ozone] lokeshj1703 commented on a change in pull request #1702: HDDS-4369. Datanode should store the delete transaction as is in rocksDB
lokeshj1703 commented on a change in pull request #1702:
URL: https://github.com/apache/ozone/pull/1702#discussion_r544178673
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/DeleteBlocksCommandHandler.java
##########
@@ -129,7 +134,16 @@ public void handle(SCMCommand command, OzoneContainer container,
cont.getContainerData();
cont.writeLock();
try {
- deleteKeyValueContainerBlocks(containerData, entry);
+ if (containerData.getSchemaVersion().equals(SCHEMA_V1)) {
+ deleteKeyValueContainerBlocks(containerData, entry);
+ } else if (containerData.getSchemaVersion().equals(SCHEMA_V2)) {
+ putTxns(containerData, entry, newDeletionBlocks,
Review comment:
https://github.com/apache/ozone/pull/1702/files#diff-489818524a6cd8c72f568ade8c114d13436222ce4adb3a9c23a787f28c0b2fb0R118-R119
I think we can club multiple transactions for the same container into a single batch operation.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/DeleteBlocksCommandHandler.java
##########
@@ -283,7 +319,58 @@ private void deleteKeyValueContainerBlocks(
// update pending deletion blocks count and delete transaction ID in
// in-memory container status
containerData.updateDeleteTransactionId(delTX.getTxID());
+ containerData.incrPendingDeletionBlocks(newDeletionBlocks);
+ }
+ }
+ }
+ }
+
+ private void updateMetaData(
Review comment:
This should be a common function for both the schemas and should be called for schema 1 as well.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java
##########
@@ -20,26 +20,24 @@
import java.io.File;
import java.io.IOException;
+import java.util.UUID;
import java.util.LinkedList;
+import java.util.Objects;
+import java.util.ArrayList;
import java.util.List;
import java.util.Map;
-import java.util.Objects;
-import java.util.UUID;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import org.apache.hadoop.hdds.conf.ConfigurationSource;
import org.apache.hadoop.hdds.scm.ScmConfigKeys;
import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
-import org.apache.hadoop.hdds.utils.BackgroundService;
-import org.apache.hadoop.hdds.utils.BackgroundTask;
-import org.apache.hadoop.hdds.utils.BackgroundTaskQueue;
-import org.apache.hadoop.hdds.utils.BackgroundTaskResult;
-import org.apache.hadoop.hdds.utils.db.BatchOperation;
-import org.apache.hadoop.hdds.utils.MetadataKeyFilters;
+import org.apache.hadoop.hdds.utils.*;
Review comment:
Star import.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaTwoDBDefinition.java
##########
@@ -78,4 +97,9 @@ protected DatanodeSchemaTwoDBDefinition(String dbPath) {
getDeletedBlocksColumnFamily() {
return DELETED_BLOCKS;
}
+
+ public DBColumnFamilyDefinition<Long, DeletedBlocksTransaction>
+ getTxnBlocksColumnFamily() {
Review comment:
Let's rename it to getDeleteTransactionsColumnFamily.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/DeleteBlocksCommandHandler.java
##########
@@ -283,7 +319,58 @@ private void deleteKeyValueContainerBlocks(
// update pending deletion blocks count and delete transaction ID in
// in-memory container status
containerData.updateDeleteTransactionId(delTX.getTxID());
+ containerData.incrPendingDeletionBlocks(newDeletionBlocks);
+ }
+ }
+ }
+ }
+
+ private void updateMetaData(
+ KeyValueContainerData containerData, DeletedBlocksTransaction delTX,
+ int newDeletionBlocks) throws IOException {
+ long containerId = delTX.getContainerID();
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Processing Container : {}, DB path : {}", containerId,
+ containerData.getMetadataPath());
+ }
+
+ if (delTX.getTxID() < containerData.getDeleteTransactionId()) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug(String.format("Ignoring delete blocks for containerId: %d."
+ + " Outdated delete transactionId %d < %d", containerId,
+ delTX.getTxID(), containerData.getDeleteTransactionId()));
+ }
+ return;
+ }
Review comment:
This is a pre-processing step. I think this is also a common functionality for the schemas and should be called before any db operations. If containerTxnId is greater than the txnId to be processed we would ignore the transaction for both the schemas.
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestStorageContainerManagerHelper.java
##########
@@ -137,6 +143,29 @@ public TestStorageContainerManagerHelper(MiniOzoneCluster cluster,
return allBlocks;
}
+ public boolean verifyBlocksWithTxnTable(Map<Long, List<Long>> containerBlocks)
+ throws IOException {
+ Set<Long> containerIDs = containerBlocks.keySet();
+ for (Long entry : containerIDs) {
+ ReferenceCountedDB meta = getContainerMetadata(entry);
+ DatanodeStore ds = meta.getStore();
+ DatanodeStoreSchemaTwoImpl dnStoreTwoImpl =
+ (DatanodeStoreSchemaTwoImpl) ds;
+ List<? extends Table.KeyValue<Long, DeletedBlocksTransaction>> kvs =
+ dnStoreTwoImpl.getTxnTable()
+ .getRangeKVs(null, Integer.MAX_VALUE, null);
+ List<Long> conID = new ArrayList<>();
Review comment:
Let's rename it to blocksInTxnTable?
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java
##########
@@ -312,9 +338,102 @@ public BackgroundTaskResult call() throws Exception {
}
crr.addAll(succeedBlocks);
return crr;
- } finally {
- container.writeUnlock();
+ } catch (IOException exception) {
+ LOG.warn(
+ "Deletion operation was not successful for container: " + container
+ .getContainerData().getContainerID());
+ }
+ return crr;
+ }
+
+ public ContainerBackgroundTaskResult deleteViaSchema2(
+ ReferenceCountedDB meta, Container container, File dataDir,
+ long startTime) {
+ ContainerBackgroundTaskResult crr = new ContainerBackgroundTaskResult();
+ try {
+ Table<String, BlockData> blockDataTable =
+ meta.getStore().getBlockDataTable();
+ DatanodeStore ds = meta.getStore();
+ DatanodeStoreSchemaTwoImpl dnStoreTwoImpl =
+ (DatanodeStoreSchemaTwoImpl) ds;
+ Table<Long, DeletedBlocksTransaction>
+ delTxTable = dnStoreTwoImpl.getTxnTable();
+ List<DeletedBlocksTransaction> delBlocks = new ArrayList<>();
+ int totalBlocks = 0;
+ try (TableIterator<Long,
+ ? extends Table.KeyValue<Long, DeletedBlocksTransaction>> iter =
+ dnStoreTwoImpl.getTxnTable().iterator()) {
+ while (iter.hasNext() && (totalBlocks < blockLimitPerTask)) {
+ DeletedBlocksTransaction delTx = iter.next().getValue();
+ totalBlocks += delTx.getLocalIDList().size();
+ delBlocks.add(delTx);
+ }
+ }
+
+ if (delBlocks.isEmpty()) {
+ LOG.debug("No transaction found in container : {}",
+ containerData.getContainerID());
+ }
+
+ int deleteBlockCount = 0;
+ LOG.debug("Container : {}, To-Delete blocks : {}",
+ containerData.getContainerID(), delBlocks.size());
+ if (!dataDir.exists() || !dataDir.isDirectory()) {
+ LOG.error("Invalid container data dir {} : "
+ + "does not exist or not a directory", dataDir.getAbsolutePath());
+ return crr;
+ }
+
+ Handler handler = Objects.requireNonNull(ozoneContainer.getDispatcher()
+ .getHandler(container.getContainerType()));
+
+ for (DeletedBlocksTransaction entry: delBlocks) {
Review comment:
Let's create a private function like deleteTransaction here. We also need to handle cases where only some blocks are deleted. Lets say the txn had 100 blocks and we successfully deleted 10 blocks then we remove only the 10 blocks from the db. Txn is removed only if all blocks are successfully deleted.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java
##########
@@ -312,9 +338,102 @@ public BackgroundTaskResult call() throws Exception {
}
crr.addAll(succeedBlocks);
return crr;
- } finally {
- container.writeUnlock();
+ } catch (IOException exception) {
+ LOG.warn(
+ "Deletion operation was not successful for container: " + container
+ .getContainerData().getContainerID());
+ }
+ return crr;
+ }
+
+ public ContainerBackgroundTaskResult deleteViaSchema2(
+ ReferenceCountedDB meta, Container container, File dataDir,
+ long startTime) {
+ ContainerBackgroundTaskResult crr = new ContainerBackgroundTaskResult();
+ try {
+ Table<String, BlockData> blockDataTable =
+ meta.getStore().getBlockDataTable();
+ DatanodeStore ds = meta.getStore();
+ DatanodeStoreSchemaTwoImpl dnStoreTwoImpl =
+ (DatanodeStoreSchemaTwoImpl) ds;
+ Table<Long, DeletedBlocksTransaction>
+ delTxTable = dnStoreTwoImpl.getTxnTable();
+ List<DeletedBlocksTransaction> delBlocks = new ArrayList<>();
+ int totalBlocks = 0;
+ try (TableIterator<Long,
+ ? extends Table.KeyValue<Long, DeletedBlocksTransaction>> iter =
+ dnStoreTwoImpl.getTxnTable().iterator()) {
+ while (iter.hasNext() && (totalBlocks < blockLimitPerTask)) {
+ DeletedBlocksTransaction delTx = iter.next().getValue();
+ totalBlocks += delTx.getLocalIDList().size();
+ delBlocks.add(delTx);
+ }
+ }
+
+ if (delBlocks.isEmpty()) {
+ LOG.debug("No transaction found in container : {}",
+ containerData.getContainerID());
+ }
+
+ int deleteBlockCount = 0;
+ LOG.debug("Container : {}, To-Delete blocks : {}",
+ containerData.getContainerID(), delBlocks.size());
+ if (!dataDir.exists() || !dataDir.isDirectory()) {
+ LOG.error("Invalid container data dir {} : "
+ + "does not exist or not a directory", dataDir.getAbsolutePath());
+ return crr;
+ }
+
+ Handler handler = Objects.requireNonNull(ozoneContainer.getDispatcher()
+ .getHandler(container.getContainerType()));
+
+ for (DeletedBlocksTransaction entry: delBlocks) {
+ for (Long blkLong : entry.getLocalIDList()) {
+ String blk = blkLong.toString();
+ BlockData blkInfo = blockDataTable.get(blk);
+ LOG.debug("Deleting block {}", blk);
+ try {
+ handler.deleteBlock(container, blkInfo);
+ deleteBlockCount++;
+ } catch (InvalidProtocolBufferException e) {
+ LOG.error("Failed to parse block info for block {}", blk, e);
+ } catch (IOException e) {
+ LOG.error("Failed to delete files for block {}", blk, e);
+ }
+ }
+ }
+
+ // Once blocks are deleted... remove the blockID from blockDataTable
+ // and also remove the transactions from txnTable.
+ try(BatchOperation batch = meta.getStore().getBatchHandler()
+ .initBatchOperation()) {
+ for (DeletedBlocksTransaction delTx : delBlocks) {
+ delTxTable.deleteWithBatch(batch, delTx.getTxID());
+ for (Long blk : delTx.getLocalIDList()) {
+ String bID = blk.toString();
+ meta.getStore().getBlockDataTable().deleteWithBatch(batch, bID);
+ }
+ meta.getStore().getBatchHandler().commitBatchOperation(batch);
+ }
+ containerData.updateAndCommitDBCounters(meta, batch,
+ deleteBlockCount);
+ // update count of pending deletion blocks and block count in
+ // in-memory container status.
+ containerData.decrPendingDeletionBlocks(deleteBlockCount);
+ containerData.decrKeyCount(deleteBlockCount);
+ }
+
+ if (deleteBlockCount > 0) {
+ LOG.info("Container: {}, deleted blocks: {}, task elapsed time: {}ms",
+ containerData.getContainerID(), deleteBlockCount,
+ Time.monotonicNow() - startTime);
+ }
+ } catch (IOException exception) {
+ LOG.warn(
+ "Deletion operation was not successful for container: " + container
+ .getContainerData().getContainerID());
Review comment:
Also print exception.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java
##########
@@ -312,9 +338,102 @@ public BackgroundTaskResult call() throws Exception {
}
crr.addAll(succeedBlocks);
return crr;
- } finally {
- container.writeUnlock();
+ } catch (IOException exception) {
+ LOG.warn(
+ "Deletion operation was not successful for container: " + container
+ .getContainerData().getContainerID());
+ }
+ return crr;
+ }
+
+ public ContainerBackgroundTaskResult deleteViaSchema2(
+ ReferenceCountedDB meta, Container container, File dataDir,
+ long startTime) {
+ ContainerBackgroundTaskResult crr = new ContainerBackgroundTaskResult();
+ try {
+ Table<String, BlockData> blockDataTable =
+ meta.getStore().getBlockDataTable();
+ DatanodeStore ds = meta.getStore();
+ DatanodeStoreSchemaTwoImpl dnStoreTwoImpl =
+ (DatanodeStoreSchemaTwoImpl) ds;
+ Table<Long, DeletedBlocksTransaction>
+ delTxTable = dnStoreTwoImpl.getTxnTable();
+ List<DeletedBlocksTransaction> delBlocks = new ArrayList<>();
+ int totalBlocks = 0;
+ try (TableIterator<Long,
+ ? extends Table.KeyValue<Long, DeletedBlocksTransaction>> iter =
+ dnStoreTwoImpl.getTxnTable().iterator()) {
+ while (iter.hasNext() && (totalBlocks < blockLimitPerTask)) {
+ DeletedBlocksTransaction delTx = iter.next().getValue();
+ totalBlocks += delTx.getLocalIDList().size();
+ delBlocks.add(delTx);
+ }
+ }
+
+ if (delBlocks.isEmpty()) {
+ LOG.debug("No transaction found in container : {}",
+ containerData.getContainerID());
+ }
+
+ int deleteBlockCount = 0;
+ LOG.debug("Container : {}, To-Delete blocks : {}",
+ containerData.getContainerID(), delBlocks.size());
+ if (!dataDir.exists() || !dataDir.isDirectory()) {
+ LOG.error("Invalid container data dir {} : "
+ + "does not exist or not a directory", dataDir.getAbsolutePath());
+ return crr;
+ }
+
+ Handler handler = Objects.requireNonNull(ozoneContainer.getDispatcher()
+ .getHandler(container.getContainerType()));
+
+ for (DeletedBlocksTransaction entry: delBlocks) {
Review comment:
We can populate the batch operation in this function itself.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java
##########
@@ -312,9 +338,102 @@ public BackgroundTaskResult call() throws Exception {
}
crr.addAll(succeedBlocks);
return crr;
- } finally {
- container.writeUnlock();
+ } catch (IOException exception) {
+ LOG.warn(
+ "Deletion operation was not successful for container: " + container
+ .getContainerData().getContainerID());
+ }
+ return crr;
+ }
+
+ public ContainerBackgroundTaskResult deleteViaSchema2(
+ ReferenceCountedDB meta, Container container, File dataDir,
+ long startTime) {
+ ContainerBackgroundTaskResult crr = new ContainerBackgroundTaskResult();
+ try {
+ Table<String, BlockData> blockDataTable =
+ meta.getStore().getBlockDataTable();
+ DatanodeStore ds = meta.getStore();
+ DatanodeStoreSchemaTwoImpl dnStoreTwoImpl =
+ (DatanodeStoreSchemaTwoImpl) ds;
+ Table<Long, DeletedBlocksTransaction>
+ delTxTable = dnStoreTwoImpl.getTxnTable();
+ List<DeletedBlocksTransaction> delBlocks = new ArrayList<>();
+ int totalBlocks = 0;
+ try (TableIterator<Long,
+ ? extends Table.KeyValue<Long, DeletedBlocksTransaction>> iter =
+ dnStoreTwoImpl.getTxnTable().iterator()) {
+ while (iter.hasNext() && (totalBlocks < blockLimitPerTask)) {
+ DeletedBlocksTransaction delTx = iter.next().getValue();
+ totalBlocks += delTx.getLocalIDList().size();
+ delBlocks.add(delTx);
+ }
+ }
+
+ if (delBlocks.isEmpty()) {
+ LOG.debug("No transaction found in container : {}",
Review comment:
Debug log should be enclosed in `if (LOG.isDebugEnabled())` check. Similarly for debug statement below.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java
##########
@@ -312,9 +338,102 @@ public BackgroundTaskResult call() throws Exception {
}
crr.addAll(succeedBlocks);
return crr;
- } finally {
- container.writeUnlock();
+ } catch (IOException exception) {
+ LOG.warn(
+ "Deletion operation was not successful for container: " + container
+ .getContainerData().getContainerID());
+ }
+ return crr;
+ }
+
+ public ContainerBackgroundTaskResult deleteViaSchema2(
+ ReferenceCountedDB meta, Container container, File dataDir,
+ long startTime) {
+ ContainerBackgroundTaskResult crr = new ContainerBackgroundTaskResult();
+ try {
+ Table<String, BlockData> blockDataTable =
+ meta.getStore().getBlockDataTable();
+ DatanodeStore ds = meta.getStore();
+ DatanodeStoreSchemaTwoImpl dnStoreTwoImpl =
+ (DatanodeStoreSchemaTwoImpl) ds;
+ Table<Long, DeletedBlocksTransaction>
+ delTxTable = dnStoreTwoImpl.getTxnTable();
+ List<DeletedBlocksTransaction> delBlocks = new ArrayList<>();
Review comment:
Let's rename it to deleteTxns.
##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java
##########
@@ -158,65 +190,167 @@ private void createToDeleteBlocks(ContainerSet containerSet,
}
byte[] arr = randomAlphanumeric(1048576).getBytes(UTF_8);
ChunkBuffer buffer = ChunkBuffer.wrap(ByteBuffer.wrap(arr));
+ int txnID = 0;
for (int x = 0; x < numOfContainers; x++) {
long containerID = ContainerTestHelper.getTestContainerID();
KeyValueContainerData data =
new KeyValueContainerData(containerID, layout,
ContainerTestHelper.CONTAINER_MAX_SIZE,
UUID.randomUUID().toString(), datanodeUuid);
data.closeContainer();
+ data.setSchemaVersion(schemaVersion);
KeyValueContainer container = new KeyValueContainer(data, conf);
container.create(volumeSet,
new RoundRobinVolumeChoosingPolicy(), scmId);
containerSet.addContainer(container);
data = (KeyValueContainerData) containerSet.getContainer(
containerID).getContainerData();
- long chunkLength = 100;
- try(ReferenceCountedDB metadata = BlockUtils.getDB(data, conf)) {
- for (int j = 0; j < numOfBlocksPerContainer; j++) {
+ if (data.getSchemaVersion().equals(SCHEMA_V1)) {
+ try(ReferenceCountedDB metadata = BlockUtils.getDB(data, conf)) {
+ for (int j = 0; j < numOfBlocksPerContainer; j++) {
+ BlockID blockID =
+ ContainerTestHelper.getTestBlockID(containerID);
+ String deleteStateName = OzoneConsts.DELETING_KEY_PREFIX +
+ blockID.getLocalID();
+ BlockData kd = new BlockData(blockID);
+ List<ContainerProtos.ChunkInfo> chunks = Lists.newArrayList();
+ putChunksInBlock(numOfChunksPerBlock, j, chunks, buffer,
+ chunkManager, container, blockID);
+ kd.setChunks(chunks);
+ metadata.getStore().getBlockDataTable().put(
+ deleteStateName, kd);
+ container.getContainerData().incrPendingDeletionBlocks(1);
+ }
+ updateMetaData(data, container, numOfBlocksPerContainer,
+ numOfChunksPerBlock);
+ }
+ } else if (data.getSchemaVersion().equals(SCHEMA_V2)) {
+ Map<Long, List<Long>> containerBlocks = new HashMap<>();
+ int blockCount = 0;
+ for (int i = 0; i < numOfBlocksPerContainer; i++) {
+ txnID = txnID + 1;
BlockID blockID =
ContainerTestHelper.getTestBlockID(containerID);
- String deleteStateName = OzoneConsts.DELETING_KEY_PREFIX +
- blockID.getLocalID();
BlockData kd = new BlockData(blockID);
List<ContainerProtos.ChunkInfo> chunks = Lists.newArrayList();
- for (int k = 0; k < numOfChunksPerBlock; k++) {
- final String chunkName = String.format("block.%d.chunk.%d", j, k);
- final long offset = k * chunkLength;
- ContainerProtos.ChunkInfo info =
- ContainerProtos.ChunkInfo.newBuilder()
- .setChunkName(chunkName)
- .setLen(chunkLength)
- .setOffset(offset)
- .setChecksumData(Checksum.getNoChecksumDataProto())
- .build();
- chunks.add(info);
- ChunkInfo chunkInfo = new ChunkInfo(chunkName, offset, chunkLength);
- ChunkBuffer chunkData = buffer.duplicate(0, (int) chunkLength);
- chunkManager.writeChunk(container, blockID, chunkInfo, chunkData,
- WRITE_STAGE);
- chunkManager.writeChunk(container, blockID, chunkInfo, chunkData,
- COMMIT_STAGE);
- }
+ putChunksInBlock(numOfChunksPerBlock, i, chunks, buffer, chunkManager,
+ container, blockID);
kd.setChunks(chunks);
- metadata.getStore().getBlockDataTable().put(
- deleteStateName, kd);
+ try(ReferenceCountedDB metadata = BlockUtils.getDB(data, conf)) {
+ String bID = blockID.getLocalID()+"";
+ metadata.getStore().getBlockDataTable().put(bID, kd);
+ }
container.getContainerData().incrPendingDeletionBlocks(1);
+
+ // In below if statements we are checking if a single container
+ // consists of more blocks than 'blockLimitPerTask' then we create
+ // (totalBlocksInContainer / blockLimitPerTask) transactions which
+ // consists of blocks equal to blockLimitPerTask and last transaction
+ // consists of blocks equal to
+ // (totalBlocksInContainer % blockLimitPerTask).
+ if (blockCount < blockLimitPerTask) {
+ if (containerBlocks.containsKey(containerID)) {
+ containerBlocks.get(containerID).add(blockID.getLocalID());
+ } else {
+ List<Long> item = new ArrayList<>();
+ item.add(blockID.getLocalID());
+ containerBlocks.put(containerID, item);
+ }
+ blockCount++;
+ }
+ boolean flag = false;
+ if (blockCount == blockLimitPerTask) {
+ createTxn(data, containerBlocks, txnID);
+ containerBlocks.clear();
+ blockCount = 0;
+ flag = true;
+ }
+ if (i == (numOfBlocksPerContainer - 1) && !flag) {
+ createTxn(data, containerBlocks, txnID);
+ }
Review comment:
```suggestion
containerBlocks.add(blockID.getLocalID());
blockCount++;
if (blockCount == blockLimitPerTask || i == (numOfBlocksPerContainer - 1)) {
createTxn(data, containerBlocks, txnID);
containerBlocks.clear();
blockCount = 0;
flag = true;
}
```
I think we can simplify the code here. We can use a List for storing containerBlocks.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaTwoDBDefinition.java
##########
@@ -58,10 +61,26 @@
ChunkInfoList.class,
new ChunkInfoListCodec());
+ public static final DBColumnFamilyDefinition<Long, DeletedBlocksTransaction>
+ TXN =
Review comment:
Let's rename it to DELETE_TRANSACTION or sth better.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaTwoImpl.java
##########
@@ -30,6 +33,8 @@
*/
public class DatanodeStoreSchemaTwoImpl extends AbstractDatanodeStore {
+ private TypedTable<Long, DeletedBlocksTransaction> txnTable;
Review comment:
Let's rename it to deleteTransactionTable and similarly the getter can be renamed as well.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java
##########
@@ -312,9 +338,102 @@ public BackgroundTaskResult call() throws Exception {
}
crr.addAll(succeedBlocks);
return crr;
- } finally {
- container.writeUnlock();
+ } catch (IOException exception) {
+ LOG.warn(
+ "Deletion operation was not successful for container: " + container
+ .getContainerData().getContainerID());
Review comment:
exception needs to be printed.
----------------------------------------------------------------
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.
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