kohlmu-pivotal commented on a change in pull request #7424:
URL: https://github.com/apache/geode/pull/7424#discussion_r821258289



##########
File path: 
geode-core/src/main/java/org/apache/geode/internal/lang/SystemPropertyHelper.java
##########
@@ -113,6 +113,14 @@
    */
   public static final String RE_AUTHENTICATE_WAIT_TIME = 
"reauthenticate.wait.time";
 
+  /**
+   * Maximum number of concurrent executions in a server of wan-copy region 
commands.
+   * Once the maximum number is reached, subsequent executions will be halted 
until
+   * a thread for any of the ongoing executions is released.
+   */
+  public static final String WAN_COPY_REGION_FUNCTION_MAX_CONCURRENT_THREADS =

Review comment:
       Just to make sure that we don't get the property wrong, I would want to 
make sure that this property is `geode.wan.copy-region.max-threads`. So we 
might require a few more contextual property "levels".
   
   @upthewaterspout @pivotal-jbarrett what are your thoughts here? I know that 
we've never really formally discussed contextual property hierarchies, but is 
this something that we might consider starting here?

##########
File path: 
geode-wan/src/main/java/org/apache/geode/cache/wan/internal/WanCopyRegionFunctionService.java
##########
@@ -43,7 +45,9 @@
   public boolean init(Cache cache) {
     String WAN_COPY_REGION_FUNCTION_EXECUTION_PROCESSOR_THREAD_PREFIX =
         "WAN Copy Region Function Execution Processor";
-    int WAN_COPY_REGION_FUNCTION_MAX_CONCURRENT_THREADS = 10;
+    int WAN_COPY_REGION_FUNCTION_MAX_CONCURRENT_THREADS = SystemProperty
+        .getProductIntegerProperty(
+            
SystemPropertyHelper.WAN_COPY_REGION_FUNCTION_MAX_CONCURRENT_THREADS, 10);

Review comment:
       I seem to be missing some tests here. Or maybe there is one already 
confirming concurrent threads. Is this something that we want to test out? The 
behavior when changing the max threads setting? Is there any potential for 
failure if we run with 1 thread vs 10 threads vs 100 threads?
   
   Also, I don't know how I feel if we have an unlimited thread count setting? 
Maybe we can initially cap the max threads? Or maybe have another (internal) 
property that would allow us to change the max-thread cap?




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


Reply via email to