External Storage Integration: Activity worker, client by cconstable · Pull Request #3020 · temporalio/sdk-java

cconstable · 2026-08-19T18:01:42Z

What was changed

Main integration points are in WorkflowClientInternalImpl, HeartbeatContextImpl, and ActivityWorker.
Heartbeat store requests are canceled if they pass a timeout (as defined by the caller/heartbeat code).
Lot's of plumbing in other adjacent files.

Why?

Activity workers and clients should offload payloads with external storage.

Checklist

Added lots of tests.

cconstable · 2026-08-19T18:27:59Z

+  public ListWorkflowExecutionsResponse listWorkflowExecutions(
+      ListWorkflowExecutionsRequest listRequest) {
+    return next.listWorkflowExecutions(listRequest);
+  }


Addressed the previous concern of eagerly fetching external payloads #2978 (comment). They are now loaded lazily.

cconstable · 2026-08-19T18:47:42Z


      try {
-        sendReply(taskToken, result, metricsScope);
+        sendReply(taskToken, result, metricsScope, activityStorageTarget(pollResponse));


Previous comment about pivoting this target based on standalone vs workflow activity has been addressed #2978 (comment)

activityStorageTarget delegates to storageTargetForActivityTask (which can be seen above)

cconstable · 2026-08-27T20:10:37Z

+ * <p>This This is an internal class that is not exposed to users or workflow code. The intent is to
+ * use this data converter to consolidate extstore usage within the SDK.
+ */
+public final class ExternalStorageDataConverter implements DataConverter {


This is the new abstraction that replaces the previous External Storage client decorator. It wraps the data converter supplied to the WorkflowClient in RootWorkflowClientInvoker

cconstable · 2026-08-27T20:16:13Z

+  public RootWorkflowClientInvoker(
+      GenericWorkflowClient genericClient,
+      WorkflowClientOptions clientOptions,
+      WorkerFactoryRegistry workerFactoryRegistry,
+      @Nullable ExternalStorageRunner externalStorage) {
+    this.externalStorage = externalStorage;


This class inherits the external storage runner from WorkflowClientInternalImpl and then uses it to create the new ExternalStorageDataConverter

cconstable · 2026-08-27T20:56:14Z

  Stream<HistoryEvent> streamHistory(WorkflowExecution execution) {
    Preconditions.checkNotNull(execution, "execution is required");

    GetWorkflowExecutionHistoryIterator iterator =
        new GetWorkflowExecutionHistoryIterator(
            options.getNamespace(), execution, null, genericClient);
    iterator.init();


This no longer runs through external storage as it was previously being handled by the client decorator. The worker still uses extstore when replaying it's own history (see https://github.com/temporalio/sdk-java/pull/3017/changes#diff-4928a50c4af33d91cd6b57a121d20ed2594cd314e118beb01c259d5e7427728c).

Is this just used for offline replay? Do we need to use extstore here?

I think we discussed offline we don't need to? At least that is what other SDKs did so we should be consistent

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

Couple nits, generally looks good. Going to do an AI review as well. Would want Justin to look as well of course

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

Codex identified these two additional issues

  1. High — Manual completion uses the wrong storage target for workflow
     activities

     The factory paths

     (https://github.com/temporalio/sdk-java/blob/0c7b3087f75afaf6d30a8cc6034686e9f847a13e/temporal-sdk/src/main/java/io/temporal/internal/client/external/ManualActivityCompletionClientFactoryImpl.java#L49-L117)
     and local-manual-completion path

     (https://github.com/temporalio/sdk-java/blob/0c7b3087f75afaf6d30a8cc6034686e9f847a13e/temporal-sdk/src/main/java/io/temporal/internal/activity/ActivityExecutionContextImpl.java#L154-L170)
     always create StorageDriverActivityInfo. For workflow-scheduled activities,
     the established policy is to use StorageDriverWorkflowInfo, as
     ActivityWorker does

     (https://github.com/temporalio/sdk-java/blob/0c7b3087f75afaf6d30a8cc6034686e9f847a13e/temporal-sdk/src/main/java/io/temporal/internal/worker/ActivityWorker.java#L274-L289).

     This means synchronous and manual completion of the same workflow activity
     can select different drivers or storage layouts. The context/execution
     already contains enough information to distinguish workflow from standalone
     activities. I’d centralize that pivot and add tests covering both activity
     kinds across task-token, by-ID, and local manual completion.

  2. Medium — TestActivityEnvironment heartbeat listeners cannot decode
     offloaded details

     The environment now passes external storage into the heartbeat context

     (https://github.com/temporalio/sdk-java/blob/0c7b3087f75afaf6d30a8cc6034686e9f847a13e/temporal-testing/src/main/java/io/temporal/testing/TestActivityEnvironmentInternal.java#L104-L116),
     so sufficiently large heartbeat details reach the mock service as
     references. However, the heartbeat listener still decodes them with the
     ordinary DataConverter

     (https://github.com/temporalio/sdk-java/blob/0c7b3087f75afaf6d30a8cc6034686e9f847a13e/temporal-testing/src/main/java/io/temporal/testing/TestActivityEnvironmentInternal.java#L135-L152)
     without retrieving them first. With threshold 0, for example,
     setActivityHeartbeatListener will receive an external-storage reference and
     fail conversion. Resolve references before listener conversion and add an
     environment regression test.

At a quick glance they look legit, but please let me know.

cconstable · 2026-08-31T20:40:02Z

The first issue that codex identified led me to do a little refactoring. I believe all the feedback has been addressed here.

…t pipelines.

…TestActivityEnvironment

…rator, address some feedback points.

…o store.

…ly have one exposed path for getting the data converter.

…ogic.

…y use extstore.

jmaeagle99 · 2026-09-04T20:28:32Z

    }

    private ActivityTaskHandler.Result handleActivity(ActivityTask task, Scope metricsScope) {
+      task = retrieveInboundPayloads(task);


If external storage throws, then there is no recovery. Exceptions here should proactively fail the activity task.

jmaeagle99 · 2026-09-04T20:38:11Z

-        sendReply(taskToken, result, metricsScope);
+        sendReply(taskToken, result, metricsScope, activityStorageTarget(pollResponse));
+      } catch (ExternalStorageTaskFailure e) {
+        sendStorageFailure(taskToken, pollResponse, metricsScope, e.getCause());


Should be consistent with the other PRs and send the storage failure without using external storage.

jmaeagle99 · 2026-09-04T20:39:40Z

-                .setWorkerVersion(options.workerVersionStamp())
-                .build();
+                .setWorkerVersion(options.workerVersionStamp());
+        storeOutboundPayloads(completedBuilder, storageTarget);


This and all other call sites should recover and send activity task failure without external storage.

jmaeagle99 · 2026-09-04T22:34:27Z

+   * Offloads large heartbeat payloads aborting if the store call runs longer than the heartbeat
+   * interval or if the activity is cancelled.
+   */
+  private void offloadHeartbeat(RecordActivityTaskHeartbeatRequest.Builder builder) {


I believe this runs under the heartbeat lock. And the cancellation below looks to be relying on the heartbeat interval, which has a default of 30 seconds. This is could also starve out other operations like getHeartbeatDetails as they queue up. Also, if too many activities get stuck on external storage during heart beating, it might exhaust the thread pool and activity cancels may not run.

jmaeagle99 · 2026-09-04T23:53:25Z

+            .setIdentity(identity)
+            .setNamespace(namespace)
            .setRequestId(UUID.randomUUID().toString())
            .setHeader(HeaderUtils.toHeaderGrpc(input.getHeader(), null));


Need to convert headers. This cannot be done with the DataConverter because they are already Payloads. Haven't looked for all instances but pointing out this one category.

jmaeagle99 · 2026-09-04T23:59:17Z

+    this.baseConverter =
+        externalStorage == null
+            ? clientOptions.getDataConverter()
+            : new ExternalStorageDataConverter(clientOptions.getDataConverter(), externalStorage);


I'm thinking we might need to update the ExternalStroageDataConverter to be used regardless of it external storage is enabled or not AND perform the TMRPL1005 check when not configured. Otherwise clients calls will not deserialize properly and probably get a not as helpful message.

jmaeagle99 · 2026-09-05T00:04:40Z

-            info -> new WorkflowExecutionMetadata(info, clientOptions.getDataConverter()));
+            info ->
+                new WorkflowExecutionMetadata(
+                    info, workflowConverter(info.getExecution(), info.getType().getName())));


This is gaining a WorkflowSerializationContext when it did not have one before. That would be changing the semantics of this API call. It is probably the right thing to do, but shouldn't be slipped in as an undocumented bug fix. So either construct one without the serialization context or we should document it.

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

cconstable mentioned this pull request Aug 19, 2026

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

Merged

cconstable changed the title ~~extstore/activity client~~ External Storage Integration: Activity worker, client Aug 19, 2026

cconstable commented Aug 19, 2026

View reviewed changes

cconstable force-pushed the extstore/activity-client branch from 37ae3c7 to 7069249 Compare August 19, 2026 18:45

cconstable commented Aug 19, 2026

View reviewed changes

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/activity-client branch from 7069249 to 550e4f2 Compare August 21, 2026 21:57

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

cconstable force-pushed the extstore/activity-client branch 3 times, most recently from ffafefa to 69a8a3a Compare August 24, 2026 19:38

cconstable commented Aug 26, 2026

View reviewed changes

Comment thread ...src/main/java/io/temporal/internal/client/external/ExternalStorageGenericWorkflowClient.java Outdated

cconstable force-pushed the extstore/activity-client branch from fad5ed3 to 8d1399c Compare August 27, 2026 20:07

cconstable commented Aug 27, 2026

View reviewed changes

cconstable force-pushed the extstore/activity-client branch from 8d1399c to 0c7b308 Compare August 27, 2026 21:28

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

View reviewed changes

Comment thread ...k/src/main/java/io/temporal/internal/client/external/ManualActivityCompletionClientImpl.java Outdated

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

View reviewed changes

Comment thread temporal-sdk/src/main/java/io/temporal/internal/client/RootWorkflowClientInvoker.java

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

View reviewed changes

Comment thread temporal-testing/src/main/java/io/temporal/testing/TestActivityEnvironmentInternal.java

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/activity-client branch 2 times, most recently from f527885 to 2cedf19 Compare August 31, 2026 15:46

cconstable mentioned this pull request Aug 31, 2026

External Storage Integration: NexusWorker #3018

Merged

cconstable added 8 commits September 3, 2026 13:59

feat(extstore): integrate into activity worker, heartbeats, and clien…

0efe1ea

…t pipelines.

fix(extstore): derive external storage runner from data converter in …

409b1a8

…TestActivityEnvironment

use ExternalStorageDataConverter, remove external storage client deco…

08f587a

…rator, address some feedback points.

externalStorage -> externalStorageRunner

fbf464d

fix(extstore): send a RespondActivityTaskFailed when extstore fails t…

8f3c23e

…o store.

refactor(extstore): add a client data converter factory so that we on…

9931808

…ly have one exposed path for getting the data converter.

fix(extstore): fix storage targets and add a factory to consolidate l…

04b2fe2

…ogic.

fix(extstore): fix TestActivityEnvironment heartbeat listeners so the…

050c637

…y use extstore.

cconstable force-pushed the extstore/activity-client branch from ec3528f to 050c637 Compare September 3, 2026 18:01

small nit refactor

a4fe03a

jmaeagle99 reviewed Sep 5, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Quinn-With-Two-Ns commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

cconstable 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

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

edited

Loading