You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/10/26 15:23:33 UTC

[GitHub] [ozone] sodonnel opened a new pull request, #3891: HDDS-7402. Adapt CommandQueue to track the count of each queued command type

sodonnel opened a new pull request, #3891:
URL: https://github.com/apache/ozone/pull/3891

   ## What changes were proposed in this pull request?
   
   Currently, all commands queued for a datanode are stored in an ArrayList, which is sent to the DN on each heartbeat.
   
   In order to limit the number of commands queued on a DN at any time, it would be useful to be able to query the current queued count of each command type, allowing Replication Manager, for example, to stop sending more work to the DN until it has free capacity.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-7402
   
   ## How was this patch tested?
   
   New unit tests
   


-- 
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


[GitHub] [ozone] sodonnel commented on a diff in pull request #3891: HDDS-7402. Adapt CommandQueue to track the count of each queued command type

Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3891:
URL: https://github.com/apache/ozone/pull/3891#discussion_r1006225317


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/CommandQueue.java:
##########
@@ -38,9 +40,8 @@
  * there where queued.
  */
 public class CommandQueue {
-  // This list is used as default return value.
-  private static final List<SCMCommand> DEFAULT_LIST = new ArrayList<>();
   private final Map<UUID, Commands> commandMap;
+  private final Map<UUID, Map<SCMCommandProto.Type, Integer>> summaryMap;

Review Comment:
   I guess that is something we could do in the future if there were more things to track on a datanode by datanode basis. I'd like to avoid too much change here, and a class per DN like a DatanodeQueue would look fairly similar I think. Something like this could easily be added later if we need it, as it will be behind the CommandQueue interface, and the changes would just be internal to the class. 



-- 
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


[GitHub] [ozone] swamirishi commented on a diff in pull request #3891: HDDS-7402. Adapt CommandQueue to track the count of each queued command type

Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #3891:
URL: https://github.com/apache/ozone/pull/3891#discussion_r1006231085


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/CommandQueue.java:
##########
@@ -38,9 +40,8 @@
  * there where queued.
  */
 public class CommandQueue {
-  // This list is used as default return value.
-  private static final List<SCMCommand> DEFAULT_LIST = new ArrayList<>();
   private final Map<UUID, Commands> commandMap;
+  private final Map<UUID, Map<SCMCommandProto.Type, Integer>> summaryMap;

Review Comment:
   Maybe the summary could go into the commands class, since we already have that



-- 
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


[GitHub] [ozone] swamirishi commented on a diff in pull request #3891: HDDS-7402. Adapt CommandQueue to track the count of each queued command type

Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #3891:
URL: https://github.com/apache/ozone/pull/3891#discussion_r1006231898


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/CommandQueue.java:
##########
@@ -38,9 +40,8 @@
  * there where queued.
  */
 public class CommandQueue {
-  // This list is used as default return value.
-  private static final List<SCMCommand> DEFAULT_LIST = new ArrayList<>();
   private final Map<UUID, Commands> commandMap;
+  private final Map<UUID, Map<SCMCommandProto.Type, Integer>> summaryMap;

Review Comment:
   I see we are doing the same map operation twice here



-- 
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


[GitHub] [ozone] sodonnel commented on a diff in pull request #3891: HDDS-7402. Adapt CommandQueue to track the count of each queued command type

Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3891:
URL: https://github.com/apache/ozone/pull/3891#discussion_r1006242390


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/CommandQueue.java:
##########
@@ -38,9 +40,8 @@
  * there where queued.
  */
 public class CommandQueue {
-  // This list is used as default return value.
-  private static final List<SCMCommand> DEFAULT_LIST = new ArrayList<>();
   private final Map<UUID, Commands> commandMap;
+  private final Map<UUID, Map<SCMCommandProto.Type, Integer>> summaryMap;

Review Comment:
   Yea putting the summary into the commands class might make it tidier. I will try that tomorrow.



-- 
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


[GitHub] [ozone] umamaheswararao merged pull request #3891: HDDS-7402. Adapt CommandQueue to track the count of each queued command type

Posted by GitBox <gi...@apache.org>.
umamaheswararao merged PR #3891:
URL: https://github.com/apache/ozone/pull/3891


-- 
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


[GitHub] [ozone] swamirishi commented on a diff in pull request #3891: HDDS-7402. Adapt CommandQueue to track the count of each queued command type

Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #3891:
URL: https://github.com/apache/ozone/pull/3891#discussion_r1006220521


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/CommandQueue.java:
##########
@@ -38,9 +40,8 @@
  * there where queued.
  */
 public class CommandQueue {
-  // This list is used as default return value.
-  private static final List<SCMCommand> DEFAULT_LIST = new ArrayList<>();
   private final Map<UUID, Commands> commandMap;
+  private final Map<UUID, Map<SCMCommandProto.Type, Integer>> summaryMap;

Review Comment:
   Would it be better if we add another class to handle Commands for one Datanode separately? Like something like DatanodeQueue. So summary and commands can go together in one place.



-- 
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


[GitHub] [ozone] swamirishi commented on a diff in pull request #3891: HDDS-7402. Adapt CommandQueue to track the count of each queued command type

Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #3891:
URL: https://github.com/apache/ozone/pull/3891#discussion_r1006220191


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/CommandQueue.java:
##########
@@ -38,9 +40,8 @@
  * there where queued.
  */
 public class CommandQueue {
-  // This list is used as default return value.
-  private static final List<SCMCommand> DEFAULT_LIST = new ArrayList<>();
   private final Map<UUID, Commands> commandMap;
+  private final Map<UUID, Map<SCMCommandProto.Type, Integer>> summaryMap;

Review Comment:
   Would it be better if we add another class to handle Commands for one Datanode separately? Like something like DatanodeQueue. So summary and commands can go together in one place.



-- 
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