zpinto commented on code in PR #2745:
URL: https://github.com/apache/helix/pull/2745#discussion_r1470096107
##########
helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/MaintenanceManagementService.java:
##########
@@ -67,10 +67,17 @@
public class MaintenanceManagementService {
+ private static class MaintenanceExecutorService {
+ static final int totalInstance = 1000;
Review Comment:
static var should be in format `TOTAL_INSTANCES`
##########
helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/MaintenanceManagementService.java:
##########
@@ -67,10 +67,17 @@
public class MaintenanceManagementService {
+ private static class MaintenanceExecutorService {
+ static final int totalInstance = 1000;
+ static final int faultZones = 20;
Review Comment:
It may be overcomplicating things to have a notion of total instances and
fault zones here. If a user is configuring this, they know how many threads
they can afford to have in the pool. They may not know how the number of
totalInstances and faultZones determine the number of threads without reading
the code.
##########
helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/MaintenanceManagementService.java:
##########
@@ -67,10 +67,17 @@
public class MaintenanceManagementService {
+ private static class MaintenanceExecutorService {
+ static final int totalInstance = 1000;
+ static final int faultZones = 20;
+ public static ExecutorService getExecutorPool() {
+ return Executors.newFixedThreadPool(totalInstance / faultZones);
+ }
+ }
private static final Logger LOG =
LoggerFactory.getLogger(MaintenanceManagementService.class);
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
- private static final ExecutorService POOL = Executors.newCachedThreadPool();
+ private static final ExecutorService POOL =
MaintenanceExecutorService.getExecutorPool();
Review Comment:
Please add comments which address how this POOL is used. Is it 1 thread per
instance that is having checks run on it?
In the place where the pool is used, explain what happens if all threads are
being used. Explain how we are queuing the next checks to run.
--
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]
