Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2739#discussion_r198977745
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/supervisor/Slot.java ---
@@ -241,64 +255,54 @@ static DynamicState
prepareForNewAssignmentNoWorkersRunning(DynamicState dynamic
.withState(MachineState.WAITING_FOR_BLOB_LOCALIZATION);
}
- /**
- * Kill the current container and start downloading what the new
assignment needs, if there is a new assignment.
- * PRECONDITION: container != null
- * @param dynamicState current state
- * @param staticState static data
- * @return the next state
- * @throws Exception
- */
- static DynamicState killContainerForChangedAssignment(DynamicState
dynamicState, StaticState staticState) throws Exception {
- assert (dynamicState.container != null);
+ private static DynamicState killContainerFor(KillReason reason,
DynamicState dynamicState, StaticState staticState)
+ throws Exception {
+ assert dynamicState.container != null;
- staticState.iSupervisor.killedWorker(staticState.port);
- dynamicState.container.kill();
- Future<Void> pendingDownload = null;
- if (dynamicState.newAssignment != null) {
- pendingDownload =
staticState.localizer.requestDownloadTopologyBlobs(dynamicState.newAssignment,
staticState.port,
-
staticState.changingCallback);
+ //Skip special case if `storm kill_workers` is already invoked
+ Boolean isDead = dynamicState.container.areAllProcessesDead();
+ if (!isDead) {
+ //TODO: Is this necessary?
--- End diff --
Okay. I just want to make sure that this is indeed the desired behavior.
---