feat(extstore): implement core extstore logic that uses payload visitor to batch and store/retrieve payloads. by cconstable · Pull Request #2976 · temporalio/sdk-java

cconstable · 2026-07-29T17:03:09Z

What was changed

Added internal/payload/storage/ExternalStorageMessageConverter.java to transform payloads in messages.
Added internal/payload/storage/ExternalStoragePayloadConverter.java to handle the actual transformation.
Refactored to use new TaskScope for handling parent-child and sibling task cancellation.

Why?

Unblocks extstore integration

Checklist

Added new tests

…or to batch and store/retrieve payloads.

cconstable · 2026-07-29T17:46:41Z

+
+  private CompletableFuture<List<IndexedValue<Payload>>> runStoreDrivers(
+      Map<String, Batch<Payload>> batches, @Nullable StorageDriverTargetInfo target) {
+    return TaskScope.withScope(


The previous logic here needed to manually account for external cancellation and handle sibling cancellation. The TaskScope handles that automatically. The scope.awaitAll() at the end has fail-fast semantics so it will immediately cancel other siblings on first error.

Copilot

Pull request overview

This PR introduces the core “extstore” plumbing in the Java SDK by adding internal converters that traverse proto messages, batch payloads by driver, and offload/retrieve large payloads via external storage drivers, with structured cancellation support.

Changes:

Added internal payload converters and reference encoding/decoding to offload payloads to external storage and restore them on retrieval.
Exposed payload-visitor utilities to support traversing nested proto payload fields and applying conversions with bounded concurrency.
Extended storage driver contexts with cancellation tokens and added tests covering batching, thresholds, ordering, and cancellation behavior.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
temporal-sdk/src/main/java/io/temporal/payload/storage/StorageDriverStoreContext.java	Adds cancellation token to store context for aborting in-flight driver calls.
temporal-sdk/src/main/java/io/temporal/payload/storage/StorageDriverRetrieveContext.java	Adds cancellation token to retrieve context for aborting in-flight driver calls.
temporal-sdk/src/main/java/io/temporal/payload/storage/StorageDriver.java	Documents cancellation token usage for driver implementers.
temporal-sdk/src/main/java/io/temporal/internal/payload/visitor/PayloadVisitors.java	Makes payload visitor entrypoints public for cross-package internal reuse.
temporal-sdk/src/main/java/io/temporal/internal/payload/visitor/PayloadVisitorOptions.java	Makes visitor options public to configure traversal/concurrency/skips.
temporal-sdk/src/main/java/io/temporal/internal/payload/visitor/PayloadVisitor.java	Makes the payload visitor functional interface public.
temporal-sdk/src/main/java/io/temporal/internal/payload/storage/StorageDriverStoreContextImpl.java	Implements store context with target info + cancellation token wiring.
temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageReferences.java	Encodes/decodes external storage reference payloads.
temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStoragePayloadConverter.java	Core batching logic to store/retrieve payload lists with TaskScope cancellation.
temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageMessageConverter.java	Bridges payload-list conversion over arbitrary proto messages via PayloadVisitors.
temporal-sdk/src/test/java/io/temporal/payload/storage/ExternalStorageOptionsTest.java	Updates tests for new cancellation token requirement in store context.
temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageReferencesTest.java	Adds tests for reference round-tripping and validation behavior.
temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStoragePayloadConverterTest.java	Adds tests for batching, thresholds, multi-driver routing, and cancellation on failure.
temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageMessageConverterTest.java	Adds tests for message traversal, nested payload conversion, and skipping search attributes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cconstable · 2026-07-29T18:37:03Z

+  static boolean isReference(@Nonnull Payload payload) {
+    return payload.getExternalPayloadsCount() > 0;
+  }


I think the correct thing to do it replace isReference() + fromReferencePayload() with a single tryParseReference(Payload) that checks both external_payloads > 0 and messageType == temporal.api.sdk.v1.ExternalStorageReference and returns null instead of throwing. Will update this soon.

cconstable · 2026-07-29T18:38:09Z

+  static boolean isReference(@Nonnull Payload payload) {
+    return payload.getExternalPayloadsCount() > 0;
+  }


Going to replace this. See thread in conversation tab.

cconstable · 2026-07-29T18:38:30Z

+  static ParsedReference fromReferencePayload(@Nonnull Payload payload) {
+    ByteString messageType = payload.getMetadataMap().get(EncodingKeys.METADATA_MESSAGE_TYPE_KEY);
+    if (messageType == null || !REFERENCE_MESSAGE_TYPE.equals(messageType.toStringUtf8())) {
+      throw new IllegalArgumentException(
+          "Payload is not an external storage reference; expected messageType '"
+              + REFERENCE_MESSAGE_TYPE
+              + "' but was '"
+              + (messageType == null ? "" : messageType.toStringUtf8())
+              + "'");
+    }


Going to replace this. See thread in conversations tab.

jmaeagle99 · 2026-07-31T04:05:31Z

+ * Converts one payload list between inline payloads and external-storage references by routing
+ * entries to storage drivers.
+ */
+final class ExternalStoragePayloadConverter {


I might call this something like ExternalStoragePayloadTransformer or something to that effect. Avoid PayloadConverter since that is an actual concept already that is tangentially related.

jmaeagle99 · 2026-07-31T16:00:19Z

+   * storage reference this SDK understands.
+   */
+  static @Nullable ParsedReference tryParseReference(@Nonnull Payload payload) {
+    if (payload.getExternalPayloadsCount() == 0) {


Remove this check. This is not a requirement for exchanging a reference for the externally stored payload.

swapping back to the original implementation

jmaeagle99 · 2026-07-31T16:02:38Z

+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+final class StorageDriverStoreContextImpl implements StorageDriverStoreContext {


Any particular reason this one is in its own separate file but the StorageDriverRetrieveContextImpl is a nested static class of ExternalStoragePayloadConverter?

No, but I'll make them consistent

jmaeagle99 · 2026-07-31T16:05:37Z

   * Stores {@code payloads} and returns one {@link StorageDriverClaim} per payload, in the same
   * order. The returned list must be the same length as {@code payloads}.
+   *
+   * <p>Drivers should use {@link StorageDriverStoreContext#getCancellationToken()} to abort


This comment reads as "I should call getCancellationToken to abort in-flight requests". I think it should be more like "call getCancellationToken() to get a token to observe when cancellation is requested". Could use better wording.

jmaeagle99 · 2026-07-31T16:17:21Z

 */
 @Experimental
-public interface StorageDriverRetrieveContext {}
+public interface StorageDriverRetrieveContext {


I looked over the other SDKs and this is the first one where a user would have to implement an interface (that isn't structural) in their testing. And I totally acknowledge that I suggested it be as such. However, that means if we ever needed to add something to the interface after GA, it would technically be a breaking change. Wondering if we should document it as such that we are free to add more to it over time or if we should convert it to a class that allows users to instantiate for testing. Maybe not too much of an issue for Java since it has default interface implementations. Just something to think through for maintenance purposes.

jmaeagle99 · 2026-07-31T16:27:46Z

+  CompletableFuture<List<Payload>> store(
+      @Nullable StorageDriverTargetInfo target, List<Payload> payloads) {
+    StorageDriverStoreContext context =
+        new StorageDriverStoreContextImpl(target, CancellationToken.none());


I think cancellation token should be plumbed through the store method and used here. Or are we not doing that because there are no callers that can offer cancellation? Might still do that so the API has the correct shape and then have the caller pass CancellationToken.none().

I would imagine that at least worker shutdown has something to plumb into, if not some kind of cancellation per workflow or task.

jmaeagle99 · 2026-07-31T16:47:16Z

+    this.payloadVisitConcurrency = payloadVisitConcurrency;
+  }
+
+  <T extends Message> CompletableFuture<T> store(


Any possible efficiency in allowing support of Message.Builder as well? Might avoid a bunch of copying.

jmaeagle99 · 2026-07-31T16:51:27Z

+    if (payload.getExternalPayloadsCount() == 0) {
+      return null;
+    }
+    ByteString messageType = payload.getMetadataMap().get(EncodingKeys.METADATA_MESSAGE_TYPE_KEY);


Should we also be checking encoding == ENCODING_PROTOBUF_JSON?

…ss pr feedback.

cconstable added 3 commits July 29, 2026 13:02

feat(extstore): implement core extstore logic that uses payload visit…

3032838

…or to batch and store/retrieve payloads.

remove legacy payload detection.

9cf1358

update comments.

2d150c2

cconstable commented Jul 29, 2026

View reviewed changes

cconstable marked this pull request as ready for review July 29, 2026 17:48

cconstable requested a review from a team as a code owner July 29, 2026 17:48

cconstable requested review from Copilot, jmaeagle99 and maciejdudko July 29, 2026 17:48

Copilot started reviewing on behalf of cconstable July 29, 2026 17:48 View session

Copilot AI reviewed Jul 29, 2026

View reviewed changes

cconstable commented Jul 29, 2026

View reviewed changes

fix extstore reference checking logic.

5dbe465

jmaeagle99 reviewed Jul 31, 2026

View reviewed changes

fix(extstore): rename message/payload converter to transformer. addre…

eba1217

…ss pr feedback.

Uh oh!

Conversation

cconstable commented Jul 29, 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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

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!

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!

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 Jul 29, 2026 •

edited

Loading