Skip to content

External Storage Integration: NexusWorker - #3018

Open
cconstable wants to merge 22 commits into
mainfrom
extstore/nexus-worker
Open

External Storage Integration: NexusWorker#3018
cconstable wants to merge 22 commits into
mainfrom
extstore/nexus-worker

Conversation

@cconstable

@cconstable cconstable commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

What was changed

  • NexusWorker now stores and retrieves payloads.

Why?

  • Nexus workers should use external storage.

Checklist

@cconstable
cconstable changed the base branch from main to extstore/foundation August 19, 2026 17:51
@cconstable cconstable changed the title extstore/nexus worker External Storage Integration: NexusWorker Aug 19, 2026
@cconstable
cconstable marked this pull request as ready for review August 19, 2026 18:57
@cconstable
cconstable requested a review from a team as a code owner August 19, 2026 18:57
…former to ExternalStorage, create a lazy extstore resolving data converter.
@cconstable
cconstable force-pushed the extstore/foundation branch from 460bfbf to b3804da Compare August 21, 2026 20:35
@cconstable
cconstable force-pushed the extstore/nexus-worker branch from 609161f to c553b3d Compare August 21, 2026 21:56
@cconstable
cconstable force-pushed the extstore/foundation branch from b3804da to ca09b50 Compare August 24, 2026 01:23
@cconstable
cconstable force-pushed the extstore/nexus-worker branch from c553b3d to 15f3980 Compare August 24, 2026 01:23
…h to workers. we've got the dataconverter already so we can just derive it where its needed.
@cconstable
cconstable force-pushed the extstore/nexus-worker branch from 15f3980 to e8155ee Compare August 24, 2026 19:37
…ce payload makes it to fromPayload without being retrieved. This is a guard against SDK features that fail or fail to correctly integrate external storage. Normal misconfiguration errors should be caught at a level above fromPayload.
… make sure users don't see a misleading error is nice it could also cause some nondeterminism if we fix the error and came with its own set of problems. maybe in the future we can find a better way to catch missing external storage integration in a better way.
@cconstable
cconstable force-pushed the extstore/nexus-worker branch from e8155ee to 5ef0bd6 Compare August 27, 2026 18:34
@cconstable

Copy link
Copy Markdown
Contributor Author

Missing: NexusClient integration. Adding now.

@cconstable
cconstable force-pushed the extstore/nexus-worker branch from 41fa991 to 94eaaca Compare August 27, 2026 21:15
…tions.java

Co-authored-by: Justin Anderson <44687433+jmaeagle99@users.noreply.github.com>
task.getCompletionCallback());
}

private void storeOutbound(Message.Builder builder) {

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.

Why does this storeOutbound work differently then this one ? Should we just make one common helper ?

@Quinn-With-Two-Ns

Copy link
Copy Markdown
Contributor

Is there any tests showing external storage working for a Nexus operation? or what PR would that be in?

@Quinn-With-Two-Ns

Copy link
Copy Markdown
Contributor

Codex flagged these issues as well

I found two actionable issues. Your question about integration tests was
  warranted.

  1. High — Storage failures never produce a retryable Nexus task failure

     ExternalStorageTaskFailure says it is reported as a retryable handler error

     (https://github.com/temporalio/sdk-java/blob/94eaaca69c34a780b00803f1ddac129112ad2107/temporal-sdk/src/main/java/io/temporal/internal/worker/NexusWorker.java#L287-L295),
     but the class is unused. Both inbound retrieval and outbound storage errors

     (https://github.com/temporalio/sdk-java/blob/94eaaca69c34a780b00803f1ddac129112ad2107/temporal-sdk/src/main/java/io/temporal/internal/worker/NexusWorker.java#L553-L574)
     escape the worker. The generic poll executor only forwards them to the
     uncaught-exception handler; it does not call RespondNexusTaskFailed.

     Recovery therefore waits for task timeout/redelivery. For an outbound
     failure, the user handler has already executed, so redelivery can repeat
     its side effects. Catch storage errors and immediately send a small,
     inline, retryable INTERNAL handler failure—without passing that failure
     through external storage again. Add transient retrieve/store failure
     integration tests.

  2. Medium — External retrieval is excluded from the used-slot lifecycle

     markSlotUsed now happens after retrieveInboundPayloads

     (https://github.com/temporalio/sdk-java/blob/94eaaca69c34a780b00803f1ddac129112ad2107/temporal-sdk/src/main/java/io/temporal/internal/worker/NexusWorker.java#L325-L350).
     During a slow retrieval, used-slot metrics and custom supplier state
     under-report active work. If retrieval fails, the completion callback
     releases the permit as taskComplete, but TrackingSlotSupplier has no slot
     info because it was never marked used

     (https://github.com/temporalio/sdk-java/blob/94eaaca69c34a780b00803f1ddac129112ad2107/temporal-sdk/src/main/java/io/temporal/internal/worker/TrackingSlotSupplier.java#L70-L91).

     Mark the slot used before retrieval, or release failed pre-handler
     retrievals through an explicit never-used/error path.

@cconstable
cconstable force-pushed the extstore/foundation branch from 2d2d90e to 504467c Compare August 28, 2026 16:10
Base automatically changed from extstore/foundation to main August 28, 2026 17:27
@cconstable
cconstable force-pushed the extstore/nexus-worker branch from 94eaaca to 78778d3 Compare August 28, 2026 21:27
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