chasm(callback): implement backoff and invocation executor by spkane31 · Pull Request #8499 · temporalio/temporal

spkane31 · 2025-10-16T18:36:58Z

What changed?

Adding an InvocationTaskExecutor and BackoffTaskExecutor to chasm/lib/callback

Why?

Second step of migrating callback from HSM -> CHASM

How did you test it?

Potential risks

None, this is not integrated.

…ck-executors

bergundy

Please use chasm/statemachine.go and copy over all of the state transitions from the HSM implementation.

bergundy · 2025-10-20T20:38:20Z

+	_, err := chasm.ReadComponent(
+		ctx,
+		invokerRef,
+		func(c *Callback, ctx chasm.Context, _ any) (struct{}, error) {


Just return what you need here instead of cloning the entire proto IMHO. I typically would rather not cloning components for use outside of the chasm contexts.

bergundy · 2025-10-20T20:38:30Z

+	task *callbackspb.InvocationTask,
+) error {
+	var ns *namespace.Namespace
+	// var invoker *Invoker


Leftover comment.

bergundy · 2025-10-20T20:38:49Z

+) error {
+	var ns *namespace.Namespace
+	// var invoker *Invoker
+	var callback *Callback


This can be the return type from ReadComponent.

bergundy · 2025-10-20T20:40:19Z

+	_ chasm.TaskAttributes,
+	_ *callbackspb.InvocationTask,
+) (bool, error) {
+	return callback.Status == callbackspb.CALLBACK_STATUS_SCHEDULED, nil


We should also validate the attempt matches, copy the attempt to the invocation task when scheduled. This would prevent duplicate tasks from being considered valid in some cases.

bergundy · 2025-10-20T20:42:44Z

+	case *callbackspb.Callback_Nexus:
+		// Parse URL to extract scheme and host, matching HSM's behavior
+		// from statemachine.go:86-90
+		u, err := url.Parse(variant.Nexus.Url)


@chaptersix we should follow up and change this to use the destination in the token in case the URL is temporal://system.

bergundy · 2025-10-20T20:45:14Z

+    map<string, string> header = 2;
+}
+
+message InvokerState {


What is this?

bergundy · 2025-10-20T22:19:13Z

+
+	// Fields from HSM's nexusInvocation struct (nexus_invocation.go:35-40)
+	// These hold the invocation context needed for the HTTP request
+	completion nexusrpc.OperationCompletion


Don't hold this here, create a separate struct for that as we did in the HSM version.

bergundy · 2025-10-20T22:20:54Z

+// - HSM invocationResultOK -> CHASM CALLBACK_STATUS_SUCCEEDED
+// - HSM invocationResultRetry -> CHASM CALLBACK_STATUS_BACKING_OFF
+// - HSM invocationResultFail -> CHASM CALLBACK_STATUS_FAILED
+func (c *Callback) invoke(


This shouldn't be a method on the component. I know we did this pattern in scheduler but I do not want us to reference components in general outside of the chasm context, it's too error prone and hard to reason about.

bergundy · 2025-10-20T22:24:22Z

+	)
+	defer cancel()
+
+	result := callback.invoke(callCtx, ns, e, taskAttributes, task)


You're missing the chasm variant that @lina-temporal recently added.

bergundy · 2025-10-20T22:24:58Z

+
+    string workflow_id = 10;
+    string run_id = 11;
+
+    string namespace_id = 12;


What is this for?

Namespace is used to get request timeout from dynamic config on a per-namespace basis

Callback is not guaranteed to be attached to a workflow.
Wait for #8533 to be merged and use ExecutionKey() instead.

…ck-executors

bergundy · 2025-10-24T16:42:06Z

+	WrapError(result invocationResult, err error) error
+}
+
+func (e InvocationTaskExecutor) executeInvocationTask(


You're going to want to call this Execute but you'll figure that out when you register it in the library.

bergundy · 2025-10-24T16:42:45Z

+	return invokable.WrapError(result, saveErr)
+}
+
+func (e InvocationTaskExecutor) loadInvocationArgs(


nit: I would write this as a function that you pass to chasm.ReadComponent to save the closure.

Well... now that you've refactored this function to work within a lock, you can just make it a method of the Callback struct. Same thing for saveResult. You don't actually need e here.

This makes for an awkward (imo):

invokable, err := chasm.ReadComponent( ctx, ref, (*Callback).loadInvocationArgs, ctx, )

any issues with that?

bergundy · 2025-10-24T16:43:08Z

+	)
+}
+
+func (e InvocationTaskExecutor) saveResult(


Same here, write this as a function that you pass to UpdateComponent.

bergundy · 2025-10-24T16:43:49Z

+	_ *callbackspb.BackoffTask,
+) (bool, error) {
+	// Validate that the callback is in BACKING_OFF state
+	return callback.Status == callbackspb.CALLBACK_STATUS_BACKING_OFF, nil


Let's also check that the attempt matches.

bergundy · 2025-10-24T17:05:47Z

 	*callbackspb.CallbackState
+
+	// Interface to retrieve Nexus operation completion data
+	CanGetNexusCompletion chasm.Field[CanGetNexusCompletion]


Call this CompletionSource or something more informative?

bergundy · 2025-10-24T17:25:28Z

+			},
+			expectedMetricOutcome: "status:200",
+			setupCallback: func(cb *Callback) {
+				cb.Status = callbackspb.CALLBACK_STATUS_SCHEDULED


Might as well set the status up for all test cases.

bergundy · 2025-10-24T17:26:56Z

You are missing a bunch of test cases that exist in the HSM implementation.

Still not seeing all of the tests ported here.

HSM CHASM

config_test.go config_test.go (copied directly)

TestValidTransitions TestValidTransitions (copied)

TestCompareState I don't think this test applies, is there a chasm.CompareState equivalence?

TestProcessInvocationTaskNexus_Outcomes TestExecuteInvocationTaskNexus_Outcomes (renamed)

TestProcessInvocationTaskHsm_Outcomes Not relevant right?

TestProcessBackoffTask TestProcessBackoffTask

TestProcessInvocationTaskChasm_Outcomes TestExecuteInvocationTaskChasm_Outcomes (added now)

bergundy · 2025-10-24T17:28:05Z

+
+    string workflow_id = 10;
+    string run_id = 11;
+
+    string namespace_id = 12;


Callback is not guaranteed to be attached to a workflow.
Wait for #8533 to be merged and use ExecutionKey() instead.

bergundy · 2025-10-24T17:28:28Z

-message BackoffTask {}
+message BackoffTask {
+    // deadline is the time at which the backoff period ends.
+    google.protobuf.Timestamp deadline = 1;


Don't need this, it's part of the task attributes.

And please add attempt for verification.

bergundy · 2025-10-24T17:28:38Z

-    // Will have other meanings as more callback use cases are added.
-    string url = 1;
+    // The destination for callbacks. Can be a URL for nexus callbacks or temporal:// for internal callbacks.
+    string destination = 1;


Don't need this, it's part of the task attributes.

…ck-executors

spkane31 · 2025-10-27T22:17:25Z

Changes to this are triggering our proto breaking change CI check, do we have a way to force skip that check?

Yes, you can disable in buf.yaml.

bergundy · 2025-10-27T22:22:30Z

+//
+// This is the CHASM port of HSM's nexusInvocation struct from nexus_invocation.go:25-32.


Suggested change

//

// This is the CHASM port of HSM's nexusInvocation struct from nexus_invocation.go:25-32.

The comment was incorrect.

bergundy · 2025-10-27T22:25:31Z

+		return nil, err
+	}
+
+	completion, err := target.GetNexusCompletion(ctx, component.RequestId)


Ah I see that the context object is used here, we should be able to use the chasm context here, that's going to be tech debt we need to track. CC @yycptt.

bergundy · 2025-10-27T22:26:45Z

+	return invokable.WrapError(result, saveErr)
+}
+
+func (e InvocationTaskExecutor) loadInvocationArgs(


Well... now that you've refactored this function to work within a lock, you can just make it a method of the Callback struct. Same thing for saveResult. You don't actually need e here.

bergundy · 2025-10-27T22:28:05Z

+	ctx chasm.MutableContext,
+	result invocationResult,
+) (struct{}, error) {
+	switch result.(type) {


Go through the statemachine transitions here.

bergundy · 2025-10-27T22:29:17Z

+
+	// Convert the CHASM task to the internal BackoffTask type
+	// Note: BackoffTask proto is empty, deadline comes from NextAttemptScheduleTime in callback
+	backoffTask := &callbackspb.BackoffTask{


What is this for?

bergundy · 2025-10-27T22:29:38Z

+	executor := InvocationTaskExecutor{
+		InvocationTaskExecutorOptions: InvocationTaskExecutorOptions{
+			Config:         e.Config,
+			MetricsHandler: e.MetricsHandler,
+			Logger:         e.Logger,
+		},
+	}


Why create another executor?

bergundy · 2025-10-27T22:29:52Z

+	return callback.Status == callbackspb.CALLBACK_STATUS_BACKING_OFF && callback.Attempt == task.Attempt, nil
+}
+
+func (e InvocationTaskExecutor) executeBackoffTask(


This can just be BackoffTaskExecutor.Execute.

bergundy · 2025-10-28T00:03:57Z

Yes, you can disable in buf.yaml.

…cs to Callback

…ck-executors

bergundy · 2025-10-29T21:31:44Z

    - chasm/lib/scheduler/proto/v1/message.proto
+    - chasm/lib/callback/proto/v1/tasks.proto


We should have a TODO to remove these after merging this PR. I'd be fine to leave the scheduler one though and let @lina-temporal handle that.

bergundy · 2025-10-29T21:52:25Z

+func (c *Callback) saveResult(
+	ctx chasm.MutableContext,
+	result invocationResult,
+) (struct{}, error) {


Not blocking this PR but we need to add a chasm.NoValue type that is *struct{} and just return nil from methods that don't have a return value.

I added this type FTR.

bergundy · 2025-10-29T21:57:27Z

+}
+
+// Test saveResult transitions
+func TestSaveResult(t *testing.T) {


This is redundant IMHO, just test the entire executor.

bergundy · 2025-10-29T21:59:30Z

+}
+
+// Test loadInvocationArgs with ComponentRef
+func TestLoadInvocationArgs(t *testing.T) {


Not blocking: This is redundant IMHO.

If you really want to test load and save (which I think isn't required) move the tests to component_test.go.

bergundy · 2025-10-29T22:00:04Z

Still not seeing all of the tests ported here.

bergundy · 2025-10-29T22:00:34Z

+	"chasm_callback_outbound_requests",
+	metrics.WithDescription("The number of CHASM outbound callback requests made by the history service."),
+)
+var RequestLatencyHistogram = metrics.NewTimerDef(
+	"chasm_callback_outbound_latency",


Suggested change

"chasm_callback_outbound_requests",

metrics.WithDescription("The number of CHASM outbound callback requests made by the history service."),

)

var RequestLatencyHistogram = metrics.NewTimerDef(

"chasm_callback_outbound_latency",

"callback_outbound_requests",

metrics.WithDescription("The number of outbound callback requests made by the history service."),

)

var RequestLatencyHistogram = metrics.NewTimerDef(

"callback_outbound_latency",

This would conflict with the existing HSM metric, do we want to emit these metrics with the same name or differentiate the two?

Same, it's a transparent change.

bergundy · 2025-10-29T22:01:05Z

+)
+var RequestLatencyHistogram = metrics.NewTimerDef(
+	"chasm_callback_outbound_latency",
+	metrics.WithDescription("Latency histogram of CHASM outbound callback requests made by the history service."),


Suggested change

metrics.WithDescription("Latency histogram of CHASM outbound callback requests made by the history service."),

metrics.WithDescription("Latency histogram of outbound callback requests made by the history service."),

same as above, did this to clarify the difference between the HSM and CHASM version.

…ck-executors

bergundy

Approved with comments.

bergundy · 2025-10-30T16:44:25Z

+func (c *Callback) saveResult(
+	ctx chasm.MutableContext,
+	result invocationResult,
+) (struct{}, error) {


I added this type FTR.

bergundy · 2025-10-30T16:46:02Z

+	"chasm_callback_outbound_requests",
+	metrics.WithDescription("The number of CHASM outbound callback requests made by the history service."),
+)
+var RequestLatencyHistogram = metrics.NewTimerDef(
+	"chasm_callback_outbound_latency",


Same, it's a transparent change.

bergundy · 2025-10-30T16:47:03Z

 	[]callbackspb.CallbackStatus{callbackspb.CALLBACK_STATUS_SCHEDULED},
 	callbackspb.CALLBACK_STATUS_FAILED,
-	func(cb *Callback, ctx chasm.MutableContext, event EventFailed) error {
+	func(cb *Callback, mctx chasm.MutableContext, event EventFailed) error {


nit: rename back to ctx? also below please.

spkane31 added 7 commits October 15, 2025 16:28

chasm(callback): implement backoff and invocation executor

42e7488

linter

0c2d63f

invoker pseudo-implementation

db9a903

Merge branch 'main' of github.com:temporalio/temporal into spk/callba…

465211c

…ck-executors

adding more to the fields and invoker

8b29e4c

docstrings

7c4fd60

nexus headers

fe14c15

spkane31 requested a review from lina-temporal October 16, 2025 23:14

spkane31 added 3 commits October 17, 2025 10:22

combining the invoker into callback component

0861623

adding tests

7543071

move Nexus to its own message, fix TODO

78eab6f

spkane31 requested a review from bergundy October 20, 2025 15:39

state machine tests

85d1298

spkane31 marked this pull request as ready for review October 20, 2025 16:17

spkane31 requested review from a team as code owners October 20, 2025 16:17

Merge branch 'main' of github.com:temporalio/temporal into spk/callba…

62e8b97

…ck-executors

spkane31 requested a review from pdoerner October 20, 2025 19:26

bergundy reviewed Oct 20, 2025

View reviewed changes

spkane31 added 11 commits October 21, 2025 09:56

pruning tests

cf433d6

a few of roeys comments

fa5adcd

Merge branch 'main' of github.com:temporalio/temporal into spk/callba…

9b05d60

…ck-executors

statemachine and statemachine tests

8fe3fca

copy tasks, library, and executors from hsm implementation

b629e02

direct port of hsm components

0193601

use same statemachine changes as other PR

58211a2

use same statemachine changes as other PR

8a7bef1

revert protobuf changes and remove tasks file

c25faf4

more fixins and removing task structs in favor of protobufs

3b73167

mutable context

bff89f2

spkane31 added 3 commits October 23, 2025 10:14

linter

5a29dac

merge conflicts

8819c11

remove fx registration

1eb9a13

spkane31 requested a review from bergundy October 23, 2025 22:59

Merge branch 'main' of github.com:temporalio/temporal into spk/callba…

92dd765

…ck-executors

bergundy reviewed Oct 24, 2025

View reviewed changes

spkane31 added 6 commits October 27, 2025 12:10

first portion of feedback from roey

e937924

Merge branch 'main' of github.com:temporalio/temporal into spk/callba…

6b0d9e2

…ck-executors

fix tests for new implementation

d6d0655

nolint and adding back tests

0449938

formatting

42ab4f1

remove workflow and run id in favor of chasm context portion

7561df2

spkane31 commented Oct 27, 2025

View reviewed changes

spkane31 requested a review from bergundy October 27, 2025 22:17

bergundy reviewed Oct 28, 2025

View reviewed changes

spkane31 added 2 commits October 28, 2025 16:23

using state transitions, allow breaking change in protobufs, move fun…

3adf7e6

…cs to Callback

Merge branch 'main' of github.com:temporalio/temporal into spk/callba…

faad40d

…ck-executors

spkane31 requested a review from bergundy October 29, 2025 15:30

bergundy reviewed Oct 29, 2025

View reviewed changes

spkane31 mentioned this pull request Oct 29, 2025

Add a chasm.NoValue type #8566

Closed

spkane31 added 2 commits October 29, 2025 17:03

port over one missing test, rename

07b8e4c

Merge branch 'main' of github.com:temporalio/temporal into spk/callba…

28a4374

…ck-executors

spkane31 requested a review from bergundy October 30, 2025 15:18

spkane31 added 2 commits October 30, 2025 09:19

merge conflicts

890765c

type error

510eea1

bergundy approved these changes Oct 30, 2025

View reviewed changes

mctx -> ctx, remove chasm prefix

4fb3383

spkane31 enabled auto-merge (squash) October 30, 2025 17:05

spkane31 merged commit 1b33d6a into main Oct 30, 2025
57 checks passed

spkane31 deleted the spk/callback-executors branch October 30, 2025 17:27

HSM	CHASM
config_test.go	config_test.go (copied directly)
TestValidTransitions	TestValidTransitions (copied)
TestCompareState	I don't think this test applies, is there a `chasm.CompareState` equivalence?
TestProcessInvocationTaskNexus_Outcomes	TestExecuteInvocationTaskNexus_Outcomes (renamed)
TestProcessInvocationTaskHsm_Outcomes	Not relevant right?
TestProcessBackoffTask	TestProcessBackoffTask
TestProcessInvocationTaskChasm_Outcomes	TestExecuteInvocationTaskChasm_Outcomes (added now)

		//
		// This is the CHASM port of HSM's nexusInvocation struct from nexus_invocation.go:25-32.

		- chasm/lib/scheduler/proto/v1/message.proto
		- chasm/lib/callback/proto/v1/tasks.proto

	metrics.WithDescription("Latency histogram of CHASM outbound callback requests made by the history service."),
	metrics.WithDescription("Latency histogram of outbound callback requests made by the history service."),

Conversation

spkane31 commented Oct 16, 2025

What changed?

Why?

How did you test it?

Potential risks

Uh oh!

bergundy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spkane31 Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spkane31 Oct 21, 2025 •

edited

Loading