External Storage Integration: NexusWorker by cconstable · Pull Request #3018 · temporalio/sdk-java

cconstable · 2026-08-19T17:50:49Z

What was changed

NexusWorker now stores and retrieves payloads.

Why?

Nexus workers should use external storage.

Checklist

cconstable · 2026-08-27T20:51:06Z

Missing: NexusClient integration. Adding now.

Quinn-With-Two-Ns · 2026-08-28T15:38:48Z

+          task.getCompletionCallback());
+    }
+
+    private void storeOutbound(Message.Builder builder) {


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

Some callers of storeOutbound have already built messages and some have builders. Some also need a target (ActivityWorker) and others also need a visitor (the WorkflowWorker one). We could create a MessageOrBuilder type for this (maybe there already is one) and do some refactoring. Personally, I'd like that to be a follow-up if that is the direction we want to head in.

Quinn-With-Two-Ns · 2026-08-28T15:39:42Z

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

Quinn-With-Two-Ns · 2026-08-28T15:49:24Z

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 · 2026-08-31T16:35:49Z

The codex finds are interesting. I think it's correct, it doesn't look like the we are catching failures. Adding some logic to send a nexus task failed so the server can retry instead of waiting for timeout.

… failures properly.

…cks a retryable failure.

cconstable · 2026-08-31T17:36:58Z

Addressed the feedback and issues codex found. One thing to note @Quinn-With-Two-Ns is that when we fail the nexus task (because of an external storage issue which happens AFTER the task is handled + "complete") and respond with a RespondNexusTaskFailed this means that the server can retry which means there will be issues if the handler is not doing idempotent work. Is that ok?

Another interesting idea @jmaeagle99 is that we could have a fallback for scenarios like this where we would skip external storage since the nexus task already completed for scenarios where making an idempotent task is more of a detriment than just skipping external storage... so external storage would be configured with a "do your best to offload these payloads but if you cant (because the provider is down, etc) its ok to send them through as regular payloads".

cconstable · 2026-08-31T17:41:15Z

Also added some tests @Quinn-With-Two-Ns to address your feedback.

cconstable · 2026-08-31T18:13:58Z

The other PRs #3017 and #3020 also needed to handle the extstore store failures in the same way we were handling here. There is some duplication but I can follow up with a PR to consolidate all that once these PRs are merged.

jmaeagle99 · 2026-08-31T23:58:04Z

Addressed the feedback and issues codex found. One thing to note @Quinn-With-Two-Ns is that when we fail the nexus task (because of an external storage issue which happens AFTER the task is handled + "complete") and respond with a RespondNexusTaskFailed this means that the server can retry which means there will be issues if the handler is not doing idempotent work. Is that ok?

Another interesting idea @jmaeagle99 is that we could have a fallback for scenarios like this where we would skip external storage since the nexus task already completed for scenarios where making an idempotent task is more of a detriment than just skipping external storage... so external storage would be configured with a "do your best to offload these payloads but if you cant (because the provider is down, etc) its ok to send them through as regular payloads".

I don't think this is a new error mode for Nexus Operation handlers though. They already have at-least-once attempt semantics. The operation's work needs to be idempotent regardless of external storage.

On skipping external storage, if the result is too large, then the task should be failed by the server (have to double check that) in a terminal way. Even if it doesn't, it will likely poison downstream consumers.

jmaeagle99 · 2026-08-31T23:15:03Z

+          .inc(1);
+      HandlerException handlerException =
+          new HandlerException(HandlerException.ErrorType.INTERNAL, "External storage failed", e);
+      sendReply(


I think this needs to handle ExternalStorageTaskFailure in case the failure cannot be externally stored and then submit a minimal NexusTaskHandler.Result from a small exception instance.

jmaeagle99 · 2026-08-31T23:25:52Z

+          task = retrieveInboundPayloads(task);
+        } catch (Throwable e) {
+          taskFailed = true;
+          sendStorageFailure(


This is going to log "External storage failed for a nexus task" but external storage is not configured, so the log is misleading.

jmaeagle99 · 2026-08-31T23:32:31Z

+    ExternalStorage externalStorageConfig = options.getExternalStorage();
+    if (externalStorageConfig != null) {
+      options =
+          NexusClientOptions.newBuilder(options)


Should this setExternalStorage(null)?

jmaeagle99 · 2026-08-31T23:34:21Z

  }

+  @Nullable
+  public ExternalStorage getExternalStorage() {


Needs javadoc

jmaeagle99 · 2026-08-31T23:34:41Z

      return this;
    }

+    public NexusClientOptions.Builder setExternalStorage(


Needs javadoc

jmaeagle99 · 2026-08-31T23:38:15Z

+  public void interruptingShutdownCancelsInFlightStorage() throws Exception {
+    NexusWorker worker = worker();
+
+    worker.shutdown(new ShutdownManager(), true).get();


These should be closed, probably in a finally, in every test.

jmaeagle99 · 2026-09-01T00:03:22Z

+        slotSupplier.markSlotUsed(
+            new NexusSlotInfo(
+                service, operation, taskQueue, options.getIdentity(), options.getBuildId()),
+            task.getPermit());


This code and above used to be outside of the try block. Not sure it needs to be inside. It might incidentally fix some existing problems. But its not the focus of this PR. If it doesn't need to be in the try to enable external storage, then let's move it back. If it is fixing something separately, then open a different PR for it.

cconstable changed the base branch from main to extstore/foundation August 19, 2026 17:51

cconstable mentioned this pull request Aug 19, 2026

External Storage Integration: Lazy resolving references, general refactoring #3016

Merged

cconstable changed the title ~~extstore/nexus worker~~ External Storage Integration: NexusWorker Aug 19, 2026

cconstable marked this pull request as ready for review August 19, 2026 18:57

cconstable requested a review from a team as a code owner August 19, 2026 18:57

cconstable force-pushed the extstore/foundation branch from 460bfbf to b3804da Compare August 21, 2026 20:35

cconstable force-pushed the extstore/nexus-worker branch from 609161f to c553b3d Compare August 21, 2026 21:56

cconstable force-pushed the extstore/foundation branch from b3804da to ca09b50 Compare August 24, 2026 01:23

cconstable force-pushed the extstore/nexus-worker branch 3 times, most recently from e8155ee to 5ef0bd6 Compare August 27, 2026 18:34

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

Quinn-With-Two-Ns reviewed Aug 28, 2026

View reviewed changes

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 force-pushed the extstore/nexus-worker branch from 94eaaca to 78778d3 Compare August 28, 2026 21:27

cconstable added 5 commits August 30, 2026 21:57

feat(extstore): integrate into nexus pipeline.

d201670

Explicitly pass cancellation tokens to extstore.

9bdc2d8

add more cancellation threading

4841678

NexusClientImpl integration

fe7fcc4

externalStorage -> externalStorageRunner

d877ec7

cconstable force-pushed the extstore/nexus-worker branch from 78778d3 to d877ec7 Compare August 31, 2026 15:42

add nexus e2e tests

f3f6f30

cconstable added 3 commits August 31, 2026 12:52

fix(extstore): handle extstore errors for inbound nexus tasks. report…

1e230e5

… failures properly.

fix(extstore): markSlotUsed before payload retrieval.

1a798a1

fix(extstore): handle outbound extstore failures properly. reports ba…

be50571

…cks a retryable failure.

formatting

2af26ff

jmaeagle99 reviewed Aug 31, 2026

View reviewed changes

jmaeagle99 reviewed Sep 1, 2026

View reviewed changes

Uh oh!

Conversation

cconstable commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was changed

Why?

Checklist

Uh oh!

cconstable commented Aug 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Quinn-With-Two-Ns commented Aug 28, 2026

Uh oh!

Quinn-With-Two-Ns commented Aug 28, 2026

Uh oh!

cconstable commented Aug 31, 2026

Uh oh!

cconstable commented Aug 31, 2026

Uh oh!

cconstable commented Aug 31, 2026

Uh oh!

cconstable commented Aug 31, 2026

Uh oh!

jmaeagle99 commented Aug 31, 2026

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cconstable commented Aug 19, 2026 •

edited

Loading