gus-asf commented on code in PR #2379: URL: https://github.com/apache/solr/pull/2379#discussion_r1560189150
########## solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java: ########## @@ -140,58 +150,64 @@ public void submit( srsp.setShard(shard); SimpleSolrResponse ssr = new SimpleSolrResponse(); srsp.setSolrResponse(ssr); + synchronized (RESPONSE_CANCELABLE_LOCK) { + pending.incrementAndGet(); + // if there are no shards available for a slice, urls.size()==0 + if (urls.isEmpty()) { + // TODO: what's the right error code here? We should use the same thing when + // all of the servers for a shard are down. + SolrException exception = + new SolrException( + SolrException.ErrorCode.SERVICE_UNAVAILABLE, "no servers hosting shard: " + shard); + srsp.setException(exception); + srsp.setResponseCode(exception.code()); + responses.add(srsp); + return; + } - pending.incrementAndGet(); - // if there are no shards available for a slice, urls.size()==0 - if (urls.isEmpty()) { - // TODO: what's the right error code here? We should use the same thing when - // all of the servers for a shard are down. - SolrException exception = - new SolrException( - SolrException.ErrorCode.SERVICE_UNAVAILABLE, "no servers hosting shard: " + shard); - srsp.setException(exception); - srsp.setResponseCode(exception.code()); - responses.add(srsp); - return; - } + // all variables that set inside this listener must be at least volatile + responseCancellableMap.put( Review Comment: I didn't feel comfortable with that solution because this map is updated while looping on an Atomic integer `pending` and also is updated immediately after `responses.take()` all of which appear to need to happen as a unit. The prior code wasn't touching it from the child threads, but since I'm changing that I want to be sure that all of these operations happen as a unit to avoid potentially weird cases where the loop fails to exit but then the map is empty etc... -- 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