agresch commented on a change in pull request #3374:
URL: https://github.com/apache/storm/pull/3374#discussion_r570497395



##########
File path: 
storm-server/src/main/java/org/apache/storm/daemon/supervisor/Slot.java
##########
@@ -1288,6 +1296,18 @@ public DynamicState 
withPendingChangingBlobs(Set<Future<Void>> pendingChangingBl
                                     pendingChangingBlobs,
                                     pendingChangingBlobsAssignment, 
this.slotMetrics);
         }
+
+        private void cancelPendingBlobs() {
+            // Make sure any downloading blobs in the background are stopped.
+            // This prevents a race condition where we could be adding 
references to a
+            // delayed downloading blob after the slot gets released, causing 
orphaned blobs.
+            for (Future future : pendingChangingBlobs) {
+                if (!future.isDone()) {
+                    LOG.info("Canceling download of {}", future);
+                    future.cancel(true);
+                }
+            }
+        }

Review comment:
       This causes various asserts in the integration tests.  I tried creating 
new DynamicStates without any futures, and that caused other problems.  This is 
beyond my understanding, but having the futures canceled should prevent the 
race conditions.




----------------------------------------------------------------
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.

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


Reply via email to