xichen01 commented on PR #4939: URL: https://github.com/apache/ozone/pull/4939#issuecomment-1867590059
> > > May be we can check for lesser queue size at SCM to send like limit "2" to avoid duplicate for the change. > > > > > > If [HDDS-8882](https://issues.apache.org/jira/browse/HDDS-8882) can be merged, maybe this doesn't need to be added What do you think? > > @xichen01 [HDDS-8882](https://issues.apache.org/jira/browse/HDDS-8882) is merged now. Related to this, we need some improvement in check for this PR. > > My concern was: getTotalDatanodeCommandCount() returns DN queue count + command count at SCM. eg: DN count = 2 SCM count=1 Total count=3 Default max count=5 -- in this case, it will add new set of command to SCM as meet the criteria and as result, SCM count=2 in command queue at SCM. > > We can optimize the logic as, getTotalDatanodeCommandCount() < 5 && getCommandQueueCount() == 0 <-- this ensure that new command is not added at SCM till previous one is send to DN. @sumitagrawl I understand, but is it too strict to limit `getCommandQueueCount() == 0`, because there may be a delay in updating the DN's queue information, which may leave the DN idle, I think it could be `getTotalDatanodeCommandCount() < queueLen && getCommandQueueCount() < ceil(queueLen/2)`. what do you think? -- 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]
