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

Reply via email to