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



##########
File path: lucene/test-framework/src/java/org/apache/lucene/util/TestUtil.java
##########
@@ -321,6 +326,11 @@ public static void syncConcurrentMerges(MergeScheduler ms) 
{
       checker.setDoSlowChecks(doSlowChecks);
       checker.setFailFast(failFast);
       checker.setInfoStream(new PrintStream(output, false, IOUtils.UTF_8), 
false);
+      if (concurrent) {
+        checker.setThreadCount(RandomizedTest.randomIntBetween(1, 5));
+      } else {
+        checker.setThreadCount(0);

Review comment:
       Hmm I still don't think we should special-case `0` to mean "use a single 
thread" -- I think that's kinda counter-intuitive.
   
   In fact I think `0` should not even be allowed (user should get an 
`IllegalArgumentException`) -- how can one make progress with no threads :)
   
   I think future users would indeed understand that `-threadCount 1` means 
single-threaded, and that that most closely matches how `CheckIndex` functions 
before this awesome addition of optional concurrency.
   
   I suppose we could debate whether `-threadCount 1` means "use the incoming 
(main) thread to do the checking" or it means "make an executor with one worker 
thread", but I think that's really an implementation detail.  From the uer's 
standpoint, only one thread is making progress on checking.




-- 
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: issues-unsubscr...@lucene.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



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

Reply via email to