Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -354,10 +354,11 @@ private void processCmd(DeleteCmdInfo cmd) {
DeletedContainerBlocksSummary summary =
DeletedContainerBlocksSummary.getFrom(containerBlocks);
LOG.info("Summary of deleting container blocks, numOfTransactions={}, "
+ "numOfContainers={}, numOfBlocks={}",
+ "numOfContainers={}, numOfBlocks={}, commandId={}.",
summary.getNumOfTxs(),
summary.getNumOfContainers(),
summary.getNumOfBlocks());
summary.getNumOfBlocks(),
cmd.getCmd().getId());
if (LOG.isDebugEnabled()) {
LOG.debug("Start to delete container blocks, TXIDs={}",
summary.getTxIDSummary());
Expand All @@ -384,7 +385,8 @@ private void processCmd(DeleteCmdInfo cmd) {
LOG.debug("Sending following block deletion ACK to SCM");
for (DeleteBlockTransactionResult result : blockDeletionACK
.getResultsList()) {
LOG.debug("{} : {}", result.getTxID(), result.getSuccess());
LOG.debug("TxId = {} : ContainerId = {} : {}",
result.getTxID(), result.getContainerID(), result.getSuccess());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ void addTransactionToDN(UUID dnID, DeletedBlocksTransaction tx) {
blocksDeleted += tx.getLocalIDCount();
if (SCMBlockDeletingService.LOG.isDebugEnabled()) {
SCMBlockDeletingService.LOG
.debug("Transaction added: {} <- TX({})", dnID, tx.getTxID());
.debug("Transaction added: {} <- TX({}), DN {} <- blocksDeleted Add {}.",
dnID, tx.getTxID(), dnID, tx.getLocalIDCount());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.CommandStatus;
import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerBlocksDeletionACKProto;
import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerBlocksDeletionACKProto.DeleteBlockTransactionResult;
import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.DeletedBlocksTransaction;
import org.apache.hadoop.hdds.scm.command.CommandStatusReportHandler.DeleteBlockStatus;
import org.apache.hadoop.hdds.scm.container.ContainerInfo;
Expand Down Expand Up @@ -200,20 +199,6 @@ private DeletedBlocksTransaction constructNewTransaction(
.build();
}

private boolean isTransactionFailed(DeleteBlockTransactionResult result) {
if (LOG.isDebugEnabled()) {
LOG.debug(
"Got block deletion ACK from datanode, TXIDs={}, " + "success={}",
result.getTxID(), result.getSuccess());
}
if (!result.getSuccess()) {
LOG.warn("Got failed ACK for TXID={}, prepare to resend the "
+ "TX in next interval", result.getTxID());
return true;
}
return false;
}

@Override
public int getNumOfValidTransactions() throws IOException {
lock.lock();
Expand Down Expand Up @@ -300,6 +285,26 @@ private void getTransaction(DeletedBlocksTransaction tx,
.setCount(transactionStatusManager.getOrDefaultRetryCount(
tx.getTxID(), 0))
.build();

// We have made an improvement here, and we expect that all replicas
// of the Container being sent will be included in the dnList.
// This change benefits ACK confirmation and improves deletion speed.
// The principle behind it is that
// DN can receive the command to delete a certain Container at the same time and provide
// feedback to SCM at roughly the same time.
// This avoids the issue of deletion blocking,
// where some replicas of a Container are deleted while others do not receive the delete command.
long containerId = tx.getContainerID();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have already provided relevant explanations regarding the core logic, which clarifies the purpose of this PR.
Line 289-307

for (ContainerReplica replica : replicas) {
DatanodeDetails datanodeDetails = replica.getDatanodeDetails();
if (!dnList.contains(datanodeDetails)) {
Comment thread
sumitagrawl marked this conversation as resolved.
DatanodeDetails dnDetail = replica.getDatanodeDetails();
LOG.debug("Skip Container = {}, because DN= {} is not in dnList.",
Comment thread
xichen01 marked this conversation as resolved.
Outdated
containerId, dnDetail.getUuid());
return;
}
}

for (ContainerReplica replica : replicas) {
DatanodeDetails details = replica.getDatanodeDetails();
if (!dnList.contains(details)) {
Comment thread
xichen01 marked this conversation as resolved.
Outdated
Expand Down Expand Up @@ -358,8 +363,8 @@ public DatanodeDeletedBlockTransactions getTransactions(
// HDDS-7126. When container is under replicated, it is possible
// that container is deleted, but transactions are not deleted.
if (containerManager.getContainer(id).isDeleted()) {
LOG.warn("Container: " + id + " was deleted for the " +
"transaction: " + txn);
LOG.warn("Container: {} was deleted for the " +
"transaction: {}.", id, txn);
txIDs.add(txn.getTxID());
} else if (txn.getCount() > -1 && txn.getCount() <= maxRetry
&& !containerManager.getContainer(id).isOpen()) {
Expand All @@ -373,8 +378,7 @@ public DatanodeDeletedBlockTransactions getTransactions(
txn, transactions, dnList, replicas, commandStatus);
}
} catch (ContainerNotFoundException ex) {
LOG.warn("Container: " + id + " was not found for the transaction: "
+ txn);
LOG.warn("Container: {} was not found for the transaction: {}.", id, txn);
txIDs.add(txn.getTxID());
}
}
Expand Down