szetszwo commented on code in PR #4403:
URL: https://github.com/apache/ozone/pull/4403#discussion_r1144595675
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLog.java:
##########
@@ -43,11 +44,14 @@ public interface DeletedBlockLog extends Closeable {
* Scan entire log once and returns TXs to DatanodeDeletedBlockTransactions.
* Once DatanodeDeletedBlockTransactions is full, the scan behavior will
* stop.
+ *
* @param blockDeletionLimit Maximum number of blocks to fetch
+ * @param dnUUIDMap map of uuid and DN details
Review Comment:
From the type, we already know that it is a map of uuid and DN details. We
should talk about what this is for. BTW, I suggest to change it to `Set` and
rename it to `included` as mentioned in another comment.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java:
##########
@@ -134,14 +135,19 @@ public EmptyTaskResult call() throws Exception {
if (LOG.isDebugEnabled()) {
LOG.debug("Running DeletedBlockTransactionScanner");
}
- // TODO - DECOMM - should we be deleting blocks from decom nodes
- // and what about entering maintenance.
List<DatanodeDetails> datanodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
if (datanodes != null) {
+ // When DN node is healthy and in-service, and previous commands
+ // are handled for deleteBlocks Type, then it will be considered
+ // in this iteration
+ Map<UUID, DatanodeDetails> dnUUIDMap = datanodes.stream().filter(
Review Comment:
Let's declare it as `Set<DatanodeDetails>` and rename it to `included`, i.e.
```java
final Set<DatanodeDetails> included = datanodes.stream()
.filter(dn -> nodeManager.getCommandQueueCount(
dn.getUuid(), Type.deleteBlocksCommand) == 0)
.collect(Collectors.toSet());
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java:
##########
@@ -87,6 +87,9 @@ public void onMessage(final DatanodeDetails datanodeDetails,
if (!nodeManager.getNodeStatus(datanodeDetails).isInMaintenance()) {
removeContainerReplicas(datanodeDetails);
}
+
+ // remove commands in command queue for the DN
+ nodeManager.getCommandQueue(datanodeDetails.getUuid());
Review Comment:
Add a log message for dropping the commands.
```
final List<SCMCommand> commands =
nodeManager.getCommandQueue(datanodeDetails.getUuid());
LOG.info(...);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
