From ce1780313fe777b7555da8a1692f31ba7906e5d4 Mon Sep 17 00:00:00 2001 From: xichen01 Date: Wed, 28 Jun 2023 00:03:49 +0800 Subject: [PATCH 01/23] HDDS-8882. Add state management of SCM's DeleteBlocksCommand to avoid sending duplicate delete transactions to the DN --- .../report/CommandStatusReportPublisher.java | 2 +- .../hdds/scm/block/DeletedBlockLog.java | 15 + .../hdds/scm/block/DeletedBlockLogImpl.java | 137 +++++-- .../scm/block/SCMBlockDeletingService.java | 11 +- .../SCMDeleteBlocksCommandStatusManager.java | 355 ++++++++++++++++++ .../command/CommandStatusReportHandler.java | 34 +- .../hadoop/hdds/scm/node/SCMNodeManager.java | 12 + 7 files changed, 527 insertions(+), 39 deletions(-) create mode 100644 hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/report/CommandStatusReportPublisher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/report/CommandStatusReportPublisher.java index 19fde7146861..ee08c9f79dbc 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/report/CommandStatusReportPublisher.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/report/CommandStatusReportPublisher.java @@ -78,9 +78,9 @@ protected CommandStatusReportsProto getReport() { // If status is still pending then don't remove it from map as // CommandHandler will change its status when it works on this command. if (!cmdStatus.getStatus().equals(Status.PENDING)) { - builder.addCmdStatus(cmdStatus.getProtoBufMessage()); map.remove(key); } + builder.addCmdStatus(cmdStatus.getProtoBufMessage()); }); return builder.getCmdStatusCount() > 0 ? builder.build() : null; } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java index 20a68e5010ca..6e45e8e4f85e 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java @@ -19,6 +19,7 @@ import java.util.Set; 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 .DeleteBlockTransactionResult; @@ -98,6 +99,20 @@ void incrementCount(List txIDs) void commitTransactions(List transactionResults, UUID dnID); + /** + * Commits DeleteBlocksCommand to update the DeleteBlocksCommand status + * by SCMDeleteBlocksCommandStatusManager. + * @param deleteBlockStatus the list of DeleteBlocksCommand + * @param dnID + */ + void commitCommandStatus(List deleteBlockStatus, UUID dnID); + + /** + * Get ScmDeleteBlocksCommandStatusManager. + * @return an Object of ScmDeleteBlocksCommandStatusManager + */ + SCMDeleteBlocksCommandStatusManager getScmCommandStatusManager(); + /** * Creates block deletion transactions for a set of containers, * add into the log and persist them atomically. An object key diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java index 52b8be9d645d..9135e929a150 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java @@ -18,12 +18,14 @@ package org.apache.hadoop.hdds.scm.block; import java.io.IOException; +import java.time.Duration; +import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.UUID; import java.util.Set; import java.util.Map; -import java.util.LinkedHashSet; import java.util.ArrayList; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeoutException; @@ -57,6 +59,7 @@ import static java.lang.Math.min; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_BLOCK_DELETION_MAX_RETRY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_BLOCK_DELETION_MAX_RETRY_DEFAULT; +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus; import static org.apache.hadoop.hdds.scm.ha.SequenceIdGenerator.DEL_TXN_ID; import org.slf4j.Logger; @@ -91,6 +94,9 @@ public class DeletedBlockLogImpl private final SCMContext scmContext; private final SequenceIdGenerator sequenceIdGen; private final ScmBlockDeletingServiceMetrics metrics; + private final SCMDeleteBlocksCommandStatusManager + scmDeleteBlocksCommandStatusManager; + private final long scmCommandTimeoutMs = Duration.ofSeconds(300).toMillis(); private static final int LIST_ALL_FAILED_TRANSACTIONS = -1; @@ -114,6 +120,8 @@ public DeletedBlockLogImpl(ConfigurationSource conf, // maps transaction to dns which have committed it. transactionToDNsCommitMap = new ConcurrentHashMap<>(); transactionToRetryCountMap = new ConcurrentHashMap<>(); + this.scmDeleteBlocksCommandStatusManager = + new SCMDeleteBlocksCommandStatusManager(); this.deletedBlockLogStateManager = DeletedBlockLogStateManagerImpl .newBuilder() .setConfiguration(conf) @@ -226,6 +234,42 @@ private DeletedBlocksTransaction constructNewTransaction( .build(); } + @Override + public SCMDeleteBlocksCommandStatusManager getScmCommandStatusManager() { + return scmDeleteBlocksCommandStatusManager; + } + + @Override + public void commitCommandStatus( + List deleteBlockStatus, UUID dnID) { + lock.lock(); + try { + processSCMCommandStatus(deleteBlockStatus, dnID); + getScmCommandStatusManager().cleanSCMCommandForDn( + dnID, scmCommandTimeoutMs); + } finally { + lock.unlock(); + } + } + + private void processSCMCommandStatus( + List deleteBlockStatus, UUID dnID) { + Map lastStatus = new HashMap<>(); + Map summary = new HashMap<>(); + + // The CommandStatus is ordered in the report. So we can focus only on the + // last status in the command report. + deleteBlockStatus.forEach(cmdStatus -> { + lastStatus.put(cmdStatus.getCmdId(), cmdStatus); + summary.put(cmdStatus.getCmdId(), cmdStatus.getStatus()); + }); + LOG.debug("CommandStatus {} from Datanode {} ", summary, dnID); + for (Map.Entry entry : lastStatus.entrySet()) { + scmDeleteBlocksCommandStatusManager.updateStatusByDNCommandStatus( + dnID, entry.getKey(), entry.getValue().getStatus()); + } + } + /** * {@inheritDoc} * @@ -404,7 +448,8 @@ public void close() throws IOException { private void getTransaction( DeletedBlocksTransaction tx, DatanodeDeletedBlockTransactions transactions, - Set dnList) { + Set dnList, + Map> commandStatus) { try { DeletedBlocksTransaction updatedTxn = DeletedBlocksTransaction .newBuilder(tx) @@ -412,19 +457,12 @@ private void getTransaction( .build(); Set replicas = containerManager .getContainerReplicas( - ContainerID.valueOf(updatedTxn.getContainerID())); + ContainerID.valueOf(tx.getContainerID())); for (ContainerReplica replica : replicas) { - UUID dnID = replica.getDatanodeDetails().getUuid(); - if (!dnList.contains(replica.getDatanodeDetails())) { - continue; - } - Set dnsWithTransactionCommitted = - transactionToDNsCommitMap.get(updatedTxn.getTxID()); - if (dnsWithTransactionCommitted == null || !dnsWithTransactionCommitted - .contains(dnID)) { - // Transaction need not be sent to dns which have - // already committed it - transactions.addTransactionToDN(dnID, updatedTxn); + DatanodeDetails details = replica.getDatanodeDetails(); + if (shouldAddTransactionToDN(details, + updatedTxn.getTxID(), dnList, commandStatus)) { + transactions.addTransactionToDN(details.getUuid(), tx); } } } catch (IOException e) { @@ -432,6 +470,37 @@ private void getTransaction( } } + private boolean shouldAddTransactionToDN(DatanodeDetails dnDetail, long tx, + Set dnLists, + Map> commandStatus) { + if (!dnLists.contains(dnDetail)) { + return false; + } + if (inProcessing(dnDetail.getUuid(), tx, commandStatus)) { + return false; + } + Set dnsWithTransactionCommitted = + transactionToDNsCommitMap.get(tx); + if (dnsWithTransactionCommitted != null && dnsWithTransactionCommitted + .contains(dnDetail.getUuid())) { + // Transaction need not be sent to dns which have + // already committed it + return false; + } + return true; + } + + private boolean inProcessing(UUID dnId, long deletedBlocksTxId, + Map> commandStatus) { + Map deletedBlocksTxStatus = commandStatus.get(dnId); + if (deletedBlocksTxStatus == null || + deletedBlocksTxStatus.get(deletedBlocksTxId) == null) { + return false; + } + return deletedBlocksTxStatus.get(deletedBlocksTxId) != + CmdStatus.NEED_RESEND; + } + @Override public DatanodeDeletedBlockTransactions getTransactions( int blockDeletionLimit, Set dnList) @@ -443,6 +512,11 @@ public DatanodeDeletedBlockTransactions getTransactions( try (TableIterator> iter = deletedBlockLogStateManager.getReadOnlyIterator()) { + // Get the CmdStatus status of the aggregation, so that the current + // status of the specified transaction can be found faster + Map> commandStatus = + getScmCommandStatusManager().getCommandStatusByTxId(dnList.stream(). + map(DatanodeDetails::getUuid).collect(Collectors.toSet())); ArrayList txIDs = new ArrayList<>(); // Here takes block replica count as the threshold to avoid the case // that part of replicas committed the TXN and recorded in the @@ -461,7 +535,7 @@ public DatanodeDeletedBlockTransactions getTransactions( txIDs.add(txn.getTxID()); } else if (txn.getCount() > -1 && txn.getCount() <= maxRetry && !containerManager.getContainer(id).isOpen()) { - getTransaction(txn, transactions, dnList); + getTransaction(txn, transactions, dnList, commandStatus); transactionToDNsCommitMap .putIfAbsent(txn.getTxID(), new LinkedHashSet<>()); } @@ -476,6 +550,10 @@ public DatanodeDeletedBlockTransactions getTransactions( metrics.incrBlockDeletionTransactionCompleted(txIDs.size()); } } + // Here we can clean up the Datanode timeout command that no longer + // reports heartbeats + getScmCommandStatusManager().cleanAllTimeoutSCMCommand( + scmCommandTimeoutMs); return transactions; } finally { lock.unlock(); @@ -489,19 +567,24 @@ public void onMessage( LOG.warn("Skip commit transactions since current SCM is not leader."); return; } + DatanodeDetails details = deleteBlockStatus.getDatanodeDetails(); - CommandStatus.Status status = deleteBlockStatus.getCmdStatus().getStatus(); - if (status == CommandStatus.Status.EXECUTED) { - ContainerBlocksDeletionACKProto ackProto = - deleteBlockStatus.getCmdStatus().getBlockDeletionAck(); - commitTransactions(ackProto.getResultsList(), - UUID.fromString(ackProto.getDnId())); - metrics.incrBlockDeletionCommandSuccess(); - } else if (status == CommandStatus.Status.FAILED) { - metrics.incrBlockDeletionCommandFailure(); - } else { - LOG.error("Delete Block Command is not executed yet."); - return; + for (CommandStatus commandStatus : deleteBlockStatus.getCmdStatus()) { + CommandStatus.Status status = commandStatus.getStatus(); + if (status == CommandStatus.Status.EXECUTED) { + ContainerBlocksDeletionACKProto ackProto = + commandStatus.getBlockDeletionAck(); + commitTransactions(ackProto.getResultsList(), + UUID.fromString(ackProto.getDnId())); + metrics.incrBlockDeletionCommandSuccess(); + } else if (status == CommandStatus.Status.FAILED) { + metrics.incrBlockDeletionCommandFailure(); + } else { + LOG.debug("Delete Block Command {} is not executed on the Datanode {}.", + commandStatus.getCmdId(), details.getUuid()); + } + + commitCommandStatus(deleteBlockStatus.getCmdStatus(), details.getUuid()); } } } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java index 0a1222e38005..41a5a1ddc56c 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java @@ -48,7 +48,6 @@ import org.apache.hadoop.hdds.utils.BackgroundTaskResult.EmptyTaskResult; import org.apache.hadoop.ozone.protocol.commands.CommandForDatanode; import org.apache.hadoop.ozone.protocol.commands.DeleteBlocksCommand; -import org.apache.hadoop.ozone.protocol.commands.SCMCommand; import org.apache.hadoop.util.Time; import com.google.common.annotations.VisibleForTesting; @@ -57,6 +56,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.createScmTxStateMachine; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_SERVICE_TIMEOUT; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_SERVICE_TIMEOUT_DEFAULT; @@ -176,13 +176,16 @@ public EmptyTaskResult call() throws Exception { UUID dnId = entry.getKey(); List dnTXs = entry.getValue(); if (!dnTXs.isEmpty()) { - processedTxIDs.addAll(dnTXs.stream() + Set dnTxSet = dnTXs.stream() .map(DeletedBlocksTransaction::getTxID) - .collect(Collectors.toSet())); - SCMCommand command = new DeleteBlocksCommand(dnTXs); + .collect(Collectors.toSet()); + processedTxIDs.addAll(dnTxSet); + DeleteBlocksCommand command = new DeleteBlocksCommand(dnTXs); command.setTerm(scmContext.getTermOfLeader()); eventPublisher.fireEvent(SCMEvents.DATANODE_COMMAND, new CommandForDatanode<>(dnId, command)); + deletedBlockLog.getScmCommandStatusManager().recordScmCommand( + createScmTxStateMachine(dnId, command.getId(), dnTxSet)); metrics.incrBlockDeletionCommandSent(); metrics.incrBlockDeletionTransactionSent(dnTXs.size()); if (LOG.isDebugEnabled()) { diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java new file mode 100644 index 000000000000..a93fad0ae413 --- /dev/null +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java @@ -0,0 +1,355 @@ +package org.apache.hadoop.hdds.scm.block; + +import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.CommandStatus; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.time.Duration; +import java.time.Instant; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; + +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.EXECUTED; +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.NEED_RESEND; +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.PENDING_EXECUTED; +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.SENT; +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.TO_BE_SENT; + +/** + * SCM DeleteBlocksCommand manager. + */ +public class SCMDeleteBlocksCommandStatusManager { + public static final Logger LOG = + LoggerFactory.getLogger(SCMDeleteBlocksCommandStatusManager.class); + + /** + * Status of SCMDeleteBlocksCommand. + */ + public enum CmdStatus { + // The DeleteBlocksCommand has not yet been sent. + // This is the initial status of the command after it's created. + TO_BE_SENT, + // This status indicates that the DeleteBlocksCommand has been sent + // to the DataNode, but the Datanode has not reported any new status + // for the DeleteBlocksCommand. + SENT, + // The DeleteBlocksCommand has been received by Datanode and + // is waiting for executed. + PENDING_EXECUTED, + // The DeleteBlocksCommand was executed, and the execution was successful + EXECUTED, + // The DeleteBlocksCommand was executed but failed to execute, + // or was lost before it was executed. + NEED_RESEND + } + + private static final CmdStatus DEFAULT_STATUS = TO_BE_SENT; + + private final Set statusesRequiringTimeout = new HashSet<>( + Arrays.asList(SENT, PENDING_EXECUTED)); + private final Set finialStatuses = new HashSet<>( + Arrays.asList(EXECUTED, NEED_RESEND)); + private final Set failedStatuses = new HashSet<>( + Arrays.asList(NEED_RESEND)); + + private final Map> scmCmdStatusRecord; + + public SCMDeleteBlocksCommandStatusManager() { + this.scmCmdStatusRecord = new ConcurrentHashMap<>(); + } + + public static CmdStatusData createScmTxStateMachine( + UUID dnId, long scmCmdId, Set deletedBlocksTxIds) { + return new CmdStatusData(dnId, scmCmdId, deletedBlocksTxIds); + } + + private static final class CmdStatusData { + private final UUID dnId; + private final long scmCmdId; + private final Set deletedBlocksTxIds; + private Instant updateTime; + private CmdStatus status; + + private CmdStatusData( + UUID dnId, long scmTxID, Set deletedBlocksTxIds) { + this.dnId = dnId; + this.scmCmdId = scmTxID; + this.deletedBlocksTxIds = deletedBlocksTxIds; + setStatus(DEFAULT_STATUS); + } + + public boolean isContainDeletedBlocksTx(long deletedBlocksTxId) { + return deletedBlocksTxIds.contains(deletedBlocksTxId); + } + + public Set getDeletedBlocksTxIds() { + return deletedBlocksTxIds; + } + + public UUID getDnId() { + return dnId; + } + + public long getScmCmdId() { + return scmCmdId; + } + + public CmdStatus getStatus() { + return status; + } + + public void setStatus(CmdStatus status) { + this.updateTime = Instant.now(); + this.status = status; + } + + public Instant getUpdateTime() { + return updateTime; + } + + @Override + public String toString() { + return "ScmTxStateMachine" + + "{dnId=" + dnId + + ", scmTxID=" + scmCmdId + + ", deletedBlocksTxIds=" + deletedBlocksTxIds + + ", updateTime=" + updateTime + + ", status=" + status + + '}'; + } + } + + public void recordScmCommand(CmdStatusData statusData) { + LOG.debug("Record ScmCommand: {}", statusData); + scmCmdStatusRecord.computeIfAbsent(statusData.getDnId(), k -> + new ConcurrentHashMap<>()).put(statusData.getScmCmdId(), statusData); + } + + public Map> getCommandStatusByTxId( + Set dnIds) { + Map> result = + new HashMap<>(scmCmdStatusRecord.size()); + + for (UUID dnId : dnIds) { + if (scmCmdStatusRecord.get(dnId) == null) { + continue; + } + Map dnStatusMap = new HashMap<>(); + Map record = scmCmdStatusRecord.get(dnId); + for (CmdStatusData statusData : record.values()) { + CmdStatus status = statusData.getStatus(); + for (Long deletedBlocksTxId : statusData.getDeletedBlocksTxIds()) { + dnStatusMap.put(deletedBlocksTxId, status); + } + } + result.put(dnId, dnStatusMap); + } + + return result; + } + + public void onSent(UUID dnId, long scmCmdId) { + updateStatus(dnId, scmCmdId, SENT); + } + + public void updateStatusByDNCommandStatus(UUID dnId, long scmCmdId, + CommandStatus.Status newState) { + CmdStatus status = fromProtoCommandStatus(newState); + if (status != null) { + updateStatus(dnId, scmCmdId, status); + } + } + + public void cleanAllTimeoutSCMCommand(long timeoutMs) { + for (UUID dnId : scmCmdStatusRecord.keySet()) { + for (CmdStatus status : statusesRequiringTimeout) { + removeTimeoutScmCommand( + dnId, getScmCommandIds(dnId, status), timeoutMs); + } + } + } + + public void cleanSCMCommandForDn(UUID dnId, long timeoutMs) { + cleanTimeoutSCMCommand(dnId, timeoutMs); + cleanFinalStatusSCMCommand(dnId); + cleanFailedCMCommandState(dnId); + } + + private void cleanTimeoutSCMCommand(UUID dnId, long timeoutMs) { + for (CmdStatus status : statusesRequiringTimeout) { + removeTimeoutScmCommand( + dnId, getScmCommandIds(dnId, status), timeoutMs); + } + } + + private void cleanFinalStatusSCMCommand(UUID dnId) { + for (CmdStatus status : finialStatuses) { + for (Long scmCmdId : getScmCommandIds(dnId, status)) { + CmdStatusData stateData = removeScmCommand(dnId, scmCmdId); + LOG.debug("Clean SCMCommand status: {} for DN: {}, stateData: {}", + status, dnId, stateData); + } + } + } + + private void cleanFailedCMCommandState(UUID dnId) { + for (CmdStatus status : failedStatuses) { + for (Long scmCmdId : getScmCommandIds(dnId, status)) { + CmdStatusData stateData = removeScmCommand(dnId, scmCmdId); + LOG.debug("Clean SCMCommand status: {} for DN: {}, stateData: {}", + status, dnId, stateData); + } + } + } + + private Set getScmCommandIds(UUID dnId, CmdStatus status) { + Set scmCmdIds = new HashSet<>(); + if (scmCmdStatusRecord.get(dnId) == null) { + return scmCmdIds; + } + for (CmdStatusData statusData : scmCmdStatusRecord.get(dnId).values()) { + if (statusData.getStatus().equals(status)) { + scmCmdIds.add(statusData.getScmCmdId()); + } + } + return scmCmdIds; + } + + private Instant getUpdateTime(UUID dnId, long scmCmdId) { + if (scmCmdStatusRecord.get(dnId) == null || + scmCmdStatusRecord.get(dnId).get(scmCmdId) == null) { + return null; + } + return scmCmdStatusRecord.get(dnId).get(scmCmdId).getUpdateTime(); + } + + private void updateStatus(UUID dnId, long scmCmdId, CmdStatus newStatus) { + Map recordForDn = scmCmdStatusRecord.get(dnId); + if (recordForDn == null) { + LOG.warn("Unknown Datanode: {} scmCmdId {} newStatus {}", + dnId, scmCmdId, newStatus); + return; + } + if (recordForDn.get(scmCmdId) == null) { + LOG.warn("Unknown SCM Command: {} Datanode {} newStatus {}", + scmCmdId, dnId, newStatus); + return; + } + + boolean changed = false; + CmdStatusData statusData = recordForDn.get(scmCmdId); + CmdStatus oldStatus = statusData.getStatus(); + + switch (newStatus) { + case SENT: + if (oldStatus == TO_BE_SENT) { + // TO_BE_SENT -> SENT: The DeleteBlocksCommand is sent by SCM, + // The follow-up status has not been updated by Datanode. + statusData.setStatus(SENT); + changed = true; + } + break; + case PENDING_EXECUTED: + if (oldStatus == SENT || oldStatus == PENDING_EXECUTED) { + // SENT -> PENDING_EXECUTED: The DeleteBlocksCommand is sent and + // received by the Datanode, but the command is not executed by the + // Datanode, the command is waiting to be executed. + + // PENDING_EXECUTED -> PENDING_EXECUTED: The DeleteBlocksCommand + // continues to wait to be executed by Datanode. + statusData.setStatus(PENDING_EXECUTED); + changed = true; + } + break; + case NEED_RESEND: + if (oldStatus == SENT || oldStatus == PENDING_EXECUTED) { + // SENT -> NEED_RESEND: The DeleteBlocksCommand is sent and lost before + // it is received by the DN. + + // PENDING_EXECUTED -> NEED_RESEND: The DeleteBlocksCommand waited for + // a while and was executed, but the execution failed;. + // Or the DeleteBlocksCommand was lost while waiting(such as the + // Datanode restart). + statusData.setStatus(NEED_RESEND); + changed = true; + } + break; + case EXECUTED: + if (oldStatus == SENT || oldStatus == PENDING_EXECUTED) { + // PENDING_EXECUTED -> EXECUTED: The Command waits for a period of + // time on the DN and is executed successfully. + + // SENT -> EXECUTED: The DeleteBlocksCommand has been sent to Datanode, + // executed by DN, and executed successfully. + statusData.setStatus(EXECUTED); + changed = true; + } + break; + default: + LOG.error("Can not update to Unknown new Status: {}", newStatus); + break; + } + if (!changed) { + LOG.warn("Cannot update illegal status for DN: {} ScmCommandId {} " + + "Status From {} to {}", dnId, scmCmdId, oldStatus, newStatus); + } else { + LOG.debug("Successful update DN: {} ScmCommandId {} Status From {} to {}", + dnId, scmCmdId, oldStatus, newStatus); + } + } + + private void removeTimeoutScmCommand(UUID dnId, + Set scmCmdIds, long timeoutMs) { + Instant now = Instant.now(); + for (Long scmCmdId : scmCmdIds) { + Instant updateTime = getUpdateTime(dnId, scmCmdId); + if (updateTime != null && + Duration.between(updateTime, now).toMillis() > timeoutMs) { + updateStatus(dnId, scmCmdId, NEED_RESEND); + CmdStatusData state = removeScmCommand(dnId, scmCmdId); + LOG.warn("Remove Timeout SCM BlockDeletionCommand {} for DN {} " + + "after without update {}ms}", state, dnId, timeoutMs); + } + } + } + + private CmdStatusData removeScmCommand(UUID dnId, long scmCmdId) { + if (scmCmdStatusRecord.get(dnId) == null || + scmCmdStatusRecord.get(dnId).get(scmCmdId) == null) { + return null; + } + + CmdStatus status = scmCmdStatusRecord.get(dnId).get(scmCmdId).getStatus(); + if (!failedStatuses.contains( + scmCmdStatusRecord.get(dnId).get(scmCmdId).getStatus())) { + LOG.error("Cannot Remove ScmCommand {} Non-final Status {} for DN: {}." + + " final Status {}", scmCmdId, status, dnId, failedStatuses); + return null; + } + + CmdStatusData statusData = scmCmdStatusRecord.get(dnId).remove(scmCmdId); + LOG.debug("Remove ScmCommand {} for DN: {} ", statusData, dnId); + return statusData; + } + + private static CmdStatus fromProtoCommandStatus( + CommandStatus.Status protoCmdStatus) { + switch (protoCmdStatus) { + case PENDING: + return CmdStatus.PENDING_EXECUTED; + case EXECUTED: + return CmdStatus.EXECUTED; + case FAILED: + return CmdStatus.NEED_RESEND; + default: + LOG.error("Unknown protoCmdStatus: {} cannot convert " + + "to ScmDeleteBlockCommandStatus", protoCmdStatus); + return null; + } + } +} diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/command/CommandStatusReportHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/command/CommandStatusReportHandler.java index d43311265d7d..96bd33c57b9d 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/command/CommandStatusReportHandler.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/command/CommandStatusReportHandler.java @@ -18,6 +18,8 @@ package org.apache.hadoop.hdds.scm.command; import com.google.common.base.Preconditions; +import org.apache.hadoop.hdds.HddsIdFactory; +import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto .StorageContainerDatanodeProtocolProtos.SCMCommandProto; import org.apache.hadoop.hdds.protocol.proto @@ -31,6 +33,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.ArrayList; import java.util.List; /** @@ -54,32 +57,41 @@ public void onMessage(CommandStatusReportFromDatanode report, } // Route command status to its watchers. + List deleteBlocksCommandStatus = new ArrayList<>(); cmdStatusList.forEach(cmdStatus -> { if (LOGGER.isTraceEnabled()) { LOGGER.trace("Emitting command status for id:{} type: {}", cmdStatus .getCmdId(), cmdStatus.getType()); } if (cmdStatus.getType() == SCMCommandProto.Type.deleteBlocksCommand) { - publisher.fireEvent(SCMEvents.DELETE_BLOCK_STATUS, - new DeleteBlockStatus(cmdStatus)); + deleteBlocksCommandStatus.add(cmdStatus); } else { LOGGER.debug("CommandStatus of type:{} not handled in " + "CommandStatusReportHandler.", cmdStatus.getType()); } }); + + /** + * The purpose of aggregating all CommandStatus to commit is to reduce the + * Thread switching. When the Datanode queue has a large number of commands + * , there will have many {@link CommandStatus#Status#PENDING} status + * CommandStatus in report + */ + publisher.fireEvent(SCMEvents.DELETE_BLOCK_STATUS, new DeleteBlockStatus( + deleteBlocksCommandStatus, report.getDatanodeDetails())); } /** * Wrapper event for CommandStatus. */ public static class CommandStatusEvent implements IdentifiableEventPayload { - private CommandStatus cmdStatus; + private final List cmdStatus; - CommandStatusEvent(CommandStatus cmdStatus) { + CommandStatusEvent(List cmdStatus) { this.cmdStatus = cmdStatus; } - public CommandStatus getCmdStatus() { + public List getCmdStatus() { return cmdStatus; } @@ -90,7 +102,7 @@ public String toString() { @Override public long getId() { - return cmdStatus.getCmdId(); + return HddsIdFactory.getLongId(); } } @@ -98,8 +110,16 @@ public long getId() { * Wrapper event for DeleteBlock Command. */ public static class DeleteBlockStatus extends CommandStatusEvent { - public DeleteBlockStatus(CommandStatus cmdStatus) { + private final DatanodeDetails datanodeDetails; + + public DeleteBlockStatus(List cmdStatus, + DatanodeDetails datanodeDetails) { super(cmdStatus); + this.datanodeDetails = datanodeDetails; + } + + public DatanodeDetails getDatanodeDetails() { + return datanodeDetails; } } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java index 3466fa423426..69ba2c666473 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java @@ -36,6 +36,7 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.VersionInfo; +import org.apache.hadoop.hdds.scm.block.DeletedBlockLog; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeMetric; import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeStat; @@ -515,6 +516,17 @@ public List processHeartbeat(DatanodeDetails datanodeDetails, commandQueue.getDatanodeCommandSummary(datanodeDetails.getUuid()); List commands = commandQueue.getCommand(datanodeDetails.getUuid()); + + // Update the SCMCommand of deleteBlocksCommand Status + for (SCMCommand command : commands) { + if (command.getType() == SCMCommandProto.Type.deleteBlocksCommand) { + DeletedBlockLog scmBlockDeletingService = scmContext.getScm(). + getScmBlockManager().getDeletedBlockLog(); + scmBlockDeletingService.getScmCommandStatusManager() + .onSent(datanodeDetails.getUuid(), command.getId()); + } + } + if (queueReport != null) { processNodeCommandQueueReport(datanodeDetails, queueReport, summary); } From d5968991f7b6d2a34cf18c54bc74ad0b8d557d3d Mon Sep 17 00:00:00 2001 From: xichen01 Date: Wed, 28 Jun 2023 16:07:51 +0800 Subject: [PATCH 02/23] added licensed for new file --- .../SCMDeleteBlocksCommandStatusManager.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java index a93fad0ae413..7da515af11ce 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java @@ -1,3 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + package org.apache.hadoop.hdds.scm.block; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.CommandStatus; From 2807f2a42f903bc891e3043658f819baf8a42259 Mon Sep 17 00:00:00 2001 From: xichen01 Date: Fri, 30 Jun 2023 01:47:54 +0800 Subject: [PATCH 03/23] Add unit test --- .../hdds/scm/block/DeletedBlockLogImpl.java | 1 + .../SCMDeleteBlocksCommandStatusManager.java | 24 +- ...stSCMDeleteBlocksCommandStatusManager.java | 273 ++++++++++++++++++ 3 files changed, 292 insertions(+), 6 deletions(-) create mode 100644 hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java index 9135e929a150..c6690853700d 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java @@ -404,6 +404,7 @@ public void reinitialize( public void onBecomeLeader() { transactionToDNsCommitMap.clear(); transactionToRetryCountMap.clear(); + scmDeleteBlocksCommandStatusManager.clear(); } /** diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java index 7da515af11ce..994651b02dae 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hdds.scm.block; +import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.CommandStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -87,7 +88,7 @@ public static CmdStatusData createScmTxStateMachine( return new CmdStatusData(dnId, scmCmdId, deletedBlocksTxIds); } - private static final class CmdStatusData { + protected static final class CmdStatusData { private final UUID dnId; private final long scmCmdId; private final Set deletedBlocksTxIds; @@ -177,7 +178,7 @@ public void onSent(UUID dnId, long scmCmdId) { } public void updateStatusByDNCommandStatus(UUID dnId, long scmCmdId, - CommandStatus.Status newState) { + CommandStatus.Status newState) { CmdStatus status = fromProtoCommandStatus(newState); if (status != null) { updateStatus(dnId, scmCmdId, status); @@ -206,6 +207,10 @@ private void cleanTimeoutSCMCommand(UUID dnId, long timeoutMs) { } } + public void clear() { + scmCmdStatusRecord.clear(); + } + private void cleanFinalStatusSCMCommand(UUID dnId) { for (CmdStatus status : finialStatuses) { for (Long scmCmdId : getScmCommandIds(dnId, status)) { @@ -323,7 +328,7 @@ private void updateStatus(UUID dnId, long scmCmdId, CmdStatus newStatus) { } private void removeTimeoutScmCommand(UUID dnId, - Set scmCmdIds, long timeoutMs) { + Set scmCmdIds, long timeoutMs) { Instant now = Instant.now(); for (Long scmCmdId : scmCmdIds) { Instant updateTime = getUpdateTime(dnId, scmCmdId); @@ -333,6 +338,9 @@ private void removeTimeoutScmCommand(UUID dnId, CmdStatusData state = removeScmCommand(dnId, scmCmdId); LOG.warn("Remove Timeout SCM BlockDeletionCommand {} for DN {} " + "after without update {}ms}", state, dnId, timeoutMs); + } else { + LOG.warn("FFFFF Timeout SCM scmCmdIds {} for DN {} " + + "after without update {}ms}", scmCmdIds, dnId, timeoutMs); } } } @@ -344,10 +352,9 @@ private CmdStatusData removeScmCommand(UUID dnId, long scmCmdId) { } CmdStatus status = scmCmdStatusRecord.get(dnId).get(scmCmdId).getStatus(); - if (!failedStatuses.contains( - scmCmdStatusRecord.get(dnId).get(scmCmdId).getStatus())) { + if (!finialStatuses.contains(status)) { LOG.error("Cannot Remove ScmCommand {} Non-final Status {} for DN: {}." + - " final Status {}", scmCmdId, status, dnId, failedStatuses); + " final Status {}", scmCmdId, status, dnId, finialStatuses); return null; } @@ -371,4 +378,9 @@ private static CmdStatus fromProtoCommandStatus( return null; } } + + @VisibleForTesting + Map> getScmCmdStatusRecord() { + return scmCmdStatusRecord; + } } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java new file mode 100644 index 000000000000..757fdec5abef --- /dev/null +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java @@ -0,0 +1,273 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hdds.scm.block; + +import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.BeforeEach; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.NEED_RESEND; +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.PENDING_EXECUTED; +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.SENT; +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.EXECUTED; +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.TO_BE_SENT; +import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatusData; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * A test for SCMDeleteBlocksCommandStatusManager. + */ +public class TestSCMDeleteBlocksCommandStatusManager { + + private SCMDeleteBlocksCommandStatusManager manager; + private static UUID dnId1; + private static UUID dnId2; + private static long scmCmdId1; + private static long scmCmdId2; + private static long scmCmdId3; + private static long scmCmdId4; + private static Set deletedBlocksTxIds1; + private static Set deletedBlocksTxIds2; + private static Set deletedBlocksTxIds3; + private static Set deletedBlocksTxIds4; + + @BeforeEach + public void setup() throws Exception { + manager = new SCMDeleteBlocksCommandStatusManager(); + // Create test data + dnId1 = UUID.randomUUID(); + dnId2 = UUID.randomUUID(); + scmCmdId1 = 1L; + scmCmdId2 = 2L; + scmCmdId3 = 3L; + scmCmdId4 = 4L; + deletedBlocksTxIds1 = new HashSet<>(); + deletedBlocksTxIds1.add(100L); + deletedBlocksTxIds2 = new HashSet<>(); + deletedBlocksTxIds2.add(200L); + deletedBlocksTxIds3 = new HashSet<>(); + deletedBlocksTxIds3.add(300L); + deletedBlocksTxIds4 = new HashSet<>(); + deletedBlocksTxIds4.add(400L); + } + + @Test + public void testRecordScmCommand() { + CmdStatusData statusData = + SCMDeleteBlocksCommandStatusManager.createScmTxStateMachine( + dnId1, scmCmdId1, deletedBlocksTxIds1); + + manager.recordScmCommand(statusData); + + assertNotNull(manager.getScmCmdStatusRecord().get(dnId1)); + assertEquals(1, manager.getScmCmdStatusRecord().get(dnId1).size()); + CmdStatusData cmdStatusData = + manager.getScmCmdStatusRecord().get(dnId1).get(scmCmdId1); + assertNotNull(cmdStatusData); + assertEquals(dnId1, statusData.getDnId()); + assertEquals(scmCmdId1, statusData.getScmCmdId()); + assertEquals(deletedBlocksTxIds1, statusData.getDeletedBlocksTxIds()); + // The default status is `CmdStatus.TO_BE_SENT` + assertEquals(TO_BE_SENT, statusData.getStatus()); + } + + @Test + public void testOnSent() { + CmdStatusData statusData = + SCMDeleteBlocksCommandStatusManager.createScmTxStateMachine( + dnId1, scmCmdId1, deletedBlocksTxIds1); + manager.recordScmCommand(statusData); + + Map dnStatusRecord = + manager.getScmCmdStatusRecord().get(dnId1); + // After the Command is sent by SCM, the status of the Command + // will change from TO_BE_SENT to SENT + assertEquals(TO_BE_SENT, dnStatusRecord.get(scmCmdId1).getStatus()); + manager.onSent(dnId1, scmCmdId1); + assertEquals(SENT, dnStatusRecord.get(scmCmdId1).getStatus()); + } + + @Test + public void testUpdateStatusByDNCommandStatus() { + // Test all Status update by Datanode Heartbeat report. + // SENT -> PENDING_EXECUTED: The DeleteBlocksCommand is sent and received + // by the Datanode, but the command is not executed by the Datanode, + // the command is waiting to be executed. + + // SENT -> NEED_RESEND: The DeleteBlocksCommand is sent and lost before + // it is received by the DN. + // SENT -> EXECUTED: The DeleteBlocksCommand has been sent to Datanode, + // executed by DN, and executed successfully. + // + // PENDING_EXECUTED -> PENDING_EXECUTED: The DeleteBlocksCommand continues + // to wait to be executed by Datanode. + // PENDING_EXECUTED -> NEED_RESEND: The DeleteBlocksCommand waited for a + // while and was executed, but the execution failed; Or the + // DeleteBlocksCommand was lost while waiting(such as the Datanode restart) + // + // PENDING_EXECUTED -> EXECUTED: The Command waits for a period of + // time on the DN and is executed successfully. + + recordAndSentCommand(manager, dnId1, + Arrays.asList(scmCmdId1, scmCmdId2, scmCmdId3, scmCmdId4), + Arrays.asList(deletedBlocksTxIds1, deletedBlocksTxIds2, + deletedBlocksTxIds3, deletedBlocksTxIds4)); + + Map dnStatusRecord = + manager.getScmCmdStatusRecord().get(dnId1); + assertEquals(SENT, dnStatusRecord.get(scmCmdId1).getStatus()); + assertEquals(SENT, dnStatusRecord.get(scmCmdId2).getStatus()); + assertEquals(SENT, dnStatusRecord.get(scmCmdId3).getStatus()); + assertEquals(SENT, dnStatusRecord.get(scmCmdId4).getStatus()); + + // SENT -> PENDING_EXECUTED + manager.updateStatusByDNCommandStatus(dnId1, scmCmdId1, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.PENDING); + // SENT -> EXECUTED + manager.updateStatusByDNCommandStatus(dnId1, scmCmdId2, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.EXECUTED); + // SENT -> NEED_RESEND + manager.updateStatusByDNCommandStatus(dnId1, scmCmdId3, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.FAILED); + // SENT -> PENDING_EXECUTED + manager.updateStatusByDNCommandStatus(dnId1, scmCmdId4, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.PENDING); + + assertEquals(PENDING_EXECUTED, dnStatusRecord.get(scmCmdId1).getStatus()); + assertEquals(EXECUTED, dnStatusRecord.get(scmCmdId2).getStatus()); + assertEquals(NEED_RESEND, dnStatusRecord.get(scmCmdId3).getStatus()); + assertEquals(PENDING_EXECUTED, dnStatusRecord.get(scmCmdId4).getStatus()); + + // PENDING_EXECUTED -> PENDING_EXECUTED + manager.updateStatusByDNCommandStatus(dnId1, scmCmdId1, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.PENDING); + assertEquals(PENDING_EXECUTED, dnStatusRecord.get(scmCmdId1).getStatus()); + + // PENDING_EXECUTED -> EXECUTED + manager.updateStatusByDNCommandStatus(dnId1, scmCmdId1, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.EXECUTED); + assertEquals(EXECUTED, dnStatusRecord.get(scmCmdId1).getStatus()); + + // PENDING_EXECUTED -> NEED_RESEND + manager.updateStatusByDNCommandStatus(dnId1, scmCmdId4, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.FAILED); + assertEquals(NEED_RESEND, dnStatusRecord.get(scmCmdId4).getStatus()); + } + + @Test + public void testCleanSCMCommandForDn() { + // Transactions in states EXECUTED and NEED_RESEND will be cleaned up + // directly, while transactions in states PENDING_EXECUTED and SENT + // will be cleaned up after timeout + recordAndSentCommand(manager, dnId1, + Arrays.asList(scmCmdId1, scmCmdId2, scmCmdId3, scmCmdId4), + Arrays.asList(deletedBlocksTxIds1, deletedBlocksTxIds2, + deletedBlocksTxIds3, deletedBlocksTxIds4)); + + // SENT -> PENDING_EXECUTED + manager.updateStatusByDNCommandStatus(dnId1, scmCmdId1, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.PENDING); + // SENT -> EXECUTED + manager.updateStatusByDNCommandStatus(dnId1, scmCmdId2, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.EXECUTED); + // SENT -> NEED_RESEND + manager.updateStatusByDNCommandStatus(dnId1, scmCmdId3, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.FAILED); + + Map dnStatusRecord = + manager.getScmCmdStatusRecord().get(dnId1); + assertNotNull(dnStatusRecord.get(scmCmdId1)); + assertNotNull(dnStatusRecord.get(scmCmdId2)); + assertNotNull(dnStatusRecord.get(scmCmdId3)); + assertNotNull(dnStatusRecord.get(scmCmdId4)); + + manager.cleanSCMCommandForDn(dnId1, Long.MAX_VALUE); + + // scmCmdId1 is PENDING_EXECUTED will be cleaned up after timeout + assertNotNull(dnStatusRecord.get(scmCmdId1)); + assertNull(dnStatusRecord.get(scmCmdId3)); + assertNull(dnStatusRecord.get(scmCmdId2)); + // scmCmdId4 is SENT will be cleaned up after timeout + assertNotNull(dnStatusRecord.get(scmCmdId4)); + + manager.cleanSCMCommandForDn(dnId1, -1); + assertNull(dnStatusRecord.get(scmCmdId1)); + assertNull(dnStatusRecord.get(scmCmdId4)); + } + + @Test + public void testCleanAllTimeoutSCMCommand() { + // Test All EXECUTED and NEED_RESEND status in the DN will be cleaned up + + // Transactions in states EXECUTED and NEED_RESEND will be cleaned up + // directly, while transactions in states PENDING_EXECUTED and SENT + // will be cleaned up after timeout + recordAndSentCommand(manager, dnId1, Arrays.asList(scmCmdId1), + Arrays.asList(deletedBlocksTxIds1)); + recordAndSentCommand(manager, dnId2, Arrays.asList(scmCmdId2), + Arrays.asList(deletedBlocksTxIds2)); + + Map dn1StatusRecord = + manager.getScmCmdStatusRecord().get(dnId1); + Map dn2StatusRecord = + manager.getScmCmdStatusRecord().get(dnId2); + + // Only let the scmCmdId1 have a Heartbeat report, its status will be + // updated, the scmCmdId2 still in SENT status. + // SENT -> PENDING_EXECUTED + manager.updateStatusByDNCommandStatus(dnId1, scmCmdId1, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.PENDING); + + manager.cleanAllTimeoutSCMCommand(Long.MAX_VALUE); + // scmCmdId1 is PENDING_EXECUTED will be cleaned up after timeout + assertNotNull(dn1StatusRecord.get(scmCmdId1)); + assertNotNull(dn2StatusRecord.get(scmCmdId2)); + + // scmCmdId2 is SENT will be cleaned up after timeout + manager.cleanAllTimeoutSCMCommand(-1); + assertNull(dn1StatusRecord.get(scmCmdId1)); + assertNull(dn2StatusRecord.get(scmCmdId2)); + + } + + private void recordAndSentCommand( + SCMDeleteBlocksCommandStatusManager statusManager, + UUID dnId, List scmCmdIds, List> txIds) { + assertEquals(scmCmdIds.size(), txIds.size()); + for (int i = 0; i < scmCmdIds.size(); i++) { + long scmCmdId = scmCmdIds.get(i); + Set deletedBlocksTxIds = txIds.get(i); + CmdStatusData statusData = + SCMDeleteBlocksCommandStatusManager.createScmTxStateMachine( + dnId, scmCmdId, deletedBlocksTxIds); + statusManager.recordScmCommand(statusData); + statusManager.onSent(dnId, scmCmdId); + } + } + +} From a94d6cbaf067b14cd81d3aabdd033401649dee6b Mon Sep 17 00:00:00 2001 From: xichen01 Date: Fri, 30 Jun 2023 01:55:31 +0800 Subject: [PATCH 04/23] Fix integration test --- .../org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java index c6690853700d..2298a64abd0f 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java @@ -458,12 +458,12 @@ private void getTransaction( .build(); Set replicas = containerManager .getContainerReplicas( - ContainerID.valueOf(tx.getContainerID())); + ContainerID.valueOf(updatedTxn.getContainerID())); for (ContainerReplica replica : replicas) { DatanodeDetails details = replica.getDatanodeDetails(); if (shouldAddTransactionToDN(details, updatedTxn.getTxID(), dnList, commandStatus)) { - transactions.addTransactionToDN(details.getUuid(), tx); + transactions.addTransactionToDN(details.getUuid(), updatedTxn); } } } catch (IOException e) { From 2021a3b3cbef8085f16bf1550e680162d5ff9760 Mon Sep 17 00:00:00 2001 From: xichen01 Date: Fri, 30 Jun 2023 11:49:49 +0800 Subject: [PATCH 05/23] Fix findbugs --- ...stSCMDeleteBlocksCommandStatusManager.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java index 757fdec5abef..4671cb0b43f1 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java @@ -45,16 +45,16 @@ public class TestSCMDeleteBlocksCommandStatusManager { private SCMDeleteBlocksCommandStatusManager manager; - private static UUID dnId1; - private static UUID dnId2; - private static long scmCmdId1; - private static long scmCmdId2; - private static long scmCmdId3; - private static long scmCmdId4; - private static Set deletedBlocksTxIds1; - private static Set deletedBlocksTxIds2; - private static Set deletedBlocksTxIds3; - private static Set deletedBlocksTxIds4; + private UUID dnId1; + private UUID dnId2; + private long scmCmdId1; + private long scmCmdId2; + private long scmCmdId3; + private long scmCmdId4; + private Set deletedBlocksTxIds1; + private Set deletedBlocksTxIds2; + private Set deletedBlocksTxIds3; + private Set deletedBlocksTxIds4; @BeforeEach public void setup() throws Exception { From 1ba8e088607b1fae3bba319f6cbde742e93ba9b2 Mon Sep 17 00:00:00 2001 From: xichen01 Date: Fri, 30 Jun 2023 16:49:05 +0800 Subject: [PATCH 06/23] Add integration test --- .../hdds/scm/block/DeletedBlockLog.java | 2 +- .../hdds/scm/block/DeletedBlockLogImpl.java | 20 ++- .../scm/block/SCMBlockDeletingService.java | 4 +- .../SCMDeleteBlocksCommandStatusManager.java | 2 +- .../hdds/scm/block/TestDeletedBlockLog.java | 167 ++++++++++++++++++ ...stSCMDeleteBlocksCommandStatusManager.java | 6 +- 6 files changed, 187 insertions(+), 14 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java index a4c9dda8fbaf..523d86cd8365 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java @@ -104,7 +104,7 @@ void commitTransactions(List transactionResults, * @param deleteBlockStatus the list of DeleteBlocksCommand * @param dnID */ - void commitCommandStatus(List deleteBlockStatus, UUID dnID); + void commitSCMCommandStatus(List deleteBlockStatus, UUID dnID); /** * Get ScmDeleteBlocksCommandStatusManager. diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java index e6a01a0e89c2..2cd2341091cb 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java @@ -95,7 +95,7 @@ public class DeletedBlockLogImpl private final ScmBlockDeletingServiceMetrics metrics; private final SCMDeleteBlocksCommandStatusManager scmDeleteBlocksCommandStatusManager; - private final long scmCommandTimeoutMs = Duration.ofSeconds(300).toMillis(); + private long scmCommandTimeoutMs = Duration.ofSeconds(300).toMillis(); private static final int LIST_ALL_FAILED_TRANSACTIONS = -1; @@ -239,7 +239,7 @@ public SCMDeleteBlocksCommandStatusManager getScmCommandStatusManager() { } @Override - public void commitCommandStatus( + public void commitSCMCommandStatus( List deleteBlockStatus, UUID dnID) { lock.lock(); try { @@ -507,6 +507,10 @@ public DatanodeDeletedBlockTransactions getTransactions( throws IOException { lock.lock(); try { + // Here we can clean up the Datanode timeout command that no longer + // reports heartbeats + getScmCommandStatusManager().cleanAllTimeoutSCMCommand( + scmCommandTimeoutMs); DatanodeDeletedBlockTransactions transactions = new DatanodeDeletedBlockTransactions(); try (TableIterator(dnId, command)); deletedBlockLog.getScmCommandStatusManager().recordScmCommand( - createScmTxStateMachine(dnId, command.getId(), dnTxSet)); + createScmCmdStatusData(dnId, command.getId(), dnTxSet)); metrics.incrBlockDeletionCommandSent(); metrics.incrBlockDeletionTransactionSent(dnTXs.size()); if (LOG.isDebugEnabled()) { diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java index 994651b02dae..f2277ce0e338 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java @@ -83,7 +83,7 @@ public SCMDeleteBlocksCommandStatusManager() { this.scmCmdStatusRecord = new ConcurrentHashMap<>(); } - public static CmdStatusData createScmTxStateMachine( + public static CmdStatusData createScmCmdStatusData( UUID dnId, long scmCmdId, Set deletedBlocksTxIds) { return new CmdStatusData(dnId, scmCmdId, deletedBlocksTxIds); } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java index e519d7c56eb1..b1643aaa2407 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java @@ -22,6 +22,8 @@ import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.hdds.client.StandaloneReplicationConfig; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.SCMCommandProto.Type; import org.apache.hadoop.hdds.protocol.proto .StorageContainerDatanodeProtocolProtos.ContainerReplicaProto; import org.apache.hadoop.hdds.scm.ScmConfigKeys; @@ -45,6 +47,8 @@ .DeleteBlockTransactionResult; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.TableIterator; +import org.apache.hadoop.ozone.protocol.commands.CommandStatus; +import org.apache.hadoop.ozone.protocol.commands.DeleteBlocksCommand; import org.apache.ozone.test.GenericTestUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; @@ -58,6 +62,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -67,6 +72,7 @@ import java.util.UUID; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.BiConsumer; import java.util.stream.Collectors; import static org.apache.hadoop.hdds.scm.ScmConfigKeys @@ -441,6 +447,167 @@ public void testCommitTransactions() throws Exception { Assertions.assertEquals(0, blocks.size()); } + private void recordScmCommandToStatusManager( + UUID dnId, DeleteBlocksCommand command) { + Set dnTxSet = command.blocksTobeDeleted() + .stream().map(DeletedBlocksTransaction::getTxID) + .collect(Collectors.toSet()); + deletedBlockLog.getScmCommandStatusManager().recordScmCommand( + SCMDeleteBlocksCommandStatusManager.createScmCmdStatusData( + dnId, command.getId(), dnTxSet)); + } + + private void sendSCMDeleteBlocksCommand(UUID dnId, long scmCmdId) { + deletedBlockLog.getScmCommandStatusManager().onSent( + dnId, scmCmdId); + } + + private void assertNoDuplicateTransactions( + DatanodeDeletedBlockTransactions transactions1, + DatanodeDeletedBlockTransactions transactions2) { + Map> map1 = + transactions1.getDatanodeTransactionMap(); + Map> map2 = + transactions2.getDatanodeTransactionMap(); + + for (UUID dnId : map1.keySet()) { + Set txSet1 = new HashSet<>(map1.get(dnId)); + Set txSet2 = new HashSet<>(map2.get(dnId)); + + txSet1.retainAll(txSet2); + Assertions.assertEquals(0, txSet1.size(), + String.format("Duplicate Transactions found first transactions %s " + + "second transactions %s for Dn %s", txSet1, txSet2, dnId)); + } + } + + private void assertContainsAllTransactions( + DatanodeDeletedBlockTransactions transactions1, + DatanodeDeletedBlockTransactions transactions2) { + Map> map1 = + transactions1.getDatanodeTransactionMap(); + Map> map2 = + transactions2.getDatanodeTransactionMap(); + + for (UUID dnId : map1.keySet()) { + Set txSet1 = new HashSet<>(map1.get(dnId)); + Set txSet2 = new HashSet<>(map2.get(dnId)); + + Assertions.assertTrue(txSet1.containsAll(txSet2)); + } + } + + private void commitSCMCommandStatus(Long scmCmdId, UUID dnID, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status status) { + List deleteBlockStatus = new ArrayList<>(); + deleteBlockStatus.add(CommandStatus.CommandStatusBuilder.newBuilder() + .setCmdId(scmCmdId) + .setType(Type.deleteBlocksCommand) + .setStatus(status) + .build() + .getProtoBufMessage()); + + deletedBlockLog.commitSCMCommandStatus(deleteBlockStatus, dnID); + } + + private void createDeleteBlocksCommandAndAction( + DatanodeDeletedBlockTransactions transactions, + BiConsumer afterCreate) { + for (Map.Entry> entry : + transactions.getDatanodeTransactionMap().entrySet()) { + UUID dnId = entry.getKey(); + List dnTXs = entry.getValue(); + DeleteBlocksCommand command = new DeleteBlocksCommand(dnTXs); + afterCreate.accept(dnId, command); + } + } + + @Test + public void testNoDuplicateTransactionsForInProcessingSCMCommand() + throws Exception { + // The SCM will not resend these transactions in blow case: + // - If the command has not been sent; + // - The DN does not report the status of the command via heartbeat + // After the command is sent; + // - If the DN reports the command status as PENDING; + addTransactions(generateData(10), true); + int blockLimit = 2 * BLOCKS_PER_TXN * THREE; + + // If the command has not been sent + DatanodeDeletedBlockTransactions transactions1 = + deletedBlockLog.getTransactions(blockLimit, new HashSet<>(dnList)); + createDeleteBlocksCommandAndAction(transactions1, + this::recordScmCommandToStatusManager); + + // - The DN does not report the status of the command via heartbeat + // After the command is sent + DatanodeDeletedBlockTransactions transactions2 = + deletedBlockLog.getTransactions(blockLimit, new HashSet<>(dnList)); + assertNoDuplicateTransactions(transactions1, transactions2); + createDeleteBlocksCommandAndAction(transactions2, (dnId, command) -> { + recordScmCommandToStatusManager(dnId, command); + sendSCMDeleteBlocksCommand(dnId, command.getId()); + }); + + // - If the DN reports the command status as PENDING + DatanodeDeletedBlockTransactions transactions3 = + deletedBlockLog.getTransactions(blockLimit, new HashSet<>(dnList)); + assertNoDuplicateTransactions(transactions1, transactions3); + createDeleteBlocksCommandAndAction(transactions3, (dnId, command) -> { + recordScmCommandToStatusManager(dnId, command); + sendSCMDeleteBlocksCommand(dnId, command.getId()); + commitSCMCommandStatus(command.getId(), dnId, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.PENDING); + }); + assertNoDuplicateTransactions(transactions3, transactions1); + assertNoDuplicateTransactions(transactions3, transactions2); + + DatanodeDeletedBlockTransactions transactions4 = + deletedBlockLog.getTransactions(blockLimit, new HashSet<>(dnList)); + assertNoDuplicateTransactions(transactions4, transactions1); + assertNoDuplicateTransactions(transactions4, transactions2); + assertNoDuplicateTransactions(transactions4, transactions3); + } + + @Test + public void testFailedAndTimeoutSCMCommandCanBeResend() throws Exception { + // The SCM will be resent these transactions in blow case: + // - Executed failed commands; + // - DN does not refresh the PENDING state for more than a period of time; + deletedBlockLog.setScmCommandTimeoutMs(Long.MAX_VALUE); + addTransactions(generateData(10), true); + int blockLimit = 2 * BLOCKS_PER_TXN * THREE; + + // - DN does not refresh the PENDING state for more than a period of time; + DatanodeDeletedBlockTransactions transactions = + deletedBlockLog.getTransactions(blockLimit, new HashSet<>(dnList)); + createDeleteBlocksCommandAndAction(transactions, (dnId, command) -> { + recordScmCommandToStatusManager(dnId, command); + sendSCMDeleteBlocksCommand(dnId, command.getId()); + commitSCMCommandStatus(command.getId(), dnId, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.PENDING); + }); + + // - Executed failed commands; + DatanodeDeletedBlockTransactions transactions2 = + deletedBlockLog.getTransactions(blockLimit, new HashSet<>(dnList)); + createDeleteBlocksCommandAndAction(transactions2, (dnId, command) -> { + recordScmCommandToStatusManager(dnId, command); + sendSCMDeleteBlocksCommand(dnId, command.getId()); + commitSCMCommandStatus(command.getId(), dnId, + StorageContainerDatanodeProtocolProtos.CommandStatus.Status.FAILED); + }); + + deletedBlockLog.setScmCommandTimeoutMs(-1L); + DatanodeDeletedBlockTransactions transactions3 = + deletedBlockLog.getTransactions(Integer.MAX_VALUE, + new HashSet<>(dnList)); + assertNoDuplicateTransactions(transactions, transactions2); + assertContainsAllTransactions(transactions3, transactions); + assertContainsAllTransactions(transactions3, transactions2); + } + @Test public void testDNOnlyOneNodeHealthy() throws Exception { Map> deletedBlocks = generateData(50); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java index 4671cb0b43f1..689a6c2b59d5 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java @@ -79,7 +79,7 @@ public void setup() throws Exception { @Test public void testRecordScmCommand() { CmdStatusData statusData = - SCMDeleteBlocksCommandStatusManager.createScmTxStateMachine( + SCMDeleteBlocksCommandStatusManager.createScmCmdStatusData( dnId1, scmCmdId1, deletedBlocksTxIds1); manager.recordScmCommand(statusData); @@ -99,7 +99,7 @@ public void testRecordScmCommand() { @Test public void testOnSent() { CmdStatusData statusData = - SCMDeleteBlocksCommandStatusManager.createScmTxStateMachine( + SCMDeleteBlocksCommandStatusManager.createScmCmdStatusData( dnId1, scmCmdId1, deletedBlocksTxIds1); manager.recordScmCommand(statusData); @@ -263,7 +263,7 @@ private void recordAndSentCommand( long scmCmdId = scmCmdIds.get(i); Set deletedBlocksTxIds = txIds.get(i); CmdStatusData statusData = - SCMDeleteBlocksCommandStatusManager.createScmTxStateMachine( + SCMDeleteBlocksCommandStatusManager.createScmCmdStatusData( dnId, scmCmdId, deletedBlocksTxIds); statusManager.recordScmCommand(statusData); statusManager.onSent(dnId, scmCmdId); From a1117a2d472d955c02ca75dbcdb5bda7ef45764b Mon Sep 17 00:00:00 2001 From: xichen01 Date: Fri, 30 Jun 2023 17:02:36 +0800 Subject: [PATCH 07/23] Fix test --- .../block/SCMDeleteBlocksCommandStatusManager.java | 11 ----------- .../hdds/scm/command/CommandStatusReportHandler.java | 6 ++++-- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java index f2277ce0e338..9a4d73aa4ade 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java @@ -197,7 +197,6 @@ public void cleanAllTimeoutSCMCommand(long timeoutMs) { public void cleanSCMCommandForDn(UUID dnId, long timeoutMs) { cleanTimeoutSCMCommand(dnId, timeoutMs); cleanFinalStatusSCMCommand(dnId); - cleanFailedCMCommandState(dnId); } private void cleanTimeoutSCMCommand(UUID dnId, long timeoutMs) { @@ -221,16 +220,6 @@ private void cleanFinalStatusSCMCommand(UUID dnId) { } } - private void cleanFailedCMCommandState(UUID dnId) { - for (CmdStatus status : failedStatuses) { - for (Long scmCmdId : getScmCommandIds(dnId, status)) { - CmdStatusData stateData = removeScmCommand(dnId, scmCmdId); - LOG.debug("Clean SCMCommand status: {} for DN: {}, stateData: {}", - status, dnId, stateData); - } - } - } - private Set getScmCommandIds(UUID dnId, CmdStatus status) { Set scmCmdIds = new HashSet<>(); if (scmCmdStatusRecord.get(dnId) == null) { diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/command/CommandStatusReportHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/command/CommandStatusReportHandler.java index 96bd33c57b9d..5d737659ddce 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/command/CommandStatusReportHandler.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/command/CommandStatusReportHandler.java @@ -77,8 +77,10 @@ public void onMessage(CommandStatusReportFromDatanode report, * , there will have many {@link CommandStatus#Status#PENDING} status * CommandStatus in report */ - publisher.fireEvent(SCMEvents.DELETE_BLOCK_STATUS, new DeleteBlockStatus( - deleteBlocksCommandStatus, report.getDatanodeDetails())); + if (!deleteBlocksCommandStatus.isEmpty()) { + publisher.fireEvent(SCMEvents.DELETE_BLOCK_STATUS, new DeleteBlockStatus( + deleteBlocksCommandStatus, report.getDatanodeDetails())); + } } /** From 0bf45459bf597996e7cb1d763001daf0bd0ba06d Mon Sep 17 00:00:00 2001 From: xichen01 Date: Sat, 1 Jul 2023 01:00:59 +0800 Subject: [PATCH 08/23] Fix test --- .../common/report/TestReportPublisher.java | 4 ++-- .../hdds/scm/block/TestDeletedBlockLog.java | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/report/TestReportPublisher.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/report/TestReportPublisher.java index d611caf126f9..c0be73382543 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/report/TestReportPublisher.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/report/TestReportPublisher.java @@ -173,8 +173,8 @@ public void testCommandStatusPublisher() throws InterruptedException { .build(); cmdStatusMap.put(obj1.getCmdId(), obj1); cmdStatusMap.put(obj2.getCmdId(), obj2); - // We are not sending the commands whose status is PENDING. - Assertions.assertEquals(1, + // We will sending the commands whose status is PENDING and EXECUTED + Assertions.assertEquals(2, ((CommandStatusReportPublisher) publisher).getReport() .getCmdStatusCount(), "Should publish report with 2 status objects"); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java index b1643aaa2407..27b3bd7eed14 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java @@ -470,17 +470,20 @@ private void assertNoDuplicateTransactions( Map> map2 = transactions2.getDatanodeTransactionMap(); - for (UUID dnId : map1.keySet()) { - Set txSet1 = new HashSet<>(map1.get(dnId)); + for (Map.Entry> entry : + map1.entrySet()) { + UUID dnId = entry.getKey(); + Set txSet1 = new HashSet<>(entry.getValue()); Set txSet2 = new HashSet<>(map2.get(dnId)); txSet1.retainAll(txSet2); Assertions.assertEquals(0, txSet1.size(), String.format("Duplicate Transactions found first transactions %s " + - "second transactions %s for Dn %s", txSet1, txSet2, dnId)); + "second transactions %s for Dn %s", txSet1, txSet2, dnId)); } } + private void assertContainsAllTransactions( DatanodeDeletedBlockTransactions transactions1, DatanodeDeletedBlockTransactions transactions2) { @@ -489,8 +492,10 @@ private void assertContainsAllTransactions( Map> map2 = transactions2.getDatanodeTransactionMap(); - for (UUID dnId : map1.keySet()) { - Set txSet1 = new HashSet<>(map1.get(dnId)); + for (Map.Entry> entry : + map1.entrySet()) { + UUID dnId = entry.getKey(); + Set txSet1 = new HashSet<>(entry.getValue()); Set txSet2 = new HashSet<>(map2.get(dnId)); Assertions.assertTrue(txSet1.containsAll(txSet2)); From 5f51a922a43e28ed0e813b47f5693a6e770bfc9f Mon Sep 17 00:00:00 2001 From: xichen01 Date: Sat, 1 Jul 2023 20:40:28 +0800 Subject: [PATCH 09/23] Fix test --- .../java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java | 4 ++-- .../org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java index 69ba2c666473..bb998494ee8e 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java @@ -520,9 +520,9 @@ public List processHeartbeat(DatanodeDetails datanodeDetails, // Update the SCMCommand of deleteBlocksCommand Status for (SCMCommand command : commands) { if (command.getType() == SCMCommandProto.Type.deleteBlocksCommand) { - DeletedBlockLog scmBlockDeletingService = scmContext.getScm(). + DeletedBlockLog deletedBlockLog = scmContext.getScm(). getScmBlockManager().getDeletedBlockLog(); - scmBlockDeletingService.getScmCommandStatusManager() + deletedBlockLog.getScmCommandStatusManager() .onSent(datanodeDetails.getUuid(), command.getId()); } } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java index 2f677629a035..cf487db4bf92 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java @@ -957,16 +957,17 @@ scmStorageConfig, eventPublisher, new NetworkTopologyImpl(conf), @Test public void testProcessCommandQueueReport() - throws IOException, NodeNotFoundException { + throws IOException, NodeNotFoundException, AuthenticationException { OzoneConfiguration conf = new OzoneConfiguration(); SCMStorageConfig scmStorageConfig = mock(SCMStorageConfig.class); when(scmStorageConfig.getClusterID()).thenReturn("xyz111"); EventPublisher eventPublisher = mock(EventPublisher.class); HDDSLayoutVersionManager lvm = new HDDSLayoutVersionManager(scmStorageConfig.getLayoutVersion()); + createNodeManager(getConf()); SCMNodeManager nodeManager = new SCMNodeManager(conf, scmStorageConfig, eventPublisher, new NetworkTopologyImpl(conf), - SCMContext.emptyContext(), lvm); + scmContext, lvm); LayoutVersionProto layoutInfo = toLayoutVersionProto( lvm.getMetadataLayoutVersion(), lvm.getSoftwareLayoutVersion()); From b7a9c1c653eb15e67dd864baf49b1b230a755f90 Mon Sep 17 00:00:00 2001 From: xichen01 Date: Thu, 13 Jul 2023 19:25:41 +0800 Subject: [PATCH 10/23] Add status cleanup when datanode is dead --- .../SCMDeleteBlocksCommandStatusManager.java | 31 ++++++++++++------- .../hadoop/hdds/scm/node/DeadNodeHandler.java | 21 +++++++++++++ .../hdds/scm/node/TestDeadNodeHandler.java | 19 +++++++++++- 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java index 9a4d73aa4ade..307ca9cb5679 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java @@ -27,6 +27,7 @@ import java.time.Duration; import java.time.Instant; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -108,7 +109,7 @@ public boolean isContainDeletedBlocksTx(long deletedBlocksTxId) { } public Set getDeletedBlocksTxIds() { - return deletedBlocksTxIds; + return Collections.unmodifiableSet(deletedBlocksTxIds); } public UUID getDnId() { @@ -156,11 +157,11 @@ public Map> getCommandStatusByTxId( new HashMap<>(scmCmdStatusRecord.size()); for (UUID dnId : dnIds) { - if (scmCmdStatusRecord.get(dnId) == null) { + Map record = scmCmdStatusRecord.get(dnId); + if (record == null) { continue; } Map dnStatusMap = new HashMap<>(); - Map record = scmCmdStatusRecord.get(dnId); for (CmdStatusData statusData : record.values()) { CmdStatus status = statusData.getStatus(); for (Long deletedBlocksTxId : statusData.getDeletedBlocksTxIds()) { @@ -177,6 +178,11 @@ public void onSent(UUID dnId, long scmCmdId) { updateStatus(dnId, scmCmdId, SENT); } + public void onDatanodeDead(UUID dnId) { + LOG.info("Clean SCMCommand record for DN: {}", dnId); + scmCmdStatusRecord.remove(dnId); + } + public void updateStatusByDNCommandStatus(UUID dnId, long scmCmdId, CommandStatus.Status newState) { CmdStatus status = fromProtoCommandStatus(newState); @@ -222,10 +228,11 @@ private void cleanFinalStatusSCMCommand(UUID dnId) { private Set getScmCommandIds(UUID dnId, CmdStatus status) { Set scmCmdIds = new HashSet<>(); - if (scmCmdStatusRecord.get(dnId) == null) { + Map record = scmCmdStatusRecord.get(dnId); + if (record == null) { return scmCmdIds; } - for (CmdStatusData statusData : scmCmdStatusRecord.get(dnId).values()) { + for (CmdStatusData statusData : record.values()) { if (statusData.getStatus().equals(status)) { scmCmdIds.add(statusData.getScmCmdId()); } @@ -234,11 +241,11 @@ private Set getScmCommandIds(UUID dnId, CmdStatus status) { } private Instant getUpdateTime(UUID dnId, long scmCmdId) { - if (scmCmdStatusRecord.get(dnId) == null || - scmCmdStatusRecord.get(dnId).get(scmCmdId) == null) { + Map record = scmCmdStatusRecord.get(dnId); + if (record == null || record.get(scmCmdId) == null) { return null; } - return scmCmdStatusRecord.get(dnId).get(scmCmdId).getUpdateTime(); + return record.get(scmCmdId).getUpdateTime(); } private void updateStatus(UUID dnId, long scmCmdId, CmdStatus newStatus) { @@ -335,19 +342,19 @@ private void removeTimeoutScmCommand(UUID dnId, } private CmdStatusData removeScmCommand(UUID dnId, long scmCmdId) { - if (scmCmdStatusRecord.get(dnId) == null || - scmCmdStatusRecord.get(dnId).get(scmCmdId) == null) { + Map record = scmCmdStatusRecord.get(dnId); + if (record == null || record.get(scmCmdId) == null) { return null; } - CmdStatus status = scmCmdStatusRecord.get(dnId).get(scmCmdId).getStatus(); + CmdStatus status = record.get(scmCmdId).getStatus(); if (!finialStatuses.contains(status)) { LOG.error("Cannot Remove ScmCommand {} Non-final Status {} for DN: {}." + " final Status {}", scmCmdId, status, dnId, finialStatuses); return null; } - CmdStatusData statusData = scmCmdStatusRecord.get(dnId).remove(scmCmdId); + CmdStatusData statusData = record.remove(scmCmdId); LOG.debug("Remove ScmCommand {} for DN: {} ", statusData, dnId); return statusData; } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java index b95998c1da11..d5b3c278781b 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java @@ -24,6 +24,7 @@ import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.scm.block.DeletedBlockLog; import org.apache.hadoop.hdds.scm.container.ContainerException; import org.apache.hadoop.hdds.scm.container.ContainerInfo; import org.apache.hadoop.hdds.scm.container.ContainerManager; @@ -41,6 +42,8 @@ import com.google.common.base.Preconditions; +import javax.annotation.Nullable; + import static org.apache.hadoop.hdds.scm.events.SCMEvents.CLOSE_CONTAINER; /** @@ -51,6 +54,8 @@ public class DeadNodeHandler implements EventHandler { private final NodeManager nodeManager; private final PipelineManager pipelineManager; private final ContainerManager containerManager; + @Nullable + private final DeletedBlockLog deletedBlockLog; private static final Logger LOG = LoggerFactory.getLogger(DeadNodeHandler.class); @@ -58,9 +63,17 @@ public class DeadNodeHandler implements EventHandler { public DeadNodeHandler(final NodeManager nodeManager, final PipelineManager pipelineManager, final ContainerManager containerManager) { + this(nodeManager, pipelineManager, containerManager, null); + } + + public DeadNodeHandler(final NodeManager nodeManager, + final PipelineManager pipelineManager, + final ContainerManager containerManager, + @Nullable final DeletedBlockLog deletedBlockLog) { this.nodeManager = nodeManager; this.pipelineManager = pipelineManager; this.containerManager = containerManager; + this.deletedBlockLog = deletedBlockLog; } @Override @@ -95,6 +108,14 @@ public void onMessage(final DatanodeDetails datanodeDetails, LOG.info("Clearing command queue of size {} for DN {}", cmdList.size(), datanodeDetails); + // remove DeleteBlocksCommand associated with the dead node unless it + // is IN_MAINTENANCE + if (deletedBlockLog != null && + !nodeManager.getNodeStatus(datanodeDetails).isInMaintenance()) { + deletedBlockLog.getScmCommandStatusManager() + .onDatanodeDead(datanodeDetails.getUuid()); + } + //move dead datanode out of ClusterNetworkTopology NetworkTopology nt = nodeManager.getClusterNetworkTopologyMap(); if (nt.contains(datanodeDetails)) { diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java index 168fdd11a57b..cc6234648164 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java @@ -47,6 +47,8 @@ .StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.HddsTestUtils; +import org.apache.hadoop.hdds.scm.block.DeletedBlockLog; +import org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException; @@ -90,6 +92,7 @@ public class TestDeadNodeHandler { private EventQueue eventQueue; private String storageDir; private SCMContext scmContext; + private SCMDeleteBlocksCommandStatusManager deleteBlocksCommandStatusManager; @BeforeEach public void setup() throws IOException, AuthenticationException { @@ -117,8 +120,13 @@ public void setup() throws IOException, AuthenticationException { pipelineManager.setPipelineProvider(RATIS, mockRatisProvider); containerManager = scm.getContainerManager(); + DeletedBlockLog deletedBlockLog = Mockito.mock(DeletedBlockLog.class); + deleteBlocksCommandStatusManager = + Mockito.mock(SCMDeleteBlocksCommandStatusManager.class); + Mockito.when(deletedBlockLog.getScmCommandStatusManager()) + .thenReturn(deleteBlocksCommandStatusManager); deadNodeHandler = new DeadNodeHandler(nodeManager, - Mockito.mock(PipelineManager.class), containerManager); + Mockito.mock(PipelineManager.class), containerManager, deletedBlockLog); healthyReadOnlyNodeHandler = new HealthyReadOnlyNodeHandler(nodeManager, pipelineManager); @@ -134,6 +142,7 @@ public void teardown() { } @Test + @SuppressWarnings("checkstyle:MethodLength") public void testOnMessage() throws Exception { //GIVEN DatanodeDetails datanode1 = MockDatanodeDetails.randomDatanodeDetails(); @@ -233,6 +242,10 @@ public void testOnMessage() throws Exception { Assertions.assertFalse( nodeManager.getClusterNetworkTopologyMap().contains(datanode1)); + Mockito.verify(deleteBlocksCommandStatusManager, + Mockito.times(0)) + .onDatanodeDead(datanode1.getUuid()); + Set container1Replicas = containerManager .getContainerReplicas(ContainerID.valueOf(container1.getContainerID())); Assertions.assertEquals(2, container1Replicas.size()); @@ -260,6 +273,10 @@ public void testOnMessage() throws Exception { Assertions.assertEquals(0, nodeManager.getCommandQueueCount(datanode1.getUuid(), cmd.getType())); + Mockito.verify(deleteBlocksCommandStatusManager, + Mockito.times(1)) + .onDatanodeDead(datanode1.getUuid()); + container1Replicas = containerManager .getContainerReplicas(ContainerID.valueOf(container1.getContainerID())); Assertions.assertEquals(1, container1Replicas.size()); From 391dca9669ae7519ca23cabff171955b2d4aef03 Mon Sep 17 00:00:00 2001 From: xichen01 Date: Sun, 16 Jul 2023 03:14:22 +0800 Subject: [PATCH 11/23] Merge transactionToDNsCommitMap to SCMDeleteBlocksCommandStatusManager --- .../hdds/scm/block/DeletedBlockLog.java | 23 -- .../hdds/scm/block/DeletedBlockLogImpl.java | 190 +-------------- .../SCMDeleteBlocksCommandStatusManager.java | 218 +++++++++++++++++- .../scm/server/StorageContainerManager.java | 3 +- .../hdds/scm/block/TestDeletedBlockLog.java | 8 +- ...stSCMDeleteBlocksCommandStatusManager.java | 10 +- 6 files changed, 237 insertions(+), 215 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java index 523d86cd8365..f4226ec84664 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java @@ -19,10 +19,6 @@ import java.util.Set; 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 - .DeleteBlockTransactionResult; import org.apache.hadoop.hdds.protocol.proto .StorageContainerDatanodeProtocolProtos.DeletedBlocksTransaction; import org.apache.hadoop.hdds.utils.db.Table; @@ -31,7 +27,6 @@ import java.io.IOException; import java.util.List; import java.util.Map; -import java.util.UUID; /** * The DeletedBlockLog is a persisted log in SCM to keep tracking @@ -88,24 +83,6 @@ void incrementCount(List txIDs) */ int resetCount(List txIDs) throws IOException; - /** - * Commits a transaction means to delete all footprints of a transaction - * from the log. This method doesn't guarantee all transactions can be - * successfully deleted, it tolerate failures and tries best efforts to. - * @param transactionResults - delete block transaction results. - * @param dnID - ID of datanode which acknowledges the delete block command. - */ - void commitTransactions(List transactionResults, - UUID dnID); - - /** - * Commits DeleteBlocksCommand to update the DeleteBlocksCommand status - * by SCMDeleteBlocksCommandStatusManager. - * @param deleteBlockStatus the list of DeleteBlocksCommand - * @param dnID - */ - void commitSCMCommandStatus(List deleteBlockStatus, UUID dnID); - /** * Get ScmDeleteBlocksCommandStatusManager. * @return an Object of ScmDeleteBlocksCommandStatusManager diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java index 2cd2341091cb..0bd3690d8cd1 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java @@ -19,9 +19,7 @@ import java.io.IOException; import java.time.Duration; -import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; import java.util.UUID; import java.util.Set; @@ -35,12 +33,8 @@ import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; -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.protocol.proto.StorageContainerDatanodeProtocolProtos.CommandStatus; -import org.apache.hadoop.hdds.scm.command.CommandStatusReportHandler.DeleteBlockStatus; -import org.apache.hadoop.hdds.scm.container.ContainerInfo; import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException; @@ -49,13 +43,10 @@ import org.apache.hadoop.hdds.scm.ha.SCMRatisServer; import org.apache.hadoop.hdds.scm.ha.SequenceIdGenerator; import org.apache.hadoop.hdds.scm.metadata.DBTransactionBuffer; -import org.apache.hadoop.hdds.server.events.EventHandler; -import org.apache.hadoop.hdds.server.events.EventPublisher; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.TableIterator; import com.google.common.collect.Lists; -import static java.lang.Math.min; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_BLOCK_DELETION_MAX_RETRY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_BLOCK_DELETION_MAX_RETRY_DEFAULT; import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus; @@ -74,8 +65,7 @@ * equally same chance to be retrieved which only depends on the nature * order of the transaction ID. */ -public class DeletedBlockLogImpl - implements DeletedBlockLog, EventHandler { +public class DeletedBlockLogImpl implements DeletedBlockLog { public static final Logger LOG = LoggerFactory.getLogger(DeletedBlockLogImpl.class); @@ -83,9 +73,6 @@ public class DeletedBlockLogImpl private final int maxRetry; private final ContainerManager containerManager; private final Lock lock; - // Maps txId to set of DNs which are successful in committing the transaction - private final Map> transactionToDNsCommitMap; - // Maps txId to its retry counts; private final Map transactionToRetryCountMap; // The access to DeletedBlocksTXTable is protected by // DeletedBlockLogStateManager. @@ -113,14 +100,7 @@ public DeletedBlockLogImpl(ConfigurationSource conf, this.containerManager = containerManager; this.lock = new ReentrantLock(); - // transactionToDNsCommitMap is updated only when - // transaction is added to the log and when it is removed. - - // maps transaction to dns which have committed it. - transactionToDNsCommitMap = new ConcurrentHashMap<>(); transactionToRetryCountMap = new ConcurrentHashMap<>(); - this.scmDeleteBlocksCommandStatusManager = - new SCMDeleteBlocksCommandStatusManager(); this.deletedBlockLogStateManager = DeletedBlockLogStateManagerImpl .newBuilder() .setConfiguration(conf) @@ -132,6 +112,9 @@ public DeletedBlockLogImpl(ConfigurationSource conf, this.scmContext = scmContext; this.sequenceIdGen = sequenceIdGen; this.metrics = metrics; + this.scmDeleteBlocksCommandStatusManager = + new SCMDeleteBlocksCommandStatusManager(deletedBlockLogStateManager, + metrics, containerManager, lock, scmContext, scmCommandTimeoutMs); } @Override @@ -238,120 +221,6 @@ public SCMDeleteBlocksCommandStatusManager getScmCommandStatusManager() { return scmDeleteBlocksCommandStatusManager; } - @Override - public void commitSCMCommandStatus( - List deleteBlockStatus, UUID dnID) { - lock.lock(); - try { - processSCMCommandStatus(deleteBlockStatus, dnID); - getScmCommandStatusManager().cleanSCMCommandForDn( - dnID, scmCommandTimeoutMs); - } finally { - lock.unlock(); - } - } - - private void processSCMCommandStatus( - List deleteBlockStatus, UUID dnID) { - Map lastStatus = new HashMap<>(); - Map summary = new HashMap<>(); - - // The CommandStatus is ordered in the report. So we can focus only on the - // last status in the command report. - deleteBlockStatus.forEach(cmdStatus -> { - lastStatus.put(cmdStatus.getCmdId(), cmdStatus); - summary.put(cmdStatus.getCmdId(), cmdStatus.getStatus()); - }); - LOG.debug("CommandStatus {} from Datanode {} ", summary, dnID); - for (Map.Entry entry : lastStatus.entrySet()) { - scmDeleteBlocksCommandStatusManager.updateStatusByDNCommandStatus( - dnID, entry.getKey(), entry.getValue().getStatus()); - } - } - - /** - * {@inheritDoc} - * - * @param transactionResults - transaction IDs. - * @param dnID - Id of Datanode which has acknowledged - * a delete block command. - * @throws IOException - */ - @Override - public void commitTransactions( - List transactionResults, UUID dnID) { - lock.lock(); - try { - ArrayList txIDsToBeDeleted = new ArrayList<>(); - Set dnsWithCommittedTxn; - for (DeleteBlockTransactionResult transactionResult : - transactionResults) { - if (isTransactionFailed(transactionResult)) { - metrics.incrBlockDeletionTransactionFailure(); - continue; - } - try { - metrics.incrBlockDeletionTransactionSuccess(); - long txID = transactionResult.getTxID(); - // set of dns which have successfully committed transaction txId. - dnsWithCommittedTxn = transactionToDNsCommitMap.get(txID); - final ContainerID containerId = ContainerID.valueOf( - transactionResult.getContainerID()); - if (dnsWithCommittedTxn == null) { - // Mostly likely it's a retried delete command response. - if (LOG.isDebugEnabled()) { - LOG.debug( - "Transaction txId={} commit by dnId={} for containerID={}" - + " failed. Corresponding entry not found.", txID, dnID, - containerId); - } - continue; - } - - dnsWithCommittedTxn.add(dnID); - final ContainerInfo container = - containerManager.getContainer(containerId); - final Set replicas = - containerManager.getContainerReplicas(containerId); - // The delete entry can be safely removed from the log if all the - // corresponding nodes commit the txn. It is required to check that - // the nodes returned in the pipeline match the replication factor. - if (min(replicas.size(), dnsWithCommittedTxn.size()) - >= container.getReplicationConfig().getRequiredNodes()) { - List containerDns = replicas.stream() - .map(ContainerReplica::getDatanodeDetails) - .map(DatanodeDetails::getUuid) - .collect(Collectors.toList()); - if (dnsWithCommittedTxn.containsAll(containerDns)) { - transactionToDNsCommitMap.remove(txID); - transactionToRetryCountMap.remove(txID); - if (LOG.isDebugEnabled()) { - LOG.debug("Purging txId={} from block deletion log", txID); - } - txIDsToBeDeleted.add(txID); - } - } - if (LOG.isDebugEnabled()) { - LOG.debug("Datanode txId={} containerId={} committed by dnId={}", - txID, containerId, dnID); - } - } catch (IOException e) { - LOG.warn("Could not commit delete block transaction: " + - transactionResult.getTxID(), e); - } - } - try { - deletedBlockLogStateManager.removeTransactionsFromDB(txIDsToBeDeleted); - metrics.incrBlockDeletionTransactionCompleted(txIDsToBeDeleted.size()); - } catch (IOException e) { - LOG.warn("Could not commit delete block transactions: " - + txIDsToBeDeleted, e); - } - } finally { - lock.unlock(); - } - } - private boolean isTransactionFailed(DeleteBlockTransactionResult result) { if (LOG.isDebugEnabled()) { LOG.debug( @@ -390,7 +259,7 @@ public int getNumOfValidTransactions() throws IOException { @Override public void reinitialize( Table deletedTable) { - // we don't need handle transactionToDNsCommitMap and + // we don't need to handle ScmDeleteBlocksCommandStatusManager and // deletedBlockLogStateManager, since they will be cleared // when becoming leader. deletedBlockLogStateManager.reinitialize(deletedTable); @@ -401,7 +270,6 @@ public void reinitialize( * leader. */ public void onBecomeLeader() { - transactionToDNsCommitMap.clear(); transactionToRetryCountMap.clear(); scmDeleteBlocksCommandStatusManager.clear(); } @@ -476,18 +344,10 @@ private boolean shouldAddTransactionToDN(DatanodeDetails dnDetail, long tx, if (!dnLists.contains(dnDetail)) { return false; } - if (inProcessing(dnDetail.getUuid(), tx, commandStatus)) { - return false; - } - Set dnsWithTransactionCommitted = - transactionToDNsCommitMap.get(tx); - if (dnsWithTransactionCommitted != null && dnsWithTransactionCommitted - .contains(dnDetail.getUuid())) { - // Transaction need not be sent to dns which have - // already committed it + if (getScmCommandStatusManager().alreadyExecuted(dnDetail.getUuid(), tx)) { return false; } - return true; + return !inProcessing(dnDetail.getUuid(), tx, commandStatus); } private boolean inProcessing(UUID dnId, long deletedBlocksTxId, @@ -524,7 +384,8 @@ public DatanodeDeletedBlockTransactions getTransactions( ArrayList txIDs = new ArrayList<>(); // Here takes block replica count as the threshold to avoid the case // that part of replicas committed the TXN and recorded in the - // transactionToDNsCommitMap, while they are counted in the threshold. + // ScmDeleteBlocksCommandStatusManager, while they are counted in the + // threshold. while (iter.hasNext() && transactions.getBlocksDeleted() < blockDeletionLimit) { Table.KeyValue keyValue = iter.next(); @@ -540,8 +401,8 @@ public DatanodeDeletedBlockTransactions getTransactions( } else if (txn.getCount() > -1 && txn.getCount() <= maxRetry && !containerManager.getContainer(id).isOpen()) { getTransaction(txn, transactions, dnList, commandStatus); - transactionToDNsCommitMap - .putIfAbsent(txn.getTxID(), new LinkedHashSet<>()); + getScmCommandStatusManager(). + recordTransactionToDNsCommitMap(txn.getTxID()); } } catch (ContainerNotFoundException ex) { LOG.warn("Container: " + id + " was not found for the transaction: " @@ -560,35 +421,6 @@ public DatanodeDeletedBlockTransactions getTransactions( } } - @Override - public void onMessage( - DeleteBlockStatus deleteBlockStatus, EventPublisher publisher) { - if (!scmContext.isLeader()) { - LOG.warn("Skip commit transactions since current SCM is not leader."); - return; - } - DatanodeDetails details = deleteBlockStatus.getDatanodeDetails(); - - for (CommandStatus commandStatus : deleteBlockStatus.getCmdStatus()) { - CommandStatus.Status status = commandStatus.getStatus(); - if (status == CommandStatus.Status.EXECUTED) { - ContainerBlocksDeletionACKProto ackProto = - commandStatus.getBlockDeletionAck(); - commitTransactions(ackProto.getResultsList(), - UUID.fromString(ackProto.getDnId())); - metrics.incrBlockDeletionCommandSuccess(); - } else if (status == CommandStatus.Status.FAILED) { - metrics.incrBlockDeletionCommandFailure(); - } else { - LOG.debug("Delete Block Command {} is not executed on the Datanode {}.", - commandStatus.getCmdId(), details.getUuid()); - } - - commitSCMCommandStatus( - deleteBlockStatus.getCmdStatus(), details.getUuid()); - } - } - public void setScmCommandTimeoutMs(long scmCommandTimeoutMs) { this.scmCommandTimeoutMs = scmCommandTimeoutMs; } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java index 307ca9cb5679..ad107ef3bb3d 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java @@ -20,21 +20,39 @@ package org.apache.hadoop.hdds.scm.block; import com.google.common.annotations.VisibleForTesting; +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.scm.command.CommandStatusReportHandler.DeleteBlockStatus; +import org.apache.hadoop.hdds.scm.container.ContainerID; +import org.apache.hadoop.hdds.scm.container.ContainerInfo; +import org.apache.hadoop.hdds.scm.container.ContainerManager; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; +import org.apache.hadoop.hdds.scm.ha.SCMContext; +import org.apache.hadoop.hdds.server.events.EventHandler; +import org.apache.hadoop.hdds.server.events.EventPublisher; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.IOException; import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.stream.Collectors; +import static java.lang.Math.min; import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.EXECUTED; import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.NEED_RESEND; import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.PENDING_EXECUTED; @@ -44,10 +62,22 @@ /** * SCM DeleteBlocksCommand manager. */ -public class SCMDeleteBlocksCommandStatusManager { +public class SCMDeleteBlocksCommandStatusManager + implements EventHandler { public static final Logger LOG = LoggerFactory.getLogger(SCMDeleteBlocksCommandStatusManager.class); + // Maps txId to set of DNs which are successful in committing the transaction + private final Map> transactionToDNsCommitMap; + // Maps txId to its retry counts; + private final Map transactionToRetryCountMap; + // The access to DeletedBlocksTXTable is protected by + // DeletedBlockLogStateManager. + private final DeletedBlockLogStateManager deletedBlockLogStateManager; + private final ContainerManager containerManager; + private final ScmBlockDeletingServiceMetrics metrics; + private final SCMContext scmContext; + /** * Status of SCMDeleteBlocksCommand. */ @@ -77,10 +107,25 @@ public enum CmdStatus { Arrays.asList(EXECUTED, NEED_RESEND)); private final Set failedStatuses = new HashSet<>( Arrays.asList(NEED_RESEND)); - + private int failureTransactionCount; + private Lock lock; + private long scmCommandTimeoutMs; private final Map> scmCmdStatusRecord; - public SCMDeleteBlocksCommandStatusManager() { + public SCMDeleteBlocksCommandStatusManager( + DeletedBlockLogStateManager deletedBlockLogStateManager, + ScmBlockDeletingServiceMetrics metrics, + ContainerManager containerManager, Lock lock, SCMContext scmContext, + long scmCommandTimeoutMs) { + // maps transaction to dns which have committed it. + this.deletedBlockLogStateManager = deletedBlockLogStateManager; + this.metrics = metrics; + this.containerManager = containerManager; + this.scmContext = scmContext; + this.lock = lock; + this.scmCommandTimeoutMs = scmCommandTimeoutMs; + transactionToDNsCommitMap = new ConcurrentHashMap<>(); + transactionToRetryCountMap = new ConcurrentHashMap<>(); this.scmCmdStatusRecord = new ConcurrentHashMap<>(); } @@ -104,10 +149,6 @@ private CmdStatusData( setStatus(DEFAULT_STATUS); } - public boolean isContainDeletedBlocksTx(long deletedBlocksTxId) { - return deletedBlocksTxIds.contains(deletedBlocksTxId); - } - public Set getDeletedBlocksTxIds() { return Collections.unmodifiableSet(deletedBlocksTxIds); } @@ -214,6 +255,7 @@ private void cleanTimeoutSCMCommand(UUID dnId, long timeoutMs) { public void clear() { scmCmdStatusRecord.clear(); + transactionToDNsCommitMap.clear(); } private void cleanFinalStatusSCMCommand(UUID dnId) { @@ -375,6 +417,168 @@ private static CmdStatus fromProtoCommandStatus( } } + public void recordTransactionToDNsCommitMap(long txId) { + transactionToDNsCommitMap + .putIfAbsent(txId, new LinkedHashSet<>()); + } + + public boolean alreadyExecuted(UUID dnId, long txId) { + Set dnsWithTransactionCommitted = + transactionToDNsCommitMap.get(txId); + return dnsWithTransactionCommitted != null && dnsWithTransactionCommitted + .contains(dnId); + } + + /** + * Commits a transaction means to delete all footprints of a transaction + * from the log. This method doesn't guarantee all transactions can be + * successfully deleted, it tolerate failures and tries best efforts to. + * @param transactionResults - delete block transaction results. + * @param dnId - ID of datanode which acknowledges the delete block command. + */ + public void commitTransactions( + List transactionResults, UUID dnId) { + + ArrayList txIDsToBeDeleted = new ArrayList<>(); + Set dnsWithCommittedTxn; + for (DeleteBlockTransactionResult transactionResult : + transactionResults) { + if (isTransactionFailed(transactionResult)) { + metrics.incrBlockDeletionTransactionFailure(); + continue; + } + try { + metrics.incrBlockDeletionTransactionSuccess(); + long txID = transactionResult.getTxID(); + // set of dns which have successfully committed transaction txId. + dnsWithCommittedTxn = transactionToDNsCommitMap.get(txID); + final ContainerID containerId = ContainerID.valueOf( + transactionResult.getContainerID()); + if (dnsWithCommittedTxn == null) { + // Mostly likely it's a retried delete command response. + if (LOG.isDebugEnabled()) { + LOG.debug( + "Transaction txId={} commit by dnId={} for containerID={}" + + " failed. Corresponding entry not found.", txID, dnId, + containerId); + } + continue; + } + + dnsWithCommittedTxn.add(dnId); + final ContainerInfo container = + containerManager.getContainer(containerId); + final Set replicas = + containerManager.getContainerReplicas(containerId); + // The delete entry can be safely removed from the log if all the + // corresponding nodes commit the txn. It is required to check that + // the nodes returned in the pipeline match the replication factor. + if (min(replicas.size(), dnsWithCommittedTxn.size()) + >= container.getReplicationConfig().getRequiredNodes()) { + List containerDns = replicas.stream() + .map(ContainerReplica::getDatanodeDetails) + .map(DatanodeDetails::getUuid) + .collect(Collectors.toList()); + if (dnsWithCommittedTxn.containsAll(containerDns)) { + transactionToDNsCommitMap.remove(txID); + transactionToRetryCountMap.remove(txID); + if (LOG.isDebugEnabled()) { + LOG.debug("Purging txId={} from block deletion log", txID); + } + txIDsToBeDeleted.add(txID); + } + } + if (LOG.isDebugEnabled()) { + LOG.debug("Datanode txId={} containerId={} committed by dnId={}", + txID, containerId, dnId); + } + } catch (IOException e) { + LOG.warn("Could not commit delete block transaction: " + + transactionResult.getTxID(), e); + } + } + try { + deletedBlockLogStateManager.removeTransactionsFromDB(txIDsToBeDeleted); + metrics.incrBlockDeletionTransactionCompleted(txIDsToBeDeleted.size()); + } catch (IOException e) { + LOG.warn("Could not commit delete block transactions: " + + txIDsToBeDeleted, e); + } + } + + @Override + public void onMessage( + DeleteBlockStatus deleteBlockStatus, EventPublisher publisher) { + if (!scmContext.isLeader()) { + LOG.warn("Skip commit transactions since current SCM is not leader."); + return; + } + + DatanodeDetails details = deleteBlockStatus.getDatanodeDetails(); + UUID dnId = details.getUuid(); + for (CommandStatus commandStatus : deleteBlockStatus.getCmdStatus()) { + CommandStatus.Status status = commandStatus.getStatus(); + lock.lock(); + try { + if (status == CommandStatus.Status.EXECUTED) { + ContainerBlocksDeletionACKProto ackProto = + commandStatus.getBlockDeletionAck(); + commitTransactions(ackProto.getResultsList(), dnId); + metrics.incrBlockDeletionCommandSuccess(); + } else if (status == CommandStatus.Status.FAILED) { + metrics.incrBlockDeletionCommandFailure(); + } else { + LOG.debug("Delete Block Command {} is not executed on the Datanode" + + " {}.", commandStatus.getCmdId(), dnId); + } + + commitSCMCommandStatus(deleteBlockStatus.getCmdStatus(), dnId); + } finally { + lock.unlock(); + } + } + } + + public void commitSCMCommandStatus(List deleteBlockStatus, + UUID dnId) { + processSCMCommandStatus(deleteBlockStatus, dnId); + cleanSCMCommandForDn(dnId, scmCommandTimeoutMs); + } + + private void processSCMCommandStatus(List deleteBlockStatus, + UUID dnID) { + Map lastStatus = new HashMap<>(); + Map summary = new HashMap<>(); + + // The CommandStatus is ordered in the report. So we can focus only on the + // last status in the command report. + deleteBlockStatus.forEach(cmdStatus -> { + lastStatus.put(cmdStatus.getCmdId(), cmdStatus); + summary.put(cmdStatus.getCmdId(), cmdStatus.getStatus()); + }); + LOG.debug("CommandStatus {} from Datanode {} ", summary, dnID); + for (Map.Entry entry : lastStatus.entrySet()) { + CommandStatus.Status status = entry.getValue().getStatus(); + updateStatusByDNCommandStatus( + dnID, entry.getKey(), status); + + } + } + + 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; + } + @VisibleForTesting Map> getScmCmdStatusRecord() { return scmCmdStatusRecord; diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index b3985c854ff5..f5a626d4f443 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -96,7 +96,6 @@ import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.block.BlockManager; import org.apache.hadoop.hdds.scm.block.BlockManagerImpl; -import org.apache.hadoop.hdds.scm.block.DeletedBlockLogImpl; import org.apache.hadoop.hdds.scm.command.CommandStatusReportHandler; import org.apache.hadoop.hdds.scm.container.CloseContainerEventHandler; import org.apache.hadoop.hdds.scm.container.ContainerActionsHandler; @@ -558,7 +557,7 @@ private void initializeEventHandlers() { datanodeStartAdminHandler); eventQueue.addHandler(SCMEvents.CMD_STATUS_REPORT, cmdStatusReportHandler); eventQueue.addHandler(SCMEvents.DELETE_BLOCK_STATUS, - (DeletedBlockLogImpl) scmBlockManager.getDeletedBlockLog()); + scmBlockManager.getDeletedBlockLog().getScmCommandStatusManager()); eventQueue.addHandler(SCMEvents.PIPELINE_ACTIONS, pipelineActionHandler); eventQueue.addHandler(SCMEvents.PIPELINE_REPORT, pipelineReportHandler); eventQueue.addHandler(SCMEvents.CRL_STATUS_REPORT, crlStatusReportHandler); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java index 27b3bd7eed14..ad2e14f50c24 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java @@ -250,7 +250,7 @@ private void commitTransactions( List transactionResults, DatanodeDetails... dns) throws IOException { for (DatanodeDetails dnDetails : dns) { - deletedBlockLog + deletedBlockLog.getScmCommandStatusManager() .commitTransactions(transactionResults, dnDetails.getUuid()); } scmHADBTransactionBuffer.flush(); @@ -282,7 +282,8 @@ private void commitTransactions( private void commitTransactions(DatanodeDeletedBlockTransactions transactions) { transactions.getDatanodeTransactionMap().forEach((uuid, - deletedBlocksTransactions) -> deletedBlockLog + deletedBlocksTransactions) -> + deletedBlockLog.getScmCommandStatusManager() .commitTransactions(deletedBlocksTransactions.stream() .map(this::createDeleteBlockTransactionResult) .collect(Collectors.toList()), uuid)); @@ -513,7 +514,8 @@ private void commitSCMCommandStatus(Long scmCmdId, UUID dnID, .build() .getProtoBufMessage()); - deletedBlockLog.commitSCMCommandStatus(deleteBlockStatus, dnID); + deletedBlockLog.getScmCommandStatusManager() + .commitSCMCommandStatus(deleteBlockStatus, dnID); } private void createDeleteBlocksCommandAndAction( diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java index 689a6c2b59d5..ee6ac1830238 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java @@ -19,8 +19,11 @@ package org.apache.hadoop.hdds.scm.block; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos; +import org.apache.hadoop.hdds.scm.container.ContainerManager; +import org.apache.hadoop.hdds.scm.ha.SCMContext; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.BeforeEach; +import org.mockito.Mockito; import java.util.Arrays; import java.util.HashSet; @@ -28,6 +31,7 @@ import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.concurrent.locks.Lock; import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.NEED_RESEND; import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.PENDING_EXECUTED; @@ -58,7 +62,11 @@ public class TestSCMDeleteBlocksCommandStatusManager { @BeforeEach public void setup() throws Exception { - manager = new SCMDeleteBlocksCommandStatusManager(); + manager = new SCMDeleteBlocksCommandStatusManager( + Mockito.mock(DeletedBlockLogStateManager.class), + Mockito.mock(ScmBlockDeletingServiceMetrics.class), + Mockito.mock(ContainerManager.class), Mockito.mock(Lock.class), + Mockito.mock(SCMContext.class), 300); // Create test data dnId1 = UUID.randomUUID(); dnId2 = UUID.randomUUID(); From f792ccc0934b3a645ce6dfc858505372c46f319b Mon Sep 17 00:00:00 2001 From: xichen01 Date: Mon, 17 Jul 2023 00:32:43 +0800 Subject: [PATCH 12/23] Split TestSCMDeleteBlocksCommandStatusManager and TestSCMDeleteBlocksCommandStatusManager --- .../hdds/scm/block/DeletedBlockLog.java | 7 +- .../hdds/scm/block/DeletedBlockLogImpl.java | 66 +- .../scm/block/SCMBlockDeletingService.java | 5 +- .../SCMDeleteBlocksCommandStatusManager.java | 586 ---------------- ...MDeletedBlockTransactionStatusManager.java | 659 ++++++++++++++++++ .../hadoop/hdds/scm/node/DeadNodeHandler.java | 2 +- .../hadoop/hdds/scm/node/SCMNodeManager.java | 2 +- .../scm/server/StorageContainerManager.java | 4 +- .../hdds/scm/block/TestDeletedBlockLog.java | 13 +- ...stSCMDeleteBlocksCommandStatusManager.java | 25 +- .../hdds/scm/node/TestDeadNodeHandler.java | 9 +- 11 files changed, 714 insertions(+), 664 deletions(-) delete mode 100644 hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java create mode 100644 hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java index f4226ec84664..be67f05cd893 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java @@ -84,10 +84,11 @@ void incrementCount(List txIDs) int resetCount(List txIDs) throws IOException; /** - * Get ScmDeleteBlocksCommandStatusManager. - * @return an Object of ScmDeleteBlocksCommandStatusManager + * Get SCMDeletedBlockTransactionStatusManager. + * @return an Object of SCMDeletedBlockTransactionStatusManager */ - SCMDeleteBlocksCommandStatusManager getScmCommandStatusManager(); + SCMDeletedBlockTransactionStatusManager + getSCMDeletedBlockTransactionStatusManager(); /** * Creates block deletion transactions for a set of containers, diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java index 0bd3690d8cd1..6457adc35dac 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java @@ -49,7 +49,7 @@ import com.google.common.collect.Lists; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_BLOCK_DELETION_MAX_RETRY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_BLOCK_DELETION_MAX_RETRY_DEFAULT; -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus; import static org.apache.hadoop.hdds.scm.ha.SequenceIdGenerator.DEL_TXN_ID; import org.slf4j.Logger; @@ -80,8 +80,8 @@ public class DeletedBlockLogImpl implements DeletedBlockLog { private final SCMContext scmContext; private final SequenceIdGenerator sequenceIdGen; private final ScmBlockDeletingServiceMetrics metrics; - private final SCMDeleteBlocksCommandStatusManager - scmDeleteBlocksCommandStatusManager; + private final SCMDeletedBlockTransactionStatusManager + transactionStatusManager; private long scmCommandTimeoutMs = Duration.ofSeconds(300).toMillis(); private static final int LIST_ALL_FAILED_TRANSACTIONS = -1; @@ -112,9 +112,11 @@ public DeletedBlockLogImpl(ConfigurationSource conf, this.scmContext = scmContext; this.sequenceIdGen = sequenceIdGen; this.metrics = metrics; - this.scmDeleteBlocksCommandStatusManager = - new SCMDeleteBlocksCommandStatusManager(deletedBlockLogStateManager, - metrics, containerManager, lock, scmContext, scmCommandTimeoutMs); + this.transactionStatusManager = + new SCMDeletedBlockTransactionStatusManager(deletedBlockLogStateManager, + containerManager, scmContext, transactionToRetryCountMap, metrics, + lock, + scmCommandTimeoutMs); } @Override @@ -217,8 +219,9 @@ private DeletedBlocksTransaction constructNewTransaction( } @Override - public SCMDeleteBlocksCommandStatusManager getScmCommandStatusManager() { - return scmDeleteBlocksCommandStatusManager; + public SCMDeletedBlockTransactionStatusManager + getSCMDeletedBlockTransactionStatusManager() { + return transactionStatusManager; } private boolean isTransactionFailed(DeleteBlockTransactionResult result) { @@ -259,7 +262,7 @@ public int getNumOfValidTransactions() throws IOException { @Override public void reinitialize( Table deletedTable) { - // we don't need to handle ScmDeleteBlocksCommandStatusManager and + // we don't need to handle SCMDeletedBlockTransactionStatusManager and // deletedBlockLogStateManager, since they will be cleared // when becoming leader. deletedBlockLogStateManager.reinitialize(deletedTable); @@ -271,7 +274,7 @@ public void reinitialize( */ public void onBecomeLeader() { transactionToRetryCountMap.clear(); - scmDeleteBlocksCommandStatusManager.clear(); + transactionStatusManager.clear(); } /** @@ -328,8 +331,11 @@ private void getTransaction( ContainerID.valueOf(updatedTxn.getContainerID())); for (ContainerReplica replica : replicas) { DatanodeDetails details = replica.getDatanodeDetails(); - if (shouldAddTransactionToDN(details, - updatedTxn.getTxID(), dnList, commandStatus)) { + if (!dnList.contains(details)) { + continue; + } + if (!transactionStatusManager.isDuplication( + details, updatedTxn.getTxID(), commandStatus)) { transactions.addTransactionToDN(details.getUuid(), updatedTxn); } } @@ -338,29 +344,6 @@ private void getTransaction( } } - private boolean shouldAddTransactionToDN(DatanodeDetails dnDetail, long tx, - Set dnLists, - Map> commandStatus) { - if (!dnLists.contains(dnDetail)) { - return false; - } - if (getScmCommandStatusManager().alreadyExecuted(dnDetail.getUuid(), tx)) { - return false; - } - return !inProcessing(dnDetail.getUuid(), tx, commandStatus); - } - - private boolean inProcessing(UUID dnId, long deletedBlocksTxId, - Map> commandStatus) { - Map deletedBlocksTxStatus = commandStatus.get(dnId); - if (deletedBlocksTxStatus == null || - deletedBlocksTxStatus.get(deletedBlocksTxId) == null) { - return false; - } - return deletedBlocksTxStatus.get(deletedBlocksTxId) != - CmdStatus.NEED_RESEND; - } - @Override public DatanodeDeletedBlockTransactions getTransactions( int blockDeletionLimit, Set dnList) @@ -369,7 +352,7 @@ public DatanodeDeletedBlockTransactions getTransactions( try { // Here we can clean up the Datanode timeout command that no longer // reports heartbeats - getScmCommandStatusManager().cleanAllTimeoutSCMCommand( + getSCMDeletedBlockTransactionStatusManager().cleanAllTimeoutSCMCommand( scmCommandTimeoutMs); DatanodeDeletedBlockTransactions transactions = new DatanodeDeletedBlockTransactions(); @@ -379,13 +362,14 @@ public DatanodeDeletedBlockTransactions getTransactions( // Get the CmdStatus status of the aggregation, so that the current // status of the specified transaction can be found faster Map> commandStatus = - getScmCommandStatusManager().getCommandStatusByTxId(dnList.stream(). + getSCMDeletedBlockTransactionStatusManager() + .getCommandStatusByTxId(dnList.stream(). map(DatanodeDetails::getUuid).collect(Collectors.toSet())); ArrayList txIDs = new ArrayList<>(); // Here takes block replica count as the threshold to avoid the case // that part of replicas committed the TXN and recorded in the - // ScmDeleteBlocksCommandStatusManager, while they are counted in the - // threshold. + // SCMDeletedBlockTransactionStatusManager, while they are counted + // in the threshold. while (iter.hasNext() && transactions.getBlocksDeleted() < blockDeletionLimit) { Table.KeyValue keyValue = iter.next(); @@ -401,8 +385,8 @@ public DatanodeDeletedBlockTransactions getTransactions( } else if (txn.getCount() > -1 && txn.getCount() <= maxRetry && !containerManager.getContainer(id).isOpen()) { getTransaction(txn, transactions, dnList, commandStatus); - getScmCommandStatusManager(). - recordTransactionToDNsCommitMap(txn.getTxID()); + getSCMDeletedBlockTransactionStatusManager(). + recordTransactionCommitted(txn.getTxID()); } } catch (ContainerNotFoundException ex) { LOG.warn("Container: " + id + " was not found for the transaction: " diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java index 78b8e8d88712..1a9f8331c001 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java @@ -56,7 +56,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.createScmCmdStatusData; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_SERVICE_TIMEOUT; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_SERVICE_TIMEOUT_DEFAULT; @@ -184,8 +183,8 @@ public EmptyTaskResult call() throws Exception { command.setTerm(scmContext.getTermOfLeader()); eventPublisher.fireEvent(SCMEvents.DATANODE_COMMAND, new CommandForDatanode<>(dnId, command)); - deletedBlockLog.getScmCommandStatusManager().recordScmCommand( - createScmCmdStatusData(dnId, command.getId(), dnTxSet)); + deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() + .recordTransactionCreated(dnId, command.getId(), dnTxSet); metrics.incrBlockDeletionCommandSent(); metrics.incrBlockDeletionTransactionSent(dnTXs.size()); if (LOG.isDebugEnabled()) { diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java deleted file mode 100644 index ad107ef3bb3d..000000000000 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeleteBlocksCommandStatusManager.java +++ /dev/null @@ -1,586 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -package org.apache.hadoop.hdds.scm.block; - -import com.google.common.annotations.VisibleForTesting; -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.scm.command.CommandStatusReportHandler.DeleteBlockStatus; -import org.apache.hadoop.hdds.scm.container.ContainerID; -import org.apache.hadoop.hdds.scm.container.ContainerInfo; -import org.apache.hadoop.hdds.scm.container.ContainerManager; -import org.apache.hadoop.hdds.scm.container.ContainerReplica; -import org.apache.hadoop.hdds.scm.ha.SCMContext; -import org.apache.hadoop.hdds.server.events.EventHandler; -import org.apache.hadoop.hdds.server.events.EventPublisher; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.io.IOException; -import java.time.Duration; -import java.time.Instant; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.UUID; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.locks.Lock; -import java.util.stream.Collectors; - -import static java.lang.Math.min; -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.EXECUTED; -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.NEED_RESEND; -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.PENDING_EXECUTED; -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.SENT; -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.TO_BE_SENT; - -/** - * SCM DeleteBlocksCommand manager. - */ -public class SCMDeleteBlocksCommandStatusManager - implements EventHandler { - public static final Logger LOG = - LoggerFactory.getLogger(SCMDeleteBlocksCommandStatusManager.class); - - // Maps txId to set of DNs which are successful in committing the transaction - private final Map> transactionToDNsCommitMap; - // Maps txId to its retry counts; - private final Map transactionToRetryCountMap; - // The access to DeletedBlocksTXTable is protected by - // DeletedBlockLogStateManager. - private final DeletedBlockLogStateManager deletedBlockLogStateManager; - private final ContainerManager containerManager; - private final ScmBlockDeletingServiceMetrics metrics; - private final SCMContext scmContext; - - /** - * Status of SCMDeleteBlocksCommand. - */ - public enum CmdStatus { - // The DeleteBlocksCommand has not yet been sent. - // This is the initial status of the command after it's created. - TO_BE_SENT, - // This status indicates that the DeleteBlocksCommand has been sent - // to the DataNode, but the Datanode has not reported any new status - // for the DeleteBlocksCommand. - SENT, - // The DeleteBlocksCommand has been received by Datanode and - // is waiting for executed. - PENDING_EXECUTED, - // The DeleteBlocksCommand was executed, and the execution was successful - EXECUTED, - // The DeleteBlocksCommand was executed but failed to execute, - // or was lost before it was executed. - NEED_RESEND - } - - private static final CmdStatus DEFAULT_STATUS = TO_BE_SENT; - - private final Set statusesRequiringTimeout = new HashSet<>( - Arrays.asList(SENT, PENDING_EXECUTED)); - private final Set finialStatuses = new HashSet<>( - Arrays.asList(EXECUTED, NEED_RESEND)); - private final Set failedStatuses = new HashSet<>( - Arrays.asList(NEED_RESEND)); - private int failureTransactionCount; - private Lock lock; - private long scmCommandTimeoutMs; - private final Map> scmCmdStatusRecord; - - public SCMDeleteBlocksCommandStatusManager( - DeletedBlockLogStateManager deletedBlockLogStateManager, - ScmBlockDeletingServiceMetrics metrics, - ContainerManager containerManager, Lock lock, SCMContext scmContext, - long scmCommandTimeoutMs) { - // maps transaction to dns which have committed it. - this.deletedBlockLogStateManager = deletedBlockLogStateManager; - this.metrics = metrics; - this.containerManager = containerManager; - this.scmContext = scmContext; - this.lock = lock; - this.scmCommandTimeoutMs = scmCommandTimeoutMs; - transactionToDNsCommitMap = new ConcurrentHashMap<>(); - transactionToRetryCountMap = new ConcurrentHashMap<>(); - this.scmCmdStatusRecord = new ConcurrentHashMap<>(); - } - - public static CmdStatusData createScmCmdStatusData( - UUID dnId, long scmCmdId, Set deletedBlocksTxIds) { - return new CmdStatusData(dnId, scmCmdId, deletedBlocksTxIds); - } - - protected static final class CmdStatusData { - private final UUID dnId; - private final long scmCmdId; - private final Set deletedBlocksTxIds; - private Instant updateTime; - private CmdStatus status; - - private CmdStatusData( - UUID dnId, long scmTxID, Set deletedBlocksTxIds) { - this.dnId = dnId; - this.scmCmdId = scmTxID; - this.deletedBlocksTxIds = deletedBlocksTxIds; - setStatus(DEFAULT_STATUS); - } - - public Set getDeletedBlocksTxIds() { - return Collections.unmodifiableSet(deletedBlocksTxIds); - } - - public UUID getDnId() { - return dnId; - } - - public long getScmCmdId() { - return scmCmdId; - } - - public CmdStatus getStatus() { - return status; - } - - public void setStatus(CmdStatus status) { - this.updateTime = Instant.now(); - this.status = status; - } - - public Instant getUpdateTime() { - return updateTime; - } - - @Override - public String toString() { - return "ScmTxStateMachine" + - "{dnId=" + dnId + - ", scmTxID=" + scmCmdId + - ", deletedBlocksTxIds=" + deletedBlocksTxIds + - ", updateTime=" + updateTime + - ", status=" + status + - '}'; - } - } - - public void recordScmCommand(CmdStatusData statusData) { - LOG.debug("Record ScmCommand: {}", statusData); - scmCmdStatusRecord.computeIfAbsent(statusData.getDnId(), k -> - new ConcurrentHashMap<>()).put(statusData.getScmCmdId(), statusData); - } - - public Map> getCommandStatusByTxId( - Set dnIds) { - Map> result = - new HashMap<>(scmCmdStatusRecord.size()); - - for (UUID dnId : dnIds) { - Map record = scmCmdStatusRecord.get(dnId); - if (record == null) { - continue; - } - Map dnStatusMap = new HashMap<>(); - for (CmdStatusData statusData : record.values()) { - CmdStatus status = statusData.getStatus(); - for (Long deletedBlocksTxId : statusData.getDeletedBlocksTxIds()) { - dnStatusMap.put(deletedBlocksTxId, status); - } - } - result.put(dnId, dnStatusMap); - } - - return result; - } - - public void onSent(UUID dnId, long scmCmdId) { - updateStatus(dnId, scmCmdId, SENT); - } - - public void onDatanodeDead(UUID dnId) { - LOG.info("Clean SCMCommand record for DN: {}", dnId); - scmCmdStatusRecord.remove(dnId); - } - - public void updateStatusByDNCommandStatus(UUID dnId, long scmCmdId, - CommandStatus.Status newState) { - CmdStatus status = fromProtoCommandStatus(newState); - if (status != null) { - updateStatus(dnId, scmCmdId, status); - } - } - - public void cleanAllTimeoutSCMCommand(long timeoutMs) { - for (UUID dnId : scmCmdStatusRecord.keySet()) { - for (CmdStatus status : statusesRequiringTimeout) { - removeTimeoutScmCommand( - dnId, getScmCommandIds(dnId, status), timeoutMs); - } - } - } - - public void cleanSCMCommandForDn(UUID dnId, long timeoutMs) { - cleanTimeoutSCMCommand(dnId, timeoutMs); - cleanFinalStatusSCMCommand(dnId); - } - - private void cleanTimeoutSCMCommand(UUID dnId, long timeoutMs) { - for (CmdStatus status : statusesRequiringTimeout) { - removeTimeoutScmCommand( - dnId, getScmCommandIds(dnId, status), timeoutMs); - } - } - - public void clear() { - scmCmdStatusRecord.clear(); - transactionToDNsCommitMap.clear(); - } - - private void cleanFinalStatusSCMCommand(UUID dnId) { - for (CmdStatus status : finialStatuses) { - for (Long scmCmdId : getScmCommandIds(dnId, status)) { - CmdStatusData stateData = removeScmCommand(dnId, scmCmdId); - LOG.debug("Clean SCMCommand status: {} for DN: {}, stateData: {}", - status, dnId, stateData); - } - } - } - - private Set getScmCommandIds(UUID dnId, CmdStatus status) { - Set scmCmdIds = new HashSet<>(); - Map record = scmCmdStatusRecord.get(dnId); - if (record == null) { - return scmCmdIds; - } - for (CmdStatusData statusData : record.values()) { - if (statusData.getStatus().equals(status)) { - scmCmdIds.add(statusData.getScmCmdId()); - } - } - return scmCmdIds; - } - - private Instant getUpdateTime(UUID dnId, long scmCmdId) { - Map record = scmCmdStatusRecord.get(dnId); - if (record == null || record.get(scmCmdId) == null) { - return null; - } - return record.get(scmCmdId).getUpdateTime(); - } - - private void updateStatus(UUID dnId, long scmCmdId, CmdStatus newStatus) { - Map recordForDn = scmCmdStatusRecord.get(dnId); - if (recordForDn == null) { - LOG.warn("Unknown Datanode: {} scmCmdId {} newStatus {}", - dnId, scmCmdId, newStatus); - return; - } - if (recordForDn.get(scmCmdId) == null) { - LOG.warn("Unknown SCM Command: {} Datanode {} newStatus {}", - scmCmdId, dnId, newStatus); - return; - } - - boolean changed = false; - CmdStatusData statusData = recordForDn.get(scmCmdId); - CmdStatus oldStatus = statusData.getStatus(); - - switch (newStatus) { - case SENT: - if (oldStatus == TO_BE_SENT) { - // TO_BE_SENT -> SENT: The DeleteBlocksCommand is sent by SCM, - // The follow-up status has not been updated by Datanode. - statusData.setStatus(SENT); - changed = true; - } - break; - case PENDING_EXECUTED: - if (oldStatus == SENT || oldStatus == PENDING_EXECUTED) { - // SENT -> PENDING_EXECUTED: The DeleteBlocksCommand is sent and - // received by the Datanode, but the command is not executed by the - // Datanode, the command is waiting to be executed. - - // PENDING_EXECUTED -> PENDING_EXECUTED: The DeleteBlocksCommand - // continues to wait to be executed by Datanode. - statusData.setStatus(PENDING_EXECUTED); - changed = true; - } - break; - case NEED_RESEND: - if (oldStatus == SENT || oldStatus == PENDING_EXECUTED) { - // SENT -> NEED_RESEND: The DeleteBlocksCommand is sent and lost before - // it is received by the DN. - - // PENDING_EXECUTED -> NEED_RESEND: The DeleteBlocksCommand waited for - // a while and was executed, but the execution failed;. - // Or the DeleteBlocksCommand was lost while waiting(such as the - // Datanode restart). - statusData.setStatus(NEED_RESEND); - changed = true; - } - break; - case EXECUTED: - if (oldStatus == SENT || oldStatus == PENDING_EXECUTED) { - // PENDING_EXECUTED -> EXECUTED: The Command waits for a period of - // time on the DN and is executed successfully. - - // SENT -> EXECUTED: The DeleteBlocksCommand has been sent to Datanode, - // executed by DN, and executed successfully. - statusData.setStatus(EXECUTED); - changed = true; - } - break; - default: - LOG.error("Can not update to Unknown new Status: {}", newStatus); - break; - } - if (!changed) { - LOG.warn("Cannot update illegal status for DN: {} ScmCommandId {} " + - "Status From {} to {}", dnId, scmCmdId, oldStatus, newStatus); - } else { - LOG.debug("Successful update DN: {} ScmCommandId {} Status From {} to {}", - dnId, scmCmdId, oldStatus, newStatus); - } - } - - private void removeTimeoutScmCommand(UUID dnId, - Set scmCmdIds, long timeoutMs) { - Instant now = Instant.now(); - for (Long scmCmdId : scmCmdIds) { - Instant updateTime = getUpdateTime(dnId, scmCmdId); - if (updateTime != null && - Duration.between(updateTime, now).toMillis() > timeoutMs) { - updateStatus(dnId, scmCmdId, NEED_RESEND); - CmdStatusData state = removeScmCommand(dnId, scmCmdId); - LOG.warn("Remove Timeout SCM BlockDeletionCommand {} for DN {} " + - "after without update {}ms}", state, dnId, timeoutMs); - } else { - LOG.warn("FFFFF Timeout SCM scmCmdIds {} for DN {} " + - "after without update {}ms}", scmCmdIds, dnId, timeoutMs); - } - } - } - - private CmdStatusData removeScmCommand(UUID dnId, long scmCmdId) { - Map record = scmCmdStatusRecord.get(dnId); - if (record == null || record.get(scmCmdId) == null) { - return null; - } - - CmdStatus status = record.get(scmCmdId).getStatus(); - if (!finialStatuses.contains(status)) { - LOG.error("Cannot Remove ScmCommand {} Non-final Status {} for DN: {}." + - " final Status {}", scmCmdId, status, dnId, finialStatuses); - return null; - } - - CmdStatusData statusData = record.remove(scmCmdId); - LOG.debug("Remove ScmCommand {} for DN: {} ", statusData, dnId); - return statusData; - } - - private static CmdStatus fromProtoCommandStatus( - CommandStatus.Status protoCmdStatus) { - switch (protoCmdStatus) { - case PENDING: - return CmdStatus.PENDING_EXECUTED; - case EXECUTED: - return CmdStatus.EXECUTED; - case FAILED: - return CmdStatus.NEED_RESEND; - default: - LOG.error("Unknown protoCmdStatus: {} cannot convert " + - "to ScmDeleteBlockCommandStatus", protoCmdStatus); - return null; - } - } - - public void recordTransactionToDNsCommitMap(long txId) { - transactionToDNsCommitMap - .putIfAbsent(txId, new LinkedHashSet<>()); - } - - public boolean alreadyExecuted(UUID dnId, long txId) { - Set dnsWithTransactionCommitted = - transactionToDNsCommitMap.get(txId); - return dnsWithTransactionCommitted != null && dnsWithTransactionCommitted - .contains(dnId); - } - - /** - * Commits a transaction means to delete all footprints of a transaction - * from the log. This method doesn't guarantee all transactions can be - * successfully deleted, it tolerate failures and tries best efforts to. - * @param transactionResults - delete block transaction results. - * @param dnId - ID of datanode which acknowledges the delete block command. - */ - public void commitTransactions( - List transactionResults, UUID dnId) { - - ArrayList txIDsToBeDeleted = new ArrayList<>(); - Set dnsWithCommittedTxn; - for (DeleteBlockTransactionResult transactionResult : - transactionResults) { - if (isTransactionFailed(transactionResult)) { - metrics.incrBlockDeletionTransactionFailure(); - continue; - } - try { - metrics.incrBlockDeletionTransactionSuccess(); - long txID = transactionResult.getTxID(); - // set of dns which have successfully committed transaction txId. - dnsWithCommittedTxn = transactionToDNsCommitMap.get(txID); - final ContainerID containerId = ContainerID.valueOf( - transactionResult.getContainerID()); - if (dnsWithCommittedTxn == null) { - // Mostly likely it's a retried delete command response. - if (LOG.isDebugEnabled()) { - LOG.debug( - "Transaction txId={} commit by dnId={} for containerID={}" - + " failed. Corresponding entry not found.", txID, dnId, - containerId); - } - continue; - } - - dnsWithCommittedTxn.add(dnId); - final ContainerInfo container = - containerManager.getContainer(containerId); - final Set replicas = - containerManager.getContainerReplicas(containerId); - // The delete entry can be safely removed from the log if all the - // corresponding nodes commit the txn. It is required to check that - // the nodes returned in the pipeline match the replication factor. - if (min(replicas.size(), dnsWithCommittedTxn.size()) - >= container.getReplicationConfig().getRequiredNodes()) { - List containerDns = replicas.stream() - .map(ContainerReplica::getDatanodeDetails) - .map(DatanodeDetails::getUuid) - .collect(Collectors.toList()); - if (dnsWithCommittedTxn.containsAll(containerDns)) { - transactionToDNsCommitMap.remove(txID); - transactionToRetryCountMap.remove(txID); - if (LOG.isDebugEnabled()) { - LOG.debug("Purging txId={} from block deletion log", txID); - } - txIDsToBeDeleted.add(txID); - } - } - if (LOG.isDebugEnabled()) { - LOG.debug("Datanode txId={} containerId={} committed by dnId={}", - txID, containerId, dnId); - } - } catch (IOException e) { - LOG.warn("Could not commit delete block transaction: " + - transactionResult.getTxID(), e); - } - } - try { - deletedBlockLogStateManager.removeTransactionsFromDB(txIDsToBeDeleted); - metrics.incrBlockDeletionTransactionCompleted(txIDsToBeDeleted.size()); - } catch (IOException e) { - LOG.warn("Could not commit delete block transactions: " - + txIDsToBeDeleted, e); - } - } - - @Override - public void onMessage( - DeleteBlockStatus deleteBlockStatus, EventPublisher publisher) { - if (!scmContext.isLeader()) { - LOG.warn("Skip commit transactions since current SCM is not leader."); - return; - } - - DatanodeDetails details = deleteBlockStatus.getDatanodeDetails(); - UUID dnId = details.getUuid(); - for (CommandStatus commandStatus : deleteBlockStatus.getCmdStatus()) { - CommandStatus.Status status = commandStatus.getStatus(); - lock.lock(); - try { - if (status == CommandStatus.Status.EXECUTED) { - ContainerBlocksDeletionACKProto ackProto = - commandStatus.getBlockDeletionAck(); - commitTransactions(ackProto.getResultsList(), dnId); - metrics.incrBlockDeletionCommandSuccess(); - } else if (status == CommandStatus.Status.FAILED) { - metrics.incrBlockDeletionCommandFailure(); - } else { - LOG.debug("Delete Block Command {} is not executed on the Datanode" + - " {}.", commandStatus.getCmdId(), dnId); - } - - commitSCMCommandStatus(deleteBlockStatus.getCmdStatus(), dnId); - } finally { - lock.unlock(); - } - } - } - - public void commitSCMCommandStatus(List deleteBlockStatus, - UUID dnId) { - processSCMCommandStatus(deleteBlockStatus, dnId); - cleanSCMCommandForDn(dnId, scmCommandTimeoutMs); - } - - private void processSCMCommandStatus(List deleteBlockStatus, - UUID dnID) { - Map lastStatus = new HashMap<>(); - Map summary = new HashMap<>(); - - // The CommandStatus is ordered in the report. So we can focus only on the - // last status in the command report. - deleteBlockStatus.forEach(cmdStatus -> { - lastStatus.put(cmdStatus.getCmdId(), cmdStatus); - summary.put(cmdStatus.getCmdId(), cmdStatus.getStatus()); - }); - LOG.debug("CommandStatus {} from Datanode {} ", summary, dnID); - for (Map.Entry entry : lastStatus.entrySet()) { - CommandStatus.Status status = entry.getValue().getStatus(); - updateStatusByDNCommandStatus( - dnID, entry.getKey(), status); - - } - } - - 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; - } - - @VisibleForTesting - Map> getScmCmdStatusRecord() { - return scmCmdStatusRecord; - } -} diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java new file mode 100644 index 000000000000..3960acceb90d --- /dev/null +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java @@ -0,0 +1,659 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.hadoop.hdds.scm.block; + +import com.google.common.annotations.VisibleForTesting; +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.scm.command.CommandStatusReportHandler.DeleteBlockStatus; +import org.apache.hadoop.hdds.scm.container.ContainerID; +import org.apache.hadoop.hdds.scm.container.ContainerInfo; +import org.apache.hadoop.hdds.scm.container.ContainerManager; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; +import org.apache.hadoop.hdds.scm.ha.SCMContext; +import org.apache.hadoop.hdds.server.events.EventHandler; +import org.apache.hadoop.hdds.server.events.EventPublisher; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.time.Duration; +import java.time.Instant; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.stream.Collectors; + +import static java.lang.Math.min; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.EXECUTED; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.NEED_RESEND; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.PENDING_EXECUTED; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.SENT; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.TO_BE_SENT; + +/** + * This is a class to manage the status of DeletedBlockTransaction, + * the purpose of this class is to reduce the number of duplicate + * DeletedBlockTransaction sent to the DN. + */ +public class SCMDeletedBlockTransactionStatusManager + implements EventHandler { + public static final Logger LOG = + LoggerFactory.getLogger(SCMDeletedBlockTransactionStatusManager.class); + // Maps txId to set of DNs which are successful in committing the transaction + private final Map> transactionToDNsCommitMap; + // Maps txId to its retry counts; + private final Map transactionToRetryCountMap; + // The access to DeletedBlocksTXTable is protected by + // DeletedBlockLogStateManager. + private final DeletedBlockLogStateManager deletedBlockLogStateManager; + private final ContainerManager containerManager; + private final ScmBlockDeletingServiceMetrics metrics; + private final SCMContext scmContext; + private final Lock lock; + private final long scmCommandTimeoutMs; + + /** + * Before the DeletedBlockTransaction is executed on DN and reported to + * SCM, it is managed by this {@link SCMDeleteBlocksCommandStatusManager}. + * + * After the DeletedBlocksTransaction in the DeleteBlocksCommand is + * committed on the SCM, it is managed by + * {@link SCMDeletedBlockTransactionStatusManager#transactionToDNsCommitMap} + */ + private final SCMDeleteBlocksCommandStatusManager + scmDeleteBlocksCommandStatusManager; + + public SCMDeletedBlockTransactionStatusManager( + DeletedBlockLogStateManager deletedBlockLogStateManager, + ContainerManager containerManager, SCMContext scmContext, + Map transactionToRetryCountMap, + ScmBlockDeletingServiceMetrics metrics, + Lock lock, long scmCommandTimeoutMs) { + // maps transaction to dns which have committed it. + this.deletedBlockLogStateManager = deletedBlockLogStateManager; + this.metrics = metrics; + this.containerManager = containerManager; + this.scmContext = scmContext; + this.lock = lock; + this.scmCommandTimeoutMs = scmCommandTimeoutMs; + this.transactionToDNsCommitMap = new ConcurrentHashMap<>(); + this.transactionToRetryCountMap = transactionToRetryCountMap; + this.scmDeleteBlocksCommandStatusManager = + new SCMDeleteBlocksCommandStatusManager(); + } + + /** + * A class that manages the status of a DeletedBlockTransaction based + * on DeleteBlocksCommand. + */ + protected static class SCMDeleteBlocksCommandStatusManager { + public static final Logger LOG = + LoggerFactory.getLogger(SCMDeleteBlocksCommandStatusManager.class); + private final Map> scmCmdStatusRecord; + + private static final CmdStatus DEFAULT_STATUS = TO_BE_SENT; + private static final Set STATUSES_REQUIRING_TIMEOUT = + new HashSet<>(Arrays.asList(SENT, PENDING_EXECUTED)); + private static final Set FINIAL_STATUSES = new HashSet<>( + Arrays.asList(EXECUTED, NEED_RESEND)); + + public SCMDeleteBlocksCommandStatusManager() { + this.scmCmdStatusRecord = new ConcurrentHashMap<>(); + } + + /** + * Status of SCMDeleteBlocksCommand. + */ + public enum CmdStatus { + // The DeleteBlocksCommand has not yet been sent. + // This is the initial status of the command after it's created. + TO_BE_SENT, + // This status indicates that the DeleteBlocksCommand has been sent + // to the DataNode, but the Datanode has not reported any new status + // for the DeleteBlocksCommand. + SENT, + // The DeleteBlocksCommand has been received by Datanode and + // is waiting for executed. + PENDING_EXECUTED, + // The DeleteBlocksCommand was executed, and the execution was successful + EXECUTED, + // The DeleteBlocksCommand was executed but failed to execute, + // or was lost before it was executed. + NEED_RESEND + } + + protected static final class CmdStatusData { + private final UUID dnId; + private final long scmCmdId; + private final Set deletedBlocksTxIds; + private Instant updateTime; + private CmdStatus status; + + private CmdStatusData( + UUID dnId, long scmTxID, Set deletedBlocksTxIds) { + this.dnId = dnId; + this.scmCmdId = scmTxID; + this.deletedBlocksTxIds = deletedBlocksTxIds; + setStatus(DEFAULT_STATUS); + } + + public Set getDeletedBlocksTxIds() { + return Collections.unmodifiableSet(deletedBlocksTxIds); + } + + public UUID getDnId() { + return dnId; + } + + public long getScmCmdId() { + return scmCmdId; + } + + public CmdStatus getStatus() { + return status; + } + + public void setStatus(CmdStatus status) { + this.updateTime = Instant.now(); + this.status = status; + } + + public Instant getUpdateTime() { + return updateTime; + } + + @Override + public String toString() { + return "ScmTxStateMachine" + + "{dnId=" + dnId + + ", scmTxID=" + scmCmdId + + ", deletedBlocksTxIds=" + deletedBlocksTxIds + + ", updateTime=" + updateTime + + ", status=" + status + + '}'; + } + } + + protected static CmdStatusData createScmCmdStatusData( + UUID dnId, long scmCmdId, Set deletedBlocksTxIds) { + return new CmdStatusData(dnId, scmCmdId, deletedBlocksTxIds); + } + + protected void recordScmCommand(CmdStatusData statusData) { + LOG.debug("Record ScmCommand: {}", statusData); + scmCmdStatusRecord.computeIfAbsent(statusData.getDnId(), k -> + new ConcurrentHashMap<>()).put(statusData.getScmCmdId(), statusData); + } + + protected void onSent(UUID dnId, long scmCmdId) { + updateStatus(dnId, scmCmdId, SENT); + } + + protected void onDatanodeDead(UUID dnId) { + LOG.info("Clean SCMCommand record for DN: {}", dnId); + scmCmdStatusRecord.remove(dnId); + } + + protected void updateStatusByDNCommandStatus(UUID dnId, long scmCmdId, + CommandStatus.Status newState) { + CmdStatus status = fromProtoCommandStatus(newState); + if (status != null) { + updateStatus(dnId, scmCmdId, status); + } + } + + protected void cleanAllTimeoutSCMCommand(long timeoutMs) { + for (UUID dnId : scmCmdStatusRecord.keySet()) { + for (CmdStatus status : STATUSES_REQUIRING_TIMEOUT) { + removeTimeoutScmCommand( + dnId, getScmCommandIds(dnId, status), timeoutMs); + } + } + } + + protected void cleanSCMCommandForDn(UUID dnId, long timeoutMs) { + cleanTimeoutSCMCommand(dnId, timeoutMs); + cleanFinalStatusSCMCommand(dnId); + } + + private void cleanTimeoutSCMCommand(UUID dnId, long timeoutMs) { + for (CmdStatus status : STATUSES_REQUIRING_TIMEOUT) { + removeTimeoutScmCommand( + dnId, getScmCommandIds(dnId, status), timeoutMs); + } + } + + private void cleanFinalStatusSCMCommand(UUID dnId) { + for (CmdStatus status : FINIAL_STATUSES) { + for (Long scmCmdId : getScmCommandIds(dnId, status)) { + CmdStatusData stateData = removeScmCommand(dnId, scmCmdId); + LOG.debug("Clean SCMCommand status: {} for DN: {}, stateData: {}", + status, dnId, stateData); + } + } + } + + private Set getScmCommandIds(UUID dnId, CmdStatus status) { + Set scmCmdIds = new HashSet<>(); + Map record = scmCmdStatusRecord.get(dnId); + if (record == null) { + return scmCmdIds; + } + for (CmdStatusData statusData : record.values()) { + if (statusData.getStatus().equals(status)) { + scmCmdIds.add(statusData.getScmCmdId()); + } + } + return scmCmdIds; + } + + private Instant getUpdateTime(UUID dnId, long scmCmdId) { + Map record = scmCmdStatusRecord.get(dnId); + if (record == null || record.get(scmCmdId) == null) { + return null; + } + return record.get(scmCmdId).getUpdateTime(); + } + + private void updateStatus(UUID dnId, long scmCmdId, CmdStatus newStatus) { + Map recordForDn = scmCmdStatusRecord.get(dnId); + if (recordForDn == null) { + LOG.warn("Unknown Datanode: {} scmCmdId {} newStatus {}", + dnId, scmCmdId, newStatus); + return; + } + if (recordForDn.get(scmCmdId) == null) { + LOG.warn("Unknown SCM Command: {} Datanode {} newStatus {}", + scmCmdId, dnId, newStatus); + return; + } + + boolean changed = false; + CmdStatusData statusData = recordForDn.get(scmCmdId); + CmdStatus oldStatus = statusData.getStatus(); + + switch (newStatus) { + case SENT: + if (oldStatus == TO_BE_SENT) { + // TO_BE_SENT -> SENT: The DeleteBlocksCommand is sent by SCM, + // The follow-up status has not been updated by Datanode. + statusData.setStatus(SENT); + changed = true; + } + break; + case PENDING_EXECUTED: + if (oldStatus == SENT || oldStatus == PENDING_EXECUTED) { + // SENT -> PENDING_EXECUTED: The DeleteBlocksCommand is sent and + // received by the Datanode, but the command is not executed by the + // Datanode, the command is waiting to be executed. + + // PENDING_EXECUTED -> PENDING_EXECUTED: The DeleteBlocksCommand + // continues to wait to be executed by Datanode. + statusData.setStatus(PENDING_EXECUTED); + changed = true; + } + break; + case NEED_RESEND: + if (oldStatus == SENT || oldStatus == PENDING_EXECUTED) { + // SENT -> NEED_RESEND: The DeleteBlocksCommand is sent and lost + // before it is received by the DN. + + // PENDING_EXECUTED -> NEED_RESEND: The DeleteBlocksCommand waited for + // a while and was executed, but the execution failed;. + // Or the DeleteBlocksCommand was lost while waiting(such as the + // Datanode restart). + statusData.setStatus(NEED_RESEND); + changed = true; + } + break; + case EXECUTED: + if (oldStatus == SENT || oldStatus == PENDING_EXECUTED) { + // PENDING_EXECUTED -> EXECUTED: The Command waits for a period of + // time on the DN and is executed successfully. + + // SENT -> EXECUTED: The DeleteBlocksCommand has been sent to + // Datanode, executed by DN, and executed successfully. + statusData.setStatus(EXECUTED); + changed = true; + } + break; + default: + LOG.error("Can not update to Unknown new Status: {}", newStatus); + break; + } + if (!changed) { + LOG.warn("Cannot update illegal status for DN: {} ScmCommandId {} " + + "Status From {} to {}", dnId, scmCmdId, oldStatus, newStatus); + } else { + LOG.debug("Successful update DN: {} ScmCommandId {} Status From {} to" + + " {}", dnId, scmCmdId, oldStatus, newStatus); + } + } + + private void removeTimeoutScmCommand(UUID dnId, + Set scmCmdIds, long timeoutMs) { + Instant now = Instant.now(); + for (Long scmCmdId : scmCmdIds) { + Instant updateTime = getUpdateTime(dnId, scmCmdId); + if (updateTime != null && + Duration.between(updateTime, now).toMillis() > timeoutMs) { + updateStatus(dnId, scmCmdId, NEED_RESEND); + CmdStatusData state = removeScmCommand(dnId, scmCmdId); + LOG.warn("Remove Timeout SCM BlockDeletionCommand {} for DN {} " + + "after without update {}ms}", state, dnId, timeoutMs); + } else { + LOG.warn("Timeout SCM scmCmdIds {} for DN {} " + + "after without update {}ms}", scmCmdIds, dnId, timeoutMs); + } + } + } + + private CmdStatusData removeScmCommand(UUID dnId, long scmCmdId) { + Map record = scmCmdStatusRecord.get(dnId); + if (record == null || record.get(scmCmdId) == null) { + return null; + } + + CmdStatus status = record.get(scmCmdId).getStatus(); + if (!FINIAL_STATUSES.contains(status)) { + LOG.error("Cannot Remove ScmCommand {} Non-final Status {} for DN: {}" + + ". final Status {}", scmCmdId, status, dnId, FINIAL_STATUSES); + return null; + } + + CmdStatusData statusData = record.remove(scmCmdId); + LOG.debug("Remove ScmCommand {} for DN: {} ", statusData, dnId); + return statusData; + } + + private static CmdStatus fromProtoCommandStatus( + CommandStatus.Status protoCmdStatus) { + switch (protoCmdStatus) { + case PENDING: + return CmdStatus.PENDING_EXECUTED; + case EXECUTED: + return CmdStatus.EXECUTED; + case FAILED: + return CmdStatus.NEED_RESEND; + default: + LOG.error("Unknown protoCmdStatus: {} cannot convert " + + "to ScmDeleteBlockCommandStatus", protoCmdStatus); + return null; + } + } + + public Map> getCommandStatusByTxId( + Set dnIds) { + Map> result = + new HashMap<>(scmCmdStatusRecord.size()); + + for (UUID dnId : dnIds) { + Map record = scmCmdStatusRecord.get(dnId); + if (record == null) { + continue; + } + Map dnStatusMap = new HashMap<>(); + for (CmdStatusData statusData : record.values()) { + CmdStatus status = statusData.getStatus(); + for (Long deletedBlocksTxId : statusData.getDeletedBlocksTxIds()) { + dnStatusMap.put(deletedBlocksTxId, status); + } + } + result.put(dnId, dnStatusMap); + } + + return result; + } + + private void clear() { + scmCmdStatusRecord.clear(); + } + + @VisibleForTesting + Map> getScmCmdStatusRecord() { + return scmCmdStatusRecord; + } + } + + public void onSent(UUID dnId, long scmCmdId) { + scmDeleteBlocksCommandStatusManager.updateStatus(dnId, scmCmdId, SENT); + } + + public Map> getCommandStatusByTxId( + Set dnIds) { + return scmDeleteBlocksCommandStatusManager.getCommandStatusByTxId(dnIds); + } + + public void recordTransactionCreated( + UUID dnId, long scmCmdId, Set dnTxSet) { + scmDeleteBlocksCommandStatusManager.recordScmCommand( + SCMDeleteBlocksCommandStatusManager + .createScmCmdStatusData(dnId, scmCmdId, dnTxSet)); + } + + public void recordTransactionCommitted(long txId) { + transactionToDNsCommitMap + .putIfAbsent(txId, new LinkedHashSet<>()); + } + + public void clear() { + scmDeleteBlocksCommandStatusManager.clear(); + transactionToDNsCommitMap.clear(); + } + + public void cleanAllTimeoutSCMCommand(long timeoutMs) { + scmDeleteBlocksCommandStatusManager.cleanAllTimeoutSCMCommand(timeoutMs); + } + + public void onDatanodeDead(UUID dnId) { + scmDeleteBlocksCommandStatusManager.onDatanodeDead(dnId); + } + + public boolean isDuplication(DatanodeDetails dnDetail, long tx, + Map> commandStatus) { + if (alreadyExecuted(dnDetail.getUuid(), tx)) { + return true; + } + return inProcessing(dnDetail.getUuid(), tx, commandStatus); + } + + public boolean alreadyExecuted(UUID dnId, long txId) { + Set dnsWithTransactionCommitted = + transactionToDNsCommitMap.get(txId); + return dnsWithTransactionCommitted != null && dnsWithTransactionCommitted + .contains(dnId); + } + + /** + * Commits a transaction means to delete all footprints of a transaction + * from the log. This method doesn't guarantee all transactions can be + * successfully deleted, it tolerate failures and tries best efforts to. + * @param transactionResults - delete block transaction results. + * @param dnId - ID of datanode which acknowledges the delete block command. + */ + @VisibleForTesting + public void commitTransactions( + List transactionResults, UUID dnId) { + + ArrayList txIDsToBeDeleted = new ArrayList<>(); + Set dnsWithCommittedTxn; + for (DeleteBlockTransactionResult transactionResult : + transactionResults) { + if (isTransactionFailed(transactionResult)) { + metrics.incrBlockDeletionTransactionFailure(); + continue; + } + try { + metrics.incrBlockDeletionTransactionSuccess(); + long txID = transactionResult.getTxID(); + // set of dns which have successfully committed transaction txId. + dnsWithCommittedTxn = transactionToDNsCommitMap.get(txID); + final ContainerID containerId = ContainerID.valueOf( + transactionResult.getContainerID()); + if (dnsWithCommittedTxn == null) { + // Mostly likely it's a retried delete command response. + if (LOG.isDebugEnabled()) { + LOG.debug( + "Transaction txId={} commit by dnId={} for containerID={}" + + " failed. Corresponding entry not found.", txID, dnId, + containerId); + } + continue; + } + + dnsWithCommittedTxn.add(dnId); + final ContainerInfo container = + containerManager.getContainer(containerId); + final Set replicas = + containerManager.getContainerReplicas(containerId); + // The delete entry can be safely removed from the log if all the + // corresponding nodes commit the txn. It is required to check that + // the nodes returned in the pipeline match the replication factor. + if (min(replicas.size(), dnsWithCommittedTxn.size()) + >= container.getReplicationConfig().getRequiredNodes()) { + List containerDns = replicas.stream() + .map(ContainerReplica::getDatanodeDetails) + .map(DatanodeDetails::getUuid) + .collect(Collectors.toList()); + if (dnsWithCommittedTxn.containsAll(containerDns)) { + transactionToDNsCommitMap.remove(txID); + transactionToRetryCountMap.remove(txID); + if (LOG.isDebugEnabled()) { + LOG.debug("Purging txId={} from block deletion log", txID); + } + txIDsToBeDeleted.add(txID); + } + } + if (LOG.isDebugEnabled()) { + LOG.debug("Datanode txId={} containerId={} committed by dnId={}", + txID, containerId, dnId); + } + } catch (IOException e) { + LOG.warn("Could not commit delete block transaction: " + + transactionResult.getTxID(), e); + } + } + try { + deletedBlockLogStateManager.removeTransactionsFromDB(txIDsToBeDeleted); + metrics.incrBlockDeletionTransactionCompleted(txIDsToBeDeleted.size()); + } catch (IOException e) { + LOG.warn("Could not commit delete block transactions: " + + txIDsToBeDeleted, e); + } + } + + @Override + public void onMessage( + DeleteBlockStatus deleteBlockStatus, EventPublisher publisher) { + if (!scmContext.isLeader()) { + LOG.warn("Skip commit transactions since current SCM is not leader."); + return; + } + + DatanodeDetails details = deleteBlockStatus.getDatanodeDetails(); + UUID dnId = details.getUuid(); + for (CommandStatus commandStatus : deleteBlockStatus.getCmdStatus()) { + CommandStatus.Status status = commandStatus.getStatus(); + lock.lock(); + try { + if (status == CommandStatus.Status.EXECUTED) { + ContainerBlocksDeletionACKProto ackProto = + commandStatus.getBlockDeletionAck(); + commitTransactions(ackProto.getResultsList(), dnId); + metrics.incrBlockDeletionCommandSuccess(); + } else if (status == CommandStatus.Status.FAILED) { + metrics.incrBlockDeletionCommandFailure(); + } else { + LOG.debug("Delete Block Command {} is not executed on the Datanode" + + " {}.", commandStatus.getCmdId(), dnId); + } + + commitSCMCommandStatus(deleteBlockStatus.getCmdStatus(), dnId); + } finally { + lock.unlock(); + } + } + } + + @VisibleForTesting + public void commitSCMCommandStatus(List deleteBlockStatus, + UUID dnId) { + processSCMCommandStatus(deleteBlockStatus, dnId); + scmDeleteBlocksCommandStatusManager. + cleanSCMCommandForDn(dnId, scmCommandTimeoutMs); + } + + private boolean inProcessing(UUID dnId, long deletedBlocksTxId, + Map> commandStatus) { + Map deletedBlocksTxStatus = commandStatus.get(dnId); + if (deletedBlocksTxStatus == null || + deletedBlocksTxStatus.get(deletedBlocksTxId) == null) { + return false; + } + return deletedBlocksTxStatus.get(deletedBlocksTxId) != + CmdStatus.NEED_RESEND; + } + + private void processSCMCommandStatus(List deleteBlockStatus, + UUID dnID) { + Map lastStatus = new HashMap<>(); + Map summary = new HashMap<>(); + + // The CommandStatus is ordered in the report. So we can focus only on the + // last status in the command report. + deleteBlockStatus.forEach(cmdStatus -> { + lastStatus.put(cmdStatus.getCmdId(), cmdStatus); + summary.put(cmdStatus.getCmdId(), cmdStatus.getStatus()); + }); + LOG.debug("CommandStatus {} from Datanode {} ", summary, dnID); + for (Map.Entry entry : lastStatus.entrySet()) { + CommandStatus.Status status = entry.getValue().getStatus(); + scmDeleteBlocksCommandStatusManager.updateStatusByDNCommandStatus( + dnID, entry.getKey(), status); + } + } + + 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; + } +} diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java index d5b3c278781b..d15d41404f53 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java @@ -112,7 +112,7 @@ public void onMessage(final DatanodeDetails datanodeDetails, // is IN_MAINTENANCE if (deletedBlockLog != null && !nodeManager.getNodeStatus(datanodeDetails).isInMaintenance()) { - deletedBlockLog.getScmCommandStatusManager() + deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() .onDatanodeDead(datanodeDetails.getUuid()); } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java index bb998494ee8e..407f0f4681bf 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java @@ -522,7 +522,7 @@ public List processHeartbeat(DatanodeDetails datanodeDetails, if (command.getType() == SCMCommandProto.Type.deleteBlocksCommand) { DeletedBlockLog deletedBlockLog = scmContext.getScm(). getScmBlockManager().getDeletedBlockLog(); - deletedBlockLog.getScmCommandStatusManager() + deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() .onSent(datanodeDetails.getUuid(), command.getId()); } } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index f5a626d4f443..18cc64495c50 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -556,8 +556,8 @@ private void initializeEventHandlers() { eventQueue.addHandler(SCMEvents.START_ADMIN_ON_NODE, datanodeStartAdminHandler); eventQueue.addHandler(SCMEvents.CMD_STATUS_REPORT, cmdStatusReportHandler); - eventQueue.addHandler(SCMEvents.DELETE_BLOCK_STATUS, - scmBlockManager.getDeletedBlockLog().getScmCommandStatusManager()); + eventQueue.addHandler(SCMEvents.DELETE_BLOCK_STATUS, scmBlockManager + .getDeletedBlockLog().getSCMDeletedBlockTransactionStatusManager()); eventQueue.addHandler(SCMEvents.PIPELINE_ACTIONS, pipelineActionHandler); eventQueue.addHandler(SCMEvents.PIPELINE_REPORT, pipelineReportHandler); eventQueue.addHandler(SCMEvents.CRL_STATUS_REPORT, crlStatusReportHandler); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java index ad2e14f50c24..6c687016d370 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java @@ -250,7 +250,7 @@ private void commitTransactions( List transactionResults, DatanodeDetails... dns) throws IOException { for (DatanodeDetails dnDetails : dns) { - deletedBlockLog.getScmCommandStatusManager() + deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() .commitTransactions(transactionResults, dnDetails.getUuid()); } scmHADBTransactionBuffer.flush(); @@ -283,7 +283,7 @@ private void commitTransactions(DatanodeDeletedBlockTransactions transactions) { transactions.getDatanodeTransactionMap().forEach((uuid, deletedBlocksTransactions) -> - deletedBlockLog.getScmCommandStatusManager() + deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() .commitTransactions(deletedBlocksTransactions.stream() .map(this::createDeleteBlockTransactionResult) .collect(Collectors.toList()), uuid)); @@ -453,13 +453,12 @@ private void recordScmCommandToStatusManager( Set dnTxSet = command.blocksTobeDeleted() .stream().map(DeletedBlocksTransaction::getTxID) .collect(Collectors.toSet()); - deletedBlockLog.getScmCommandStatusManager().recordScmCommand( - SCMDeleteBlocksCommandStatusManager.createScmCmdStatusData( - dnId, command.getId(), dnTxSet)); + deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() + .recordTransactionCreated(dnId, command.getId(), dnTxSet); } private void sendSCMDeleteBlocksCommand(UUID dnId, long scmCmdId) { - deletedBlockLog.getScmCommandStatusManager().onSent( + deletedBlockLog.getSCMDeletedBlockTransactionStatusManager().onSent( dnId, scmCmdId); } @@ -514,7 +513,7 @@ private void commitSCMCommandStatus(Long scmCmdId, UUID dnID, .build() .getProtoBufMessage()); - deletedBlockLog.getScmCommandStatusManager() + deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() .commitSCMCommandStatus(deleteBlockStatus, dnID); } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java index ee6ac1830238..6aa396610c32 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java @@ -19,11 +19,8 @@ package org.apache.hadoop.hdds.scm.block; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos; -import org.apache.hadoop.hdds.scm.container.ContainerManager; -import org.apache.hadoop.hdds.scm.ha.SCMContext; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.BeforeEach; -import org.mockito.Mockito; import java.util.Arrays; import java.util.HashSet; @@ -31,14 +28,14 @@ import java.util.Map; import java.util.Set; import java.util.UUID; -import java.util.concurrent.locks.Lock; - -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.NEED_RESEND; -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.PENDING_EXECUTED; -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.SENT; -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.EXECUTED; -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatus.TO_BE_SENT; -import static org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager.CmdStatusData; + +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.NEED_RESEND; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.PENDING_EXECUTED; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.SENT; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.EXECUTED; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.TO_BE_SENT; +import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatusData; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -62,11 +59,7 @@ public class TestSCMDeleteBlocksCommandStatusManager { @BeforeEach public void setup() throws Exception { - manager = new SCMDeleteBlocksCommandStatusManager( - Mockito.mock(DeletedBlockLogStateManager.class), - Mockito.mock(ScmBlockDeletingServiceMetrics.class), - Mockito.mock(ContainerManager.class), Mockito.mock(Lock.class), - Mockito.mock(SCMContext.class), 300); + manager = new SCMDeleteBlocksCommandStatusManager(); // Create test data dnId1 = UUID.randomUUID(); dnId2 = UUID.randomUUID(); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java index cc6234648164..46d2a27700e7 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java @@ -48,7 +48,7 @@ import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.HddsTestUtils; import org.apache.hadoop.hdds.scm.block.DeletedBlockLog; -import org.apache.hadoop.hdds.scm.block.SCMDeleteBlocksCommandStatusManager; +import org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException; @@ -92,7 +92,8 @@ public class TestDeadNodeHandler { private EventQueue eventQueue; private String storageDir; private SCMContext scmContext; - private SCMDeleteBlocksCommandStatusManager deleteBlocksCommandStatusManager; + private SCMDeletedBlockTransactionStatusManager + deleteBlocksCommandStatusManager; @BeforeEach public void setup() throws IOException, AuthenticationException { @@ -122,8 +123,8 @@ public void setup() throws IOException, AuthenticationException { containerManager = scm.getContainerManager(); DeletedBlockLog deletedBlockLog = Mockito.mock(DeletedBlockLog.class); deleteBlocksCommandStatusManager = - Mockito.mock(SCMDeleteBlocksCommandStatusManager.class); - Mockito.when(deletedBlockLog.getScmCommandStatusManager()) + Mockito.mock(SCMDeletedBlockTransactionStatusManager.class); + Mockito.when(deletedBlockLog.getSCMDeletedBlockTransactionStatusManager()) .thenReturn(deleteBlocksCommandStatusManager); deadNodeHandler = new DeadNodeHandler(nodeManager, Mockito.mock(PipelineManager.class), containerManager, deletedBlockLog); From be5e118265b3f96d45f40ca5409090dbffe01fd2 Mon Sep 17 00:00:00 2001 From: xichen01 Date: Mon, 17 Jul 2023 02:28:38 +0800 Subject: [PATCH 13/23] Implement NodeManager#registerSendCommandNotify --- ...MDeletedBlockTransactionStatusManager.java | 9 ++++-- .../hadoop/hdds/scm/node/NodeManager.java | 13 +++++++++ .../hadoop/hdds/scm/node/SCMNodeManager.java | 21 +++++++++----- .../hdds/scm/block/TestDeletedBlockLog.java | 28 ++++++++++++++----- 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java index 3960acceb90d..06c5c39efb6d 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java @@ -22,6 +22,7 @@ import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.CommandStatus; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.SCMCommandProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerBlocksDeletionACKProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerBlocksDeletionACKProto.DeleteBlockTransactionResult; import org.apache.hadoop.hdds.scm.command.CommandStatusReportHandler.DeleteBlockStatus; @@ -32,6 +33,7 @@ import org.apache.hadoop.hdds.scm.ha.SCMContext; import org.apache.hadoop.hdds.server.events.EventHandler; import org.apache.hadoop.hdds.server.events.EventPublisher; +import org.apache.hadoop.ozone.protocol.commands.SCMCommand; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -110,6 +112,8 @@ public SCMDeletedBlockTransactionStatusManager( this.transactionToRetryCountMap = transactionToRetryCountMap; this.scmDeleteBlocksCommandStatusManager = new SCMDeleteBlocksCommandStatusManager(); + this.scmContext.getScm().getScmNodeManager().registerSendCommandNotify( + SCMCommandProto.Type.deleteBlocksCommand, this::onSent); } /** @@ -445,8 +449,9 @@ Map> getScmCmdStatusRecord() { } } - public void onSent(UUID dnId, long scmCmdId) { - scmDeleteBlocksCommandStatusManager.updateStatus(dnId, scmCmdId, SENT); + public void onSent(DatanodeDetails dnId, SCMCommand scmCommand) { + scmDeleteBlocksCommandStatusManager.updateStatus( + dnId.getUuid(), scmCommand.getId(), SENT); } public Map> getCommandStatusByTxId( diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java index b35a19b402df..936cf96d7eb3 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java @@ -47,6 +47,7 @@ import java.util.Set; import java.util.UUID; import java.util.Collection; +import java.util.function.BiConsumer; /** * A node manager supports a simple interface for managing a datanode. @@ -90,6 +91,18 @@ default RegisteredCommand register( defaultLayoutVersionProto()); } + /** + * Register a SendCommandNotify handler for a specific type of SCMCommand. + * @param type The type of the SCMCommand. + * @param scmCommand A BiConsumer that takes a DatanodeDetails and a + * SCMCommand object and performs the necessary actions. + * @return whatever the regular register command returns with default + * layout version passed in. + */ + default void registerSendCommandNotify(SCMCommandProto.Type type, + BiConsumer> scmCommand) { + } + /** * Gets all Live Datanodes that are currently communicating with SCM. * @param nodeStatus - Status of the node to return diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java index 407f0f4681bf..f712550c5a98 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java @@ -36,7 +36,6 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.VersionInfo; -import org.apache.hadoop.hdds.scm.block.DeletedBlockLog; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeMetric; import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeStat; @@ -88,6 +87,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiConsumer; import java.util.stream.Collectors; import static org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name.HTTP; @@ -131,6 +131,8 @@ public class SCMNodeManager implements NodeManager { private final HDDSLayoutVersionManager scmLayoutVersionManager; private final EventPublisher scmNodeEventPublisher; private final SCMContext scmContext; + private final Map>> sendCommandNotifyMap; /** * Lock used to synchronize some operation in Node manager to ensure a @@ -177,6 +179,13 @@ public SCMNodeManager(OzoneConfiguration conf, String dnLimit = conf.get(ScmConfigKeys.OZONE_DATANODE_PIPELINE_LIMIT); this.heavyNodeCriteria = dnLimit == null ? 0 : Integer.parseInt(dnLimit); this.scmContext = scmContext; + this.sendCommandNotifyMap = new HashMap<>(); + } + + @Override + public void registerSendCommandNotify(SCMCommandProto.Type type, + BiConsumer> scmCommand) { + this.sendCommandNotifyMap.put(type, scmCommand); } private void registerMXBean() { @@ -518,12 +527,10 @@ public List processHeartbeat(DatanodeDetails datanodeDetails, commandQueue.getCommand(datanodeDetails.getUuid()); // Update the SCMCommand of deleteBlocksCommand Status - for (SCMCommand command : commands) { - if (command.getType() == SCMCommandProto.Type.deleteBlocksCommand) { - DeletedBlockLog deletedBlockLog = scmContext.getScm(). - getScmBlockManager().getDeletedBlockLog(); - deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() - .onSent(datanodeDetails.getUuid(), command.getId()); + for (SCMCommand command : commands) { + if (sendCommandNotifyMap.get(command.getType()) != null) { + sendCommandNotifyMap.get(command.getType()) + .accept(datanodeDetails, command); } } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java index 6c687016d370..03ea18d849d2 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java @@ -32,10 +32,14 @@ import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.ContainerInfo; +import org.apache.hadoop.hdds.scm.ha.SCMContext; import org.apache.hadoop.hdds.scm.ha.SCMHADBTransactionBuffer; import org.apache.hadoop.hdds.scm.ha.SCMHADBTransactionBufferStub; +import org.apache.hadoop.hdds.scm.ha.SCMHAManagerStub; +import org.apache.hadoop.hdds.scm.node.SCMNodeManager; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; +import org.apache.hadoop.hdds.scm.server.SCMConfigurator; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -49,6 +53,7 @@ import org.apache.hadoop.hdds.utils.db.TableIterator; import org.apache.hadoop.ozone.protocol.commands.CommandStatus; import org.apache.hadoop.ozone.protocol.commands.DeleteBlocksCommand; +import org.apache.hadoop.ozone.protocol.commands.SCMCommand; import org.apache.ozone.test.GenericTestUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; @@ -109,7 +114,16 @@ public void setup() throws Exception { conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, true); conf.setInt(OZONE_SCM_BLOCK_DELETION_MAX_RETRY, 20); conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath()); - scm = HddsTestUtils.getScm(conf); + SCMConfigurator configurator = new SCMConfigurator(); + configurator.setSCMHAManager(SCMHAManagerStub.getInstance(true)); + StorageContainerManager storageContainerManager = + Mockito.mock(StorageContainerManager.class); + when(storageContainerManager.getScmNodeManager()).thenReturn(Mockito.mock( + SCMNodeManager.class)); + SCMContext context = + new SCMContext.Builder().setSCM(storageContainerManager).build(); + configurator.setScmContext(context); + scm = HddsTestUtils.getScm(conf, configurator); containerManager = Mockito.mock(ContainerManager.class); containerTable = scm.getScmMetadataStore().getContainerTable(); scmHADBTransactionBuffer = @@ -457,9 +471,9 @@ private void recordScmCommandToStatusManager( .recordTransactionCreated(dnId, command.getId(), dnTxSet); } - private void sendSCMDeleteBlocksCommand(UUID dnId, long scmCmdId) { + private void sendSCMDeleteBlocksCommand(UUID dnId, SCMCommand scmCommand) { deletedBlockLog.getSCMDeletedBlockTransactionStatusManager().onSent( - dnId, scmCmdId); + DatanodeDetails.newBuilder().setUuid(dnId).build(), scmCommand); } private void assertNoDuplicateTransactions( @@ -553,7 +567,7 @@ public void testNoDuplicateTransactionsForInProcessingSCMCommand() assertNoDuplicateTransactions(transactions1, transactions2); createDeleteBlocksCommandAndAction(transactions2, (dnId, command) -> { recordScmCommandToStatusManager(dnId, command); - sendSCMDeleteBlocksCommand(dnId, command.getId()); + sendSCMDeleteBlocksCommand(dnId, command); }); // - If the DN reports the command status as PENDING @@ -562,7 +576,7 @@ public void testNoDuplicateTransactionsForInProcessingSCMCommand() assertNoDuplicateTransactions(transactions1, transactions3); createDeleteBlocksCommandAndAction(transactions3, (dnId, command) -> { recordScmCommandToStatusManager(dnId, command); - sendSCMDeleteBlocksCommand(dnId, command.getId()); + sendSCMDeleteBlocksCommand(dnId, command); commitSCMCommandStatus(command.getId(), dnId, StorageContainerDatanodeProtocolProtos.CommandStatus.Status.PENDING); }); @@ -590,7 +604,7 @@ public void testFailedAndTimeoutSCMCommandCanBeResend() throws Exception { deletedBlockLog.getTransactions(blockLimit, new HashSet<>(dnList)); createDeleteBlocksCommandAndAction(transactions, (dnId, command) -> { recordScmCommandToStatusManager(dnId, command); - sendSCMDeleteBlocksCommand(dnId, command.getId()); + sendSCMDeleteBlocksCommand(dnId, command); commitSCMCommandStatus(command.getId(), dnId, StorageContainerDatanodeProtocolProtos.CommandStatus.Status.PENDING); }); @@ -600,7 +614,7 @@ public void testFailedAndTimeoutSCMCommandCanBeResend() throws Exception { deletedBlockLog.getTransactions(blockLimit, new HashSet<>(dnList)); createDeleteBlocksCommandAndAction(transactions2, (dnId, command) -> { recordScmCommandToStatusManager(dnId, command); - sendSCMDeleteBlocksCommand(dnId, command.getId()); + sendSCMDeleteBlocksCommand(dnId, command); commitSCMCommandStatus(command.getId(), dnId, StorageContainerDatanodeProtocolProtos.CommandStatus.Status.FAILED); }); From 5fc9b5c8fa32425b6db00a9013b2ce83e38de59e Mon Sep 17 00:00:00 2001 From: xichen01 Date: Mon, 17 Jul 2023 14:57:58 +0800 Subject: [PATCH 14/23] Fix test --- .../org/apache/hadoop/hdds/scm/HddsTestUtils.java | 14 ++++++++++++++ .../hdds/scm/block/TestDeletedBlockLog.java | 15 +-------------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java index 6021c33de16c..193eac54d70a 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java @@ -83,6 +83,7 @@ import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.authentication.client .AuthenticationException; +import org.mockito.Mockito; import java.io.File; import java.io.IOException; @@ -641,6 +642,19 @@ public static StorageContainerManager getScm(OzoneConfiguration conf, conf.set(ScmConfigKeys.OZONE_SCM_DATANODE_ADDRESS_KEY, "127.0.0.1:0"); conf.set(ScmConfigKeys.OZONE_SCM_HTTP_ADDRESS_KEY, "127.0.0.1:0"); SCMStorageConfig scmStore = new SCMStorageConfig(conf); + + // Adding ScmNodeManager to SCMContext for + // SCMDeletedBlockTransactionStatusManager + if (configurator.getScmContext().getScm() == null) { + StorageContainerManager storageContainerManager = + Mockito.mock(StorageContainerManager.class); + when(storageContainerManager.getScmNodeManager()).thenReturn(Mockito.mock( + SCMNodeManager.class)); + SCMContext context = + new SCMContext.Builder().setSCM(storageContainerManager).build(); + configurator.setScmContext(context); + } + if (scmStore.getState() != Storage.StorageState.INITIALIZED) { String clusterId = UUID.randomUUID().toString(); String scmId = UUID.randomUUID().toString(); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java index 03ea18d849d2..fc0cec69816a 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java @@ -32,14 +32,10 @@ import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.ContainerInfo; -import org.apache.hadoop.hdds.scm.ha.SCMContext; import org.apache.hadoop.hdds.scm.ha.SCMHADBTransactionBuffer; import org.apache.hadoop.hdds.scm.ha.SCMHADBTransactionBufferStub; -import org.apache.hadoop.hdds.scm.ha.SCMHAManagerStub; -import org.apache.hadoop.hdds.scm.node.SCMNodeManager; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; -import org.apache.hadoop.hdds.scm.server.SCMConfigurator; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -114,16 +110,7 @@ public void setup() throws Exception { conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, true); conf.setInt(OZONE_SCM_BLOCK_DELETION_MAX_RETRY, 20); conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath()); - SCMConfigurator configurator = new SCMConfigurator(); - configurator.setSCMHAManager(SCMHAManagerStub.getInstance(true)); - StorageContainerManager storageContainerManager = - Mockito.mock(StorageContainerManager.class); - when(storageContainerManager.getScmNodeManager()).thenReturn(Mockito.mock( - SCMNodeManager.class)); - SCMContext context = - new SCMContext.Builder().setSCM(storageContainerManager).build(); - configurator.setScmContext(context); - scm = HddsTestUtils.getScm(conf, configurator); + scm = HddsTestUtils.getScm(conf); containerManager = Mockito.mock(ContainerManager.class); containerTable = scm.getScmMetadataStore().getContainerTable(); scmHADBTransactionBuffer = From 24e0e5a71fc410f1712404731dbc3cb9cd1e64cc Mon Sep 17 00:00:00 2001 From: xichen01 Date: Mon, 17 Jul 2023 18:22:39 +0800 Subject: [PATCH 15/23] Fix test --- .../SCMDeletedBlockTransactionStatusManager.java | 4 ++-- .../hdds/scm/server/StorageContainerManager.java | 6 ++++++ .../org/apache/hadoop/hdds/scm/HddsTestUtils.java | 14 -------------- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java index 06c5c39efb6d..df4a28fb622d 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java @@ -112,8 +112,8 @@ public SCMDeletedBlockTransactionStatusManager( this.transactionToRetryCountMap = transactionToRetryCountMap; this.scmDeleteBlocksCommandStatusManager = new SCMDeleteBlocksCommandStatusManager(); - this.scmContext.getScm().getScmNodeManager().registerSendCommandNotify( - SCMCommandProto.Type.deleteBlocksCommand, this::onSent); +// this.scmContext.getScm().getScmNodeManager().registerSendCommandNotify( +// SCMCommandProto.Type.deleteBlocksCommand, this::onSent); } /** diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index 18cc64495c50..7ff4aca29464 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -37,6 +37,8 @@ import org.apache.hadoop.hdds.conf.ReconfigurationHandler; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.SCMCommandProto; import org.apache.hadoop.hdds.protocolPB.SCMSecurityProtocolClientSideTranslatorPB; import org.apache.hadoop.hdds.scm.PipelineChoosePolicy; import org.apache.hadoop.hdds.scm.PlacementPolicy; @@ -562,6 +564,10 @@ private void initializeEventHandlers() { eventQueue.addHandler(SCMEvents.PIPELINE_REPORT, pipelineReportHandler); eventQueue.addHandler(SCMEvents.CRL_STATUS_REPORT, crlStatusReportHandler); + scmNodeManager.registerSendCommandNotify( + SCMCommandProto.Type.deleteBlocksCommand, + scmBlockManager.getDeletedBlockLog() + .getSCMDeletedBlockTransactionStatusManager()::onSent); } private void initializeCertificateClient() throws IOException { diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java index 193eac54d70a..6021c33de16c 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java @@ -83,7 +83,6 @@ import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.authentication.client .AuthenticationException; -import org.mockito.Mockito; import java.io.File; import java.io.IOException; @@ -642,19 +641,6 @@ public static StorageContainerManager getScm(OzoneConfiguration conf, conf.set(ScmConfigKeys.OZONE_SCM_DATANODE_ADDRESS_KEY, "127.0.0.1:0"); conf.set(ScmConfigKeys.OZONE_SCM_HTTP_ADDRESS_KEY, "127.0.0.1:0"); SCMStorageConfig scmStore = new SCMStorageConfig(conf); - - // Adding ScmNodeManager to SCMContext for - // SCMDeletedBlockTransactionStatusManager - if (configurator.getScmContext().getScm() == null) { - StorageContainerManager storageContainerManager = - Mockito.mock(StorageContainerManager.class); - when(storageContainerManager.getScmNodeManager()).thenReturn(Mockito.mock( - SCMNodeManager.class)); - SCMContext context = - new SCMContext.Builder().setSCM(storageContainerManager).build(); - configurator.setScmContext(context); - } - if (scmStore.getState() != Storage.StorageState.INITIALIZED) { String clusterId = UUID.randomUUID().toString(); String scmId = UUID.randomUUID().toString(); From 43c72517f958e29be0a253438ad75dde1680b96f Mon Sep 17 00:00:00 2001 From: xichen01 Date: Mon, 17 Jul 2023 18:36:08 +0800 Subject: [PATCH 16/23] fix checkstyle --- .../scm/block/SCMDeletedBlockTransactionStatusManager.java | 3 --- .../apache/hadoop/hdds/scm/server/StorageContainerManager.java | 1 - 2 files changed, 4 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java index df4a28fb622d..624d058b28c1 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java @@ -22,7 +22,6 @@ import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.CommandStatus; -import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.SCMCommandProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerBlocksDeletionACKProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerBlocksDeletionACKProto.DeleteBlockTransactionResult; import org.apache.hadoop.hdds.scm.command.CommandStatusReportHandler.DeleteBlockStatus; @@ -112,8 +111,6 @@ public SCMDeletedBlockTransactionStatusManager( this.transactionToRetryCountMap = transactionToRetryCountMap; this.scmDeleteBlocksCommandStatusManager = new SCMDeleteBlocksCommandStatusManager(); -// this.scmContext.getScm().getScmNodeManager().registerSendCommandNotify( -// SCMCommandProto.Type.deleteBlocksCommand, this::onSent); } /** diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index 7ff4aca29464..e1ae392d4f56 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -37,7 +37,6 @@ import org.apache.hadoop.hdds.conf.ReconfigurationHandler; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState; -import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.SCMCommandProto; import org.apache.hadoop.hdds.protocolPB.SCMSecurityProtocolClientSideTranslatorPB; import org.apache.hadoop.hdds.scm.PipelineChoosePolicy; From bb55189124ef8c347bf4d675fe776325fe7de51f Mon Sep 17 00:00:00 2001 From: xichen01 Date: Sun, 8 Oct 2023 18:49:57 +0800 Subject: [PATCH 17/23] Simplified State of SCMDeleteBlocksCommandStatusManager --- ...MDeletedBlockTransactionStatusManager.java | 140 ++++-------------- ...stSCMDeleteBlocksCommandStatusManager.java | 34 +---- 2 files changed, 39 insertions(+), 135 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java index 624d058b28c1..27a225b17c33 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java @@ -55,9 +55,6 @@ import static java.lang.Math.min; import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus; -import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.EXECUTED; -import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.NEED_RESEND; -import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.PENDING_EXECUTED; import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.SENT; import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.TO_BE_SENT; @@ -86,7 +83,6 @@ public class SCMDeletedBlockTransactionStatusManager /** * Before the DeletedBlockTransaction is executed on DN and reported to * SCM, it is managed by this {@link SCMDeleteBlocksCommandStatusManager}. - * * After the DeletedBlocksTransaction in the DeleteBlocksCommand is * committed on the SCM, it is managed by * {@link SCMDeletedBlockTransactionStatusManager#transactionToDNsCommitMap} @@ -124,9 +120,7 @@ protected static class SCMDeleteBlocksCommandStatusManager { private static final CmdStatus DEFAULT_STATUS = TO_BE_SENT; private static final Set STATUSES_REQUIRING_TIMEOUT = - new HashSet<>(Arrays.asList(SENT, PENDING_EXECUTED)); - private static final Set FINIAL_STATUSES = new HashSet<>( - Arrays.asList(EXECUTED, NEED_RESEND)); + new HashSet<>(Arrays.asList(SENT)); public SCMDeleteBlocksCommandStatusManager() { this.scmCmdStatusRecord = new ConcurrentHashMap<>(); @@ -139,18 +133,14 @@ public enum CmdStatus { // The DeleteBlocksCommand has not yet been sent. // This is the initial status of the command after it's created. TO_BE_SENT, - // This status indicates that the DeleteBlocksCommand has been sent - // to the DataNode, but the Datanode has not reported any new status - // for the DeleteBlocksCommand. + // If the DeleteBlocksCommand has been sent but has not been executed + // completely by DN, the DeleteBlocksCommand's state will be SENT. + // Note that the state of SENT includes the following possibilities. + // - The command was sent but not received + // - The command was sent and received by the DN, + // and is waiting to be executed. + // - The Command sent and being executed by DN SENT, - // The DeleteBlocksCommand has been received by Datanode and - // is waiting for executed. - PENDING_EXECUTED, - // The DeleteBlocksCommand was executed, and the execution was successful - EXECUTED, - // The DeleteBlocksCommand was executed but failed to execute, - // or was lost before it was executed. - NEED_RESEND } protected static final class CmdStatusData { @@ -217,7 +207,7 @@ protected void recordScmCommand(CmdStatusData statusData) { } protected void onSent(UUID dnId, long scmCmdId) { - updateStatus(dnId, scmCmdId, SENT); + updateStatus(dnId, scmCmdId, CommandStatus.Status.PENDING); } protected void onDatanodeDead(UUID dnId) { @@ -227,10 +217,7 @@ protected void onDatanodeDead(UUID dnId) { protected void updateStatusByDNCommandStatus(UUID dnId, long scmCmdId, CommandStatus.Status newState) { - CmdStatus status = fromProtoCommandStatus(newState); - if (status != null) { - updateStatus(dnId, scmCmdId, status); - } + updateStatus(dnId, scmCmdId, newState); } protected void cleanAllTimeoutSCMCommand(long timeoutMs) { @@ -242,28 +229,13 @@ protected void cleanAllTimeoutSCMCommand(long timeoutMs) { } } - protected void cleanSCMCommandForDn(UUID dnId, long timeoutMs) { - cleanTimeoutSCMCommand(dnId, timeoutMs); - cleanFinalStatusSCMCommand(dnId); - } - - private void cleanTimeoutSCMCommand(UUID dnId, long timeoutMs) { + public void cleanTimeoutSCMCommand(UUID dnId, long timeoutMs) { for (CmdStatus status : STATUSES_REQUIRING_TIMEOUT) { removeTimeoutScmCommand( dnId, getScmCommandIds(dnId, status), timeoutMs); } } - private void cleanFinalStatusSCMCommand(UUID dnId) { - for (CmdStatus status : FINIAL_STATUSES) { - for (Long scmCmdId : getScmCommandIds(dnId, status)) { - CmdStatusData stateData = removeScmCommand(dnId, scmCmdId); - LOG.debug("Clean SCMCommand status: {} for DN: {}, stateData: {}", - status, dnId, stateData); - } - } - } - private Set getScmCommandIds(UUID dnId, CmdStatus status) { Set scmCmdIds = new HashSet<>(); Map record = scmCmdStatusRecord.get(dnId); @@ -286,7 +258,8 @@ private Instant getUpdateTime(UUID dnId, long scmCmdId) { return record.get(scmCmdId).getUpdateTime(); } - private void updateStatus(UUID dnId, long scmCmdId, CmdStatus newStatus) { + private void updateStatus(UUID dnId, long scmCmdId, + CommandStatus.Status newStatus) { Map recordForDn = scmCmdStatusRecord.get(dnId); if (recordForDn == null) { LOG.warn("Unknown Datanode: {} scmCmdId {} newStatus {}", @@ -302,49 +275,27 @@ private void updateStatus(UUID dnId, long scmCmdId, CmdStatus newStatus) { boolean changed = false; CmdStatusData statusData = recordForDn.get(scmCmdId); CmdStatus oldStatus = statusData.getStatus(); - switch (newStatus) { - case SENT: - if (oldStatus == TO_BE_SENT) { + case PENDING: + if (oldStatus == TO_BE_SENT || oldStatus == SENT) { // TO_BE_SENT -> SENT: The DeleteBlocksCommand is sent by SCM, // The follow-up status has not been updated by Datanode. + + // SENT -> SENT: The DeleteBlocksCommand continues to wait to be + // executed by Datanode. statusData.setStatus(SENT); changed = true; } break; - case PENDING_EXECUTED: - if (oldStatus == SENT || oldStatus == PENDING_EXECUTED) { - // SENT -> PENDING_EXECUTED: The DeleteBlocksCommand is sent and - // received by the Datanode, but the command is not executed by the - // Datanode, the command is waiting to be executed. - - // PENDING_EXECUTED -> PENDING_EXECUTED: The DeleteBlocksCommand - // continues to wait to be executed by Datanode. - statusData.setStatus(PENDING_EXECUTED); - changed = true; - } - break; - case NEED_RESEND: - if (oldStatus == SENT || oldStatus == PENDING_EXECUTED) { - // SENT -> NEED_RESEND: The DeleteBlocksCommand is sent and lost - // before it is received by the DN. - - // PENDING_EXECUTED -> NEED_RESEND: The DeleteBlocksCommand waited for - // a while and was executed, but the execution failed;. - // Or the DeleteBlocksCommand was lost while waiting(such as the - // Datanode restart). - statusData.setStatus(NEED_RESEND); - changed = true; - } - break; case EXECUTED: - if (oldStatus == SENT || oldStatus == PENDING_EXECUTED) { - // PENDING_EXECUTED -> EXECUTED: The Command waits for a period of - // time on the DN and is executed successfully. - - // SENT -> EXECUTED: The DeleteBlocksCommand has been sent to - // Datanode, executed by DN, and executed successfully. - statusData.setStatus(EXECUTED); + case FAILED: + if (oldStatus == SENT) { + // Once the DN executes DeleteBlocksCommands, regardless of whether + // DeleteBlocksCommands is executed successfully or not, + // it will be deleted from record. + // Successful DeleteBlocksCommands are recorded in + // `transactionToDNsCommitMap`. + removeScmCommand(dnId, scmCmdId); changed = true; } break; @@ -368,7 +319,6 @@ private void removeTimeoutScmCommand(UUID dnId, Instant updateTime = getUpdateTime(dnId, scmCmdId); if (updateTime != null && Duration.between(updateTime, now).toMillis() > timeoutMs) { - updateStatus(dnId, scmCmdId, NEED_RESEND); CmdStatusData state = removeScmCommand(dnId, scmCmdId); LOG.warn("Remove Timeout SCM BlockDeletionCommand {} for DN {} " + "after without update {}ms}", state, dnId, timeoutMs); @@ -384,35 +334,11 @@ private CmdStatusData removeScmCommand(UUID dnId, long scmCmdId) { if (record == null || record.get(scmCmdId) == null) { return null; } - - CmdStatus status = record.get(scmCmdId).getStatus(); - if (!FINIAL_STATUSES.contains(status)) { - LOG.error("Cannot Remove ScmCommand {} Non-final Status {} for DN: {}" + - ". final Status {}", scmCmdId, status, dnId, FINIAL_STATUSES); - return null; - } - CmdStatusData statusData = record.remove(scmCmdId); LOG.debug("Remove ScmCommand {} for DN: {} ", statusData, dnId); return statusData; } - private static CmdStatus fromProtoCommandStatus( - CommandStatus.Status protoCmdStatus) { - switch (protoCmdStatus) { - case PENDING: - return CmdStatus.PENDING_EXECUTED; - case EXECUTED: - return CmdStatus.EXECUTED; - case FAILED: - return CmdStatus.NEED_RESEND; - default: - LOG.error("Unknown protoCmdStatus: {} cannot convert " + - "to ScmDeleteBlockCommandStatus", protoCmdStatus); - return null; - } - } - public Map> getCommandStatusByTxId( Set dnIds) { Map> result = @@ -447,8 +373,8 @@ Map> getScmCmdStatusRecord() { } public void onSent(DatanodeDetails dnId, SCMCommand scmCommand) { - scmDeleteBlocksCommandStatusManager.updateStatus( - dnId.getUuid(), scmCommand.getId(), SENT); + scmDeleteBlocksCommandStatusManager.onSent( + dnId.getUuid(), scmCommand.getId()); } public Map> getCommandStatusByTxId( @@ -612,18 +538,14 @@ public void commitSCMCommandStatus(List deleteBlockStatus, UUID dnId) { processSCMCommandStatus(deleteBlockStatus, dnId); scmDeleteBlocksCommandStatusManager. - cleanSCMCommandForDn(dnId, scmCommandTimeoutMs); + cleanTimeoutSCMCommand(dnId, scmCommandTimeoutMs); } private boolean inProcessing(UUID dnId, long deletedBlocksTxId, Map> commandStatus) { Map deletedBlocksTxStatus = commandStatus.get(dnId); - if (deletedBlocksTxStatus == null || - deletedBlocksTxStatus.get(deletedBlocksTxId) == null) { - return false; - } - return deletedBlocksTxStatus.get(deletedBlocksTxId) != - CmdStatus.NEED_RESEND; + return deletedBlocksTxStatus != null && + deletedBlocksTxStatus.get(deletedBlocksTxId) != null; } private void processSCMCommandStatus(List deleteBlockStatus, diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java index 6aa396610c32..888cb42fd7de 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestSCMDeleteBlocksCommandStatusManager.java @@ -30,10 +30,7 @@ import java.util.UUID; import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager; -import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.NEED_RESEND; -import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.PENDING_EXECUTED; import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.SENT; -import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.EXECUTED; import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus.TO_BE_SENT; import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatusData; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -159,25 +156,10 @@ public void testUpdateStatusByDNCommandStatus() { manager.updateStatusByDNCommandStatus(dnId1, scmCmdId4, StorageContainerDatanodeProtocolProtos.CommandStatus.Status.PENDING); - assertEquals(PENDING_EXECUTED, dnStatusRecord.get(scmCmdId1).getStatus()); - assertEquals(EXECUTED, dnStatusRecord.get(scmCmdId2).getStatus()); - assertEquals(NEED_RESEND, dnStatusRecord.get(scmCmdId3).getStatus()); - assertEquals(PENDING_EXECUTED, dnStatusRecord.get(scmCmdId4).getStatus()); - - // PENDING_EXECUTED -> PENDING_EXECUTED - manager.updateStatusByDNCommandStatus(dnId1, scmCmdId1, - StorageContainerDatanodeProtocolProtos.CommandStatus.Status.PENDING); - assertEquals(PENDING_EXECUTED, dnStatusRecord.get(scmCmdId1).getStatus()); - - // PENDING_EXECUTED -> EXECUTED - manager.updateStatusByDNCommandStatus(dnId1, scmCmdId1, - StorageContainerDatanodeProtocolProtos.CommandStatus.Status.EXECUTED); - assertEquals(EXECUTED, dnStatusRecord.get(scmCmdId1).getStatus()); - - // PENDING_EXECUTED -> NEED_RESEND - manager.updateStatusByDNCommandStatus(dnId1, scmCmdId4, - StorageContainerDatanodeProtocolProtos.CommandStatus.Status.FAILED); - assertEquals(NEED_RESEND, dnStatusRecord.get(scmCmdId4).getStatus()); + assertEquals(SENT, dnStatusRecord.get(scmCmdId1).getStatus()); + assertNull(dnStatusRecord.get(scmCmdId2)); + assertNull(dnStatusRecord.get(scmCmdId3)); + assertEquals(SENT, dnStatusRecord.get(scmCmdId4).getStatus()); } @Test @@ -203,11 +185,11 @@ public void testCleanSCMCommandForDn() { Map dnStatusRecord = manager.getScmCmdStatusRecord().get(dnId1); assertNotNull(dnStatusRecord.get(scmCmdId1)); - assertNotNull(dnStatusRecord.get(scmCmdId2)); - assertNotNull(dnStatusRecord.get(scmCmdId3)); + assertNull(dnStatusRecord.get(scmCmdId2)); + assertNull(dnStatusRecord.get(scmCmdId3)); assertNotNull(dnStatusRecord.get(scmCmdId4)); - manager.cleanSCMCommandForDn(dnId1, Long.MAX_VALUE); + manager.cleanTimeoutSCMCommand(dnId1, Long.MAX_VALUE); // scmCmdId1 is PENDING_EXECUTED will be cleaned up after timeout assertNotNull(dnStatusRecord.get(scmCmdId1)); @@ -216,7 +198,7 @@ public void testCleanSCMCommandForDn() { // scmCmdId4 is SENT will be cleaned up after timeout assertNotNull(dnStatusRecord.get(scmCmdId4)); - manager.cleanSCMCommandForDn(dnId1, -1); + manager.cleanTimeoutSCMCommand(dnId1, -1); assertNull(dnStatusRecord.get(scmCmdId1)); assertNull(dnStatusRecord.get(scmCmdId4)); } From d353c083ea3e4549d18bd9ab60ccbb2def5b7986 Mon Sep 17 00:00:00 2001 From: xichen01 Date: Mon, 6 Nov 2023 19:32:32 +0800 Subject: [PATCH 18/23] move transactionToRetryCountMap into SCMDeletedBlockGTransactionStatusManager --- .../hdds/scm/block/DeletedBlockLogImpl.java | 35 +++--------------- ...MDeletedBlockTransactionStatusManager.java | 37 ++++++++++++++++++- 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java index 3b17f54d7a0a..2bc2c4213732 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java @@ -25,7 +25,6 @@ import java.util.Set; import java.util.Map; import java.util.ArrayList; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -76,7 +75,6 @@ public class DeletedBlockLogImpl implements DeletedBlockLog { private final int maxRetry; private final ContainerManager containerManager; private final Lock lock; - private final Map transactionToRetryCountMap; // The access to DeletedBlocksTXTable is protected by // DeletedBlockLogStateManager. private final DeletedBlockLogStateManager deletedBlockLogStateManager; @@ -103,7 +101,6 @@ public DeletedBlockLogImpl(ConfigurationSource conf, this.containerManager = containerManager; this.lock = new ReentrantLock(); - transactionToRetryCountMap = new ConcurrentHashMap<>(); this.deletedBlockLogStateManager = DeletedBlockLogStateManagerImpl .newBuilder() .setConfiguration(conf) @@ -117,9 +114,7 @@ public DeletedBlockLogImpl(ConfigurationSource conf, this.metrics = metrics; this.transactionStatusManager = new SCMDeletedBlockTransactionStatusManager(deletedBlockLogStateManager, - containerManager, scmContext, transactionToRetryCountMap, metrics, - lock, - scmCommandTimeoutMs); + containerManager, scmContext, metrics, lock, scmCommandTimeoutMs); } @Override @@ -164,25 +159,7 @@ public void incrementCount(List txIDs) throws IOException { lock.lock(); try { - ArrayList txIDsToUpdate = new ArrayList<>(); - for (Long txID : txIDs) { - int currentCount = - transactionToRetryCountMap.getOrDefault(txID, 0); - if (currentCount > maxRetry) { - continue; - } else { - currentCount += 1; - if (currentCount > maxRetry) { - txIDsToUpdate.add(txID); - } - transactionToRetryCountMap.put(txID, currentCount); - } - } - - if (!txIDsToUpdate.isEmpty()) { - deletedBlockLogStateManager - .increaseRetryCountOfTransactionInDB(txIDsToUpdate); - } + transactionStatusManager.incrementRetryCount(txIDs, maxRetry); } finally { lock.unlock(); } @@ -201,9 +178,7 @@ public int resetCount(List txIDs) throws IOException { .map(DeletedBlocksTransaction::getTxID) .collect(Collectors.toList()); } - for (Long txID: txIDs) { - transactionToRetryCountMap.computeIfPresent(txID, (key, value) -> 0); - } + transactionStatusManager.resetRetryCount(txIDs); return deletedBlockLogStateManager.resetRetryCountOfTransactionInDB( new ArrayList<>(new HashSet<>(txIDs))); } finally { @@ -276,7 +251,6 @@ public void reinitialize( * leader. */ public void onBecomeLeader() { - transactionToRetryCountMap.clear(); transactionStatusManager.clear(); } @@ -325,7 +299,8 @@ private void getTransaction(DeletedBlocksTransaction tx, Map> commandStatus) { DeletedBlocksTransaction updatedTxn = DeletedBlocksTransaction.newBuilder(tx) - .setCount(transactionToRetryCountMap.getOrDefault(tx.getTxID(), 0)) + .setCount(transactionStatusManager.getOrDefaultRetryCount( + tx.getTxID(), 0)) .build(); for (ContainerReplica replica : replicas) { DatanodeDetails details = replica.getDatanodeDetails(); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java index 27a225b17c33..701ad452a593 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java @@ -93,7 +93,6 @@ public class SCMDeletedBlockTransactionStatusManager public SCMDeletedBlockTransactionStatusManager( DeletedBlockLogStateManager deletedBlockLogStateManager, ContainerManager containerManager, SCMContext scmContext, - Map transactionToRetryCountMap, ScmBlockDeletingServiceMetrics metrics, Lock lock, long scmCommandTimeoutMs) { // maps transaction to dns which have committed it. @@ -104,7 +103,7 @@ public SCMDeletedBlockTransactionStatusManager( this.lock = lock; this.scmCommandTimeoutMs = scmCommandTimeoutMs; this.transactionToDNsCommitMap = new ConcurrentHashMap<>(); - this.transactionToRetryCountMap = transactionToRetryCountMap; + this.transactionToRetryCountMap = new ConcurrentHashMap<>(); this.scmDeleteBlocksCommandStatusManager = new SCMDeleteBlocksCommandStatusManager(); } @@ -372,6 +371,39 @@ Map> getScmCmdStatusRecord() { } } + public void incrementRetryCount(List txIDs, long maxRetry) + throws IOException { + ArrayList txIDsToUpdate = new ArrayList<>(); + for (Long txID : txIDs) { + int currentCount = + transactionToRetryCountMap.getOrDefault(txID, 0); + if (currentCount > maxRetry) { + continue; + } else { + currentCount += 1; + if (currentCount > maxRetry) { + txIDsToUpdate.add(txID); + } + transactionToRetryCountMap.put(txID, currentCount); + } + } + + if (!txIDsToUpdate.isEmpty()) { + deletedBlockLogStateManager + .increaseRetryCountOfTransactionInDB(txIDsToUpdate); + } + } + + public void resetRetryCount(List txIDs) throws IOException { + for (Long txID: txIDs) { + transactionToRetryCountMap.computeIfPresent(txID, (key, value) -> 0); + } + } + + public int getOrDefaultRetryCount(long txID, int defaultValue) { + return transactionToRetryCountMap.getOrDefault(txID, defaultValue); + } + public void onSent(DatanodeDetails dnId, SCMCommand scmCommand) { scmDeleteBlocksCommandStatusManager.onSent( dnId.getUuid(), scmCommand.getId()); @@ -395,6 +427,7 @@ public void recordTransactionCommitted(long txId) { } public void clear() { + transactionToRetryCountMap.clear(); scmDeleteBlocksCommandStatusManager.clear(); transactionToDNsCommitMap.clear(); } From d8592648815b4f09ebee135af50dd6a6e373a280 Mon Sep 17 00:00:00 2001 From: xichen01 Date: Fri, 1 Dec 2023 16:27:19 +0800 Subject: [PATCH 19/23] Merge recordTransactionCreated and recordTransactionCommitted; Remove useless code; Fix thread issue --- .../hadoop/hdds/scm/block/DeletedBlockLogImpl.java | 2 -- .../hadoop/hdds/scm/block/SCMBlockDeletingService.java | 4 ++-- .../block/SCMDeletedBlockTransactionStatusManager.java | 9 +++------ 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java index 2bc2c4213732..4790eef6e907 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java @@ -373,8 +373,6 @@ public DatanodeDeletedBlockTransactions getTransactions( } getTransaction( txn, transactions, dnList, replicas, commandStatus); - getSCMDeletedBlockTransactionStatusManager(). - recordTransactionCommitted(txn.getTxID()); } } catch (ContainerNotFoundException ex) { LOG.warn("Container: " + id + " was not found for the transaction: " diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java index f1acb9a4b46d..9015be0c99ac 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java @@ -184,10 +184,10 @@ public EmptyTaskResult call() throws Exception { processedTxIDs.addAll(dnTxSet); DeleteBlocksCommand command = new DeleteBlocksCommand(dnTXs); command.setTerm(scmContext.getTermOfLeader()); - eventPublisher.fireEvent(SCMEvents.DATANODE_COMMAND, - new CommandForDatanode<>(dnId, command)); deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() .recordTransactionCreated(dnId, command.getId(), dnTxSet); + eventPublisher.fireEvent(SCMEvents.DATANODE_COMMAND, + new CommandForDatanode<>(dnId, command)); metrics.incrBlockDeletionCommandSent(); metrics.incrBlockDeletionTransactionSent(dnTXs.size()); if (LOG.isDebugEnabled()) { diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java index 701ad452a593..ee24caf89d13 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java @@ -139,7 +139,7 @@ public enum CmdStatus { // - The command was sent and received by the DN, // and is waiting to be executed. // - The Command sent and being executed by DN - SENT, + SENT } protected static final class CmdStatusData { @@ -419,11 +419,8 @@ public void recordTransactionCreated( scmDeleteBlocksCommandStatusManager.recordScmCommand( SCMDeleteBlocksCommandStatusManager .createScmCmdStatusData(dnId, scmCmdId, dnTxSet)); - } - - public void recordTransactionCommitted(long txId) { - transactionToDNsCommitMap - .putIfAbsent(txId, new LinkedHashSet<>()); + dnTxSet.forEach(txId -> transactionToDNsCommitMap + .putIfAbsent(txId, new LinkedHashSet<>())); } public void clear() { From addbdf5549a40e97d0a839e3c009ba466076ccd9 Mon Sep 17 00:00:00 2001 From: xichen01 Date: Fri, 1 Dec 2023 16:35:05 +0800 Subject: [PATCH 20/23] Add additional processing logic in the updateStatus --- .../block/SCMDeletedBlockTransactionStatusManager.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java index ee24caf89d13..d550f60120b4 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java @@ -297,6 +297,16 @@ private void updateStatus(UUID dnId, long scmCmdId, removeScmCommand(dnId, scmCmdId); changed = true; } + if (oldStatus == TO_BE_SENT) { + // SCM receives a reply to an unsent transaction, + // which should not normally occur. + LOG.error("Received {} status for a command marked TO_BE_SENT. " + + "This indicates a potential issue in command handling. " + + "SCM Command ID: {}, Datanode ID: {}, Current Status: {}", + newStatus, scmCmdId, dnId, oldStatus); + removeScmCommand(dnId, scmCmdId); + changed = true; + } break; default: LOG.error("Can not update to Unknown new Status: {}", newStatus); From 343b67d20bc5c4202595dc506ec83f5748a819ac Mon Sep 17 00:00:00 2001 From: xichen01 Date: Sun, 3 Dec 2023 01:30:55 +0800 Subject: [PATCH 21/23] Fix test --- .../hdds/scm/block/TestDeletedBlockLog.java | 38 +++++++++++++++++-- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java index a1cdcb825456..6ce03d5e073f 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java @@ -323,6 +323,13 @@ private List getTransactions( transactions.getDatanodeTransactionMap().get(dn.getUuid())) .orElseGet(LinkedList::new)); } + // Simulated transactions are sent + for (Map.Entry> entry : + transactions.getDatanodeTransactionMap().entrySet()) { + DeleteBlocksCommand command = new DeleteBlocksCommand(entry.getValue()); + recordScmCommandToStatusManager(entry.getKey(), command); + sendSCMDeleteBlocksCommand(entry.getKey(), command); + } return txns; } @@ -439,6 +446,9 @@ public void testResetCount() throws Exception { } // Increment for the reset transactions. + // Lets the SCM delete the transaction and wait for the DN reply + // to timeout, thus allowing the transaction to resend the + deletedBlockLog.setScmCommandTimeoutMs(-1L); incrementCount(txIDs); blocks = getAllTransactions(); for (DeletedBlocksTransaction block : blocks) { @@ -450,6 +460,7 @@ public void testResetCount() throws Exception { @Test public void testCommitTransactions() throws Exception { + deletedBlockLog.setScmCommandTimeoutMs(Long.MAX_VALUE); addTransactions(generateData(50), true); mockContainerHealthResult(true); List blocks = @@ -466,6 +477,12 @@ public void testCommitTransactions() throws Exception { DatanodeDetails.newBuilder().setUuid(UUID.randomUUID()) .build()); + blocks = getTransactions(50 * BLOCKS_PER_TXN * THREE); + // SCM will not repeat a transaction until it has timed out. + Assertions.assertEquals(0, blocks.size()); + // Lets the SCM delete the transaction and wait for the DN reply + // to timeout, thus allowing the transaction to resend the + deletedBlockLog.setScmCommandTimeoutMs(-1L); blocks = getTransactions(50 * BLOCKS_PER_TXN * THREE); // only uncommitted dn have transactions Assertions.assertEquals(30, blocks.size()); @@ -566,6 +583,7 @@ public void testNoDuplicateTransactionsForInProcessingSCMCommand() // - If the DN reports the command status as PENDING; addTransactions(generateData(10), true); int blockLimit = 2 * BLOCKS_PER_TXN * THREE; + mockContainerHealthResult(true); // If the command has not been sent DatanodeDeletedBlockTransactions transactions1 = @@ -611,6 +629,7 @@ public void testFailedAndTimeoutSCMCommandCanBeResend() throws Exception { deletedBlockLog.setScmCommandTimeoutMs(Long.MAX_VALUE); addTransactions(generateData(10), true); int blockLimit = 2 * BLOCKS_PER_TXN * THREE; + mockContainerHealthResult(true); // - DN does not refresh the PENDING state for more than a period of time; DatanodeDeletedBlockTransactions transactions = @@ -670,9 +689,7 @@ public void testInadequateReplicaCommit() throws Exception { // For the first 30 txn, deletedBlockLog only has the txn from dn1 and dn2 // For the rest txn, txn will be got from all dns. // Committed txn will be: 1-40. 1-40. 31-40 - commitTransactions(deletedBlockLog.getTransactions( - 30 * BLOCKS_PER_TXN * THREE, - dnList.stream().collect(Collectors.toSet()))); + commitTransactions(getTransactions(30 * BLOCKS_PER_TXN * THREE)); // The rest txn shall be: 41-50. 41-50. 41-50 List blocks = getAllTransactions(); @@ -764,6 +781,7 @@ public void testPersistence() throws Exception { @Test public void testDeletedBlockTransactions() throws IOException, TimeoutException { + deletedBlockLog.setScmCommandTimeoutMs(Long.MAX_VALUE); mockContainerHealthResult(true); int txNum = 10; List blocks; @@ -796,9 +814,21 @@ public void testDeletedBlockTransactions() // add two transactions for same container containerID = blocks.get(0).getContainerID(); Map> deletedBlocksMap = new HashMap<>(); - deletedBlocksMap.put(containerID, new LinkedList<>()); + Random random = new Random(); + long localId = random.nextLong(); + deletedBlocksMap.put(containerID, new LinkedList<>( + Collections.singletonList(localId))); addTransactions(deletedBlocksMap, true); + blocks = getTransactions(txNum * BLOCKS_PER_TXN * ONE); + // Only newly added Blocks will be sent, as previously sent transactions + // that have not yet timed out will not be sent. + Assertions.assertEquals(1, blocks.size()); + Assertions.assertEquals(1, blocks.get(0).getLocalIDCount()); + Assertions.assertEquals(blocks.get(0).getLocalID(0), localId); + // Lets the SCM delete the transaction and wait for the DN reply + // to timeout, thus allowing the transaction to resend the + deletedBlockLog.setScmCommandTimeoutMs(-1L); // get should return two transactions for the same container blocks = getTransactions(txNum * BLOCKS_PER_TXN * ONE); Assertions.assertEquals(2, blocks.size()); From 03cca5ff050c207572bbf5bf1ea9d0e9cf702e04 Mon Sep 17 00:00:00 2001 From: xichen01 Date: Sun, 3 Dec 2023 02:10:50 +0800 Subject: [PATCH 22/23] Findbugs --- .../hadoop/hdds/scm/block/TestDeletedBlockLog.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java index 6ce03d5e073f..239f292e63d0 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java @@ -291,16 +291,6 @@ private void commitTransactions( .collect(Collectors.toList())); } - private void commitTransactions(DatanodeDeletedBlockTransactions - transactions) { - transactions.getDatanodeTransactionMap().forEach((uuid, - deletedBlocksTransactions) -> - deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() - .commitTransactions(deletedBlocksTransactions.stream() - .map(this::createDeleteBlockTransactionResult) - .collect(Collectors.toList()), uuid)); - } - private DeleteBlockTransactionResult createDeleteBlockTransactionResult( DeletedBlocksTransaction transaction) { return DeleteBlockTransactionResult.newBuilder() From 0a39cb3c8399f9e3e5e02f039780a379ccd958de Mon Sep 17 00:00:00 2001 From: xichen01 Date: Sun, 10 Dec 2023 02:17:44 +0800 Subject: [PATCH 23/23] Remove getSCMDeletedBlockTransactionStatusManager from DeletedBlockLog --- .../hdds/scm/block/DeletedBlockLog.java | 32 +++++++- .../hdds/scm/block/DeletedBlockLogImpl.java | 75 +++++++++++++++++-- .../scm/block/SCMBlockDeletingService.java | 4 +- ...MDeletedBlockTransactionStatusManager.java | 46 +----------- .../hadoop/hdds/scm/node/DeadNodeHandler.java | 3 +- .../scm/server/StorageContainerManager.java | 8 +- .../hdds/scm/block/TestDeletedBlockLog.java | 5 +- .../hdds/scm/node/TestDeadNodeHandler.java | 16 +--- 8 files changed, 110 insertions(+), 79 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java index be67f05cd893..45d53c0ef2cd 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java @@ -22,11 +22,13 @@ import org.apache.hadoop.hdds.protocol.proto .StorageContainerDatanodeProtocolProtos.DeletedBlocksTransaction; import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.ozone.protocol.commands.SCMCommand; import java.io.Closeable; import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.UUID; /** * The DeletedBlockLog is a persisted log in SCM to keep tracking @@ -84,11 +86,33 @@ void incrementCount(List txIDs) int resetCount(List txIDs) throws IOException; /** - * Get SCMDeletedBlockTransactionStatusManager. - * @return an Object of SCMDeletedBlockTransactionStatusManager + * Records the creation of a transaction for a DataNode. + * + * @param dnId The identifier of the DataNode. + * @param scmCmdId The ID of the SCM command. + * @param dnTxSet Set of transaction IDs for the DataNode. + */ + void recordTransactionCreated( + UUID dnId, long scmCmdId, Set dnTxSet); + + /** + * Handles the cleanup process when a DataNode is reported dead. This method + * is responsible for updating or cleaning up the transaction records + * associated with the dead DataNode. + * + * @param dnId The identifier of the dead DataNode. + */ + void onDatanodeDead(UUID dnId); + + /** + * Records the event of sending a block deletion command to a DataNode. This + * method is called when a command is successfully dispatched to a DataNode, + * and it helps in tracking the status of the command. + * + * @param dnId Details of the DataNode. + * @param scmCommand The block deletion command sent. */ - SCMDeletedBlockTransactionStatusManager - getSCMDeletedBlockTransactionStatusManager(); + void onSent(DatanodeDetails dnId, SCMCommand scmCommand); /** * Creates block deletion transactions for a set of containers, diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java index 4790eef6e907..2f55b76f8df1 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java @@ -30,10 +30,14 @@ import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; +import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hdds.conf.ConfigurationSource; 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; import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.ContainerID; @@ -45,6 +49,8 @@ import org.apache.hadoop.hdds.scm.ha.SCMRatisServer; import org.apache.hadoop.hdds.scm.ha.SequenceIdGenerator; import org.apache.hadoop.hdds.scm.metadata.DBTransactionBuffer; +import org.apache.hadoop.hdds.server.events.EventHandler; +import org.apache.hadoop.hdds.server.events.EventPublisher; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.TableIterator; @@ -54,6 +60,7 @@ import static org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager.SCMDeleteBlocksCommandStatusManager.CmdStatus; import static org.apache.hadoop.hdds.scm.ha.SequenceIdGenerator.DEL_TXN_ID; +import org.apache.hadoop.ozone.protocol.commands.SCMCommand; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -67,7 +74,8 @@ * equally same chance to be retrieved which only depends on the nature * order of the transaction ID. */ -public class DeletedBlockLogImpl implements DeletedBlockLog { +public class DeletedBlockLogImpl + implements DeletedBlockLog, EventHandler { public static final Logger LOG = LoggerFactory.getLogger(DeletedBlockLogImpl.class); @@ -114,7 +122,7 @@ public DeletedBlockLogImpl(ConfigurationSource conf, this.metrics = metrics; this.transactionStatusManager = new SCMDeletedBlockTransactionStatusManager(deletedBlockLogStateManager, - containerManager, scmContext, metrics, lock, scmCommandTimeoutMs); + containerManager, scmContext, metrics, scmCommandTimeoutMs); } @Override @@ -196,12 +204,6 @@ private DeletedBlocksTransaction constructNewTransaction( .build(); } - @Override - public SCMDeletedBlockTransactionStatusManager - getSCMDeletedBlockTransactionStatusManager() { - return transactionStatusManager; - } - private boolean isTransactionFailed(DeleteBlockTransactionResult result) { if (LOG.isDebugEnabled()) { LOG.debug( @@ -395,4 +397,61 @@ public void setScmCommandTimeoutMs(long scmCommandTimeoutMs) { this.scmCommandTimeoutMs = scmCommandTimeoutMs; } + @VisibleForTesting + public SCMDeletedBlockTransactionStatusManager + getSCMDeletedBlockTransactionStatusManager() { + return transactionStatusManager; + } + + @Override + public void recordTransactionCreated(UUID dnId, long scmCmdId, + Set dnTxSet) { + getSCMDeletedBlockTransactionStatusManager() + .recordTransactionCreated(dnId, scmCmdId, dnTxSet); + } + + @Override + public void onDatanodeDead(UUID dnId) { + getSCMDeletedBlockTransactionStatusManager().onDatanodeDead(dnId); + } + + @Override + public void onSent(DatanodeDetails dnId, SCMCommand scmCommand) { + getSCMDeletedBlockTransactionStatusManager().onSent(dnId, scmCommand); + } + + @Override + public void onMessage( + DeleteBlockStatus deleteBlockStatus, EventPublisher publisher) { + if (!scmContext.isLeader()) { + LOG.warn("Skip commit transactions since current SCM is not leader."); + return; + } + + DatanodeDetails details = deleteBlockStatus.getDatanodeDetails(); + UUID dnId = details.getUuid(); + for (CommandStatus commandStatus : deleteBlockStatus.getCmdStatus()) { + CommandStatus.Status status = commandStatus.getStatus(); + lock.lock(); + try { + if (status == CommandStatus.Status.EXECUTED) { + ContainerBlocksDeletionACKProto ackProto = + commandStatus.getBlockDeletionAck(); + getSCMDeletedBlockTransactionStatusManager() + .commitTransactions(ackProto.getResultsList(), dnId); + metrics.incrBlockDeletionCommandSuccess(); + } else if (status == CommandStatus.Status.FAILED) { + metrics.incrBlockDeletionCommandFailure(); + } else { + LOG.debug("Delete Block Command {} is not executed on the Datanode" + + " {}.", commandStatus.getCmdId(), dnId); + } + + getSCMDeletedBlockTransactionStatusManager() + .commitSCMCommandStatus(deleteBlockStatus.getCmdStatus(), dnId); + } finally { + lock.unlock(); + } + } + } } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java index ae6edc1518da..24e950a599ff 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java @@ -181,8 +181,8 @@ public EmptyTaskResult call() throws Exception { processedTxIDs.addAll(dnTxSet); DeleteBlocksCommand command = new DeleteBlocksCommand(dnTXs); command.setTerm(scmContext.getTermOfLeader()); - deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() - .recordTransactionCreated(dnId, command.getId(), dnTxSet); + deletedBlockLog.recordTransactionCreated(dnId, command.getId(), + dnTxSet); eventPublisher.fireEvent(SCMEvents.DATANODE_COMMAND, new CommandForDatanode<>(dnId, command)); metrics.incrBlockDeletionCommandSent(); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java index d550f60120b4..b43e91e0592f 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java @@ -22,16 +22,12 @@ import com.google.common.annotations.VisibleForTesting; 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.scm.command.CommandStatusReportHandler.DeleteBlockStatus; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.ContainerInfo; import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.ha.SCMContext; -import org.apache.hadoop.hdds.server.events.EventHandler; -import org.apache.hadoop.hdds.server.events.EventPublisher; import org.apache.hadoop.ozone.protocol.commands.SCMCommand; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,7 +46,6 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.locks.Lock; import java.util.stream.Collectors; import static java.lang.Math.min; @@ -63,8 +58,7 @@ * the purpose of this class is to reduce the number of duplicate * DeletedBlockTransaction sent to the DN. */ -public class SCMDeletedBlockTransactionStatusManager - implements EventHandler { +public class SCMDeletedBlockTransactionStatusManager { public static final Logger LOG = LoggerFactory.getLogger(SCMDeletedBlockTransactionStatusManager.class); // Maps txId to set of DNs which are successful in committing the transaction @@ -77,7 +71,6 @@ public class SCMDeletedBlockTransactionStatusManager private final ContainerManager containerManager; private final ScmBlockDeletingServiceMetrics metrics; private final SCMContext scmContext; - private final Lock lock; private final long scmCommandTimeoutMs; /** @@ -93,14 +86,12 @@ public class SCMDeletedBlockTransactionStatusManager public SCMDeletedBlockTransactionStatusManager( DeletedBlockLogStateManager deletedBlockLogStateManager, ContainerManager containerManager, SCMContext scmContext, - ScmBlockDeletingServiceMetrics metrics, - Lock lock, long scmCommandTimeoutMs) { + ScmBlockDeletingServiceMetrics metrics, long scmCommandTimeoutMs) { // maps transaction to dns which have committed it. this.deletedBlockLogStateManager = deletedBlockLogStateManager; this.metrics = metrics; this.containerManager = containerManager; this.scmContext = scmContext; - this.lock = lock; this.scmCommandTimeoutMs = scmCommandTimeoutMs; this.transactionToDNsCommitMap = new ConcurrentHashMap<>(); this.transactionToRetryCountMap = new ConcurrentHashMap<>(); @@ -540,39 +531,6 @@ public void commitTransactions( } } - @Override - public void onMessage( - DeleteBlockStatus deleteBlockStatus, EventPublisher publisher) { - if (!scmContext.isLeader()) { - LOG.warn("Skip commit transactions since current SCM is not leader."); - return; - } - - DatanodeDetails details = deleteBlockStatus.getDatanodeDetails(); - UUID dnId = details.getUuid(); - for (CommandStatus commandStatus : deleteBlockStatus.getCmdStatus()) { - CommandStatus.Status status = commandStatus.getStatus(); - lock.lock(); - try { - if (status == CommandStatus.Status.EXECUTED) { - ContainerBlocksDeletionACKProto ackProto = - commandStatus.getBlockDeletionAck(); - commitTransactions(ackProto.getResultsList(), dnId); - metrics.incrBlockDeletionCommandSuccess(); - } else if (status == CommandStatus.Status.FAILED) { - metrics.incrBlockDeletionCommandFailure(); - } else { - LOG.debug("Delete Block Command {} is not executed on the Datanode" + - " {}.", commandStatus.getCmdId(), dnId); - } - - commitSCMCommandStatus(deleteBlockStatus.getCmdStatus(), dnId); - } finally { - lock.unlock(); - } - } - } - @VisibleForTesting public void commitSCMCommandStatus(List deleteBlockStatus, UUID dnId) { diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java index d4237ed7ab75..3c40437d7f66 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java @@ -112,8 +112,7 @@ public void onMessage(final DatanodeDetails datanodeDetails, // is IN_MAINTENANCE if (deletedBlockLog != null && !nodeManager.getNodeStatus(datanodeDetails).isInMaintenance()) { - deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() - .onDatanodeDead(datanodeDetails.getUuid()); + deletedBlockLog.onDatanodeDead(datanodeDetails.getUuid()); } //move dead datanode out of ClusterNetworkTopology diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index a72f182b8b1f..24adb0ca8e15 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -99,6 +99,7 @@ import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.block.BlockManager; import org.apache.hadoop.hdds.scm.block.BlockManagerImpl; +import org.apache.hadoop.hdds.scm.block.DeletedBlockLogImpl; import org.apache.hadoop.hdds.scm.command.CommandStatusReportHandler; import org.apache.hadoop.hdds.scm.container.CloseContainerEventHandler; import org.apache.hadoop.hdds.scm.container.ContainerActionsHandler; @@ -568,16 +569,15 @@ private void initializeEventHandlers() { eventQueue.addHandler(SCMEvents.START_ADMIN_ON_NODE, datanodeStartAdminHandler); eventQueue.addHandler(SCMEvents.CMD_STATUS_REPORT, cmdStatusReportHandler); - eventQueue.addHandler(SCMEvents.DELETE_BLOCK_STATUS, scmBlockManager - .getDeletedBlockLog().getSCMDeletedBlockTransactionStatusManager()); + eventQueue.addHandler(SCMEvents.DELETE_BLOCK_STATUS, + (DeletedBlockLogImpl) scmBlockManager.getDeletedBlockLog()); eventQueue.addHandler(SCMEvents.PIPELINE_ACTIONS, pipelineActionHandler); eventQueue.addHandler(SCMEvents.PIPELINE_REPORT, pipelineReportHandler); eventQueue.addHandler(SCMEvents.CRL_STATUS_REPORT, crlStatusReportHandler); scmNodeManager.registerSendCommandNotify( SCMCommandProto.Type.deleteBlocksCommand, - scmBlockManager.getDeletedBlockLog() - .getSCMDeletedBlockTransactionStatusManager()::onSent); + scmBlockManager.getDeletedBlockLog()::onSent); } private void initializeCertificateClient() throws IOException { diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java index 239f292e63d0..987b1ddbb90d 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java @@ -487,12 +487,11 @@ private void recordScmCommandToStatusManager( Set dnTxSet = command.blocksTobeDeleted() .stream().map(DeletedBlocksTransaction::getTxID) .collect(Collectors.toSet()); - deletedBlockLog.getSCMDeletedBlockTransactionStatusManager() - .recordTransactionCreated(dnId, command.getId(), dnTxSet); + deletedBlockLog.recordTransactionCreated(dnId, command.getId(), dnTxSet); } private void sendSCMDeleteBlocksCommand(UUID dnId, SCMCommand scmCommand) { - deletedBlockLog.getSCMDeletedBlockTransactionStatusManager().onSent( + deletedBlockLog.onSent( DatanodeDetails.newBuilder().setUuid(dnId).build(), scmCommand); } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java index 46d2a27700e7..0f65cdb10870 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java @@ -48,7 +48,6 @@ import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.HddsTestUtils; import org.apache.hadoop.hdds.scm.block.DeletedBlockLog; -import org.apache.hadoop.hdds.scm.block.SCMDeletedBlockTransactionStatusManager; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException; @@ -92,8 +91,7 @@ public class TestDeadNodeHandler { private EventQueue eventQueue; private String storageDir; private SCMContext scmContext; - private SCMDeletedBlockTransactionStatusManager - deleteBlocksCommandStatusManager; + private DeletedBlockLog deletedBlockLog; @BeforeEach public void setup() throws IOException, AuthenticationException { @@ -121,11 +119,7 @@ public void setup() throws IOException, AuthenticationException { pipelineManager.setPipelineProvider(RATIS, mockRatisProvider); containerManager = scm.getContainerManager(); - DeletedBlockLog deletedBlockLog = Mockito.mock(DeletedBlockLog.class); - deleteBlocksCommandStatusManager = - Mockito.mock(SCMDeletedBlockTransactionStatusManager.class); - Mockito.when(deletedBlockLog.getSCMDeletedBlockTransactionStatusManager()) - .thenReturn(deleteBlocksCommandStatusManager); + deletedBlockLog = Mockito.mock(DeletedBlockLog.class); deadNodeHandler = new DeadNodeHandler(nodeManager, Mockito.mock(PipelineManager.class), containerManager, deletedBlockLog); healthyReadOnlyNodeHandler = @@ -243,8 +237,7 @@ public void testOnMessage() throws Exception { Assertions.assertFalse( nodeManager.getClusterNetworkTopologyMap().contains(datanode1)); - Mockito.verify(deleteBlocksCommandStatusManager, - Mockito.times(0)) + Mockito.verify(deletedBlockLog, Mockito.times(0)) .onDatanodeDead(datanode1.getUuid()); Set container1Replicas = containerManager @@ -274,8 +267,7 @@ public void testOnMessage() throws Exception { Assertions.assertEquals(0, nodeManager.getCommandQueueCount(datanode1.getUuid(), cmd.getType())); - Mockito.verify(deleteBlocksCommandStatusManager, - Mockito.times(1)) + Mockito.verify(deletedBlockLog, Mockito.times(1)) .onDatanodeDead(datanode1.getUuid()); container1Replicas = containerManager