[GitHub] [lucene] zacharymorn commented on a change in pull request #128: LUCENE-9662: CheckIndex should be concurrent - parallelizing index parts check within each segment

[GitBox <gi...@apache.org>]

zacharymorn commented on a change in pull request #128:
URL: https://github.com/apache/lucene/pull/128#discussion_r641274199



##########
File path: lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
##########
@@ -3720,6 +3957,20 @@ public static Options parseOptions(String[] args) {
         }
         i++;
         opts.dirImpl = args[i];
+      } else if ("-threadCount".equals(arg)) {
+        if (i == args.length - 1) {
+          throw new IllegalArgumentException("-threadCount requires a following number");
+        }
+        i++;
+        int providedThreadCount = Integer.parseInt(args[i]);
+        // Current implementation supports up to 11 concurrent checks at any time, and no
+        // concurrency across segments.
+        // Capping the thread count to 11 to avoid unnecessary threads to be created.
+        if (providedThreadCount > 11) {

Review comment:
       > as we add new index / codec formats, this would increase right?
   
   I think since currently most of the checks finish super fast, this max threadCount should be good for a while actually until we also support currency across segments.
   
   > Could we maybe just pull this 11 into a static final int MAX_PER_SEGMENT_CONCURRENCY = 11; constant at the top of the class?
   
   Ah yes shouldn't have left a magic number there...updated!
   




-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org