-
Notifications
You must be signed in to change notification settings - Fork 14k
[FLINK-40379][runtime] Wait for pre-restart parallelism to become available again after a rescale-triggered restart #28999
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -241,9 +241,13 @@ public static Settings of( | |
| Duration submissionStabilizationTimeoutDefault = | ||
| JobManagerOptions.SCHEDULER_SUBMISSION_RESOURCE_STABILIZATION_TIMEOUT | ||
| .defaultValue(); | ||
| Duration rescaleResourceStabilizationTimeoutDefault = | ||
| JobManagerOptions.SCHEDULER_RESCALE_RESOURCE_STABILIZATION_TIMEOUT | ||
| .defaultValue(); | ||
| if (executionMode == SchedulerExecutionMode.REACTIVE) { | ||
| submissionResourceWaitTimeoutDefault = Duration.ofMillis(-1); | ||
| submissionStabilizationTimeoutDefault = Duration.ZERO; | ||
| rescaleResourceStabilizationTimeoutDefault = Duration.ZERO; | ||
| } | ||
|
|
||
| final Duration executingCooldownTimeout = | ||
|
|
@@ -306,6 +310,11 @@ public static Settings of( | |
| JobManagerOptions | ||
| .SCHEDULER_SUBMISSION_RESOURCE_STABILIZATION_TIMEOUT) | ||
| .orElse(submissionStabilizationTimeoutDefault), | ||
| configuration | ||
| .getOptional( | ||
| JobManagerOptions | ||
| .SCHEDULER_RESCALE_RESOURCE_STABILIZATION_TIMEOUT) | ||
| .orElse(rescaleResourceStabilizationTimeoutDefault), | ||
| configuration.get(JobManagerOptions.SLOT_IDLE_TIMEOUT), | ||
| executingCooldownTimeout, | ||
| configuration.get( | ||
|
|
@@ -322,6 +331,7 @@ public static Settings of( | |
| private final SchedulerExecutionMode executionMode; | ||
| private final Duration submissionResourceWaitTimeout; | ||
| private final Duration submissionResourceStabilizationTimeout; | ||
| private final Duration rescaleResourceStabilizationTimeout; | ||
| private final Duration slotIdleTimeout; | ||
| private final Duration executingCooldownTimeout; | ||
| private final Duration executingResourceStabilizationTimeout; | ||
|
|
@@ -334,6 +344,7 @@ private Settings( | |
| SchedulerExecutionMode executionMode, | ||
| Duration submissionResourceWaitTimeout, | ||
| Duration submissionResourceStabilizationTimeout, | ||
| Duration rescaleResourceStabilizationTimeout, | ||
| Duration slotIdleTimeout, | ||
| Duration executingCooldownTimeout, | ||
| Duration executingResourceStabilizationTimeout, | ||
|
|
@@ -344,6 +355,7 @@ private Settings( | |
| this.executionMode = executionMode; | ||
| this.submissionResourceWaitTimeout = submissionResourceWaitTimeout; | ||
| this.submissionResourceStabilizationTimeout = submissionResourceStabilizationTimeout; | ||
| this.rescaleResourceStabilizationTimeout = rescaleResourceStabilizationTimeout; | ||
| this.slotIdleTimeout = slotIdleTimeout; | ||
| this.executingCooldownTimeout = executingCooldownTimeout; | ||
| this.executingResourceStabilizationTimeout = executingResourceStabilizationTimeout; | ||
|
|
@@ -365,6 +377,10 @@ public Duration getSubmissionResourceStabilizationTimeout() { | |
| return submissionResourceStabilizationTimeout; | ||
| } | ||
|
|
||
| public Duration getRescaleResourceStabilizationTimeout() { | ||
| return rescaleResourceStabilizationTimeout; | ||
| } | ||
|
|
||
| public Duration getSlotIdleTimeout() { | ||
| return slotIdleTimeout; | ||
| } | ||
|
|
@@ -1269,16 +1285,21 @@ public ArchivedExecutionGraph getArchivedExecutionGraph( | |
| } | ||
|
|
||
| @Override | ||
| public void goToWaitingForResources(@Nullable ExecutionGraph previousExecutionGraph) { | ||
| public void goToWaitingForResources( | ||
| @Nullable ExecutionGraph previousExecutionGraph, | ||
| @Nullable VertexParallelism targetVertexParallelism) { | ||
| declareDesiredResources(); | ||
|
|
||
| transitionToState( | ||
| new WaitingForResources.Factory( | ||
| this, | ||
| LOG, | ||
| settings.getSubmissionResourceWaitTimeout(), | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Side note: It seems we use SubmissionResourceWaitTimeout for all cases - not only for submission. Perhaps we should follow-up on that with something like: |
||
| this::createWaitingForResourceStateTransitionManager, | ||
| previousExecutionGraph)); | ||
| targetVertexParallelism != null | ||
| ? this::createRestartWaitingForResourceStateTransitionManager | ||
| : this::createWaitingForResourceStateTransitionManager, | ||
| previousExecutionGraph, | ||
| targetVertexParallelism)); | ||
| } | ||
|
|
||
| private StateTransitionManager createWaitingForResourceStateTransitionManager( | ||
|
|
@@ -1291,6 +1312,16 @@ private StateTransitionManager createWaitingForResourceStateTransitionManager( | |
| Duration.ZERO); // trigger immediately once the stabilization phase is over | ||
| } | ||
|
|
||
| private StateTransitionManager createRestartWaitingForResourceStateTransitionManager( | ||
| StateTransitionManager.Context ctx) { | ||
| return stateTransitionManagerFactory.create( | ||
| ctx, | ||
| clock, | ||
| Duration.ZERO, // skip cooldown phase | ||
| settings.getRescaleResourceStabilizationTimeout(), | ||
| Duration.ZERO); // trigger immediately once the stabilization phase is over | ||
| } | ||
|
|
||
| private void declareDesiredResources() { | ||
| final ResourceCounter newDesiredResources = calculateDesiredResources(); | ||
|
|
||
|
|
@@ -1643,6 +1674,17 @@ public Optional<VertexParallelism> getAvailableVertexParallelism() { | |
| jobInformation, declarativeSlotPool.getAllSlotsInformation()); | ||
| } | ||
|
|
||
| @Override | ||
| public Optional<VertexParallelism> getFreeSlotVertexParallelism() { | ||
| return slotAllocator.determineParallelism( | ||
| jobInformation, declarativeSlotPool.getFreeSlotTracker().getFreeSlotsInformation()); | ||
| } | ||
|
|
||
| @Override | ||
| public int getUpperBoundParallelism(JobVertexID jobVertexId) { | ||
| return jobInformation.getVertexInformation(jobVertexId).getParallelism(); | ||
| } | ||
|
|
||
| @Override | ||
| public void onFinished(ArchivedExecutionGraph archivedExecutionGraph) { | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -114,23 +114,31 @@ void onGloballyTerminalState(JobStatus globallyTerminalState) { | |||||
| } | ||||||
|
|
||||||
| private void goToSubsequentState() { | ||||||
| if (availableParallelismNotChanged(restartWithParallelism) | ||||||
| || context.hasDesiredResources()) { | ||||||
| // hasDesiredResources() counts all slots allocated to the job, including ones still | ||||||
| // reserved by the execution that is only now being cancelled: it must not be used as a | ||||||
| // fallback here when a restart target is known, or it would immediately undo the very | ||||||
| // guard freeSlotVertexParallelismUnchanged() exists to provide. | ||||||
| if (freeSlotVertexParallelismUnchanged() | ||||||
| || (restartWithParallelism == null && context.hasDesiredResources())) { | ||||||
| context.goToCreatingExecutionGraph(getExecutionGraph()); | ||||||
| } else { | ||||||
| context.goToWaitingForResources(getExecutionGraph()); | ||||||
| context.goToWaitingForResources(getExecutionGraph(), restartWithParallelism); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| private boolean availableParallelismNotChanged(VertexParallelism restartWithParallelism) { | ||||||
| if (this.restartWithParallelism == null) { | ||||||
| private boolean freeSlotVertexParallelismUnchanged() { | ||||||
| if (restartWithParallelism == null) { | ||||||
| return false; | ||||||
| } | ||||||
|
|
||||||
| return context.getAvailableVertexParallelism() | ||||||
| return context.getFreeSlotVertexParallelism() | ||||||
| .map( | ||||||
| vertexParallelism -> | ||||||
| vertexParallelism.getVertices().stream() | ||||||
| // Iterate over restartWithParallelism (the restart target), not | ||||||
| // vertexParallelism (the free-slot-based result): a vertex present | ||||||
| // in the target but missing from the free-slot-based result must | ||||||
| // fail the check, not be silently skipped by allMatch. | ||||||
| restartWithParallelism.getVertices().stream() | ||||||
| .allMatch( | ||||||
| vertex -> | ||||||
| restartWithParallelism.getParallelism( | ||||||
|
|
@@ -160,10 +168,10 @@ interface Context | |||||
| ScheduledFuture<?> runIfState(State expectedState, Runnable action, Duration delay); | ||||||
|
|
||||||
| /** | ||||||
| * Returns the {@link VertexParallelism} that can be provided by the currently available | ||||||
| * slots. | ||||||
| * Returns the {@link VertexParallelism} that can be achieved with the currently free slots | ||||||
| * (excluding slots still reserved by the execution that is being cancelled). | ||||||
| */ | ||||||
| Optional<VertexParallelism> getAvailableVertexParallelism(); | ||||||
| Optional<VertexParallelism> getFreeSlotVertexParallelism(); | ||||||
|
|
||||||
| /** | ||||||
| * Checks whether we have the desired resources. | ||||||
|
|
@@ -181,7 +189,7 @@ static class Factory implements StateFactory<Restarting> { | |||||
| private final ExecutionGraphHandler executionGraphHandler; | ||||||
| private final OperatorCoordinatorHandler operatorCoordinatorHandler; | ||||||
| private final Duration backoffTime; | ||||||
| private final @Nullable VertexParallelism restartWithParallelism; | ||||||
| private final @Nullable VertexParallelism targetVertexParallelism; | ||||||
| private final ClassLoader userCodeClassLoader; | ||||||
| private final List<ExceptionHistoryEntry> failureCollection; | ||||||
|
|
||||||
|
|
@@ -192,7 +200,7 @@ public Factory( | |||||
| OperatorCoordinatorHandler operatorCoordinatorHandler, | ||||||
| Logger log, | ||||||
| Duration backoffTime, | ||||||
| @Nullable VertexParallelism restartWithParallelism, | ||||||
| @Nullable VertexParallelism targetVertexParallelism, | ||||||
| ClassLoader userCodeClassLoader, | ||||||
| List<ExceptionHistoryEntry> failureCollection) { | ||||||
| this.context = context; | ||||||
|
|
@@ -201,7 +209,7 @@ public Factory( | |||||
| this.executionGraphHandler = executionGraphHandler; | ||||||
| this.operatorCoordinatorHandler = operatorCoordinatorHandler; | ||||||
| this.backoffTime = backoffTime; | ||||||
| this.restartWithParallelism = restartWithParallelism; | ||||||
| this.targetVertexParallelism = targetVertexParallelism; | ||||||
| this.userCodeClassLoader = userCodeClassLoader; | ||||||
| this.failureCollection = failureCollection; | ||||||
| } | ||||||
|
|
@@ -218,7 +226,7 @@ public Restarting getState() { | |||||
| operatorCoordinatorHandler, | ||||||
| log, | ||||||
| backoffTime, | ||||||
| restartWithParallelism, | ||||||
| targetVertexParallelism, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
We have to be consistent here - I'm ok with either leaving |
||||||
| userCodeClassLoader, | ||||||
| failureCollection); | ||||||
| } | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not keep the docs change separate because it's correlated with the
JobManagerOptionschange of the previous commit. But that's a quick fix by just running squash & rebase on the branch when merging