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