[GitHub] [ozone] sumitagrawl commented on a diff in pull request #4939: HDDS-8888. Consider Datanode queue capacity when sending DeleteBlocks command

["sumitagrawl (via GitHub)" <gi...@apache.org>]

sumitagrawl commented on code in PR #4939:
URL: https://github.com/apache/ozone/pull/4939#discussion_r1259872326


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java:
##########
@@ -273,4 +279,24 @@ public void stop() {
   public ScmBlockDeletingServiceMetrics getMetrics() {
     return this.metrics;
   }
+
+  /**
+   * Filters and returns a set of healthy datanodes that have not exceeded
+   * the deleteBlocksPendingCommandLimit.
+   *
+   * @param datanodes a list of DatanodeDetails
+   * @return a set of filtered DatanodeDetails
+   */
+  @VisibleForTesting
+  protected Set<DatanodeDetails> getDatanodesWithinCommandLimit(
+      List<DatanodeDetails> datanodes) throws NodeNotFoundException {
+    final Set<DatanodeDetails> included = new HashSet<>();
+    for (DatanodeDetails dn : datanodes) {
+      if (nodeManager.getTotalDatanodeCommandCount(dn,

Review Comment:
   @xichen01 
   1. Without processing DN HB, this will get same command again. So it is not useful to keep queue at SCM side. Else SCM will add duplicate command only, and DN will be processing duplicate commands only.
   2. DN queue is limited to "5" only as default (to limit the duplicate request queueing up when DN is slow). and handle memory in queue.
   
   Considering parallel HB and SCM deleteBlock processing,  they are synchronized using lock, 
   - HB take lock, get commands, process HB response, release lock
   - SCM deleteBlock, getCommandQueueCount() take lock, get count and then release lock
   
   So below sequence,
   Scenario 1: - No issue as queue empty and next command can be added
   1. HB comes first
   2. SCM delete block processing next
   
   Scenario 2: Here, adding same command will be duplicate and there is retry
   1. SCM delete block processing first
   2. HB comes next
   
   Considering this, we need not have queue at SCM and above changes not required.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org