shanthoosh commented on a change in pull request #1434:
URL: https://github.com/apache/samza/pull/1434#discussion_r503587552



##########
File path: 
samza-rest/src/main/java/org/apache/samza/monitor/SamzaMonitorService.java
##########
@@ -100,4 +110,19 @@ public void run() {
       }
     };
   }
+
+  /**
+   * Creates a ScheduledThreadPoolExecutor with core pool size 1
+   * @return ScheduledExecutorService
+   */
+  private ScheduledExecutorService createScheduler() {

Review comment:
       > are you aware of any existing monitors could be impacted by this 
change?
   
   
   Let's please double-check if the parallel execution of 
monitors(StateStoreGCMonitor, StateStoreDeltaMonitor) internally could 
potentially cause any correctness issues. 
   
   Other than that, updating the API spec of Monitor abstraction(& calling out 
backwards compatibility) should be sufficient. 




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to