Suggest continue-as-new before hitting maximumSignalsPerExecution#10989
Open
nitishagar wants to merge 1 commit into
Open
Suggest continue-as-new before hitting maximumSignalsPerExecution#10989nitishagar wants to merge 1 commit into
nitishagar wants to merge 1 commit into
Conversation
Add a fourth SuggestContinueAsNewReason, TOO_MANY_SIGNALS, so the server advises a workflow to continue-as-new when its signal count reaches a configurable fraction of maximumSignalsPerExecution, before the hard ErrSignalsLimitExceeded rejection bites. This is the signals analog of the existing TOO_MANY_UPDATES suggest-CAN feature. The suggestion is computed inline in getHistorySizeInfo alongside the existing history-size and history-event-count reasons (signals have no registry, so inline computation reuses the scope already established there). It fires when SignalCount >= ceil(maximumSignalsPerExecution * threshold), mirroring the updates path's rounding. The threshold is a new namespace-scoped dynamic-config key history.maximumSignalsPerExecution.suggestContinueAsNewThreshold that defaults to 0 (disabled), so the feature is opt-in and a strict no-op on upgrade. The hard-limit rejection path is unchanged. Requires go.temporal.io/api to export SUGGEST_CONTINUE_AS_NEW_REASON_TOO_MANY_SIGNALS (added in temporalio/api#822); go.mod temporarily points at a fork until that version is released. Closes temporalio#10941.
12a69e1 to
3b79549
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed?
Adds a fourth
SuggestContinueAsNewReason— TOO_MANY_SIGNALS — so the server advises a workflow to continue-as-new when its signal count reaches a configurable fraction ofmaximumSignalsPerExecution, before the hardErrSignalsLimitExceededrejection bites. This is the signals analog of the existingTOO_MANY_UPDATESsuggest-CAN feature.history.maximumSignalsPerExecution.suggestContinueAsNewThreshold(float, default0= disabled).getHistorySizeInfoalongside the existing history-size and history-event-count reasons: it fires whenSignalCount >= ceil(maximumSignalsPerExecution * threshold), mirroring the updates path's rounding.workflow_suggest_continue_as_new_countcounter with a fourth boolean tagSuggestContinueAsNewReasonTooManySignalsTag.signal_workflow_util.go) is unchanged.The new enum value
SUGGEST_CONTINUE_AS_NEW_REASON_TOO_MANY_SIGNALS = 5is added upstream in temporalio/api#822.Why?
Closes #10941. Today the only signal feedback is a hard rejection at the limit; workers get no advance warning to drain and continue-as-new. A soft, opt-in suggestion gives them that headroom, consistent with how history size, event count, and updates already behave. Defaulting the threshold to
0(disabled) makes it a strict no-op on upgrade — signals are far more common than updates, so a non-zero default would newly emit suggestions on countless existing executions.How did you test it?
service/history/workflow/workflow_task_state_machine_signals_test.gocovers: threshold 0 / max 0 disable (INV-3), below/at(inclusive>=)/above boundary, threshold 1.0 at the hard limit and below, coexistence with the history-size reason (INV-7), and continue-as-new reset (INV-4).tests/signal_suggest_can_test.go: overrides both knobs, sends signals to the boundary and assertsSuggestContinueAsNew+ theTOO_MANY_SIGNALSreason on the started event; asserts the hard limit still rejects end-to-end (INV-2); and asserts no reason appears under the disabled default.go test -tags test_dep ./service/history/workflow/...is green;golangci-lintreports 0 new issues. The functional test requires the docker test cluster to run.Potential risks
go.temporal.io/apirelease contains the enum,go.modpoints at a fork (github.com/nitishagar/api-go) via areplacedirective (the standard "Working with local API changes" flow). The replace must be swapped for the released version before merge.