gerlowskija commented on code in PR #2666: URL: https://github.com/apache/solr/pull/2666#discussion_r1777144114
########## solr/core/src/java/org/apache/solr/handler/component/ParallelHttpShardHandler.java: ########## @@ -41,57 +39,78 @@ @NotThreadSafe public class ParallelHttpShardHandler extends HttpShardHandler { + @SuppressWarnings("unused") private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private final ExecutorService commExecutor; + /* + * Unlike the basic HttpShardHandler, this class allows us to exit submit before + * pending is incremented and the responseFutureMap is updated. If the runnables that Review Comment: [Q] Taking the "this class allows us to exit submit before pending is incremented" behavior as a given, the use of these additional AtomicInt's makes sense. And the descriptive comment here is lovely. But why does the PR introduce this complication in the first place? Is it required by SOLR-17158? Is it fixing a concurrency bug that pre-exists SOLR-17158? -- 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...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org