Skip to content

Dedup retried Nexus signals - #2036

Merged
atol merged 5 commits into
mainfrom
alicelin/NEXUS-940/signal-dedup
Nov 7, 2026
Merged

atol merged 5 commits into
mainfrom
alicelin/NEXUS-940/signal-dedup

Conversation

@atol

@atol atol commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

What was changed

  • Signals and signal-with-start sent through TemporalNexusClient.client in a Temporal operation handler now reuse the Nexus operation's request ID.
  • Breaking (experimental API): each handler run can send only one such signal; a second raises a BAD_REQUEST nexusrpc.HandlerError. (Signal-with-start counts toward the limit; a failed signal doesn't; a retried Nexus task starts with a fresh limit.)
  • Signals sent through temporalio.nexus.client() are unchanged.
  • Based on De-duplicate Nexus Signal and SignalWithStart operations sdk-go#2781.

Why?

When a Nexus task is retried, the handler reruns and sends its signal again with a new random request ID, so the workflow receives it twice. Reusing the operation's request ID lets the server dedupe the retry. However, every signal in one handler run then shares that ID, so a second signal to the same workflow would be silently dropped while reported as delivered. Limiting each run to one signal turns that silent loss into a clear error.

Checklist

  1. How was this tested:
    Added integration tests.

  2. Any docs updates needed?
    No.

@atol
atol marked this pull request as ready for review October 30, 2026 15:52
@atol
atol requested a review from a team as a code owner October 30, 2026 15:52
@atol
atol force-pushed the alicelin/NEXUS-940/signal-dedup branch from fe91ac8 to d7ca329 Compare November 3, 2026 21:03
@VegetarianOrc VegetarianOrc self-assigned this Nov 3, 2026
Comment thread temporalio/nexus/_temporal_client.py Resolved
Comment thread temporalio/nexus/_temporal_client.py Resolved
Comment thread temporalio/nexus/_operation_context.py Resolved
@atol
atol force-pushed the alicelin/NEXUS-940/signal-dedup branch from d7ca329 to 0f3b9f5 Compare November 5, 2026 19:37
@atol
atol requested a review from VegetarianOrc November 5, 2026 19:43

@VegetarianOrc VegetarianOrc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Ty for the changes! Just a couple of nits I'd consider before merging.

Comment thread temporalio/nexus/_operation_handlers.py Outdated
Comment on lines +204 to +208
token = _temporal_operation_signal_slot.set(_SignalSlot())
try:
result = await self.start_operation(start_ctx, nexus_client, input)
finally:
_temporal_operation_signal_slot.reset(token)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, but I'd prefer this encapsulated in a context manager that manages the signal slot context var. could be a static method on _SignalSlot so we could do like

with _SignalSlot.limit_signals():
    result = await self.start_operation(start_ctx, nexus_client, input)

Comment thread temporalio/nexus/_operation_context.py Resolved
@atol
atol force-pushed the alicelin/NEXUS-940/signal-dedup branch from c4ef27c to 2ddc554 Compare November 6, 2026 18:17
@atol
atol force-pushed the alicelin/NEXUS-940/signal-dedup branch from 2ddc554 to abcb22c Compare November 6, 2026 22:34
@atol

atol commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Moving the dedup into _ClientImpl via _signal_request_nexus_context makes it apply to every client used inside _SignalSlot.scope(). .client is again the same object as nexus.client() and removing it isn't needed for the fix, so I restored it.

@atol
atol merged commit 3499a78 into main Nov 7, 2026
46 of 52 checks passed
@atol
atol deleted the alicelin/NEXUS-940/signal-dedup branch November 7, 2026 00:45
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.

2 participants