juergbi commented on code in PR #2160:
URL: https://github.com/apache/buildstream/pull/2160#discussion_r3705613751


##########
src/buildstream/_cas/casremote.py:
##########
@@ -92,7 +94,16 @@ def send(self, *, missing_blobs=None):
         local_cas = self._remote.casd.get_local_cas()
 
         for request in self._requests:
-            batch_response = local_cas.FetchMissingBlobs(request)
+            batch_response_future = local_cas.FetchMissingBlobs.future(request)
+
+            try:
+                while batch_response_future.running():
+                    time.sleep(1)
+            except:
+                batch_response_future.cancel()

Review Comment:
   Ah, this is actually a `grpc.Future`, not a generic Python stdlib 
`concurrent.futures.Future`. That's better than what I first thought and should 
actually perform proper gRPC cancellation.
   
   Yes, the server (buildbox-casd) side should react to this as well, but 
that's an improvement that can be done separately and it shouldn't be needed to 
fix the #2157 issue (although it may still cause some delay on shutdown). 
buildbox-casd already handles cancellation for local execution requests and as 
part of the retry mechanism. Further cancellation points can (and should) be 
added but may need some work as there may be a blocking gRPC client call in 
some nested function call.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to