JacksonYao287 commented on PR #3610: URL: https://github.com/apache/ozone/pull/3610#issuecomment-1190048762
thanks @umamaheswararao for this patch, the changes overall looks good, i have several questions: 1 `UnderReplicatedProcessor` and `OverReplicatedProcessor` now run in a single thread background service, can we add a thread pool to run these task concurrently? 2 in `ReplicationManager#processAll`, we get two new instances of queue for both overReplicated and underReplicated tasks every time, and they are protected by a lock. there may be a case that the earlier generated task queue is replaced by a later generated task queue without completing all the tasks in the earlier one. for example, the interval of `OverReplicatedProcessor` is 10m, and the interval of RM is 2m. so in 10m, RM will produce 5 task queues for overreplicated containers, but OverReplicatedProcessor can only consume the latest one. i suggest to add two current collections(Eg. PriorityBlockingQueue and LinkedBlockingQueue) for overReplicated and underReplicated containers, so that the processors can get task from this queue , meanwhile RM can add tasks into them without a lock. by the way, if we want to filter the same task generated by RM, we can use ConcurrentSkipListSet. etc. -- 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]
