Skip to content

Held a channel's listeners in one table of callbacks. - #16

Closed
moetemp wants to merge 4 commits into
moe/AI-198-srv-12-unsubscribefrom
moe/AI-198-srv-13-listener-callbacks
Closed

moetemp wants to merge 4 commits into
moe/AI-198-srv-12-unsubscribefrom
moe/AI-198-srv-13-listener-callbacks

Conversation

@moetemp

@moetemp moetemp commented Oct 3, 2026

Copy link
Copy Markdown
Owner

This PR holds a channel's workflow and callback listeners in one table of callbacks.

What changed?

  • The two listener maps on chasm/lib/channel are one Listeners map keyed by listener id, of one record Listener that holds a temporal.api.common.v1.Callback. A URL callback is the Nexus variant with its URL and headers. A workflow listener is the Internal variant, whose data is a marshalled WorkflowTarget with the namespace id, workflow id, run id and first execution run id. Delivery dispatches on the variant: Nexus through the outbound callback task, Internal through the routed DeliverChannelNotification on the run's shard.
  • RegisterWorkflowListener stays the internal entry point and writes an Internal entry under the workflow id, so a later run of the same workflow still takes the entry of the run before it. Re-key and forget decode the target, check the run and write it back.
  • DescribeChannel keeps its wire shape. The listener kind, workflow id and run id are derived from the variant, workflow listeners first, each kind in id order. The listener_kind metrics tag is derived from the variant too.
  • One id space. A callback whose request id is the id of a workflow listening on the channel is refused with InvalidArgument, and so is a workflow registering under a callback's id.
  • Ordered delivery to a URL callback holds without a change: one post is in flight per listener, a notification arriving during it folds into the one pending entry and goes out when the post returns. TestNotificationChannelCallbackDeliveriesInOrder sends a burst of rising counters to a slow endpoint and checks that the counters it saw only rise and end on the last one.
  • Re-creation after retention holds without a change: the idle task closes and deletes a channel with no listeners a retention after its last activity, and a notify after that starts a fresh channel with an empty ring. TestNotificationChannelRecreatedAfterRetention covers it with a one-second retention. A poller holding a counter from the old channel sees only the new notification.
  • tests/notification_channel_test.go reads the Listeners#<id> node, and the unit tests read the one map. TestWorkflowListenerIsAnInternalCallback pins the record shape.

Part of AI-198 (epic AI-37).

Why?

The next layer addresses a channel's owner by Execution, a workflow or a standalone activity, and a listener table with one map per kind would grow a third map for it. One record that says where a notification goes, in the shape the server already uses for completion callbacks, lets a new kind of listener be a new target inside the Internal data rather than a new table. The two checks are here because folding the table is only safe once the behaviour it had, in-order posts and the retention cycle, is pinned by tests.

How did you test it?

  • Unit Tests
  • Staging
  • End to End Tests

go build ./..., golangci-lint and the errortype vet are clean on chasm/lib/channel and tests. The channel unit tests ran, with the new record-shape case. The functional tests ran with -tags disable_grpc_modules,test_dep: the notification channel suite including the two new cases, and the linked channel, inline handoff, stream channel, describe channels and unsubscribe suites, all unchanged apart from the renamed node in the callback-pending helper.

A workflow listener is an internal callback whose data names the run, so the
table has one record shape and the delivery dispatches on the variant.
Describe and the metrics derive the kind from it.
The channel kept one post in flight per callback listener and folded what
arrived meanwhile, and a channel deleted by the idle task started fresh on
the next notify. Neither had a test.
@moetemp

moetemp commented Oct 3, 2026

Copy link
Copy Markdown
Owner Author

Replaced by #18, #19, #20, #21, #22, #23, #24, #25, #26, #27, #28, #29, #30, #31, #32, #33, #34, #35, #36 and #37.

Same content, split into 20 PRs in the v3 series: the notification channel first, then the streaming interface, then native streams and the rest. The branch stays as a pin.

@moetemp moetemp closed this Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant