Suppress CHASM caller metrics during reset/conflict-resolution re-apply#10845
Open
tekkaya wants to merge 4 commits into
Open
Suppress CHASM caller metrics during reset/conflict-resolution re-apply#10845tekkaya wants to merge 4 commits into
tekkaya wants to merge 4 commits into
Conversation
2 tasks
tekkaya
force-pushed
the
gokhan/chasm-suppress-metrics-on-cherrypick
branch
from
712b895 to
2c9a8a5
Compare
…tion
Nexus operations can be backed by either the HSM tree or the CHASM tree,
and both coexist on the same mutable state. Before this change,
reset/rebuild/reapply dispatched Nexus events only through HSM, and async
completion committed to one framework by token type. So once
nexusoperation.enableChasmWorkflowOperations is on, CHASM-backed ops were
silently not rebuilt on reset, and an op that "moved" trees via reset could
not be completed.
This wires reset, state rebuild, the event reapplier, and async completion
to support either tree using a probe-and-fallback approach keyed by the
cross-tree-stable ScheduledEventId (HSM node ID; CHASM Workflow.Operations
map key). No new event field/marker.
- Reset reapply (ndc/workflow_resetter.go): reapplyEvents probes HSM, then
falls back to the CHASM workflow registry, with an explicit
cherryPickOutcome (applied/skipped/fallback) to distinguish "recognized
but not cherry-pickable" (skip) from "not owned here" (fallback).
- Flag-driven rebuild-create (workflow/mutable_state_rebuilder.go): the
NexusOperationScheduled create event is routed by the per-namespace
enableChasmWorkflowOperations flag (configs.Config
EnableChasmNexusWorkflowOperations); on CHASM create error it falls back
to HSM. Non-create events use HSM-first / CHASM-fallback.
- Completion-token fallback, both directions, server-side in History
(service/history/handler.go):
- HSM token -> CHASM op: on HSM NotFound, recover ScheduledEventId from
the StateMachineRef and complete the op on the current run's CHASM tree.
- CHASM token -> after-reset re-resolution: a CHASM token pins the
schedule-time run via its ChasmComponentRef; a reset closes that run and
rebuilds the op on a new current run. On failure against the pinned run,
re-resolve the op by ScheduledEventId on the current run and complete it
in CHASM or fall back to the HSM handler depending on where the rebuild
placed it. Namespace/business IDs are read from the ChasmComponentRef,
not the legacy NamespaceId/WorkflowId completion fields.
- Fallback gating keeps invalid/unrelated tokens rejected rather than
silently re-targeting the current run: archetype must be the Workflow
archetype; the pinned run must actually exist (a reset closes it but
leaves it readable, whereas a bogus run id does not resolve) -- a
positive existence check, not error-type based; and namespace EnableChasm
must hold (avoids a noopChasmTree). The frontend completion handler stays
a pure router by token type.
- Event reapplier (ndc/events_reapplier.go): plumbs the real CHASM workflow
registry through EventsReapplierImpl so the replication / current-run
reapply path gets the same fallback.
- Plumbing: new ShardContext.ChasmWorkflowRegistry() accessor (interface,
ContextImpl, factory, regenerated mock, test util), fx-auto-wired.
Tests:
- Unit: handler_test.go (token/ref helpers, completion conversion);
mutable_state_rebuilder_test.go (flag-on CHASM-create-error -> HSM).
- Functional (tests/nexus_workflow_test.go):
TestNexusOperationResetCrossTreeCompletion (op created in one tree, flag
flipped, reset rebuilds into the other tree and the completion reapplies
across trees, both directions); TestNexusOperationCompletionNotFoundInEitherTree
(tampered token -> NotFound preserved); TestNexusOperationAsyncCompletionAfterReset
now enabled for the CHASM variant (live completion after reset resolves
onto the reset-created current run).
Two fixes to the server-side completion-token fallback: - Gate the HSM-token -> CHASM-op fallback (completeNexusOperationChasmFallback) on the token's schedule-time run actually existing, mirroring the CHASM-token path. Previously it re-targeted the current run using only ns/workflow/ scheduledEventId, so a bogus or tampered run ID with an otherwise valid event/request ID could complete the current-run op instead of preserving NotFound. - Preserve the canceled outcome in the CHASM-token -> HSM-op fallback. CompleteNexusOperationChasmRequest carries no explicit outcome state, so nexusOperationErrorFromChasmRequest defaulted non-OperationError failures to Failed. It now infers Canceled from CanceledFailureInfo, matching the native CHASM path (Operation.HandleNexusCompletion), so a completion records the same terminal state (canceled vs failed) regardless of which tree the reset placed the op in.
…pick) CHASM components emit metrics inside their transition callbacks via ctx.MetricsHandler() (e.g. nexusoperation TransitionSucceeded -> emitOnSucceededMetrics). When the reset / conflict-resolution re-apply (cherry-pick) path re-runs those transitions, the metrics would be emitted again and double-counted. Add a framework-level guard: a replaying flag on the chasm context, set via the new NewMutableContextForReplay constructor, that makes MetricsHandler() return metrics.NoopMetricsHandler. The flag is inherited by child contexts (withValue), so it applies to any component's transition side-effect metrics, not just Nexus. This is the guardrail only. The CHASM cherry-pick/re-apply dispatch is not wired yet (reset re-apply still goes through the HSM registry), so this has no production caller today; once that dispatch lands it should build the re-apply context with NewMutableContextForReplay. A functional reset/double-count test should be added together with that wiring. Tests: - chasm: NewMutableContextForReplay -> noop handler; live -> real handler; ContextWithValue inherits the flag. - chasm/lib/nexusoperation: a real TransitionSucceeded run under a real replay context emits nothing, while a live context emits the success counter.
Now that #10854 wires the CHASM reset/reapply dispatch, use the replay context (from the previous commit) at the two sites that re-run CHASM transitions from history: - mutable_state_rebuilder: base rebuild (def.Apply) - workflow_resetter.cherryPickChasmEvent: reset reapply (def.CherryPick) Add MutableState.ChasmWorkflowComponentForReplay, which builds the workflow component context via chasm.NewMutableContextForReplay so metrics emitted inside transitions are suppressed while re-applying events (avoiding double-counting caller-side Nexus metrics). ChasmWorkflowComponent (live callers) is unchanged; both share a private helper. Update the reset/rebuilder mock expectations accordingly.
tekkaya
force-pushed
the
gokhan/chasm-suppress-metrics-on-cherrypick
branch
from
2c9a8a5 to
30c98a7
Compare
tekkaya
changed the base branch from
main
to
gokhan/nexus-reset-chasm-fallback-draft
tekkaya
changed the title
bergundy
approved these changes
Jul 6, 2026
| // | ||
| // NOTE: Library authors should not invoke this constructor directly; it is intended for the | ||
| // reset/conflict-resolution re-apply path. | ||
| func NewMutableContextForReplay( |
Member
There was a problem hiding this comment.
If the node backend (mutable state) could indicate that it's replaying we could get rid of this top level exposed method. Not blocking.
Member
There was a problem hiding this comment.
good point. 🤔 we can probably refactoring something around mutableState.StartTransaction() and let caller specify this is a Replay transaction.
I think chasm/foundations team can look into that later. I will file a task to track.
tekkaya
force-pushed
the
gokhan/nexus-reset-chasm-fallback-draft
branch
from
11f5712 to
2e7ae0f
Compare
yycptt
reviewed
Jul 7, 2026
Comment on lines
51
to
53
Member
There was a problem hiding this comment.
sorry what's the backward compatibility concern here? We are free to change the public methods/interfaces other than those exposed as serverOptions.
Adding chasm workflow registry later feels error prone and can cause NPE.
Member
There was a problem hiding this comment.
hmm are changes in this file intended to be part of this PR? Doesn't feel like it's related to suppressing metrics on reset/reapply. I could be missing some context here though. Same Q for the changes in service/history/statemachine_environment.go
Comment on lines
810
to
811
| // Ensure the root CHASM workflow component exists before applying. | ||
| b.mutableState.EnsureChasmWorkflowComponent(ctx) |
Member
There was a problem hiding this comment.
jfyi that this method won't actually do anything today. The chasm tree by default is a workflow execution and ready to be used. I will remove it and clean up all usages soon.
| // | ||
| // NOTE: Library authors should not invoke this constructor directly; it is intended for the | ||
| // reset/conflict-resolution re-apply path. | ||
| func NewMutableContextForReplay( |
Member
There was a problem hiding this comment.
good point. 🤔 we can probably refactoring something around mutableState.StartTransaction() and let caller specify this is a Replay transaction.
I think chasm/foundations team can look into that later. I will file a task to track.
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
3 participants
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?
Suppress CHASM caller-side metric emission while events are re-applied from history (reset rebuild, cherry-pick, conflict resolution), and wire it into the reset/reapply dispatch added by #10854.
CHASM components emit metrics inside their transition callbacks via
ctx.MetricsHandler()(e.g.nexusoperation.TransitionSucceeded→emitOnSucceededMetrics). When the reset/conflict-resolution paths re-run those transitions, the metrics would be emitted again and double-counted.Framework guard (
chasm):chasm.NewMutableContextForReplay(ctx, node)builds aMutableContextwith areplayingflag;Context.MetricsHandler()returnsmetrics.NoopMetricsHandlerwhile replaying. Inherited by child contexts (withValue). Generic — suppresses metrics for any component's transitions, not just Nexus.Wiring (
service/history):MutableState.ChasmWorkflowComponentForReplay(ctx)returns the workflow component with a replay context (shares a helper withChasmWorkflowComponent; live callers unchanged).mutable_state_rebuilder(def.Apply)workflow_resetter.cherryPickChasmEvent(def.CherryPick)Why?
Mirrors the HSM caller-metrics work (#10808), where emission was kept out of
Applyentirely. CHASM emits inside transitions, so the equivalent is a replay-aware no-op handler applied at the re-apply sites. Both re-apply sites must be guarded: the base rebuild re-emits metrics for operations that were already terminal before the reset point, and the cherry-pick re-emits for post-reset-point events.How did you test it?
Potential risks
replayingdefaults to false; live constructors/callers unchanged).