bipinprasad commented on a change in pull request #3346:
URL: https://github.com/apache/storm/pull/3346#discussion_r525282016
##########
File path:
storm-server/src/main/java/org/apache/storm/scheduler/resource/strategies/scheduling/BaseResourceAwareStrategy.java
##########
@@ -428,16 +465,39 @@ protected SchedulingResult
scheduleExecutorsOnNodes(List<ExecutorDetails> ordere
}
progressIdxForExec[execIndex]++;
+ int numBoundAckerAssigned
+ = assignBoundAckersForNewWorkerSlot(exec, node,
workerSlot);
+ if (numBoundAckerAssigned == -1) {
+ // This only happens when trying to assign bound
ackers to the worker slot and failed.
+ // Free the entire worker slot and put those bound
ackers back to unassigned list
+ LOG.debug("Failed to assign bound acker for exec: {}
of topo: {} to worker: {}. Backtracking.",
+ exec, topoName, workerSlot);
+ searcherState.freeWorkerSlotWithBoundAckers(node,
workerSlot);
+ continue;
+ }
+
if (!isExecAssignmentToWorkerValid(exec, workerSlot)) {
+ // This only happens when this exec can not fit in the
workerSlot
+ // and this is not the first exec to this workerSlot.
+ // So just go to next workerSlot and don't free the
worker.
+ if (numBoundAckerAssigned > 0) {
+ LOG.debug("Failed to assign exec: {} of topo: {}
with bound ackers to worker: {}. Backtracking.",
+ exec, topoName, workerSlot);
+ searcherState.freeWorkerSlotWithBoundAckers(node,
workerSlot);
+ }
continue;
}
searcherState.incStatesSearched();
searcherState.assignCurrentExecutor(execToComp, node,
workerSlot);
+ if (numBoundAckerAssigned > 0) {
+ // This exec with its bounded ackers have all been
successfully assigned
+ searcherState.getExecsWithBoundAckers().add(exec);
+ }
if (searcherState.areAllExecsScheduled()) {
//Everything is scheduled correctly, so no need to
search any more.
LOG.info("scheduleExecutorsOnNodes: Done at loopCnt={}
in {}ms, state.elapsedtime={}, backtrackCnt={}, topo={}",
- loopCnt, System.currentTimeMillis() -
startTimeMilli,
+ loopCnt, Time.currentTimeMillis() -
startTimeMilli,
Review comment:
When testing you need both. Because Time can be advanced in (some) test,
it gives the wrong clock elapsed time - but is still useful information.
----------------------------------------------------------------
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:
[email protected]
