adoroszlai commented on code in PR #3783:
URL: https://github.com/apache/ozone/pull/3783#discussion_r996780015


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java:
##########
@@ -518,6 +525,28 @@ private void initializeEventHandlers() {
 
   }
 
+  @NotNull
+  private List<BlockingQueue<ContainerReportBase>> initContainerReportQueue() {
+    int threadPoolSize = configuration.getInt(OZONE_SCM_EVENT_PREFIX +
+            StringUtils.camelize(SCMEvents.CONTAINER_REPORT.getName()
+                + "_OR_"
+                + SCMEvents.INCREMENTAL_CONTAINER_REPORT.getName())
+            + ".thread.pool.size",
+        OZONE_SCM_EVENT_THREAD_POOL_SIZE_DEFAULT);
+    int queueSize = configuration.getInt(OZONE_SCM_EVENT_PREFIX +
+            StringUtils.camelize(SCMEvents.CONTAINER_REPORT.getName()
+                + "_OR_"
+                + SCMEvents.INCREMENTAL_CONTAINER_REPORT.getName())
+            + ".queue.size",
+        OZONE_SCM_EVENT_CONTAINER_REPORT_QUEUE_SIZE_DEFAULT);

Review Comment:
   @sumitagrawl Let me clarify:
   
   Existing code uses two separate configs for the two executores (one for 
`ContainerReport`, one for `IncrementalContainerReport`).  With this patch, 
those configs are no longer used, and a third one is being introduced with a 
more complex name (`...ContainerReport_or_IncrementalContainerReport...`).
   
   I have two concerns:
   1. existing configs are ignored
   2. the new name exposes internal implementation (that now we have a single 
thread pool instead of two) that the users may not care about
   
   Neither of those are big deals, but maybe we could do better.
   
   One possible solution, using existing config:
    * Thread pool size could be the larger one of container report thread pool 
size and incremental container report thread pool size.
    * Queue size could be sum of container report queue size and incremental 
container report queue size.



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

Reply via email to