From eee31a3ff2c4bf78786cb386461c8cc618b3003b Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Tue, 18 Aug 2026 16:24:22 -0400 Subject: [PATCH 01/23] refactor(extstore): general extstore refactoring. rename MessageTransformer to ExternalStorage, create a lazy extstore resolving data converter. --- .../PayloadAndFailureDataConverter.java | 6 + .../payload/storage/ExternalStorage.java | 162 +++++++++ .../ExternalStorageMessageTransformer.java | 75 ---- ...ExternalStorageNotConfiguredException.java | 16 + .../storage/ExternalStorageReferences.java | 11 +- .../payload/visitor/MessageVisitor.java | 2 +- .../visitor/PayloadVisitorOptions.java | 2 +- .../internal/worker/SingleWorkerOptions.java | 23 +- .../storage/ExternalStorageOptions.java | 30 +- .../ExternalStorageReferenceGuardTest.java | 51 +++ ...ExternalStorageMessageTransformerTest.java | 161 --------- .../payload/storage/ExternalStorageTest.java | 323 ++++++++++++++++++ .../storage/ExternalStorageOptionsTest.java | 18 + 13 files changed, 635 insertions(+), 245 deletions(-) create mode 100644 temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorage.java delete mode 100644 temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageMessageTransformer.java create mode 100644 temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java create mode 100644 temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java delete mode 100644 temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageMessageTransformerTest.java create mode 100644 temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageTest.java diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java index 935fd8462c..a628b4118b 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java @@ -8,6 +8,8 @@ import io.temporal.api.common.v1.Payloads; import io.temporal.api.failure.v1.Failure; import io.temporal.failure.DefaultFailureConverter; +import io.temporal.internal.payload.storage.ExternalStorageNotConfiguredException; +import io.temporal.internal.payload.storage.ExternalStorageReferences; import io.temporal.payload.context.SerializationContext; import java.lang.reflect.Type; import java.util.*; @@ -71,6 +73,10 @@ public T fromPayload(Payload payload, Class valueClass, Type valueType) return (T) new RawValue(payload); } + if (ExternalStorageReferences.isReference(payload)) { + throw new ExternalStorageNotConfiguredException(); + } + try { String encoding = payload.getMetadataOrThrow(EncodingKeys.METADATA_ENCODING_KEY).toString(UTF_8); diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorage.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorage.java new file mode 100644 index 0000000000..ec225fc837 --- /dev/null +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorage.java @@ -0,0 +1,162 @@ +package io.temporal.internal.payload.storage; + +import com.google.common.base.Throwables; +import com.google.protobuf.Message; +import io.temporal.api.common.v1.Payload; +import io.temporal.api.sdk.v1.ExternalStorageReference; +import io.temporal.common.CancellationToken; +import io.temporal.internal.payload.visitor.MessageVisitor; +import io.temporal.internal.payload.visitor.PayloadVisitorOptions; +import io.temporal.internal.payload.visitor.PayloadVisitors; +import io.temporal.payload.storage.ExternalStorageOptions; +import io.temporal.payload.storage.StorageDriver; +import io.temporal.payload.storage.StorageDriverTargetInfo; +import java.util.List; +import java.util.concurrent.CancellationException; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.ExecutionException; +import javax.annotation.Nullable; + +/** + * External storage offloads large payloads via {@link StorageDriver}s. It walks messages using + * {@link PayloadVisitors} transforming payloads to and from {@link ExternalStorageReference} using + * {@link ExternalStoragePayloadTransformer}. Use {@link ExternalStorageOptions} via {@link#create} + * to configure external storage. + */ +public final class ExternalStorage { + private final ExternalStoragePayloadTransformer payloadTransformer; + private final int payloadVisitConcurrency; + + public static ExternalStorage create(ExternalStorageOptions options) { + return new ExternalStorage( + ExternalStoragePayloadTransformer.fromOptions(options), + options.getMaxConcurrentPayloadVisits()); + } + + ExternalStorage( + ExternalStoragePayloadTransformer payloadTransformer, int payloadVisitConcurrency) { + this.payloadTransformer = payloadTransformer; + this.payloadVisitConcurrency = payloadVisitConcurrency; + } + + public T storeBlocking(T message, @Nullable StorageDriverTargetInfo target) { + return storeBlocking(message, target, CancellationToken.none()); + } + + public T storeBlocking( + T message, + @Nullable StorageDriverTargetInfo target, + CancellationToken cancellationToken) { + return getOrThrowIfCancelled(store(message, target, cancellationToken), cancellationToken); + } + + public T storeBlocking( + T message, + @Nullable StorageDriverTargetInfo target, + @Nullable MessageVisitor targetVisitor) { + CancellationToken cancellationToken = CancellationToken.none(); + return getOrThrowIfCancelled( + PayloadVisitors.visit(message, storeOptions(target, targetVisitor, cancellationToken)), + cancellationToken); + } + + public T retrieveBlocking(T message) { + CancellationToken cancellationToken = CancellationToken.none(); + return getOrThrowIfCancelled(retrieve(message, cancellationToken), cancellationToken); + } + + public CompletableFuture retrieveAsync(T message) { + return retrieve(message, CancellationToken.none()); + } + + /** + * Throws {@link ExternalStorageNotConfiguredException} if {@code message} contains any reference + * payload. Used at inbound task boundaries when external storage is not configured. + */ + public static void throwIfContainsReference(Message message) { + PayloadVisitorOptions options = + PayloadVisitorOptions.newBuilder( + (context, payloads) -> { + for (Payload payload : payloads) { + if (ExternalStorageReferences.isReference(payload)) { + CompletableFuture> found = new CompletableFuture<>(); + found.completeExceptionally(new ExternalStorageNotConfiguredException()); + return found; + } + } + return CompletableFuture.completedFuture(payloads); + }) + .setSkipSearchAttributes(true) + .build(); + try { + PayloadVisitors.visit(message.toBuilder(), options).join(); + } catch (CompletionException e) { + Throwable cause = e.getCause() != null ? e.getCause() : e; + Throwables.throwIfUnchecked(cause); + throw e; + } + } + + private static T getOrThrowIfCancelled( + CompletableFuture future, CancellationToken cancellationToken) { + try { + CompletableFuture.anyOf(future, cancellationToken.getCancellationFuture()).get(); + return future.get(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new CancellationException("External storage operation interrupted"); + } catch (ExecutionException e) { + Throwable cause = e.getCause() != null ? e.getCause() : e; + Throwables.throwIfUnchecked(cause); + throw new CompletionException(cause); + } + } + + CompletableFuture store( + T message, + @Nullable StorageDriverTargetInfo target, + CancellationToken cancellationToken) { + return PayloadVisitors.visit(message, storeOptions(target, null, cancellationToken)); + } + + CompletableFuture store( + Message.Builder builder, + @Nullable StorageDriverTargetInfo target, + CancellationToken cancellationToken) { + return PayloadVisitors.visit(builder, storeOptions(target, null, cancellationToken)); + } + + CompletableFuture retrieve( + T message, CancellationToken cancellationToken) { + return PayloadVisitors.visit(message, retrieveOptions(cancellationToken)); + } + + CompletableFuture retrieve( + Message.Builder builder, CancellationToken cancellationToken) { + return PayloadVisitors.visit(builder, retrieveOptions(cancellationToken)); + } + + private PayloadVisitorOptions storeOptions( + @Nullable StorageDriverTargetInfo target, + @Nullable MessageVisitor targetVisitor, + CancellationToken cancellationToken) { + return PayloadVisitorOptions.newBuilder( + (visitedTarget, payloads) -> + payloadTransformer.store(payloads, visitedTarget, cancellationToken)) + .setInitialContext(target) + .setMessageVisitor(targetVisitor) + .setConcurrency(payloadVisitConcurrency) + .setSkipSearchAttributes(true) + .build(); + } + + private PayloadVisitorOptions retrieveOptions( + CancellationToken cancellationToken) { + return PayloadVisitorOptions.newBuilder( + (context, payloads) -> payloadTransformer.retrieve(payloads, cancellationToken)) + .setConcurrency(payloadVisitConcurrency) + .setSkipSearchAttributes(true) + .build(); + } +} diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageMessageTransformer.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageMessageTransformer.java deleted file mode 100644 index 7385f99009..0000000000 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageMessageTransformer.java +++ /dev/null @@ -1,75 +0,0 @@ -package io.temporal.internal.payload.storage; - -import com.google.protobuf.Message; -import io.temporal.common.CancellationToken; -import io.temporal.internal.payload.visitor.PayloadVisitorOptions; -import io.temporal.internal.payload.visitor.PayloadVisitors; -import io.temporal.payload.storage.StorageDriverTargetInfo; -import java.util.concurrent.CancellationException; -import java.util.concurrent.CompletableFuture; -import javax.annotation.Nullable; - -/** - * Transforms payload lists reachable from a proto message by delegating each visited list to {@link - * ExternalStoragePayloadTransformer}. - * - *

Search attributes stay inline because the server indexes and validates their payload values. - * - *

The {@link Message.Builder} overloads transform in place; the {@link Message} overloads copy - * through a builder and complete with the copy. - */ -final class ExternalStorageMessageTransformer { - private final ExternalStoragePayloadTransformer payloadTransformer; - private final int payloadVisitConcurrency; - - ExternalStorageMessageTransformer( - ExternalStoragePayloadTransformer payloadTransformer, int payloadVisitConcurrency) { - this.payloadTransformer = payloadTransformer; - this.payloadVisitConcurrency = payloadVisitConcurrency; - } - - CompletableFuture store( - T message, - @Nullable StorageDriverTargetInfo target, - CancellationToken cancellationToken) { - return PayloadVisitors.visit(message, storeOptions(target, cancellationToken)); - } - - CompletableFuture store( - Message.Builder builder, - @Nullable StorageDriverTargetInfo target, - CancellationToken cancellationToken) { - return PayloadVisitors.visit(builder, storeOptions(target, cancellationToken)); - } - - CompletableFuture retrieve( - T message, CancellationToken cancellationToken) { - return PayloadVisitors.visit(message, retrieveOptions(cancellationToken)); - } - - CompletableFuture retrieve( - Message.Builder builder, CancellationToken cancellationToken) { - return PayloadVisitors.visit(builder, retrieveOptions(cancellationToken)); - } - - private PayloadVisitorOptions storeOptions( - @Nullable StorageDriverTargetInfo target, - CancellationToken cancellationToken) { - return PayloadVisitorOptions.newBuilder( - (visitedTarget, payloads) -> - payloadTransformer.store(payloads, visitedTarget, cancellationToken)) - .setInitialContext(target) - .setConcurrency(payloadVisitConcurrency) - .setSkipSearchAttributes(true) - .build(); - } - - private PayloadVisitorOptions retrieveOptions( - CancellationToken cancellationToken) { - return PayloadVisitorOptions.newBuilder( - (context, payloads) -> payloadTransformer.retrieve(payloads, cancellationToken)) - .setConcurrency(payloadVisitConcurrency) - .setSkipSearchAttributes(true) - .build(); - } -} diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java new file mode 100644 index 0000000000..441e41851a --- /dev/null +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java @@ -0,0 +1,16 @@ +package io.temporal.internal.payload.storage; + +import io.temporal.common.converter.DataConverterException; + +/** + * Signals that an external storage reference reached a data converter without storage configured. + * Logs a TMPRL1105 error. + */ +public final class ExternalStorageNotConfiguredException extends DataConverterException { + public ExternalStorageNotConfiguredException() { + super( + "[TMPRL1105] Encountered an external-storage reference payload but external storage is not " + + "configured. Configure WorkflowClientOptions.Builder.setExternalStorage(...) with a " + + "driver able to retrieve it."); + } +} diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageReferences.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageReferences.java index 3a68c6bb66..1a81e3b676 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageReferences.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageReferences.java @@ -9,7 +9,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -final class ExternalStorageReferences { +public final class ExternalStorageReferences { private static final String ENCODING_PROTOBUF_JSON = "json/protobuf"; private static final String REFERENCE_MESSAGE_TYPE = ExternalStorageReference.getDescriptor().getFullName(); @@ -64,8 +64,7 @@ static Payload toReferencePayload( * producer that omits it still yields a readable reference. */ static @Nullable ParsedReference tryParseReference(@Nonnull Payload payload) { - if (!hasMetadata(payload, EncodingKeys.METADATA_ENCODING_KEY, ENCODING_PROTOBUF_JSON) - || !hasMetadata(payload, EncodingKeys.METADATA_MESSAGE_TYPE_KEY, REFERENCE_MESSAGE_TYPE)) { + if (!isReference(payload)) { return null; } ExternalStorageReference.Builder builder = ExternalStorageReference.newBuilder(); @@ -79,6 +78,12 @@ static Payload toReferencePayload( reference.getDriverName(), new StorageDriverClaim(reference.getClaimDataMap())); } + /** True if {@code payload} has an external storage reference encoding and message type. */ + public static boolean isReference(Payload payload) { + return hasMetadata(payload, EncodingKeys.METADATA_ENCODING_KEY, ENCODING_PROTOBUF_JSON) + && hasMetadata(payload, EncodingKeys.METADATA_MESSAGE_TYPE_KEY, REFERENCE_MESSAGE_TYPE); + } + private static boolean hasMetadata(Payload payload, String key, String expected) { ByteString value = payload.getMetadataMap().get(key); return value != null && expected.equals(value.toStringUtf8()); diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/visitor/MessageVisitor.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/visitor/MessageVisitor.java index 21268e41d7..4bb6083e3e 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/visitor/MessageVisitor.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/visitor/MessageVisitor.java @@ -11,7 +11,7 @@ * @param type of the contextual value */ @FunctionalInterface -interface MessageVisitor { +public interface MessageVisitor { /** * Handles a message being entered and returns the contextual value for it and its contents. * diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/visitor/PayloadVisitorOptions.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/visitor/PayloadVisitorOptions.java index 4eac39be46..e4d6c89e47 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/visitor/PayloadVisitorOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/visitor/PayloadVisitorOptions.java @@ -69,7 +69,7 @@ private Builder(@Nonnull PayloadVisitor payloadVisitor) { this.payloadVisitor = Objects.requireNonNull(payloadVisitor, "payloadVisitor"); } - Builder setMessageVisitor(@Nullable MessageVisitor messageVisitor) { + public Builder setMessageVisitor(@Nullable MessageVisitor messageVisitor) { this.messageVisitor = messageVisitor; return this; } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java b/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java index 8e0288566e..9b1d055110 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java @@ -7,10 +7,12 @@ import io.temporal.common.converter.DataConverter; import io.temporal.common.converter.GlobalDataConverter; import io.temporal.common.interceptors.WorkerInterceptor; +import io.temporal.internal.payload.storage.ExternalStorage; import io.temporal.worker.PreferredVersionProvider; import io.temporal.worker.WorkerDeploymentOptions; import java.time.Duration; import java.util.List; +import javax.annotation.Nullable; public final class SingleWorkerOptions { @@ -45,6 +47,7 @@ public static final class Builder { private boolean allowActivityHeartbeatDuringShutdown; private String workerControlTaskQueue; private PreferredVersionProvider preferredVersionProvider; + private @Nullable ExternalStorage externalStorage; private Builder() {} @@ -73,6 +76,7 @@ private Builder(SingleWorkerOptions options) { this.allowActivityHeartbeatDuringShutdown = options.getAllowActivityHeartbeatDuringShutdown(); this.workerControlTaskQueue = options.getWorkerControlTaskQueue(); this.preferredVersionProvider = options.getPreferredVersionProvider(); + this.externalStorage = options.getExternalStorage(); } public Builder setIdentity(String identity) { @@ -185,6 +189,11 @@ public Builder setPreferredVersionProvider(PreferredVersionProvider preferredVer return this; } + public Builder setExternalStorage(@Nullable ExternalStorage externalStorage) { + this.externalStorage = externalStorage; + return this; + } + public SingleWorkerOptions build() { PollerOptions pollerOptions = this.pollerOptions; if (pollerOptions == null) { @@ -227,7 +236,8 @@ public SingleWorkerOptions build() { this.workerInstanceKey, this.allowActivityHeartbeatDuringShutdown, this.workerControlTaskQueue, - this.preferredVersionProvider); + this.preferredVersionProvider, + this.externalStorage); } } @@ -252,6 +262,7 @@ public SingleWorkerOptions build() { private final boolean allowActivityHeartbeatDuringShutdown; private final String workerControlTaskQueue; private final PreferredVersionProvider preferredVersionProvider; + private final @Nullable ExternalStorage externalStorage; private SingleWorkerOptions( String identity, @@ -274,7 +285,8 @@ private SingleWorkerOptions( String workerInstanceKey, boolean allowActivityHeartbeatDuringShutdown, String workerControlTaskQueue, - PreferredVersionProvider preferredVersionProvider) { + PreferredVersionProvider preferredVersionProvider, + @Nullable ExternalStorage externalStorage) { this.identity = identity; this.binaryChecksum = binaryChecksum; this.buildId = buildId; @@ -296,6 +308,7 @@ private SingleWorkerOptions( this.allowActivityHeartbeatDuringShutdown = allowActivityHeartbeatDuringShutdown; this.workerControlTaskQueue = workerControlTaskQueue; this.preferredVersionProvider = preferredVersionProvider; + this.externalStorage = externalStorage; } public String getIdentity() { @@ -393,6 +406,12 @@ public PreferredVersionProvider getPreferredVersionProvider() { return preferredVersionProvider; } + /** The external-storage message transformer for this worker, or null when disabled. */ + @Nullable + public ExternalStorage getExternalStorage() { + return externalStorage; + } + public WorkerVersioningOptions getWorkerVersioningOptions() { return new WorkerVersioningOptions( this.getBuildId(), this.isUsingBuildIdForVersioning(), this.getDeploymentOptions()); diff --git a/temporal-sdk/src/main/java/io/temporal/payload/storage/ExternalStorageOptions.java b/temporal-sdk/src/main/java/io/temporal/payload/storage/ExternalStorageOptions.java index 1486fb76b0..3abc969291 100644 --- a/temporal-sdk/src/main/java/io/temporal/payload/storage/ExternalStorageOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/payload/storage/ExternalStorageOptions.java @@ -15,6 +15,7 @@ @Experimental public final class ExternalStorageOptions { static final int DEFAULT_PAYLOAD_SIZE_THRESHOLD = 256 * 1024; + static final int DEFAULT_MAX_CONCURRENT_PAYLOAD_VISITS = 3; public static Builder newBuilder() { return new Builder(); @@ -23,14 +24,17 @@ public static Builder newBuilder() { private final @Nonnull List drivers; private final @Nonnull StorageDriverSelector driverSelector; private final int payloadSizeThreshold; + private final int maxConcurrentPayloadVisits; private ExternalStorageOptions( @Nonnull List drivers, @Nonnull StorageDriverSelector driverSelector, - int payloadSizeThreshold) { + int payloadSizeThreshold, + int maxConcurrentPayloadVisits) { this.drivers = Collections.unmodifiableList(new ArrayList<>(drivers)); this.driverSelector = driverSelector; this.payloadSizeThreshold = payloadSizeThreshold; + this.maxConcurrentPayloadVisits = maxConcurrentPayloadVisits; } @Nonnull @@ -51,10 +55,20 @@ public int getPayloadSizeThreshold() { return payloadSizeThreshold; } + /** + * Maximum number of payload lists visited concurrently while offloading or restoring the payloads + * of a single message. Defaults to 3. + */ + public int getMaxConcurrentPayloadVisits() { + return maxConcurrentPayloadVisits; + } + public static final class Builder { private List drivers = Collections.emptyList(); private StorageDriverSelector driverSelector; private int payloadSizeThreshold = ExternalStorageOptions.DEFAULT_PAYLOAD_SIZE_THRESHOLD; + private int maxConcurrentPayloadVisits = + ExternalStorageOptions.DEFAULT_MAX_CONCURRENT_PAYLOAD_VISITS; private Builder() {} @@ -84,10 +98,21 @@ public Builder setPayloadSizeThreshold(int payloadSizeThreshold) { return this; } + /** + * Maximum number of payload lists visited concurrently while offloading or restoring the + * payloads of a single message. Must be at least 1. Defaults to 3. + */ + public Builder setMaxConcurrentPayloadVisits(int maxConcurrentPayloadVisits) { + this.maxConcurrentPayloadVisits = maxConcurrentPayloadVisits; + return this; + } + public ExternalStorageOptions build() { Preconditions.checkState(!drivers.isEmpty(), "At least one driver must be provided"); Preconditions.checkState( payloadSizeThreshold >= 0, "payloadSizeThreshold must be greater than or equal to zero"); + Preconditions.checkState( + maxConcurrentPayloadVisits >= 1, "maxConcurrentPayloadVisits must be at least 1"); Set names = new HashSet<>(); for (StorageDriver driver : drivers) { String name = driver.getName(); @@ -102,7 +127,8 @@ public ExternalStorageOptions build() { StorageDriver driver = drivers.get(0); selector = (context, payload) -> driver; } - return new ExternalStorageOptions(drivers, selector, payloadSizeThreshold); + return new ExternalStorageOptions( + drivers, selector, payloadSizeThreshold, maxConcurrentPayloadVisits); } } } diff --git a/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java b/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java new file mode 100644 index 0000000000..31c9698692 --- /dev/null +++ b/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java @@ -0,0 +1,51 @@ +package io.temporal.common.converter; + +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import com.google.protobuf.ByteString; +import io.temporal.api.common.v1.Payload; +import io.temporal.api.sdk.v1.ExternalStorageReference; +import io.temporal.internal.payload.storage.ExternalStorageNotConfiguredException; +import org.junit.Test; + +/** + * When external storage is not configured, an inbound reference payload reaching value + * deserialization must fail with the clear {@code [TMPRL1105]} error instead of an opaque decoding + * failure. + */ +public class ExternalStorageReferenceGuardTest { + + private final DataConverter dataConverter = DefaultDataConverter.newDefaultInstance(); + + @Test + public void referencePayloadWithoutConfiguredStorageThrows() { + Payload reference = + Payload.newBuilder() + .putMetadata( + EncodingKeys.METADATA_ENCODING_KEY, ByteString.copyFromUtf8("json/protobuf")) + .putMetadata( + EncodingKeys.METADATA_MESSAGE_TYPE_KEY, + ByteString.copyFromUtf8(ExternalStorageReference.getDescriptor().getFullName())) + .setData(ByteString.copyFromUtf8("{}")) + .build(); + + ExternalStorageNotConfiguredException e = + assertThrows( + ExternalStorageNotConfiguredException.class, + () -> dataConverter.fromPayload(reference, String.class, String.class)); + assertTrue(e.getMessage(), e.getMessage().contains("[TMPRL1105]")); + } + + @Test + public void rawValueBypassesTheGuard() { + Payload reference = + Payload.newBuilder() + .addExternalPayloads( + Payload.ExternalPayloadDetails.newBuilder().setSizeBytes(1024).build()) + .build(); + + RawValue raw = dataConverter.fromPayload(reference, RawValue.class, RawValue.class); + assertTrue(raw.getPayload().getExternalPayloadsCount() > 0); + } +} diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageMessageTransformerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageMessageTransformerTest.java deleted file mode 100644 index f17bcff47a..0000000000 --- a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageMessageTransformerTest.java +++ /dev/null @@ -1,161 +0,0 @@ -package io.temporal.internal.payload.storage; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - -import com.google.protobuf.ByteString; -import io.temporal.api.command.v1.Command; -import io.temporal.api.command.v1.ScheduleActivityTaskCommandAttributes; -import io.temporal.api.command.v1.StartChildWorkflowExecutionCommandAttributes; -import io.temporal.api.common.v1.Payload; -import io.temporal.api.common.v1.Payloads; -import io.temporal.api.common.v1.SearchAttributes; -import io.temporal.common.CancellationToken; -import io.temporal.payload.storage.ExternalStorageOptions; -import io.temporal.payload.storage.StorageDriver; -import io.temporal.payload.storage.StorageDriverClaim; -import io.temporal.payload.storage.StorageDriverRetrieveContext; -import io.temporal.payload.storage.StorageDriverStoreContext; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.concurrent.CompletableFuture; -import org.junit.Test; - -/** Tests external storage message conversion. */ -public class ExternalStorageMessageTransformerTest { - - @Test - public void storeAndRetrieveRoundTripsOverAMessage() throws Exception { - InMemoryDriver driver = new InMemoryDriver("d1"); - ExternalStorageMessageTransformer transformer = transformer(driver, 0); - Payloads message = - Payloads.newBuilder().addPayloads(payload("a")).addPayloads(payload("b")).build(); - - Payloads stored = transformer.store(message, null, CancellationToken.none()).get(); - - assertNotNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(0))); - assertNotNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(1))); - - Payloads retrieved = transformer.retrieve(stored, CancellationToken.none()).get(); - assertEquals(message, retrieved); - } - - @Test - public void walksNestedPayloads() throws Exception { - InMemoryDriver driver = new InMemoryDriver("d1"); - ExternalStorageMessageTransformer transformer = transformer(driver, 0); - Command command = - Command.newBuilder() - .setScheduleActivityTaskCommandAttributes( - ScheduleActivityTaskCommandAttributes.newBuilder() - .setInput(Payloads.newBuilder().addPayloads(payload("deep")))) - .build(); - - Command stored = transformer.store(command, null, CancellationToken.none()).get(); - - Payload nested = stored.getScheduleActivityTaskCommandAttributes().getInput().getPayloads(0); - assertNotNull(ExternalStorageReferences.tryParseReference(nested)); - assertEquals(command, transformer.retrieve(stored, CancellationToken.none()).get()); - } - - @Test - public void payloadBelowThresholdLeavesMessageUnchanged() throws Exception { - InMemoryDriver driver = new InMemoryDriver("d1"); - ExternalStorageMessageTransformer transformer = transformer(driver, 1024); - Payloads message = Payloads.newBuilder().addPayloads(payload("small")).build(); - - Payloads stored = transformer.store(message, null, CancellationToken.none()).get(); - - assertNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(0))); - assertEquals(message, stored); - assertTrue(driver.storeBatchSizes.isEmpty()); - } - - @Test - public void searchAttributesAreNotOffloaded() throws Exception { - InMemoryDriver driver = new InMemoryDriver("d1"); - ExternalStorageMessageTransformer transformer = transformer(driver, 0); - Command command = - Command.newBuilder() - .setStartChildWorkflowExecutionCommandAttributes( - StartChildWorkflowExecutionCommandAttributes.newBuilder() - .setInput(Payloads.newBuilder().addPayloads(payload("input"))) - .setSearchAttributes( - SearchAttributes.newBuilder() - .putIndexedFields("k", payload("indexed-value")))) - .build(); - - Command stored = transformer.store(command, null, CancellationToken.none()).get(); - - StartChildWorkflowExecutionCommandAttributes attrs = - stored.getStartChildWorkflowExecutionCommandAttributes(); - assertNotNull(ExternalStorageReferences.tryParseReference(attrs.getInput().getPayloads(0))); - Payload indexed = attrs.getSearchAttributes().getIndexedFieldsOrThrow("k"); - assertNull(ExternalStorageReferences.tryParseReference(indexed)); - assertEquals(payload("indexed-value"), indexed); - } - - private static ExternalStorageMessageTransformer transformer( - StorageDriver driver, int threshold) { - ExternalStoragePayloadTransformer payloadTransformer = - ExternalStoragePayloadTransformer.fromOptions( - ExternalStorageOptions.newBuilder() - .setDriver(driver) - .setPayloadSizeThreshold(threshold) - .build()); - return new ExternalStorageMessageTransformer(payloadTransformer, 4); - } - - private static Payload payload(String data) { - return Payload.newBuilder().setData(ByteString.copyFromUtf8(data)).build(); - } - - private static final class InMemoryDriver implements StorageDriver { - private final String name; - private final Map objects = new HashMap<>(); - final List storeBatchSizes = new ArrayList<>(); - private int counter = 0; - - InMemoryDriver(String name) { - this.name = name; - } - - @Override - public String getName() { - return name; - } - - @Override - public String getType() { - return "test.inmemory"; - } - - @Override - public synchronized CompletableFuture> store( - StorageDriverStoreContext context, List payloads) { - storeBatchSizes.add(payloads.size()); - List claims = new ArrayList<>(); - for (Payload payload : payloads) { - String key = name + "-" + (counter++); - objects.put(key, payload); - claims.add(new StorageDriverClaim(Collections.singletonMap("key", key))); - } - return CompletableFuture.completedFuture(claims); - } - - @Override - public synchronized CompletableFuture> retrieve( - StorageDriverRetrieveContext context, List claims) { - List payloads = new ArrayList<>(); - for (StorageDriverClaim claim : claims) { - payloads.add(objects.get(claim.getClaimData().get("key"))); - } - return CompletableFuture.completedFuture(payloads); - } - } -} diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageTest.java new file mode 100644 index 0000000000..44576a87f8 --- /dev/null +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageTest.java @@ -0,0 +1,323 @@ +package io.temporal.internal.payload.storage; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import com.google.protobuf.ByteString; +import io.temporal.api.command.v1.Command; +import io.temporal.api.command.v1.CompleteWorkflowExecutionCommandAttributes; +import io.temporal.api.command.v1.ScheduleActivityTaskCommandAttributes; +import io.temporal.api.command.v1.ScheduleActivityTaskCommandAttributesOrBuilder; +import io.temporal.api.command.v1.StartChildWorkflowExecutionCommandAttributes; +import io.temporal.api.common.v1.ActivityType; +import io.temporal.api.common.v1.Payload; +import io.temporal.api.common.v1.Payloads; +import io.temporal.api.common.v1.SearchAttributes; +import io.temporal.api.workflowservice.v1.RespondWorkflowTaskCompletedRequest; +import io.temporal.common.CancellationToken; +import io.temporal.internal.concurrent.structured.CancelSource; +import io.temporal.internal.payload.visitor.MessageVisitor; +import io.temporal.payload.storage.ExternalStorageOptions; +import io.temporal.payload.storage.StorageDriver; +import io.temporal.payload.storage.StorageDriverActivityInfo; +import io.temporal.payload.storage.StorageDriverClaim; +import io.temporal.payload.storage.StorageDriverRetrieveContext; +import io.temporal.payload.storage.StorageDriverStoreContext; +import io.temporal.payload.storage.StorageDriverTargetInfo; +import io.temporal.payload.storage.StorageDriverWorkflowInfo; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CancellationException; +import java.util.concurrent.CompletableFuture; +import org.junit.Test; + +/** Tests external storage message conversion. */ +public class ExternalStorageTest { + + @Test + public void storeAndRetrieveRoundTripsOverAMessage() throws Exception { + InMemoryDriver driver = new InMemoryDriver("d1"); + ExternalStorage transformer = transformer(driver, 0); + Payloads message = + Payloads.newBuilder().addPayloads(payload("a")).addPayloads(payload("b")).build(); + + Payloads stored = transformer.store(message, null, CancellationToken.none()).get(); + + assertNotNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(0))); + assertNotNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(1))); + + Payloads retrieved = transformer.retrieve(stored, CancellationToken.none()).get(); + assertEquals(message, retrieved); + } + + @Test + public void walksNestedPayloads() throws Exception { + InMemoryDriver driver = new InMemoryDriver("d1"); + ExternalStorage transformer = transformer(driver, 0); + Command command = + Command.newBuilder() + .setScheduleActivityTaskCommandAttributes( + ScheduleActivityTaskCommandAttributes.newBuilder() + .setInput(Payloads.newBuilder().addPayloads(payload("deep")))) + .build(); + + Command stored = transformer.store(command, null, CancellationToken.none()).get(); + + Payload nested = stored.getScheduleActivityTaskCommandAttributes().getInput().getPayloads(0); + assertNotNull(ExternalStorageReferences.tryParseReference(nested)); + assertEquals(command, transformer.retrieve(stored, CancellationToken.none()).get()); + } + + @Test + public void payloadBelowThresholdLeavesMessageUnchanged() throws Exception { + InMemoryDriver driver = new InMemoryDriver("d1"); + ExternalStorage transformer = transformer(driver, 1024); + Payloads message = Payloads.newBuilder().addPayloads(payload("small")).build(); + + Payloads stored = transformer.store(message, null, CancellationToken.none()).get(); + + assertNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(0))); + assertEquals(message, stored); + assertTrue(driver.storeBatchSizes.isEmpty()); + } + + @Test + public void searchAttributesAreNotOffloaded() throws Exception { + InMemoryDriver driver = new InMemoryDriver("d1"); + ExternalStorage transformer = transformer(driver, 0); + Command command = + Command.newBuilder() + .setStartChildWorkflowExecutionCommandAttributes( + StartChildWorkflowExecutionCommandAttributes.newBuilder() + .setInput(Payloads.newBuilder().addPayloads(payload("input"))) + .setSearchAttributes( + SearchAttributes.newBuilder() + .putIndexedFields("k", payload("indexed-value")))) + .build(); + + Command stored = transformer.store(command, null, CancellationToken.none()).get(); + + StartChildWorkflowExecutionCommandAttributes attrs = + stored.getStartChildWorkflowExecutionCommandAttributes(); + assertNotNull(ExternalStorageReferences.tryParseReference(attrs.getInput().getPayloads(0))); + Payload indexed = attrs.getSearchAttributes().getIndexedFieldsOrThrow("k"); + assertNull(ExternalStorageReferences.tryParseReference(indexed)); + assertEquals(payload("indexed-value"), indexed); + } + + @Test + public void throwIfContainsReferenceThrowsOnReference() throws Exception { + InMemoryDriver driver = new InMemoryDriver("d1"); + ExternalStorage transformer = transformer(driver, 0); + Payloads stored = + transformer + .store( + Payloads.newBuilder().addPayloads(payload("a")).build(), + null, + CancellationToken.none()) + .get(); + + ExternalStorageNotConfiguredException e = + assertThrows( + ExternalStorageNotConfiguredException.class, + () -> ExternalStorage.throwIfContainsReference(stored)); + assertTrue(e.getMessage(), e.getMessage().contains("[TMPRL1105]")); + } + + @Test + public void throwIfContainsReferenceAllowsInlinePayloads() { + Payloads inline = Payloads.newBuilder().addPayloads(payload("a")).build(); + ExternalStorage.throwIfContainsReference(inline); + } + + @Test + public void storeAppliesPerCommandTargetFromMessageVisitor() { + TargetCapturingDriver driver = new TargetCapturingDriver("d1"); + ExternalStorage storage = transformer(driver, 0); + + RespondWorkflowTaskCompletedRequest request = + RespondWorkflowTaskCompletedRequest.newBuilder() + .addCommands( + Command.newBuilder() + .setScheduleActivityTaskCommandAttributes( + ScheduleActivityTaskCommandAttributes.newBuilder() + .setActivityId("act-1") + .setActivityType(ActivityType.newBuilder().setName("MyActivity")) + .setInput( + Payloads.newBuilder().addPayloads(payload("activity-input"))))) + .addCommands( + Command.newBuilder() + .setCompleteWorkflowExecutionCommandAttributes( + CompleteWorkflowExecutionCommandAttributes.newBuilder() + .setResult(Payloads.newBuilder().addPayloads(payload("wf-result"))))) + .build(); + + StorageDriverTargetInfo workflowTarget = + new StorageDriverWorkflowInfo("ns", "wf-1", "run-1", "MyWorkflow"); + MessageVisitor visitor = + (current, message) -> { + if (message instanceof ScheduleActivityTaskCommandAttributesOrBuilder) { + ScheduleActivityTaskCommandAttributesOrBuilder attrs = + (ScheduleActivityTaskCommandAttributesOrBuilder) message; + return new StorageDriverActivityInfo( + "ns", attrs.getActivityId(), null, attrs.getActivityType().getName()); + } + return current; + }; + + storage.storeBlocking(request, workflowTarget, visitor); + + assertEquals( + new StorageDriverActivityInfo("ns", "act-1", null, "MyActivity"), + driver.targetFor("activity-input")); + assertEquals(workflowTarget, driver.targetFor("wf-result")); + } + + @Test + public void callerCancellationAbortsStore() { + ExternalStorage storage = transformer(new HangingDriver("d1"), 0); + CancelSource caller = new CancelSource<>(CancellationException::new); + caller.cancel(); + Payloads message = Payloads.newBuilder().addPayloads(payload("big")).build(); + + assertThrows( + CancellationException.class, () -> storage.storeBlocking(message, null, caller.token())); + } + + private static ExternalStorage transformer(StorageDriver driver, int threshold) { + ExternalStoragePayloadTransformer payloadTransformer = + ExternalStoragePayloadTransformer.fromOptions( + ExternalStorageOptions.newBuilder() + .setDriver(driver) + .setPayloadSizeThreshold(threshold) + .build()); + return new ExternalStorage(payloadTransformer, 4); + } + + private static Payload payload(String data) { + return Payload.newBuilder().setData(ByteString.copyFromUtf8(data)).build(); + } + + private static final class InMemoryDriver implements StorageDriver { + private final String name; + private final Map objects = new HashMap<>(); + final List storeBatchSizes = new ArrayList<>(); + private int counter = 0; + + InMemoryDriver(String name) { + this.name = name; + } + + @Override + public String getName() { + return name; + } + + @Override + public String getType() { + return "test.inmemory"; + } + + @Override + public synchronized CompletableFuture> store( + StorageDriverStoreContext context, List payloads) { + storeBatchSizes.add(payloads.size()); + List claims = new ArrayList<>(); + for (Payload payload : payloads) { + String key = name + "-" + (counter++); + objects.put(key, payload); + claims.add(new StorageDriverClaim(Collections.singletonMap("key", key))); + } + return CompletableFuture.completedFuture(claims); + } + + @Override + public synchronized CompletableFuture> retrieve( + StorageDriverRetrieveContext context, List claims) { + List payloads = new ArrayList<>(); + for (StorageDriverClaim claim : claims) { + payloads.add(objects.get(claim.getClaimData().get("key"))); + } + return CompletableFuture.completedFuture(payloads); + } + } + + private static final class TargetCapturingDriver implements StorageDriver { + private final String name; + private final Map targetByData = new HashMap<>(); + private int counter = 0; + + TargetCapturingDriver(String name) { + this.name = name; + } + + @Override + public String getName() { + return name; + } + + @Override + public String getType() { + return "test.capture"; + } + + @Override + public synchronized CompletableFuture> store( + StorageDriverStoreContext context, List payloads) { + List claims = new ArrayList<>(); + for (Payload payload : payloads) { + targetByData.put(payload.getData().toStringUtf8(), context.getTarget()); + claims.add( + new StorageDriverClaim(Collections.singletonMap("key", name + "-" + (counter++)))); + } + return CompletableFuture.completedFuture(claims); + } + + synchronized StorageDriverTargetInfo targetFor(String data) { + return targetByData.get(data); + } + + @Override + public CompletableFuture> retrieve( + StorageDriverRetrieveContext context, List claims) { + throw new UnsupportedOperationException(); + } + } + + /** Driver whose operations never settle, so only cancellation can end a blocking call. */ + private static final class HangingDriver implements StorageDriver { + private final String name; + + HangingDriver(String name) { + this.name = name; + } + + @Override + public String getName() { + return name; + } + + @Override + public String getType() { + return "test.hanging"; + } + + @Override + public CompletableFuture> store( + StorageDriverStoreContext context, List payloads) { + return new CompletableFuture<>(); + } + + @Override + public CompletableFuture> retrieve( + StorageDriverRetrieveContext context, List claims) { + return new CompletableFuture<>(); + } + } +} diff --git a/temporal-sdk/src/test/java/io/temporal/payload/storage/ExternalStorageOptionsTest.java b/temporal-sdk/src/test/java/io/temporal/payload/storage/ExternalStorageOptionsTest.java index 2c7ffc782f..b932986ad6 100644 --- a/temporal-sdk/src/test/java/io/temporal/payload/storage/ExternalStorageOptionsTest.java +++ b/temporal-sdk/src/test/java/io/temporal/payload/storage/ExternalStorageOptionsTest.java @@ -118,4 +118,22 @@ public void negativeThresholdRejected() { .setPayloadSizeThreshold(-1) .build(); } + + @Test + public void maxConcurrentPayloadVisitsDefaultsToThree() { + assertEquals( + 3, + ExternalStorageOptions.newBuilder() + .setDriver(driver("a")) + .build() + .getMaxConcurrentPayloadVisits()); + } + + @Test(expected = IllegalStateException.class) + public void zeroMaxConcurrentPayloadVisitsRejected() { + ExternalStorageOptions.newBuilder() + .setDriver(driver("a")) + .setMaxConcurrentPayloadVisits(0) + .build(); + } } From ca09b5081e5ec7f86eefc3f574c7b19d58275233 Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Fri, 21 Aug 2026 16:34:50 -0400 Subject: [PATCH 02/23] refactor(extstore): move external storage to data converter and rename to match other sdks. --- .../common/converter/CodecDataConverter.java | 7 ++ .../common/converter/DataConverter.java | 12 ++++ .../converter/DefaultDataConverter.java | 11 +++ .../PayloadAndFailureDataConverter.java | 13 +++- .../ExternalStoragePayloadTransformer.java | 4 +- ...torage.java => ExternalStorageRunner.java} | 66 +++++------------- .../internal/worker/SingleWorkerOptions.java | 12 ++-- ...orageOptions.java => ExternalStorage.java} | 13 ++-- .../payload/storage/StorageDriver.java | 4 +- .../storage/StorageDriverSelector.java | 2 +- ...ExternalStoragePayloadTransformerTest.java | 17 ++--- ...st.java => ExternalStorageRunnerTest.java} | 68 ++++++++++--------- ...ionsTest.java => ExternalStorageTest.java} | 33 ++++----- 13 files changed, 133 insertions(+), 129 deletions(-) rename temporal-sdk/src/main/java/io/temporal/internal/payload/storage/{ExternalStorage.java => ExternalStorageRunner.java} (68%) rename temporal-sdk/src/main/java/io/temporal/payload/storage/{ExternalStorageOptions.java => ExternalStorage.java} (92%) rename temporal-sdk/src/test/java/io/temporal/internal/payload/storage/{ExternalStorageTest.java => ExternalStorageRunnerTest.java} (84%) rename temporal-sdk/src/test/java/io/temporal/payload/storage/{ExternalStorageOptionsTest.java => ExternalStorageTest.java} (79%) diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/CodecDataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/CodecDataConverter.java index a82172348b..3fe83a3d0b 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/CodecDataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/CodecDataConverter.java @@ -12,6 +12,7 @@ import io.temporal.payload.codec.ChainCodec; import io.temporal.payload.codec.PayloadCodec; import io.temporal.payload.context.SerializationContext; +import io.temporal.payload.storage.ExternalStorage; import java.lang.reflect.Type; import java.util.Collection; import java.util.Collections; @@ -190,6 +191,12 @@ public CodecDataConverter withContext(@Nonnull SerializationContext context) { return new CodecDataConverter(dataConverter, chainCodec, encodeFailureAttributes, context); } + @Override + @Nullable + public ExternalStorage getExternalStorage() { + return dataConverter.getExternalStorage(); + } + @Nonnull @Override public List encode(@Nonnull List payloads) { diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/DataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/DataConverter.java index decf6181b0..e299a3c2b7 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/DataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/DataConverter.java @@ -11,10 +11,12 @@ import io.temporal.failure.DefaultFailureConverter; import io.temporal.payload.codec.PayloadCodec; import io.temporal.payload.context.SerializationContext; +import io.temporal.payload.storage.ExternalStorage; import java.lang.reflect.Type; import java.util.Arrays; import java.util.Optional; import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** * Used by the framework to serialize/deserialize method parameters that need to be sent over the @@ -202,6 +204,16 @@ default DataConverter withContext(@Nonnull SerializationContext context) { return this; } + /** + * External storage offloads large payloads. This should not be used from inside workflow code as + * it performs nondeterministic operations. + */ + @Experimental + @Nullable + default ExternalStorage getExternalStorage() { + return null; + } + /** * @deprecated use {@link DataConverter#fromPayloads(int, Optional, Class, Type)}. This is an SDK * implementation detail and never was expected to be exposed to users. diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/DefaultDataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/DefaultDataConverter.java index f05c3f95a9..2df6393f76 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/DefaultDataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/DefaultDataConverter.java @@ -1,8 +1,10 @@ package io.temporal.common.converter; import com.google.common.base.Preconditions; +import io.temporal.payload.storage.ExternalStorage; import java.util.*; import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** * A {@link DataConverter} that delegates payload conversion to type specific {@link @@ -101,4 +103,13 @@ public DefaultDataConverter withFailureConverter(@Nonnull FailureConverter failu this.failureConverter = Preconditions.checkNotNull(failureConverter, "failureConverter"); return this; } + + /** + * Modifies this {@code DefaultDataConverter} by attaching external storage, used to offload and + * restore large payloads at task and RPC boundaries. + */ + public DefaultDataConverter withExternalStorage(@Nullable ExternalStorage externalStorage) { + this.externalStorage = externalStorage; + return this; + } } diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java index a628b4118b..e848e5d9db 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java @@ -11,6 +11,7 @@ import io.temporal.internal.payload.storage.ExternalStorageNotConfiguredException; import io.temporal.internal.payload.storage.ExternalStorageReferences; import io.temporal.payload.context.SerializationContext; +import io.temporal.payload.storage.ExternalStorage; import java.lang.reflect.Type; import java.util.*; import javax.annotation.Nonnull; @@ -24,6 +25,7 @@ class PayloadAndFailureDataConverter implements DataConverter { volatile List converters; volatile Map convertersMap; volatile FailureConverter failureConverter; + volatile @Nullable ExternalStorage externalStorage; private final @Nullable SerializationContext serializationContext; public PayloadAndFailureDataConverter(@Nonnull List converters) { @@ -149,9 +151,18 @@ public Failure exceptionToFailure(@Nonnull Throwable throwable) { .exceptionToFailure(throwable, this); } + @Override + @Nullable + public ExternalStorage getExternalStorage() { + return externalStorage; + } + @Override public @Nonnull DataConverter withContext(@Nonnull SerializationContext context) { - return new PayloadAndFailureDataConverter(converters, convertersMap, failureConverter, context); + PayloadAndFailureDataConverter copy = + new PayloadAndFailureDataConverter(converters, convertersMap, failureConverter, context); + copy.externalStorage = this.externalStorage; + return copy; } static Map createConvertersMap(List converters) { diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStoragePayloadTransformer.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStoragePayloadTransformer.java index 6e0d4d770c..ef13075f9e 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStoragePayloadTransformer.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStoragePayloadTransformer.java @@ -4,7 +4,7 @@ import io.temporal.common.CancellationToken; import io.temporal.internal.common.ListUtils; import io.temporal.internal.concurrent.structured.TaskScope; -import io.temporal.payload.storage.ExternalStorageOptions; +import io.temporal.payload.storage.ExternalStorage; import io.temporal.payload.storage.StorageDriver; import io.temporal.payload.storage.StorageDriverClaim; import io.temporal.payload.storage.StorageDriverRetrieveContext; @@ -30,7 +30,7 @@ final class ExternalStoragePayloadTransformer { private final StorageDriverSelector selector; private final int payloadSizeThreshold; - static ExternalStoragePayloadTransformer fromOptions(ExternalStorageOptions options) { + static ExternalStoragePayloadTransformer fromOptions(ExternalStorage options) { Map driversByName = new LinkedHashMap<>(); for (StorageDriver driver : options.getDrivers()) { driversByName.put(driver.getName(), driver); diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorage.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java similarity index 68% rename from temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorage.java rename to temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java index ec225fc837..3f1023cc3e 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorage.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java @@ -8,7 +8,7 @@ import io.temporal.internal.payload.visitor.MessageVisitor; import io.temporal.internal.payload.visitor.PayloadVisitorOptions; import io.temporal.internal.payload.visitor.PayloadVisitors; -import io.temporal.payload.storage.ExternalStorageOptions; +import io.temporal.payload.storage.ExternalStorage; import io.temporal.payload.storage.StorageDriver; import io.temporal.payload.storage.StorageDriverTargetInfo; import java.util.List; @@ -21,53 +21,45 @@ /** * External storage offloads large payloads via {@link StorageDriver}s. It walks messages using * {@link PayloadVisitors} transforming payloads to and from {@link ExternalStorageReference} using - * {@link ExternalStoragePayloadTransformer}. Use {@link ExternalStorageOptions} via {@link#create} - * to configure external storage. + * {@link ExternalStoragePayloadTransformer}. Use {@link ExternalStorage} via {@link#create} to + * configure external storage. */ -public final class ExternalStorage { +public final class ExternalStorageRunner { private final ExternalStoragePayloadTransformer payloadTransformer; private final int payloadVisitConcurrency; - public static ExternalStorage create(ExternalStorageOptions options) { - return new ExternalStorage( + public static ExternalStorageRunner create(ExternalStorage options) { + return new ExternalStorageRunner( ExternalStoragePayloadTransformer.fromOptions(options), options.getMaxConcurrentPayloadVisits()); } - ExternalStorage( + ExternalStorageRunner( ExternalStoragePayloadTransformer payloadTransformer, int payloadVisitConcurrency) { this.payloadTransformer = payloadTransformer; this.payloadVisitConcurrency = payloadVisitConcurrency; } - public T storeBlocking(T message, @Nullable StorageDriverTargetInfo target) { - return storeBlocking(message, target, CancellationToken.none()); + public void store(Message.Builder builder, @Nullable StorageDriverTargetInfo target) { + store(builder, target, null, CancellationToken.none()); } - public T storeBlocking( - T message, + public void store( + Message.Builder builder, @Nullable StorageDriverTargetInfo target, + @Nullable MessageVisitor targetVisitor, CancellationToken cancellationToken) { - return getOrThrowIfCancelled(store(message, target, cancellationToken), cancellationToken); - } - - public T storeBlocking( - T message, - @Nullable StorageDriverTargetInfo target, - @Nullable MessageVisitor targetVisitor) { - CancellationToken cancellationToken = CancellationToken.none(); - return getOrThrowIfCancelled( - PayloadVisitors.visit(message, storeOptions(target, targetVisitor, cancellationToken)), + getOrThrowIfCancelled( + PayloadVisitors.visit(builder, storeOptions(target, targetVisitor, cancellationToken)), cancellationToken); } - public T retrieveBlocking(T message) { - CancellationToken cancellationToken = CancellationToken.none(); - return getOrThrowIfCancelled(retrieve(message, cancellationToken), cancellationToken); + public T retrieve(T message) { + return getOrThrowIfCancelled(retrieveAsync(message), CancellationToken.none()); } public CompletableFuture retrieveAsync(T message) { - return retrieve(message, CancellationToken.none()); + return PayloadVisitors.visit(message, retrieveOptions(CancellationToken.none())); } /** @@ -113,30 +105,6 @@ private static T getOrThrowIfCancelled( } } - CompletableFuture store( - T message, - @Nullable StorageDriverTargetInfo target, - CancellationToken cancellationToken) { - return PayloadVisitors.visit(message, storeOptions(target, null, cancellationToken)); - } - - CompletableFuture store( - Message.Builder builder, - @Nullable StorageDriverTargetInfo target, - CancellationToken cancellationToken) { - return PayloadVisitors.visit(builder, storeOptions(target, null, cancellationToken)); - } - - CompletableFuture retrieve( - T message, CancellationToken cancellationToken) { - return PayloadVisitors.visit(message, retrieveOptions(cancellationToken)); - } - - CompletableFuture retrieve( - Message.Builder builder, CancellationToken cancellationToken) { - return PayloadVisitors.visit(builder, retrieveOptions(cancellationToken)); - } - private PayloadVisitorOptions storeOptions( @Nullable StorageDriverTargetInfo target, @Nullable MessageVisitor targetVisitor, diff --git a/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java b/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java index 9b1d055110..23eb93625a 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java @@ -7,7 +7,7 @@ import io.temporal.common.converter.DataConverter; import io.temporal.common.converter.GlobalDataConverter; import io.temporal.common.interceptors.WorkerInterceptor; -import io.temporal.internal.payload.storage.ExternalStorage; +import io.temporal.internal.payload.storage.ExternalStorageRunner; import io.temporal.worker.PreferredVersionProvider; import io.temporal.worker.WorkerDeploymentOptions; import java.time.Duration; @@ -47,7 +47,7 @@ public static final class Builder { private boolean allowActivityHeartbeatDuringShutdown; private String workerControlTaskQueue; private PreferredVersionProvider preferredVersionProvider; - private @Nullable ExternalStorage externalStorage; + private @Nullable ExternalStorageRunner externalStorage; private Builder() {} @@ -189,7 +189,7 @@ public Builder setPreferredVersionProvider(PreferredVersionProvider preferredVer return this; } - public Builder setExternalStorage(@Nullable ExternalStorage externalStorage) { + public Builder setExternalStorage(@Nullable ExternalStorageRunner externalStorage) { this.externalStorage = externalStorage; return this; } @@ -262,7 +262,7 @@ public SingleWorkerOptions build() { private final boolean allowActivityHeartbeatDuringShutdown; private final String workerControlTaskQueue; private final PreferredVersionProvider preferredVersionProvider; - private final @Nullable ExternalStorage externalStorage; + private final @Nullable ExternalStorageRunner externalStorage; private SingleWorkerOptions( String identity, @@ -286,7 +286,7 @@ private SingleWorkerOptions( boolean allowActivityHeartbeatDuringShutdown, String workerControlTaskQueue, PreferredVersionProvider preferredVersionProvider, - @Nullable ExternalStorage externalStorage) { + @Nullable ExternalStorageRunner externalStorage) { this.identity = identity; this.binaryChecksum = binaryChecksum; this.buildId = buildId; @@ -408,7 +408,7 @@ public PreferredVersionProvider getPreferredVersionProvider() { /** The external-storage message transformer for this worker, or null when disabled. */ @Nullable - public ExternalStorage getExternalStorage() { + public ExternalStorageRunner getExternalStorage() { return externalStorage; } diff --git a/temporal-sdk/src/main/java/io/temporal/payload/storage/ExternalStorageOptions.java b/temporal-sdk/src/main/java/io/temporal/payload/storage/ExternalStorage.java similarity index 92% rename from temporal-sdk/src/main/java/io/temporal/payload/storage/ExternalStorageOptions.java rename to temporal-sdk/src/main/java/io/temporal/payload/storage/ExternalStorage.java index 3abc969291..854254ef04 100644 --- a/temporal-sdk/src/main/java/io/temporal/payload/storage/ExternalStorageOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/payload/storage/ExternalStorage.java @@ -13,7 +13,7 @@ /** Configuration for offloading large payloads to external storage. */ @Experimental -public final class ExternalStorageOptions { +public final class ExternalStorage { static final int DEFAULT_PAYLOAD_SIZE_THRESHOLD = 256 * 1024; static final int DEFAULT_MAX_CONCURRENT_PAYLOAD_VISITS = 3; @@ -26,7 +26,7 @@ public static Builder newBuilder() { private final int payloadSizeThreshold; private final int maxConcurrentPayloadVisits; - private ExternalStorageOptions( + private ExternalStorage( @Nonnull List drivers, @Nonnull StorageDriverSelector driverSelector, int payloadSizeThreshold, @@ -66,9 +66,8 @@ public int getMaxConcurrentPayloadVisits() { public static final class Builder { private List drivers = Collections.emptyList(); private StorageDriverSelector driverSelector; - private int payloadSizeThreshold = ExternalStorageOptions.DEFAULT_PAYLOAD_SIZE_THRESHOLD; - private int maxConcurrentPayloadVisits = - ExternalStorageOptions.DEFAULT_MAX_CONCURRENT_PAYLOAD_VISITS; + private int payloadSizeThreshold = ExternalStorage.DEFAULT_PAYLOAD_SIZE_THRESHOLD; + private int maxConcurrentPayloadVisits = ExternalStorage.DEFAULT_MAX_CONCURRENT_PAYLOAD_VISITS; private Builder() {} @@ -107,7 +106,7 @@ public Builder setMaxConcurrentPayloadVisits(int maxConcurrentPayloadVisits) { return this; } - public ExternalStorageOptions build() { + public ExternalStorage build() { Preconditions.checkState(!drivers.isEmpty(), "At least one driver must be provided"); Preconditions.checkState( payloadSizeThreshold >= 0, "payloadSizeThreshold must be greater than or equal to zero"); @@ -127,7 +126,7 @@ public ExternalStorageOptions build() { StorageDriver driver = drivers.get(0); selector = (context, payload) -> driver; } - return new ExternalStorageOptions( + return new ExternalStorage( drivers, selector, payloadSizeThreshold, maxConcurrentPayloadVisits); } } diff --git a/temporal-sdk/src/main/java/io/temporal/payload/storage/StorageDriver.java b/temporal-sdk/src/main/java/io/temporal/payload/storage/StorageDriver.java index 01d851fbe6..bf332c38d7 100644 --- a/temporal-sdk/src/main/java/io/temporal/payload/storage/StorageDriver.java +++ b/temporal-sdk/src/main/java/io/temporal/payload/storage/StorageDriver.java @@ -11,8 +11,8 @@ public interface StorageDriver { /** * Name of this driver instance, unique among the drivers registered in a single {@link - * ExternalStorageOptions}. Used as the routing key recorded in a stored payload's reference and - * resolved back to this driver on retrieval. + * ExternalStorage}. Used as the routing key recorded in a stored payload's reference and resolved + * back to this driver on retrieval. */ @Nonnull String getName(); diff --git a/temporal-sdk/src/main/java/io/temporal/payload/storage/StorageDriverSelector.java b/temporal-sdk/src/main/java/io/temporal/payload/storage/StorageDriverSelector.java index 431622e2fa..966e52e68d 100644 --- a/temporal-sdk/src/main/java/io/temporal/payload/storage/StorageDriverSelector.java +++ b/temporal-sdk/src/main/java/io/temporal/payload/storage/StorageDriverSelector.java @@ -11,7 +11,7 @@ public interface StorageDriverSelector { /** * Returns the driver to store {@code payload}, which must be one of the drivers registered in the - * {@link ExternalStorageOptions}, or {@code null} to leave the payload stored inline. + * {@link ExternalStorage}, or {@code null} to leave the payload stored inline. */ @Nullable StorageDriver selectDriver(@Nonnull StorageDriverStoreContext context, @Nonnull Payload payload); diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStoragePayloadTransformerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStoragePayloadTransformerTest.java index f1632ca81e..2dcb58f384 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStoragePayloadTransformerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStoragePayloadTransformerTest.java @@ -12,7 +12,7 @@ import io.temporal.api.common.v1.Payload; import io.temporal.common.CancellationToken; import io.temporal.internal.concurrent.structured.CancelSource; -import io.temporal.payload.storage.ExternalStorageOptions; +import io.temporal.payload.storage.ExternalStorage; import io.temporal.payload.storage.StorageDriver; import io.temporal.payload.storage.StorageDriverClaim; import io.temporal.payload.storage.StorageDriverRetrieveContext; @@ -75,7 +75,7 @@ public void selectorReturningNullKeepsInline() throws Exception { InMemoryDriver driver = new InMemoryDriver("d1"); ExternalStoragePayloadTransformer transformer = ExternalStoragePayloadTransformer.fromOptions( - ExternalStorageOptions.newBuilder() + ExternalStorage.newBuilder() .setDriver(driver) .setDriverSelector((context, payload) -> null) .setPayloadSizeThreshold(0) @@ -101,7 +101,7 @@ public void multipleDriversBatchPerDriverAndPreserveOrder() throws Exception { (context, payload) -> byPrefix.get(payload.getData().toStringUtf8().substring(0, 1)); ExternalStoragePayloadTransformer transformer = ExternalStoragePayloadTransformer.fromOptions( - ExternalStorageOptions.newBuilder() + ExternalStorage.newBuilder() .setDrivers(Arrays.asList(d1, d2)) .setDriverSelector(selector) .setPayloadSizeThreshold(0) @@ -198,7 +198,7 @@ public void selectorReturningUnregisteredDriverFails() { InMemoryDriver stranger = new InMemoryDriver("d2"); ExternalStoragePayloadTransformer transformer = ExternalStoragePayloadTransformer.fromOptions( - ExternalStorageOptions.newBuilder() + ExternalStorage.newBuilder() .setDriver(registered) .setDriverSelector((context, payload) -> stranger) .setPayloadSizeThreshold(0) @@ -232,7 +232,7 @@ public CompletableFuture> store( byPrefix.put("2", doomed); ExternalStoragePayloadTransformer transformer = ExternalStoragePayloadTransformer.fromOptions( - ExternalStorageOptions.newBuilder() + ExternalStorage.newBuilder() .setDrivers(Arrays.asList(slow, doomed)) .setDriverSelector( (context, payload) -> @@ -314,7 +314,7 @@ public void selectorObservesCallerCancellationToken() { AtomicReference> observed = new AtomicReference<>(); ExternalStoragePayloadTransformer transformer = ExternalStoragePayloadTransformer.fromOptions( - ExternalStorageOptions.newBuilder() + ExternalStorage.newBuilder() .setDriver(driver) .setDriverSelector( (context, payload) -> { @@ -332,10 +332,7 @@ public void selectorObservesCallerCancellationToken() { private static ExternalStoragePayloadTransformer transformer( StorageDriver driver, int threshold) { return ExternalStoragePayloadTransformer.fromOptions( - ExternalStorageOptions.newBuilder() - .setDriver(driver) - .setPayloadSizeThreshold(threshold) - .build()); + ExternalStorage.newBuilder().setDriver(driver).setPayloadSizeThreshold(threshold).build()); } private static Payload payload(String data) { diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java similarity index 84% rename from temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageTest.java rename to temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java index 44576a87f8..643dc3436c 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java @@ -20,7 +20,7 @@ import io.temporal.common.CancellationToken; import io.temporal.internal.concurrent.structured.CancelSource; import io.temporal.internal.payload.visitor.MessageVisitor; -import io.temporal.payload.storage.ExternalStorageOptions; +import io.temporal.payload.storage.ExternalStorage; import io.temporal.payload.storage.StorageDriver; import io.temporal.payload.storage.StorageDriverActivityInfo; import io.temporal.payload.storage.StorageDriverClaim; @@ -38,28 +38,30 @@ import org.junit.Test; /** Tests external storage message conversion. */ -public class ExternalStorageTest { +public class ExternalStorageRunnerTest { @Test public void storeAndRetrieveRoundTripsOverAMessage() throws Exception { InMemoryDriver driver = new InMemoryDriver("d1"); - ExternalStorage transformer = transformer(driver, 0); + ExternalStorageRunner transformer = transformer(driver, 0); Payloads message = Payloads.newBuilder().addPayloads(payload("a")).addPayloads(payload("b")).build(); - Payloads stored = transformer.store(message, null, CancellationToken.none()).get(); + Payloads.Builder builder = message.toBuilder(); + transformer.store(builder, null); + Payloads stored = builder.build(); assertNotNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(0))); assertNotNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(1))); - Payloads retrieved = transformer.retrieve(stored, CancellationToken.none()).get(); + Payloads retrieved = transformer.retrieve(stored); assertEquals(message, retrieved); } @Test public void walksNestedPayloads() throws Exception { InMemoryDriver driver = new InMemoryDriver("d1"); - ExternalStorage transformer = transformer(driver, 0); + ExternalStorageRunner transformer = transformer(driver, 0); Command command = Command.newBuilder() .setScheduleActivityTaskCommandAttributes( @@ -67,20 +69,24 @@ public void walksNestedPayloads() throws Exception { .setInput(Payloads.newBuilder().addPayloads(payload("deep")))) .build(); - Command stored = transformer.store(command, null, CancellationToken.none()).get(); + Command.Builder builder = command.toBuilder(); + transformer.store(builder, null); + Command stored = builder.build(); Payload nested = stored.getScheduleActivityTaskCommandAttributes().getInput().getPayloads(0); assertNotNull(ExternalStorageReferences.tryParseReference(nested)); - assertEquals(command, transformer.retrieve(stored, CancellationToken.none()).get()); + assertEquals(command, transformer.retrieve(stored)); } @Test public void payloadBelowThresholdLeavesMessageUnchanged() throws Exception { InMemoryDriver driver = new InMemoryDriver("d1"); - ExternalStorage transformer = transformer(driver, 1024); + ExternalStorageRunner transformer = transformer(driver, 1024); Payloads message = Payloads.newBuilder().addPayloads(payload("small")).build(); - Payloads stored = transformer.store(message, null, CancellationToken.none()).get(); + Payloads.Builder builder = message.toBuilder(); + transformer.store(builder, null); + Payloads stored = builder.build(); assertNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(0))); assertEquals(message, stored); @@ -90,7 +96,7 @@ public void payloadBelowThresholdLeavesMessageUnchanged() throws Exception { @Test public void searchAttributesAreNotOffloaded() throws Exception { InMemoryDriver driver = new InMemoryDriver("d1"); - ExternalStorage transformer = transformer(driver, 0); + ExternalStorageRunner transformer = transformer(driver, 0); Command command = Command.newBuilder() .setStartChildWorkflowExecutionCommandAttributes( @@ -101,7 +107,9 @@ public void searchAttributesAreNotOffloaded() throws Exception { .putIndexedFields("k", payload("indexed-value")))) .build(); - Command stored = transformer.store(command, null, CancellationToken.none()).get(); + Command.Builder builder = command.toBuilder(); + transformer.store(builder, null); + Command stored = builder.build(); StartChildWorkflowExecutionCommandAttributes attrs = stored.getStartChildWorkflowExecutionCommandAttributes(); @@ -114,34 +122,30 @@ public void searchAttributesAreNotOffloaded() throws Exception { @Test public void throwIfContainsReferenceThrowsOnReference() throws Exception { InMemoryDriver driver = new InMemoryDriver("d1"); - ExternalStorage transformer = transformer(driver, 0); - Payloads stored = - transformer - .store( - Payloads.newBuilder().addPayloads(payload("a")).build(), - null, - CancellationToken.none()) - .get(); + ExternalStorageRunner transformer = transformer(driver, 0); + Payloads.Builder builder = Payloads.newBuilder().addPayloads(payload("a")); + transformer.store(builder, null); + Payloads stored = builder.build(); ExternalStorageNotConfiguredException e = assertThrows( ExternalStorageNotConfiguredException.class, - () -> ExternalStorage.throwIfContainsReference(stored)); + () -> ExternalStorageRunner.throwIfContainsReference(stored)); assertTrue(e.getMessage(), e.getMessage().contains("[TMPRL1105]")); } @Test public void throwIfContainsReferenceAllowsInlinePayloads() { Payloads inline = Payloads.newBuilder().addPayloads(payload("a")).build(); - ExternalStorage.throwIfContainsReference(inline); + ExternalStorageRunner.throwIfContainsReference(inline); } @Test public void storeAppliesPerCommandTargetFromMessageVisitor() { TargetCapturingDriver driver = new TargetCapturingDriver("d1"); - ExternalStorage storage = transformer(driver, 0); + ExternalStorageRunner storage = transformer(driver, 0); - RespondWorkflowTaskCompletedRequest request = + RespondWorkflowTaskCompletedRequest.Builder request = RespondWorkflowTaskCompletedRequest.newBuilder() .addCommands( Command.newBuilder() @@ -155,8 +159,7 @@ public void storeAppliesPerCommandTargetFromMessageVisitor() { Command.newBuilder() .setCompleteWorkflowExecutionCommandAttributes( CompleteWorkflowExecutionCommandAttributes.newBuilder() - .setResult(Payloads.newBuilder().addPayloads(payload("wf-result"))))) - .build(); + .setResult(Payloads.newBuilder().addPayloads(payload("wf-result"))))); StorageDriverTargetInfo workflowTarget = new StorageDriverWorkflowInfo("ns", "wf-1", "run-1", "MyWorkflow"); @@ -171,7 +174,7 @@ public void storeAppliesPerCommandTargetFromMessageVisitor() { return current; }; - storage.storeBlocking(request, workflowTarget, visitor); + storage.store(request, workflowTarget, visitor, CancellationToken.none()); assertEquals( new StorageDriverActivityInfo("ns", "act-1", null, "MyActivity"), @@ -181,23 +184,24 @@ public void storeAppliesPerCommandTargetFromMessageVisitor() { @Test public void callerCancellationAbortsStore() { - ExternalStorage storage = transformer(new HangingDriver("d1"), 0); + ExternalStorageRunner storage = transformer(new HangingDriver("d1"), 0); CancelSource caller = new CancelSource<>(CancellationException::new); caller.cancel(); Payloads message = Payloads.newBuilder().addPayloads(payload("big")).build(); assertThrows( - CancellationException.class, () -> storage.storeBlocking(message, null, caller.token())); + CancellationException.class, + () -> storage.store(message.toBuilder(), null, null, caller.token())); } - private static ExternalStorage transformer(StorageDriver driver, int threshold) { + private static ExternalStorageRunner transformer(StorageDriver driver, int threshold) { ExternalStoragePayloadTransformer payloadTransformer = ExternalStoragePayloadTransformer.fromOptions( - ExternalStorageOptions.newBuilder() + ExternalStorage.newBuilder() .setDriver(driver) .setPayloadSizeThreshold(threshold) .build()); - return new ExternalStorage(payloadTransformer, 4); + return new ExternalStorageRunner(payloadTransformer, 4); } private static Payload payload(String data) { diff --git a/temporal-sdk/src/test/java/io/temporal/payload/storage/ExternalStorageOptionsTest.java b/temporal-sdk/src/test/java/io/temporal/payload/storage/ExternalStorageTest.java similarity index 79% rename from temporal-sdk/src/test/java/io/temporal/payload/storage/ExternalStorageOptionsTest.java rename to temporal-sdk/src/test/java/io/temporal/payload/storage/ExternalStorageTest.java index b932986ad6..e68b13b3a0 100644 --- a/temporal-sdk/src/test/java/io/temporal/payload/storage/ExternalStorageOptionsTest.java +++ b/temporal-sdk/src/test/java/io/temporal/payload/storage/ExternalStorageTest.java @@ -12,7 +12,7 @@ import org.junit.Test; /** Tests external storage option validation and defaults. */ -public class ExternalStorageOptionsTest { +public class ExternalStorageTest { private static StorageDriverStoreContext storeContext(StorageDriverTargetInfo target) { return new StorageDriverStoreContext() { @@ -52,7 +52,7 @@ public CompletableFuture> retrieve( @Test public void singleDriverNoSelectorSynthesizesSelector() { StorageDriver a = driver("a"); - ExternalStorageOptions storage = ExternalStorageOptions.newBuilder().setDriver(a).build(); + ExternalStorage storage = ExternalStorage.newBuilder().setDriver(a).build(); assertEquals(1, storage.getDrivers().size()); StorageDriverSelector selector = storage.getDriverSelector(); assertNotNull(selector); @@ -62,8 +62,8 @@ public void singleDriverNoSelectorSynthesizesSelector() { @Test public void multipleDriversWithSelectorIsValid() { StorageDriver a = driver("a"); - ExternalStorageOptions storage = - ExternalStorageOptions.newBuilder() + ExternalStorage storage = + ExternalStorage.newBuilder() .setDrivers(Arrays.asList(a, driver("b"))) .setDriverSelector((context, payload) -> a) .build(); @@ -76,8 +76,8 @@ public void lastSetDriversWins() { StorageDriver a = driver("a"); StorageDriver b = driver("b"); StorageDriver c = driver("c"); - ExternalStorageOptions storage = - ExternalStorageOptions.newBuilder() + ExternalStorage storage = + ExternalStorage.newBuilder() .setDrivers(Arrays.asList(a, b)) .setDrivers(Collections.singletonList(c)) .build(); @@ -86,8 +86,8 @@ public void lastSetDriversWins() { @Test public void zeroThresholdStoresAll() { - ExternalStorageOptions storage = - ExternalStorageOptions.newBuilder() + ExternalStorage storage = + ExternalStorage.newBuilder() .setDrivers(Collections.singletonList(driver("a"))) .setPayloadSizeThreshold(0) .build(); @@ -96,24 +96,22 @@ public void zeroThresholdStoresAll() { @Test(expected = IllegalStateException.class) public void noDriversRejected() { - ExternalStorageOptions.newBuilder().build(); + ExternalStorage.newBuilder().build(); } @Test(expected = IllegalStateException.class) public void duplicateDriverNamesRejected() { - ExternalStorageOptions.newBuilder() - .setDrivers(Arrays.asList(driver("dup"), driver("dup"))) - .build(); + ExternalStorage.newBuilder().setDrivers(Arrays.asList(driver("dup"), driver("dup"))).build(); } @Test(expected = IllegalStateException.class) public void multipleDriversRequireSelector() { - ExternalStorageOptions.newBuilder().setDrivers(Arrays.asList(driver("a"), driver("b"))).build(); + ExternalStorage.newBuilder().setDrivers(Arrays.asList(driver("a"), driver("b"))).build(); } @Test(expected = IllegalStateException.class) public void negativeThresholdRejected() { - ExternalStorageOptions.newBuilder() + ExternalStorage.newBuilder() .setDrivers(Collections.singletonList(driver("a"))) .setPayloadSizeThreshold(-1) .build(); @@ -123,7 +121,7 @@ public void negativeThresholdRejected() { public void maxConcurrentPayloadVisitsDefaultsToThree() { assertEquals( 3, - ExternalStorageOptions.newBuilder() + ExternalStorage.newBuilder() .setDriver(driver("a")) .build() .getMaxConcurrentPayloadVisits()); @@ -131,9 +129,6 @@ public void maxConcurrentPayloadVisitsDefaultsToThree() { @Test(expected = IllegalStateException.class) public void zeroMaxConcurrentPayloadVisitsRejected() { - ExternalStorageOptions.newBuilder() - .setDriver(driver("a")) - .setMaxConcurrentPayloadVisits(0) - .build(); + ExternalStorage.newBuilder().setDriver(driver("a")).setMaxConcurrentPayloadVisits(0).build(); } } From af36622f287f02d0053c11c444a5faad0d652688 Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Mon, 24 Aug 2026 15:03:35 -0400 Subject: [PATCH 03/23] refactor(extstore): stop threading the external storage runner through to workers. we've got the dataconverter already so we can just derive it where its needed. --- .../internal/worker/SingleWorkerOptions.java | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java b/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java index 23eb93625a..39a33d99bd 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java @@ -8,6 +8,7 @@ import io.temporal.common.converter.GlobalDataConverter; import io.temporal.common.interceptors.WorkerInterceptor; import io.temporal.internal.payload.storage.ExternalStorageRunner; +import io.temporal.payload.storage.ExternalStorage; import io.temporal.worker.PreferredVersionProvider; import io.temporal.worker.WorkerDeploymentOptions; import java.time.Duration; @@ -47,7 +48,6 @@ public static final class Builder { private boolean allowActivityHeartbeatDuringShutdown; private String workerControlTaskQueue; private PreferredVersionProvider preferredVersionProvider; - private @Nullable ExternalStorageRunner externalStorage; private Builder() {} @@ -76,7 +76,6 @@ private Builder(SingleWorkerOptions options) { this.allowActivityHeartbeatDuringShutdown = options.getAllowActivityHeartbeatDuringShutdown(); this.workerControlTaskQueue = options.getWorkerControlTaskQueue(); this.preferredVersionProvider = options.getPreferredVersionProvider(); - this.externalStorage = options.getExternalStorage(); } public Builder setIdentity(String identity) { @@ -189,11 +188,6 @@ public Builder setPreferredVersionProvider(PreferredVersionProvider preferredVer return this; } - public Builder setExternalStorage(@Nullable ExternalStorageRunner externalStorage) { - this.externalStorage = externalStorage; - return this; - } - public SingleWorkerOptions build() { PollerOptions pollerOptions = this.pollerOptions; if (pollerOptions == null) { @@ -236,8 +230,7 @@ public SingleWorkerOptions build() { this.workerInstanceKey, this.allowActivityHeartbeatDuringShutdown, this.workerControlTaskQueue, - this.preferredVersionProvider, - this.externalStorage); + this.preferredVersionProvider); } } @@ -285,8 +278,7 @@ private SingleWorkerOptions( String workerInstanceKey, boolean allowActivityHeartbeatDuringShutdown, String workerControlTaskQueue, - PreferredVersionProvider preferredVersionProvider, - @Nullable ExternalStorageRunner externalStorage) { + PreferredVersionProvider preferredVersionProvider) { this.identity = identity; this.binaryChecksum = binaryChecksum; this.buildId = buildId; @@ -308,7 +300,9 @@ private SingleWorkerOptions( this.allowActivityHeartbeatDuringShutdown = allowActivityHeartbeatDuringShutdown; this.workerControlTaskQueue = workerControlTaskQueue; this.preferredVersionProvider = preferredVersionProvider; - this.externalStorage = externalStorage; + ExternalStorage externalStorageConfig = dataConverter.getExternalStorage(); + this.externalStorage = + externalStorageConfig == null ? null : ExternalStorageRunner.create(externalStorageConfig); } public String getIdentity() { @@ -406,7 +400,10 @@ public PreferredVersionProvider getPreferredVersionProvider() { return preferredVersionProvider; } - /** The external-storage message transformer for this worker, or null when disabled. */ + /** + * The external-storage runner for this worker, derived from the worker's {@link DataConverter}, + * or null when the converter has no external storage configured. + */ @Nullable public ExternalStorageRunner getExternalStorage() { return externalStorage; From 5ebc8ee14761f5917c59e19a1b1ee71e5cf3c90f Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Mon, 24 Aug 2026 15:37:11 -0400 Subject: [PATCH 04/23] Update ExternalStorageNotConfiguredException to refer to correct API for configuring external storage. --- .../storage/ExternalStorageNotConfiguredException.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java index 441e41851a..3bd67e40a6 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java @@ -10,7 +10,8 @@ public final class ExternalStorageNotConfiguredException extends DataConverterEx public ExternalStorageNotConfiguredException() { super( "[TMPRL1105] Encountered an external-storage reference payload but external storage is not " - + "configured. Configure WorkflowClientOptions.Builder.setExternalStorage(...) with a " - + "driver able to retrieve it."); + + "configured. Configure external storage on the DataConverter, for example with " + + "DefaultDataConverter.withExternalStorage(...), and provide a driver able to " + + "retrieve it."); } } From 6cc2e4a1c28cfbfdcac4f333daf9614742b50c7b Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Mon, 24 Aug 2026 17:44:56 -0400 Subject: [PATCH 05/23] Add new ExternalStorageUnhandledReferenceException for when a reference 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. --- .../converter/PayloadAndFailureDataConverter.java | 4 ++-- .../ExternalStorageNotConfiguredException.java | 11 ++++++----- ...xternalStorageUnhandledReferenceException.java | 15 +++++++++++++++ .../ExternalStorageReferenceGuardTest.java | 14 ++++++-------- .../storage/ExternalStorageRunnerTest.java | 1 - 5 files changed, 29 insertions(+), 16 deletions(-) create mode 100644 temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java index e848e5d9db..cc7de6682a 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java @@ -8,8 +8,8 @@ import io.temporal.api.common.v1.Payloads; import io.temporal.api.failure.v1.Failure; import io.temporal.failure.DefaultFailureConverter; -import io.temporal.internal.payload.storage.ExternalStorageNotConfiguredException; import io.temporal.internal.payload.storage.ExternalStorageReferences; +import io.temporal.internal.payload.storage.ExternalStorageUnhandledReferenceException; import io.temporal.payload.context.SerializationContext; import io.temporal.payload.storage.ExternalStorage; import java.lang.reflect.Type; @@ -76,7 +76,7 @@ public T fromPayload(Payload payload, Class valueClass, Type valueType) } if (ExternalStorageReferences.isReference(payload)) { - throw new ExternalStorageNotConfiguredException(); + throw new ExternalStorageUnhandledReferenceException(); } try { diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java index 3bd67e40a6..10dcca724b 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java @@ -3,15 +3,16 @@ import io.temporal.common.converter.DataConverterException; /** - * Signals that an external storage reference reached a data converter without storage configured. - * Logs a TMPRL1105 error. + * Signals that a payload referenced in external storage needs to be retrieved, but external storage + * is not configured. Logs a TMPRL1105 error. */ public final class ExternalStorageNotConfiguredException extends DataConverterException { public ExternalStorageNotConfiguredException() { super( - "[TMPRL1105] Encountered an external-storage reference payload but external storage is not " - + "configured. Configure external storage on the DataConverter, for example with " - + "DefaultDataConverter.withExternalStorage(...), and provide a driver able to " + "[TMPRL1105] Encountered a reference to a payload in external storage, but no external " + + "storage is configured to retrieve it. Configure external storage on the " + + "DataConverter with " + + "DefaultDataConverter.withExternalStorage(...) and provide a driver able to " + "retrieve it."); } } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java new file mode 100644 index 0000000000..ab31c0fbe8 --- /dev/null +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java @@ -0,0 +1,15 @@ +package io.temporal.internal.payload.storage; + +import io.temporal.common.converter.DataConverterException; + +/** + * Signals that an external storage reference reached ordinary payload conversion without being + * handled by any external storage integration. + */ +public final class ExternalStorageUnhandledReferenceException extends DataConverterException { + public ExternalStorageUnhandledReferenceException() { + super( + "[BUG] An external storage reference reached payload conversion without being handled. This" + + "is likely an SDK bug. Please file a bug report."); + } +} diff --git a/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java b/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java index 31c9698692..98a8696482 100644 --- a/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java +++ b/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java @@ -6,20 +6,19 @@ import com.google.protobuf.ByteString; import io.temporal.api.common.v1.Payload; import io.temporal.api.sdk.v1.ExternalStorageReference; -import io.temporal.internal.payload.storage.ExternalStorageNotConfiguredException; +import io.temporal.internal.payload.storage.ExternalStorageUnhandledReferenceException; import org.junit.Test; /** - * When external storage is not configured, an inbound reference payload reaching value - * deserialization must fail with the clear {@code [TMPRL1105]} error instead of an opaque decoding - * failure. + * An external storage reference reaching value deserialization indicates that an SDK integration + * failed to resolve or reject it at the appropriate boundary. */ public class ExternalStorageReferenceGuardTest { private final DataConverter dataConverter = DefaultDataConverter.newDefaultInstance(); @Test - public void referencePayloadWithoutConfiguredStorageThrows() { + public void unhandledReferencePayloadThrowsSdkBug() { Payload reference = Payload.newBuilder() .putMetadata( @@ -30,11 +29,10 @@ public void referencePayloadWithoutConfiguredStorageThrows() { .setData(ByteString.copyFromUtf8("{}")) .build(); - ExternalStorageNotConfiguredException e = + ExternalStorageUnhandledReferenceException e = assertThrows( - ExternalStorageNotConfiguredException.class, + ExternalStorageUnhandledReferenceException.class, () -> dataConverter.fromPayload(reference, String.class, String.class)); - assertTrue(e.getMessage(), e.getMessage().contains("[TMPRL1105]")); } @Test diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java index 643dc3436c..82dd9bf007 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java @@ -131,7 +131,6 @@ public void throwIfContainsReferenceThrowsOnReference() throws Exception { assertThrows( ExternalStorageNotConfiguredException.class, () -> ExternalStorageRunner.throwIfContainsReference(stored)); - assertTrue(e.getMessage(), e.getMessage().contains("[TMPRL1105]")); } @Test From 66f31c3c4b05e1d4b621de36e55aa83da19071b4 Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Tue, 25 Aug 2026 15:34:14 -0400 Subject: [PATCH 06/23] lint: fix whitespace formatting issue --- .../storage/ExternalStorageUnhandledReferenceException.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java index ab31c0fbe8..ecc5f79837 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java @@ -4,7 +4,7 @@ /** * Signals that an external storage reference reached ordinary payload conversion without being - * handled by any external storage integration. + * handled by any external storage integration. */ public final class ExternalStorageUnhandledReferenceException extends DataConverterException { public ExternalStorageUnhandledReferenceException() { From d51e3482feb5b2d7323565ef97e96ecfb13a63c6 Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Wed, 26 Aug 2026 15:10:39 -0400 Subject: [PATCH 07/23] address PR feedback --- .../payload/storage/ExternalStorageRunner.java | 12 +++++------- ...ternalStorageUnhandledReferenceException.java | 2 +- .../ExternalStorageReferenceGuardTest.java | 16 +--------------- .../storage/ExternalStorageRunnerTest.java | 4 ++-- 4 files changed, 9 insertions(+), 25 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java index 3f1023cc3e..e7518316f4 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java @@ -54,12 +54,12 @@ public void store( cancellationToken); } - public T retrieve(T message) { - return getOrThrowIfCancelled(retrieveAsync(message), CancellationToken.none()); + public T retrieve(T message, CancellationToken cancellationToken) { + return getOrThrowIfCancelled(retrieveAsync(message, cancellationToken), cancellationToken); } - public CompletableFuture retrieveAsync(T message) { - return PayloadVisitors.visit(message, retrieveOptions(CancellationToken.none())); + public CompletableFuture retrieveAsync(T message, CancellationToken cancellationToken) { + return PayloadVisitors.visit(message, retrieveOptions(cancellationToken)); } /** @@ -72,9 +72,7 @@ public static void throwIfContainsReference(Message message) { (context, payloads) -> { for (Payload payload : payloads) { if (ExternalStorageReferences.isReference(payload)) { - CompletableFuture> found = new CompletableFuture<>(); - found.completeExceptionally(new ExternalStorageNotConfiguredException()); - return found; + throw new ExternalStorageNotConfiguredException(); } } return CompletableFuture.completedFuture(payloads); diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java index ecc5f79837..5d1dd3cb5b 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java @@ -10,6 +10,6 @@ public final class ExternalStorageUnhandledReferenceException extends DataConver public ExternalStorageUnhandledReferenceException() { super( "[BUG] An external storage reference reached payload conversion without being handled. This" - + "is likely an SDK bug. Please file a bug report."); + + " is likely an SDK bug. Please file a bug report."); } } diff --git a/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java b/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java index 98a8696482..c7494a358d 100644 --- a/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java +++ b/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java @@ -1,7 +1,6 @@ package io.temporal.common.converter; import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; import com.google.protobuf.ByteString; import io.temporal.api.common.v1.Payload; @@ -29,21 +28,8 @@ public void unhandledReferencePayloadThrowsSdkBug() { .setData(ByteString.copyFromUtf8("{}")) .build(); - ExternalStorageUnhandledReferenceException e = - assertThrows( + assertThrows( ExternalStorageUnhandledReferenceException.class, () -> dataConverter.fromPayload(reference, String.class, String.class)); } - - @Test - public void rawValueBypassesTheGuard() { - Payload reference = - Payload.newBuilder() - .addExternalPayloads( - Payload.ExternalPayloadDetails.newBuilder().setSizeBytes(1024).build()) - .build(); - - RawValue raw = dataConverter.fromPayload(reference, RawValue.class, RawValue.class); - assertTrue(raw.getPayload().getExternalPayloadsCount() > 0); - } } diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java index 82dd9bf007..65159c9909 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java @@ -54,7 +54,7 @@ public void storeAndRetrieveRoundTripsOverAMessage() throws Exception { assertNotNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(0))); assertNotNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(1))); - Payloads retrieved = transformer.retrieve(stored); + Payloads retrieved = transformer.retrieve(stored, CancellationToken.none()); assertEquals(message, retrieved); } @@ -75,7 +75,7 @@ public void walksNestedPayloads() throws Exception { Payload nested = stored.getScheduleActivityTaskCommandAttributes().getInput().getPayloads(0); assertNotNull(ExternalStorageReferences.tryParseReference(nested)); - assertEquals(command, transformer.retrieve(stored)); + assertEquals(command, transformer.retrieve(stored, CancellationToken.none())); } @Test From 36224c31f52b328553a50cc106b982152dd75e0e Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Thu, 27 Aug 2026 11:57:39 -0400 Subject: [PATCH 08/23] refactor(extstore): move external storage from data converter to workflow client options --- .../client/WorkflowClientInternalImpl.java | 12 +++ .../client/WorkflowClientOptions.java | 44 +++++++++-- .../common/converter/CodecDataConverter.java | 7 -- .../common/converter/DataConverter.java | 12 --- .../converter/DefaultDataConverter.java | 11 --- .../PayloadAndFailureDataConverter.java | 9 --- .../client/WorkflowClientInternal.java | 4 + ...ExternalStorageNotConfiguredException.java | 7 +- .../storage/ExternalStorageRunner.java | 11 +-- .../internal/worker/SingleWorkerOptions.java | 22 +++--- .../main/java/io/temporal/worker/Worker.java | 32 ++++++-- ...kflowClientOptionsExternalStorageTest.java | 77 +++++++++++++++++++ .../ExternalStorageReferenceGuardTest.java | 4 +- .../storage/ExternalStorageRunnerTest.java | 17 ++-- ...WorkerPollerAutoEnrollEligibilityTest.java | 2 + .../WorkerPollerAutoEnrollStartupTest.java | 2 + .../temporal/worker/WorkerShutdownTest.java | 2 + 17 files changed, 192 insertions(+), 83 deletions(-) create mode 100644 temporal-sdk/src/test/java/io/temporal/client/WorkflowClientOptionsExternalStorageTest.java diff --git a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientInternalImpl.java b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientInternalImpl.java index e856ca4b46..a92cab89a1 100644 --- a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientInternalImpl.java +++ b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientInternalImpl.java @@ -22,8 +22,10 @@ import io.temporal.internal.client.external.GenericWorkflowClientImpl; import io.temporal.internal.client.external.ManualActivityCompletionClientFactory; import io.temporal.internal.common.PluginUtils; +import io.temporal.internal.payload.storage.ExternalStorageRunner; import io.temporal.internal.sync.StubMarker; import io.temporal.internal.worker.HeartbeatManager; +import io.temporal.payload.storage.ExternalStorage; import io.temporal.serviceclient.MetricsTag; import io.temporal.serviceclient.WorkflowServiceStubs; import io.temporal.serviceclient.WorkflowServiceStubsPlugin; @@ -56,6 +58,7 @@ final class WorkflowClientInternalImpl implements WorkflowClient, WorkflowClient private final WorkerFactoryRegistry workerFactoryRegistry = new WorkerFactoryRegistry(); private final String workerGroupingKey = java.util.UUID.randomUUID().toString(); private final @Nullable HeartbeatManager heartbeatManager; + private final @Nullable ExternalStorageRunner externalStorage; /** * Creates client that connects to an instance of the Temporal Service. Cannot be used from within @@ -106,6 +109,9 @@ public static WorkflowClient newInstance( .getOptions() .getMetricsScope() .tagged(MetricsTag.defaultTags(options.getNamespace())); + ExternalStorage externalStorageConfig = options.getExternalStorage(); + this.externalStorage = + externalStorageConfig == null ? null : ExternalStorageRunner.create(externalStorageConfig); this.genericClient = new GenericWorkflowClientImpl(workflowServiceStubs, metricsScope); this.interceptors = options.getInterceptors(); this.workflowClientCallsInvoker = initializeClientInvoker(); @@ -815,6 +821,12 @@ public HeartbeatManager getHeartbeatManager() { return heartbeatManager; } + @Override + @Nullable + public ExternalStorageRunner getExternalStorage() { + return externalStorage; + } + @Override public NexusStartWorkflowResponse startNexus( NexusStartWorkflowRequest request, Functions.Proc workflow) { diff --git a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java index e10defba51..6692d6b978 100644 --- a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java @@ -7,12 +7,14 @@ import io.temporal.common.converter.DataConverter; import io.temporal.common.converter.GlobalDataConverter; import io.temporal.common.interceptors.WorkflowClientInterceptor; +import io.temporal.payload.storage.ExternalStorage; import java.lang.management.ManagementFactory; import java.time.Duration; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Objects; +import javax.annotation.Nullable; /** Options for WorkflowClient configuration. */ public final class WorkflowClientOptions { @@ -52,6 +54,7 @@ public static final class Builder { private QueryRejectCondition queryRejectCondition; private WorkflowClientPlugin[] plugins; private Duration workerHeartbeatInterval; + private ExternalStorage externalStorage; private Builder() {} @@ -68,6 +71,7 @@ private Builder(WorkflowClientOptions options) { queryRejectCondition = options.queryRejectCondition; plugins = options.plugins; workerHeartbeatInterval = options.workerHeartbeatInterval; + externalStorage = options.externalStorage; } public Builder setNamespace(String namespace) { @@ -86,6 +90,17 @@ public Builder setDataConverter(DataConverter dataConverter) { return this; } + /** + * External storage configuration uses to store/retrieve large payloads. + * + *

Defaults to null. + */ + @Experimental + public Builder setExternalStorage(@Nullable ExternalStorage externalStorage) { + this.externalStorage = externalStorage; + return this; + } + /** * Interceptor used to intercept workflow client calls. * @@ -180,7 +195,8 @@ public WorkflowClientOptions build() { contextPropagators, queryRejectCondition, plugins == null ? EMPTY_PLUGINS : plugins, - resolveHeartbeatInterval(workerHeartbeatInterval)); + resolveHeartbeatInterval(workerHeartbeatInterval), + externalStorage); } /** @@ -207,7 +223,8 @@ public WorkflowClientOptions validateAndBuildWithDefaults() { ? QueryRejectCondition.QUERY_REJECT_CONDITION_UNSPECIFIED : queryRejectCondition, plugins == null ? EMPTY_PLUGINS : plugins, - resolveHeartbeatInterval(workerHeartbeatInterval)); + resolveHeartbeatInterval(workerHeartbeatInterval), + externalStorage); } private static Duration resolveHeartbeatInterval(Duration raw) { @@ -250,6 +267,8 @@ private static Duration resolveHeartbeatInterval(Duration raw) { private final Duration workerHeartbeatInterval; + private final @Nullable ExternalStorage externalStorage; + private WorkflowClientOptions( String namespace, DataConverter dataConverter, @@ -259,7 +278,8 @@ private WorkflowClientOptions( List contextPropagators, QueryRejectCondition queryRejectCondition, WorkflowClientPlugin[] plugins, - Duration workerHeartbeatInterval) { + Duration workerHeartbeatInterval, + @Nullable ExternalStorage externalStorage) { this.namespace = namespace; this.dataConverter = dataConverter; this.interceptors = interceptors; @@ -269,6 +289,7 @@ private WorkflowClientOptions( this.queryRejectCondition = queryRejectCondition; this.plugins = plugins; this.workerHeartbeatInterval = workerHeartbeatInterval; + this.externalStorage = externalStorage; } /** @@ -284,6 +305,15 @@ public DataConverter getDataConverter() { return dataConverter; } + /** + * External storage used to offload large payloads or null when disabled. + */ + @Experimental + @Nullable + public ExternalStorage getExternalStorage() { + return externalStorage; + } + public WorkflowClientInterceptor[] getInterceptors() { return interceptors; } @@ -359,6 +389,8 @@ public String toString() { + Arrays.toString(plugins) + ", workerHeartbeatInterval=" + workerHeartbeatInterval + + ", externalStorage=" + + externalStorage + '}'; } @@ -376,7 +408,8 @@ public boolean equals(Object o) { && queryRejectCondition == that.queryRejectCondition && Arrays.equals(plugins, that.plugins) && com.google.common.base.Objects.equal( - workerHeartbeatInterval, that.workerHeartbeatInterval); + workerHeartbeatInterval, that.workerHeartbeatInterval) + && com.google.common.base.Objects.equal(externalStorage, that.externalStorage); } @Override @@ -390,6 +423,7 @@ public int hashCode() { contextPropagators, queryRejectCondition, Arrays.hashCode(plugins), - workerHeartbeatInterval); + workerHeartbeatInterval, + externalStorage); } } diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/CodecDataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/CodecDataConverter.java index 3fe83a3d0b..a82172348b 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/CodecDataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/CodecDataConverter.java @@ -12,7 +12,6 @@ import io.temporal.payload.codec.ChainCodec; import io.temporal.payload.codec.PayloadCodec; import io.temporal.payload.context.SerializationContext; -import io.temporal.payload.storage.ExternalStorage; import java.lang.reflect.Type; import java.util.Collection; import java.util.Collections; @@ -191,12 +190,6 @@ public CodecDataConverter withContext(@Nonnull SerializationContext context) { return new CodecDataConverter(dataConverter, chainCodec, encodeFailureAttributes, context); } - @Override - @Nullable - public ExternalStorage getExternalStorage() { - return dataConverter.getExternalStorage(); - } - @Nonnull @Override public List encode(@Nonnull List payloads) { diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/DataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/DataConverter.java index e299a3c2b7..decf6181b0 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/DataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/DataConverter.java @@ -11,12 +11,10 @@ import io.temporal.failure.DefaultFailureConverter; import io.temporal.payload.codec.PayloadCodec; import io.temporal.payload.context.SerializationContext; -import io.temporal.payload.storage.ExternalStorage; import java.lang.reflect.Type; import java.util.Arrays; import java.util.Optional; import javax.annotation.Nonnull; -import javax.annotation.Nullable; /** * Used by the framework to serialize/deserialize method parameters that need to be sent over the @@ -204,16 +202,6 @@ default DataConverter withContext(@Nonnull SerializationContext context) { return this; } - /** - * External storage offloads large payloads. This should not be used from inside workflow code as - * it performs nondeterministic operations. - */ - @Experimental - @Nullable - default ExternalStorage getExternalStorage() { - return null; - } - /** * @deprecated use {@link DataConverter#fromPayloads(int, Optional, Class, Type)}. This is an SDK * implementation detail and never was expected to be exposed to users. diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/DefaultDataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/DefaultDataConverter.java index 2df6393f76..f05c3f95a9 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/DefaultDataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/DefaultDataConverter.java @@ -1,10 +1,8 @@ package io.temporal.common.converter; import com.google.common.base.Preconditions; -import io.temporal.payload.storage.ExternalStorage; import java.util.*; import javax.annotation.Nonnull; -import javax.annotation.Nullable; /** * A {@link DataConverter} that delegates payload conversion to type specific {@link @@ -103,13 +101,4 @@ public DefaultDataConverter withFailureConverter(@Nonnull FailureConverter failu this.failureConverter = Preconditions.checkNotNull(failureConverter, "failureConverter"); return this; } - - /** - * Modifies this {@code DefaultDataConverter} by attaching external storage, used to offload and - * restore large payloads at task and RPC boundaries. - */ - public DefaultDataConverter withExternalStorage(@Nullable ExternalStorage externalStorage) { - this.externalStorage = externalStorage; - return this; - } } diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java index cc7de6682a..665d8731cf 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java @@ -11,7 +11,6 @@ import io.temporal.internal.payload.storage.ExternalStorageReferences; import io.temporal.internal.payload.storage.ExternalStorageUnhandledReferenceException; import io.temporal.payload.context.SerializationContext; -import io.temporal.payload.storage.ExternalStorage; import java.lang.reflect.Type; import java.util.*; import javax.annotation.Nonnull; @@ -25,7 +24,6 @@ class PayloadAndFailureDataConverter implements DataConverter { volatile List converters; volatile Map convertersMap; volatile FailureConverter failureConverter; - volatile @Nullable ExternalStorage externalStorage; private final @Nullable SerializationContext serializationContext; public PayloadAndFailureDataConverter(@Nonnull List converters) { @@ -151,17 +149,10 @@ public Failure exceptionToFailure(@Nonnull Throwable throwable) { .exceptionToFailure(throwable, this); } - @Override - @Nullable - public ExternalStorage getExternalStorage() { - return externalStorage; - } - @Override public @Nonnull DataConverter withContext(@Nonnull SerializationContext context) { PayloadAndFailureDataConverter copy = new PayloadAndFailureDataConverter(converters, convertersMap, failureConverter, context); - copy.externalStorage = this.externalStorage; return copy; } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/client/WorkflowClientInternal.java b/temporal-sdk/src/main/java/io/temporal/internal/client/WorkflowClientInternal.java index fc034a366b..7d351ae186 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/client/WorkflowClientInternal.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/client/WorkflowClientInternal.java @@ -1,6 +1,7 @@ package io.temporal.internal.client; import io.temporal.client.WorkflowClient; +import io.temporal.internal.payload.storage.ExternalStorageRunner; import io.temporal.internal.worker.HeartbeatManager; import io.temporal.worker.WorkerFactory; import io.temporal.workflow.Functions; @@ -25,4 +26,7 @@ public interface WorkflowClientInternal { @Nullable HeartbeatManager getHeartbeatManager(); + + @Nullable + ExternalStorageRunner getExternalStorage(); } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java index 10dcca724b..7a99153926 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java @@ -10,9 +10,8 @@ public final class ExternalStorageNotConfiguredException extends DataConverterEx public ExternalStorageNotConfiguredException() { super( "[TMPRL1105] Encountered a reference to a payload in external storage, but no external " - + "storage is configured to retrieve it. Configure external storage on the " - + "DataConverter with " - + "DefaultDataConverter.withExternalStorage(...) and provide a driver able to " - + "retrieve it."); + + "storage is configured to retrieve it. Configure external storage with " + + "WorkflowClientOptions.Builder.setExternalStorage(...) and provide a driver " + + "able to retrieve it."); } } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java index e7518316f4..24d6b633a7 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java @@ -11,7 +11,6 @@ import io.temporal.payload.storage.ExternalStorage; import io.temporal.payload.storage.StorageDriver; import io.temporal.payload.storage.StorageDriverTargetInfo; -import java.util.List; import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; @@ -40,10 +39,6 @@ public static ExternalStorageRunner create(ExternalStorage options) { this.payloadVisitConcurrency = payloadVisitConcurrency; } - public void store(Message.Builder builder, @Nullable StorageDriverTargetInfo target) { - store(builder, target, null, CancellationToken.none()); - } - public void store( Message.Builder builder, @Nullable StorageDriverTargetInfo target, @@ -54,11 +49,13 @@ public void store( cancellationToken); } - public T retrieve(T message, CancellationToken cancellationToken) { + public T retrieve( + T message, CancellationToken cancellationToken) { return getOrThrowIfCancelled(retrieveAsync(message, cancellationToken), cancellationToken); } - public CompletableFuture retrieveAsync(T message, CancellationToken cancellationToken) { + public CompletableFuture retrieveAsync( + T message, CancellationToken cancellationToken) { return PayloadVisitors.visit(message, retrieveOptions(cancellationToken)); } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java b/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java index 39a33d99bd..21c9a5f60b 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java @@ -8,7 +8,6 @@ import io.temporal.common.converter.GlobalDataConverter; import io.temporal.common.interceptors.WorkerInterceptor; import io.temporal.internal.payload.storage.ExternalStorageRunner; -import io.temporal.payload.storage.ExternalStorage; import io.temporal.worker.PreferredVersionProvider; import io.temporal.worker.WorkerDeploymentOptions; import java.time.Duration; @@ -48,6 +47,7 @@ public static final class Builder { private boolean allowActivityHeartbeatDuringShutdown; private String workerControlTaskQueue; private PreferredVersionProvider preferredVersionProvider; + private @Nullable ExternalStorageRunner externalStorage; private Builder() {} @@ -76,6 +76,7 @@ private Builder(SingleWorkerOptions options) { this.allowActivityHeartbeatDuringShutdown = options.getAllowActivityHeartbeatDuringShutdown(); this.workerControlTaskQueue = options.getWorkerControlTaskQueue(); this.preferredVersionProvider = options.getPreferredVersionProvider(); + this.externalStorage = options.getExternalStorage(); } public Builder setIdentity(String identity) { @@ -188,6 +189,11 @@ public Builder setPreferredVersionProvider(PreferredVersionProvider preferredVer return this; } + public Builder setExternalStorage(@Nullable ExternalStorageRunner externalStorage) { + this.externalStorage = externalStorage; + return this; + } + public SingleWorkerOptions build() { PollerOptions pollerOptions = this.pollerOptions; if (pollerOptions == null) { @@ -230,7 +236,8 @@ public SingleWorkerOptions build() { this.workerInstanceKey, this.allowActivityHeartbeatDuringShutdown, this.workerControlTaskQueue, - this.preferredVersionProvider); + this.preferredVersionProvider, + this.externalStorage); } } @@ -278,7 +285,8 @@ private SingleWorkerOptions( String workerInstanceKey, boolean allowActivityHeartbeatDuringShutdown, String workerControlTaskQueue, - PreferredVersionProvider preferredVersionProvider) { + PreferredVersionProvider preferredVersionProvider, + @Nullable ExternalStorageRunner externalStorage) { this.identity = identity; this.binaryChecksum = binaryChecksum; this.buildId = buildId; @@ -300,9 +308,7 @@ private SingleWorkerOptions( this.allowActivityHeartbeatDuringShutdown = allowActivityHeartbeatDuringShutdown; this.workerControlTaskQueue = workerControlTaskQueue; this.preferredVersionProvider = preferredVersionProvider; - ExternalStorage externalStorageConfig = dataConverter.getExternalStorage(); - this.externalStorage = - externalStorageConfig == null ? null : ExternalStorageRunner.create(externalStorageConfig); + this.externalStorage = externalStorage; } public String getIdentity() { @@ -400,10 +406,6 @@ public PreferredVersionProvider getPreferredVersionProvider() { return preferredVersionProvider; } - /** - * The external-storage runner for this worker, derived from the worker's {@link DataConverter}, - * or null when the converter has no external storage configured. - */ @Nullable public ExternalStorageRunner getExternalStorage() { return externalStorage; diff --git a/temporal-sdk/src/main/java/io/temporal/worker/Worker.java b/temporal-sdk/src/main/java/io/temporal/worker/Worker.java index b755134448..2e0040db10 100644 --- a/temporal-sdk/src/main/java/io/temporal/worker/Worker.java +++ b/temporal-sdk/src/main/java/io/temporal/worker/Worker.java @@ -22,6 +22,8 @@ import io.temporal.common.converter.DataConverter; import io.temporal.common.converter.EncodedValues; import io.temporal.failure.TemporalFailure; +import io.temporal.internal.client.WorkflowClientInternal; +import io.temporal.internal.payload.storage.ExternalStorageRunner; import io.temporal.internal.sync.WorkflowInternal; import io.temporal.internal.sync.WorkflowThreadExecutor; import io.temporal.internal.worker.*; @@ -123,6 +125,8 @@ private static final class TaskSnapshot { this.options = WorkerOptions.newBuilder(options).validateAndBuildWithDefaults(); this.clientOptions = client.getOptions(); this.cache = cache; + ExternalStorageRunner externalStorage = + ((WorkflowClientInternal) client.getInternal()).getExternalStorage(); factoryOptions = WorkerFactoryOptions.newBuilder(factoryOptions).validateAndBuildWithDefaults(); WorkflowClientOptions clientOptions = client.getOptions(); String namespace = clientOptions.getNamespace(); @@ -150,6 +154,7 @@ private static final class TaskSnapshot { taggedScope, workerInstanceKey, workerControlTaskQueue, + externalStorage, activityTaskAutoEnrollEligible); if (this.options.isLocalActivityWorkerOnly()) { activityWorker = null; @@ -185,6 +190,7 @@ private static final class TaskSnapshot { taggedScope, workerInstanceKey, workerControlTaskQueue, + externalStorage, nexusTaskAutoEnrollEligible); SlotSupplier nexusSlotSupplier = this.options.getWorkerTuner() == null @@ -206,6 +212,7 @@ private static final class TaskSnapshot { taggedScope, workerInstanceKey, workerControlTaskQueue, + externalStorage, workflowTaskAutoEnrollEligible); SingleWorkerOptions localActivityOptions = toLocalActivityOptions( @@ -215,7 +222,8 @@ private static final class TaskSnapshot { contextPropagators, taggedScope, workerInstanceKey, - workerControlTaskQueue); + workerControlTaskQueue, + externalStorage); SlotSupplier workflowSlotSupplier = this.options.getWorkerTuner() == null @@ -915,6 +923,7 @@ private static SingleWorkerOptions toActivityOptions( Scope metricsScope, String workerInstanceKey, String workerControlTaskQueue, + @Nullable ExternalStorageRunner externalStorage, boolean autoEnrollEligible) { return toSingleWorkerOptions( factoryOptions, @@ -922,7 +931,8 @@ private static SingleWorkerOptions toActivityOptions( clientOptions, contextPropagators, workerInstanceKey, - workerControlTaskQueue) + workerControlTaskQueue, + externalStorage) .setUsingVirtualThreads(options.isUsingVirtualThreadsOnActivityWorker()) .setAllowActivityHeartbeatDuringShutdown(options.getAllowActivityHeartbeatDuringShutdown()) .setPollerOptions( @@ -948,6 +958,7 @@ private static SingleWorkerOptions toNexusOptions( Scope metricsScope, String workerInstanceKey, String workerControlTaskQueue, + @Nullable ExternalStorageRunner externalStorage, boolean autoEnrollEligible) { return toSingleWorkerOptions( factoryOptions, @@ -955,7 +966,8 @@ private static SingleWorkerOptions toNexusOptions( clientOptions, contextPropagators, workerInstanceKey, - workerControlTaskQueue) + workerControlTaskQueue, + externalStorage) .setPollerOptions( PollerOptions.newBuilder() .setPollerBehavior( @@ -980,6 +992,7 @@ private static SingleWorkerOptions toWorkflowWorkerOptions( Scope metricsScope, String workerInstanceKey, String workerControlTaskQueue, + @Nullable ExternalStorageRunner externalStorage, boolean autoEnrollEligible) { Map tags = new ImmutableMap.Builder(1).put(MetricsTag.TASK_QUEUE, taskQueue).build(); @@ -1015,7 +1028,8 @@ private static SingleWorkerOptions toWorkflowWorkerOptions( clientOptions, contextPropagators, workerInstanceKey, - workerControlTaskQueue) + workerControlTaskQueue, + externalStorage) .setPollerOptions( PollerOptions.newBuilder() .setPollerBehavior( @@ -1040,14 +1054,16 @@ private static SingleWorkerOptions toLocalActivityOptions( List contextPropagators, Scope metricsScope, String workerInstanceKey, - String workerControlTaskQueue) { + String workerControlTaskQueue, + @Nullable ExternalStorageRunner externalStorage) { return toSingleWorkerOptions( factoryOptions, options, clientOptions, contextPropagators, workerInstanceKey, - workerControlTaskQueue) + workerControlTaskQueue, + externalStorage) .setPollerOptions( PollerOptions.newBuilder() .setPollerBehavior(new PollerBehaviorSimpleMaximum(1)) @@ -1066,7 +1082,8 @@ private static SingleWorkerOptions.Builder toSingleWorkerOptions( WorkflowClientOptions clientOptions, List contextPropagators, String workerInstanceKey, - String workerControlTaskQueue) { + String workerControlTaskQueue, + @Nullable ExternalStorageRunner externalStorage) { String buildId = null; if (options.getBuildId() != null) { buildId = options.getBuildId(); @@ -1081,6 +1098,7 @@ private static SingleWorkerOptions.Builder toSingleWorkerOptions( return SingleWorkerOptions.newBuilder() .setDataConverter(clientOptions.getDataConverter()) + .setExternalStorage(externalStorage) .setIdentity(identity) .setBuildId(buildId) .setUseBuildIdForVersioning(options.isUsingBuildIdForVersioning()) diff --git a/temporal-sdk/src/test/java/io/temporal/client/WorkflowClientOptionsExternalStorageTest.java b/temporal-sdk/src/test/java/io/temporal/client/WorkflowClientOptionsExternalStorageTest.java new file mode 100644 index 0000000000..d02e0c8cb0 --- /dev/null +++ b/temporal-sdk/src/test/java/io/temporal/client/WorkflowClientOptionsExternalStorageTest.java @@ -0,0 +1,77 @@ +package io.temporal.client; + +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; + +import io.temporal.api.common.v1.Payload; +import io.temporal.payload.storage.ExternalStorage; +import io.temporal.payload.storage.StorageDriver; +import io.temporal.payload.storage.StorageDriverClaim; +import io.temporal.payload.storage.StorageDriverRetrieveContext; +import io.temporal.payload.storage.StorageDriverStoreContext; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import org.junit.Test; + +public class WorkflowClientOptionsExternalStorageTest { + + @Test + public void defaultsToDisabled() { + assertNull(WorkflowClientOptions.newBuilder().build().getExternalStorage()); + assertNull(WorkflowClientOptions.getDefaultInstance().getExternalStorage()); + } + + @Test + public void buildsWithDefaults() { + ExternalStorage storage = storage(); + + WorkflowClientOptions options = + WorkflowClientOptions.newBuilder() + .setExternalStorage(storage) + .validateAndBuildWithDefaults(); + + assertSame(storage, options.getExternalStorage()); + } + + /** Plugins reconfigure a client by rebuilding its options, so a round trip must not drop it. */ + @Test + public void survivesRoundTripThroughBuilder() { + ExternalStorage storage = storage(); + + WorkflowClientOptions original = + WorkflowClientOptions.newBuilder().setExternalStorage(storage).build(); + + assertSame(storage, original.toBuilder().build().getExternalStorage()); + assertSame(storage, WorkflowClientOptions.newBuilder(original).build().getExternalStorage()); + } + + private static ExternalStorage storage() { + return ExternalStorage.newBuilder().setDriver(driver()).build(); + } + + private static StorageDriver driver() { + return new StorageDriver() { + @Override + public String getName() { + return "test-driver"; + } + + @Override + public String getType() { + return "test"; + } + + @Override + public CompletableFuture> store( + StorageDriverStoreContext context, List payloads) { + throw new UnsupportedOperationException(); + } + + @Override + public CompletableFuture> retrieve( + StorageDriverRetrieveContext context, List claims) { + throw new UnsupportedOperationException(); + } + }; + } +} diff --git a/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java b/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java index c7494a358d..6a30e0b947 100644 --- a/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java +++ b/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java @@ -29,7 +29,7 @@ public void unhandledReferencePayloadThrowsSdkBug() { .build(); assertThrows( - ExternalStorageUnhandledReferenceException.class, - () -> dataConverter.fromPayload(reference, String.class, String.class)); + ExternalStorageUnhandledReferenceException.class, + () -> dataConverter.fromPayload(reference, String.class, String.class)); } } diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java index 65159c9909..e1c40ab005 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java @@ -48,7 +48,7 @@ public void storeAndRetrieveRoundTripsOverAMessage() throws Exception { Payloads.newBuilder().addPayloads(payload("a")).addPayloads(payload("b")).build(); Payloads.Builder builder = message.toBuilder(); - transformer.store(builder, null); + transformer.store(builder, null, null, CancellationToken.none()); Payloads stored = builder.build(); assertNotNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(0))); @@ -70,7 +70,7 @@ public void walksNestedPayloads() throws Exception { .build(); Command.Builder builder = command.toBuilder(); - transformer.store(builder, null); + transformer.store(builder, null, null, CancellationToken.none()); Command stored = builder.build(); Payload nested = stored.getScheduleActivityTaskCommandAttributes().getInput().getPayloads(0); @@ -85,7 +85,7 @@ public void payloadBelowThresholdLeavesMessageUnchanged() throws Exception { Payloads message = Payloads.newBuilder().addPayloads(payload("small")).build(); Payloads.Builder builder = message.toBuilder(); - transformer.store(builder, null); + transformer.store(builder, null, null, CancellationToken.none()); Payloads stored = builder.build(); assertNull(ExternalStorageReferences.tryParseReference(stored.getPayloads(0))); @@ -108,7 +108,7 @@ public void searchAttributesAreNotOffloaded() throws Exception { .build(); Command.Builder builder = command.toBuilder(); - transformer.store(builder, null); + transformer.store(builder, null, null, CancellationToken.none()); Command stored = builder.build(); StartChildWorkflowExecutionCommandAttributes attrs = @@ -124,13 +124,12 @@ public void throwIfContainsReferenceThrowsOnReference() throws Exception { InMemoryDriver driver = new InMemoryDriver("d1"); ExternalStorageRunner transformer = transformer(driver, 0); Payloads.Builder builder = Payloads.newBuilder().addPayloads(payload("a")); - transformer.store(builder, null); + transformer.store(builder, null, null, CancellationToken.none()); Payloads stored = builder.build(); - ExternalStorageNotConfiguredException e = - assertThrows( - ExternalStorageNotConfiguredException.class, - () -> ExternalStorageRunner.throwIfContainsReference(stored)); + assertThrows( + ExternalStorageNotConfiguredException.class, + () -> ExternalStorageRunner.throwIfContainsReference(stored)); } @Test diff --git a/temporal-sdk/src/test/java/io/temporal/worker/WorkerPollerAutoEnrollEligibilityTest.java b/temporal-sdk/src/test/java/io/temporal/worker/WorkerPollerAutoEnrollEligibilityTest.java index 46ae4d37de..b7ffce91ca 100644 --- a/temporal-sdk/src/test/java/io/temporal/worker/WorkerPollerAutoEnrollEligibilityTest.java +++ b/temporal-sdk/src/test/java/io/temporal/worker/WorkerPollerAutoEnrollEligibilityTest.java @@ -12,6 +12,7 @@ import io.temporal.api.workflowservice.v1.WorkflowServiceGrpc; import io.temporal.client.WorkflowClient; import io.temporal.client.WorkflowClientOptions; +import io.temporal.internal.client.WorkflowClientInternal; import io.temporal.internal.sync.WorkflowThreadExecutor; import io.temporal.internal.worker.NamespaceCapabilities; import io.temporal.internal.worker.WorkflowExecutorCache; @@ -43,6 +44,7 @@ private Worker buildWorker(WorkerOptions options) { when(blockingStub.withOption(any(), any())).thenReturn(blockingStub); WorkflowClient client = mock(WorkflowClient.class); + when(client.getInternal()).thenReturn(mock(WorkflowClientInternal.class)); when(client.getWorkflowServiceStubs()).thenReturn(service); when(client.getOptions()) .thenReturn( diff --git a/temporal-sdk/src/test/java/io/temporal/worker/WorkerPollerAutoEnrollStartupTest.java b/temporal-sdk/src/test/java/io/temporal/worker/WorkerPollerAutoEnrollStartupTest.java index 00ea0d69be..1d5f5df30d 100644 --- a/temporal-sdk/src/test/java/io/temporal/worker/WorkerPollerAutoEnrollStartupTest.java +++ b/temporal-sdk/src/test/java/io/temporal/worker/WorkerPollerAutoEnrollStartupTest.java @@ -21,6 +21,7 @@ import io.temporal.api.workflowservice.v1.WorkflowServiceGrpc; import io.temporal.client.WorkflowClient; import io.temporal.client.WorkflowClientOptions; +import io.temporal.internal.client.WorkflowClientInternal; import io.temporal.internal.sync.WorkflowThreadExecutor; import io.temporal.internal.worker.NamespaceCapabilities; import io.temporal.internal.worker.ShutdownManager; @@ -97,6 +98,7 @@ public void autoEnrollAtStartupSwitchesPollersToAutoscaling() throws Exception { when(blockingStub.withOption(any(), any())).thenReturn(blockingStub); WorkflowClient client = mock(WorkflowClient.class); + when(client.getInternal()).thenReturn(mock(WorkflowClientInternal.class)); when(client.getWorkflowServiceStubs()).thenReturn(service); when(client.getOptions()) .thenReturn( diff --git a/temporal-sdk/src/test/java/io/temporal/worker/WorkerShutdownTest.java b/temporal-sdk/src/test/java/io/temporal/worker/WorkerShutdownTest.java index 23a63cda8b..390efe1e7d 100644 --- a/temporal-sdk/src/test/java/io/temporal/worker/WorkerShutdownTest.java +++ b/temporal-sdk/src/test/java/io/temporal/worker/WorkerShutdownTest.java @@ -21,6 +21,7 @@ import io.temporal.api.workflowservice.v1.WorkflowServiceGrpc; import io.temporal.client.WorkflowClient; import io.temporal.client.WorkflowClientOptions; +import io.temporal.internal.client.WorkflowClientInternal; import io.temporal.internal.sync.WorkflowThreadExecutor; import io.temporal.internal.worker.NamespaceCapabilities; import io.temporal.internal.worker.ShutdownManager; @@ -92,6 +93,7 @@ public void activeTaskQueueTypesEvaluatedAtShutdownTime() throws Exception { when(blockingStub.withOption(any(), any())).thenReturn(blockingStub); WorkflowClient client = mock(WorkflowClient.class); + when(client.getInternal()).thenReturn(mock(WorkflowClientInternal.class)); when(client.getWorkflowServiceStubs()).thenReturn(service); when(client.getOptions()) .thenReturn( From b7f544e9770a311a2137aca14174ddcc107848cb Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Thu, 27 Aug 2026 12:56:45 -0400 Subject: [PATCH 09/23] remove isReference check from data converter. while adding a check to 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. --- .../common/converter/PayloadAndFailureDataConverter.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java index 665d8731cf..d2db4d2617 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java @@ -73,10 +73,6 @@ public T fromPayload(Payload payload, Class valueClass, Type valueType) return (T) new RawValue(payload); } - if (ExternalStorageReferences.isReference(payload)) { - throw new ExternalStorageUnhandledReferenceException(); - } - try { String encoding = payload.getMetadataOrThrow(EncodingKeys.METADATA_ENCODING_KEY).toString(UTF_8); From 126129482b32829f4d83cf20c25882331c6bebba Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Thu, 27 Aug 2026 14:23:46 -0400 Subject: [PATCH 10/23] remove unused tests and code --- .../client/WorkflowClientOptions.java | 4 +-- .../PayloadAndFailureDataConverter.java | 2 -- ...nalStorageUnhandledReferenceException.java | 15 -------- .../ExternalStorageReferenceGuardTest.java | 35 ------------------- .../storage/ExternalStorageRunnerTest.java | 4 +-- 5 files changed, 3 insertions(+), 57 deletions(-) delete mode 100644 temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java delete mode 100644 temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java diff --git a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java index 6692d6b978..77e8dddbc9 100644 --- a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java @@ -305,9 +305,7 @@ public DataConverter getDataConverter() { return dataConverter; } - /** - * External storage used to offload large payloads or null when disabled. - */ + /** External storage used to offload large payloads or null when disabled. */ @Experimental @Nullable public ExternalStorage getExternalStorage() { diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java index d2db4d2617..c7526fc551 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java @@ -8,8 +8,6 @@ import io.temporal.api.common.v1.Payloads; import io.temporal.api.failure.v1.Failure; import io.temporal.failure.DefaultFailureConverter; -import io.temporal.internal.payload.storage.ExternalStorageReferences; -import io.temporal.internal.payload.storage.ExternalStorageUnhandledReferenceException; import io.temporal.payload.context.SerializationContext; import java.lang.reflect.Type; import java.util.*; diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java deleted file mode 100644 index 5d1dd3cb5b..0000000000 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageUnhandledReferenceException.java +++ /dev/null @@ -1,15 +0,0 @@ -package io.temporal.internal.payload.storage; - -import io.temporal.common.converter.DataConverterException; - -/** - * Signals that an external storage reference reached ordinary payload conversion without being - * handled by any external storage integration. - */ -public final class ExternalStorageUnhandledReferenceException extends DataConverterException { - public ExternalStorageUnhandledReferenceException() { - super( - "[BUG] An external storage reference reached payload conversion without being handled. This" - + " is likely an SDK bug. Please file a bug report."); - } -} diff --git a/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java b/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java deleted file mode 100644 index 6a30e0b947..0000000000 --- a/temporal-sdk/src/test/java/io/temporal/common/converter/ExternalStorageReferenceGuardTest.java +++ /dev/null @@ -1,35 +0,0 @@ -package io.temporal.common.converter; - -import static org.junit.Assert.assertThrows; - -import com.google.protobuf.ByteString; -import io.temporal.api.common.v1.Payload; -import io.temporal.api.sdk.v1.ExternalStorageReference; -import io.temporal.internal.payload.storage.ExternalStorageUnhandledReferenceException; -import org.junit.Test; - -/** - * An external storage reference reaching value deserialization indicates that an SDK integration - * failed to resolve or reject it at the appropriate boundary. - */ -public class ExternalStorageReferenceGuardTest { - - private final DataConverter dataConverter = DefaultDataConverter.newDefaultInstance(); - - @Test - public void unhandledReferencePayloadThrowsSdkBug() { - Payload reference = - Payload.newBuilder() - .putMetadata( - EncodingKeys.METADATA_ENCODING_KEY, ByteString.copyFromUtf8("json/protobuf")) - .putMetadata( - EncodingKeys.METADATA_MESSAGE_TYPE_KEY, - ByteString.copyFromUtf8(ExternalStorageReference.getDescriptor().getFullName())) - .setData(ByteString.copyFromUtf8("{}")) - .build(); - - assertThrows( - ExternalStorageUnhandledReferenceException.class, - () -> dataConverter.fromPayload(reference, String.class, String.class)); - } -} diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java index e1c40ab005..b5c432ff80 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java @@ -128,8 +128,8 @@ public void throwIfContainsReferenceThrowsOnReference() throws Exception { Payloads stored = builder.build(); assertThrows( - ExternalStorageNotConfiguredException.class, - () -> ExternalStorageRunner.throwIfContainsReference(stored)); + ExternalStorageNotConfiguredException.class, + () -> ExternalStorageRunner.throwIfContainsReference(stored)); } @Test From 024ec40d713fe2ef0d34d18e47a98c3a781df978 Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Thu, 27 Aug 2026 16:40:43 -0400 Subject: [PATCH 11/23] small fixes --- .../PayloadAndFailureDataConverter.java | 4 +- ...ExternalStorageNotConfiguredException.java | 2 +- .../storage/ExternalStorageRunner.java | 12 ++++-- .../storage/ExternalStorageRunnerTest.java | 38 +++++++++++++++++++ 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java index c7526fc551..935fd8462c 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/common/converter/PayloadAndFailureDataConverter.java @@ -145,9 +145,7 @@ public Failure exceptionToFailure(@Nonnull Throwable throwable) { @Override public @Nonnull DataConverter withContext(@Nonnull SerializationContext context) { - PayloadAndFailureDataConverter copy = - new PayloadAndFailureDataConverter(converters, convertersMap, failureConverter, context); - return copy; + return new PayloadAndFailureDataConverter(converters, convertersMap, failureConverter, context); } static Map createConvertersMap(List converters) { diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java index 7a99153926..1f977c81db 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageNotConfiguredException.java @@ -4,7 +4,7 @@ /** * Signals that a payload referenced in external storage needs to be retrieved, but external storage - * is not configured. Logs a TMPRL1105 error. + * is not configured. */ public final class ExternalStorageNotConfiguredException extends DataConverterException { public ExternalStorageNotConfiguredException() { diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java index 24d6b633a7..c05f8c5d03 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageRunner.java @@ -20,7 +20,7 @@ /** * External storage offloads large payloads via {@link StorageDriver}s. It walks messages using * {@link PayloadVisitors} transforming payloads to and from {@link ExternalStorageReference} using - * {@link ExternalStoragePayloadTransformer}. Use {@link ExternalStorage} via {@link#create} to + * {@link ExternalStoragePayloadTransformer}. Use {@link ExternalStorage} via {@link #create} to * configure external storage. */ public final class ExternalStorageRunner { @@ -87,16 +87,22 @@ public static void throwIfContainsReference(Message message) { private static T getOrThrowIfCancelled( CompletableFuture future, CancellationToken cancellationToken) { + CompletableFuture cancellation = cancellationToken.getCancellationFuture(); try { - CompletableFuture.anyOf(future, cancellationToken.getCancellationFuture()).get(); + CompletableFuture.anyOf(future, cancellation).get(); return future.get(); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - throw new CancellationException("External storage operation interrupted"); + CancellationException cancelled = + new CancellationException("External storage operation interrupted"); + cancelled.initCause(e); + throw cancelled; } catch (ExecutionException e) { Throwable cause = e.getCause() != null ? e.getCause() : e; Throwables.throwIfUnchecked(cause); throw new CompletionException(cause); + } finally { + cancellation.complete(null); } } diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java index b5c432ff80..d6cf71eafd 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java @@ -35,6 +35,7 @@ import java.util.Map; import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.Test; /** Tests external storage message conversion. */ @@ -192,6 +193,20 @@ public void callerCancellationAbortsStore() { () -> storage.store(message.toBuilder(), null, null, caller.token())); } + @Test + public void completedOperationsReleaseTheirCancellationRegistrations() { + RegistrationCountingToken token = new RegistrationCountingToken(); + ExternalStorageRunner storage = transformer(new InMemoryDriver("d1"), 0); + + for (int i = 0; i < 5; i++) { + Payloads.Builder builder = Payloads.newBuilder().addPayloads(payload("a")); + storage.store(builder, null, null, token); + storage.retrieve(builder.build(), token); + } + + assertEquals(0, token.open()); + } + private static ExternalStorageRunner transformer(StorageDriver driver, int threshold) { ExternalStoragePayloadTransformer payloadTransformer = ExternalStoragePayloadTransformer.fromOptions( @@ -206,6 +221,29 @@ private static Payload payload(String data) { return Payload.newBuilder().setData(ByteString.copyFromUtf8(data)).build(); } + private static final class RegistrationCountingToken + implements CancellationToken { + private final AtomicInteger open = new AtomicInteger(); + + int open() { + return open.get(); + } + + @Override + public boolean isCancellationRequested() { + return false; + } + + @Override + public void throwIfCancellationRequested() {} + + @Override + public Registration onCancel(Runnable callback) { + open.incrementAndGet(); + return open::decrementAndGet; + } + } + private static final class InMemoryDriver implements StorageDriver { private final String name; private final Map objects = new HashMap<>(); From 8df7ce16749c6b0bac1e16046d1603a22ac3bd20 Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Thu, 27 Aug 2026 17:04:24 -0400 Subject: [PATCH 12/23] move ExternalStorageDataConverter to foundation PR. --- .../storage/ExternalStorageDataConverter.java | 124 +++++++++++++ .../ExternalStorageDataConverterTest.java | 169 ++++++++++++++++++ 2 files changed, 293 insertions(+) create mode 100644 temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageDataConverter.java create mode 100644 temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageDataConverterTest.java diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageDataConverter.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageDataConverter.java new file mode 100644 index 0000000000..d168d22f56 --- /dev/null +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageDataConverter.java @@ -0,0 +1,124 @@ +package io.temporal.internal.payload.storage; + +import io.temporal.api.common.v1.Payload; +import io.temporal.api.common.v1.Payloads; +import io.temporal.api.failure.v1.Failure; +import io.temporal.common.CancellationToken; +import io.temporal.common.converter.DataConverter; +import io.temporal.common.converter.DataConverterException; +import io.temporal.payload.context.SerializationContext; +import io.temporal.payload.storage.StorageDriverTargetInfo; +import java.lang.reflect.Type; +import java.util.Optional; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +/** + * A {@link DataConverter} that stores/retrieves payloads to/from external storage. + * + *

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 { + + private final DataConverter delegate; + private final ExternalStorageRunner externalStorage; + private final @Nullable StorageDriverTargetInfo storageTarget; + + public ExternalStorageDataConverter( + @Nonnull DataConverter delegate, @Nonnull ExternalStorageRunner externalStorage) { + this(delegate, externalStorage, null); + } + + private ExternalStorageDataConverter( + @Nonnull DataConverter delegate, + @Nonnull ExternalStorageRunner externalStorage, + @Nullable StorageDriverTargetInfo storageTarget) { + this.delegate = delegate; + this.externalStorage = externalStorage; + this.storageTarget = storageTarget; + } + + public ExternalStorageDataConverter withStorageTarget( + @Nullable StorageDriverTargetInfo storageTarget) { + return new ExternalStorageDataConverter(delegate, externalStorage, storageTarget); + } + + @Override + public Optional toPayload(T value) throws DataConverterException { + Optional converted = delegate.toPayload(value); + if (!converted.isPresent()) { + return converted; + } + Payloads stored = store(Payloads.newBuilder().addPayloads(converted.get()).build()); + return Optional.of(stored.getPayloads(0)); + } + + @Override + public Optional toPayloads(Object... values) throws DataConverterException { + Optional converted = delegate.toPayloads(values); + if (!converted.isPresent()) { + return converted; + } + return Optional.of(store(converted.get())); + } + + @Override + public T fromPayload(Payload payload, Class valueClass, Type valueType) + throws DataConverterException { + return delegate.fromPayload(retrieve(payload), valueClass, valueType); + } + + @Override + public T fromPayloads( + int index, Optional content, Class parameterType, Type genericParameterType) + throws DataConverterException { + if (!content.isPresent() || index >= content.get().getPayloadsCount()) { + return delegate.fromPayloads(index, content, parameterType, genericParameterType); + } + Payload resolved = retrieve(content.get().getPayloads(index)); + return delegate.fromPayload(resolved, parameterType, genericParameterType); + } + + @Override + @Nonnull + public RuntimeException failureToException(@Nonnull Failure failure) { + return delegate.failureToException(retrieveMessage(failure)); + } + + @Override + @Nonnull + public Failure exceptionToFailure(@Nonnull Throwable throwable) { + return storeMessage(delegate.exceptionToFailure(throwable)); + } + + @Override + @Nonnull + public DataConverter withContext(@Nonnull SerializationContext context) { + return new ExternalStorageDataConverter( + delegate.withContext(context), externalStorage, storageTarget); + } + + private Payload retrieve(Payload payload) { + if (!ExternalStorageReferences.isReference(payload)) { + return payload; + } + return retrieveMessage(Payloads.newBuilder().addPayloads(payload).build()).getPayloads(0); + } + + private Payloads store(Payloads payloads) { + Payloads.Builder builder = payloads.toBuilder(); + externalStorage.store(builder, storageTarget, null, CancellationToken.none()); + return builder.build(); + } + + private T retrieveMessage(T message) { + return externalStorage.retrieve(message, CancellationToken.none()); + } + + private Failure storeMessage(Failure failure) { + Failure.Builder builder = failure.toBuilder(); + externalStorage.store(builder, storageTarget, null, CancellationToken.none()); + return builder.build(); + } +} diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageDataConverterTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageDataConverterTest.java new file mode 100644 index 0000000000..03114aa9ef --- /dev/null +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageDataConverterTest.java @@ -0,0 +1,169 @@ +package io.temporal.internal.payload.storage; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import io.temporal.api.common.v1.Payload; +import io.temporal.api.common.v1.Payloads; +import io.temporal.api.failure.v1.Failure; +import io.temporal.common.converter.DataConverter; +import io.temporal.common.converter.DefaultDataConverter; +import io.temporal.failure.ApplicationFailure; +import io.temporal.payload.storage.ExternalStorage; +import io.temporal.payload.storage.StorageDriver; +import io.temporal.payload.storage.StorageDriverClaim; +import io.temporal.payload.storage.StorageDriverRetrieveContext; +import io.temporal.payload.storage.StorageDriverStoreContext; +import io.temporal.payload.storage.StorageDriverTargetInfo; +import io.temporal.payload.storage.StorageDriverWorkflowInfo; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import org.junit.Test; + +public class ExternalStorageDataConverterTest { + + private final DataConverter plain = DefaultDataConverter.newDefaultInstance(); + + @Test + public void payloadsRoundTripThroughStorage() { + RecordingDriver driver = new RecordingDriver(); + DataConverter converter = resolving(driver, 0); + + Optional stored = converter.toPayloads("a", "b"); + + assertTrue(ExternalStorageReferences.isReference(stored.get().getPayloads(0))); + assertTrue(ExternalStorageReferences.isReference(stored.get().getPayloads(1))); + + assertEquals("a", converter.fromPayloads(0, stored, String.class, String.class)); + assertEquals("b", converter.fromPayloads(1, stored, String.class, String.class)); + } + + @Test + public void payloadsBelowThresholdStayInline() { + RecordingDriver driver = new RecordingDriver(); + DataConverter converter = resolving(driver, 1024); + + Optional stored = converter.toPayloads("small"); + + assertFalse(ExternalStorageReferences.isReference(stored.get().getPayloads(0))); + assertTrue(driver.objects.isEmpty()); + assertEquals("small", converter.fromPayloads(0, stored, String.class, String.class)); + } + + @Test + public void readingOneArgumentDoesNotFetchTheRest() { + RecordingDriver driver = new RecordingDriver(); + DataConverter converter = resolving(driver, 0); + + Optional stored = converter.toPayloads("first", "second", "third"); + driver.retrievedKeys.clear(); + + assertEquals("second", converter.fromPayloads(1, stored, String.class, String.class)); + + assertEquals(1, driver.retrievedKeys.size()); + } + + @Test + public void singlePayloadRoundTrips() { + DataConverter converter = resolving(new RecordingDriver(), 0); + + Optional stored = converter.toPayload("value"); + + assertTrue(ExternalStorageReferences.isReference(stored.get())); + assertEquals("value", converter.fromPayload(stored.get(), String.class, String.class)); + } + + @Test + public void failureDetailsRoundTrip() { + DataConverter converter = resolving(new RecordingDriver(), 0); + + Failure failure = + converter.exceptionToFailure( + ApplicationFailure.newFailure("boom", "TestType", "detail-value")); + + Payload detail = failure.getApplicationFailureInfo().getDetails().getPayloads(0); + assertTrue(ExternalStorageReferences.isReference(detail)); + + RuntimeException restored = converter.failureToException(failure); + assertTrue(restored instanceof ApplicationFailure); + assertEquals("detail-value", ((ApplicationFailure) restored).getDetails().get(0, String.class)); + } + + @Test + public void storageTargetReachesTheDriver() { + RecordingDriver driver = new RecordingDriver(); + StorageDriverWorkflowInfo target = new StorageDriverWorkflowInfo("ns", "wf-1", null, null); + + ExternalStorageDataConverter converter = + new ExternalStorageDataConverter(plain, runner(driver, 0)).withStorageTarget(target); + converter.toPayloads("x"); + + assertEquals(target, driver.lastTarget); + } + + @Test + public void withoutATargetTheDriverSeesNone() { + RecordingDriver driver = new RecordingDriver(); + resolving(driver, 0).toPayloads("x"); + + assertNull(driver.lastTarget); + } + + private DataConverter resolving(StorageDriver driver, int threshold) { + return new ExternalStorageDataConverter(plain, runner(driver, threshold)); + } + + private static ExternalStorageRunner runner(StorageDriver driver, int threshold) { + return ExternalStorageRunner.create( + ExternalStorage.newBuilder().setDriver(driver).setPayloadSizeThreshold(threshold).build()); + } + + private static final class RecordingDriver implements StorageDriver { + final Map objects = new HashMap<>(); + final List retrievedKeys = new ArrayList<>(); + volatile StorageDriverTargetInfo lastTarget; + private int counter = 0; + + @Override + public String getName() { + return "test"; + } + + @Override + public String getType() { + return "test.inmemory"; + } + + @Override + public synchronized CompletableFuture> store( + StorageDriverStoreContext context, List payloads) { + lastTarget = context.getTarget(); + List claims = new ArrayList<>(); + for (Payload payload : payloads) { + String key = "k-" + (counter++); + objects.put(key, payload); + claims.add(new StorageDriverClaim(Collections.singletonMap("key", key))); + } + return CompletableFuture.completedFuture(claims); + } + + @Override + public synchronized CompletableFuture> retrieve( + StorageDriverRetrieveContext context, List claims) { + List payloads = new ArrayList<>(); + for (StorageDriverClaim claim : claims) { + String key = claim.getClaimData().get("key"); + retrievedKeys.add(key); + payloads.add(objects.get(key)); + } + return CompletableFuture.completedFuture(payloads); + } + } +} From 856524433dd8f5a746aa320c058e7c00e61fcd79 Mon Sep 17 00:00:00 2001 From: Christopher Constable Date: Fri, 28 Aug 2026 11:27:32 -0400 Subject: [PATCH 13/23] Update temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java Co-authored-by: Justin Anderson <44687433+jmaeagle99@users.noreply.github.com> --- .../src/main/java/io/temporal/client/WorkflowClientOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java index 77e8dddbc9..bc3ffa2b79 100644 --- a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java @@ -91,7 +91,7 @@ public Builder setDataConverter(DataConverter dataConverter) { } /** - * External storage configuration uses to store/retrieve large payloads. + * External storage configuration used to store/retrieve large payloads. * *

Defaults to null. */ From 2204e4d4eb0af2be6b2e9f504052e2cd9dd4e641 Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Fri, 28 Aug 2026 11:52:00 -0400 Subject: [PATCH 14/23] address naming feedback and override a few more methods on ExternalStorageDataConverter. --- .../client/WorkflowClientInternalImpl.java | 12 +-- .../client/WorkflowClientInternal.java | 2 +- .../storage/ExternalStorageDataConverter.java | 24 ++++- .../storage/ExternalStorageReferences.java | 4 +- .../internal/worker/SingleWorkerOptions.java | 20 ++-- .../main/java/io/temporal/worker/Worker.java | 32 +++---- .../ExternalStorageDataConverterTest.java | 95 +++++++++++++++++++ .../storage/ExternalStorageRunnerTest.java | 20 ++++ 8 files changed, 172 insertions(+), 37 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientInternalImpl.java b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientInternalImpl.java index a92cab89a1..c92d1d8390 100644 --- a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientInternalImpl.java +++ b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientInternalImpl.java @@ -58,7 +58,7 @@ final class WorkflowClientInternalImpl implements WorkflowClient, WorkflowClient private final WorkerFactoryRegistry workerFactoryRegistry = new WorkerFactoryRegistry(); private final String workerGroupingKey = java.util.UUID.randomUUID().toString(); private final @Nullable HeartbeatManager heartbeatManager; - private final @Nullable ExternalStorageRunner externalStorage; + private final @Nullable ExternalStorageRunner externalStorageRunner; /** * Creates client that connects to an instance of the Temporal Service. Cannot be used from within @@ -109,9 +109,9 @@ public static WorkflowClient newInstance( .getOptions() .getMetricsScope() .tagged(MetricsTag.defaultTags(options.getNamespace())); - ExternalStorage externalStorageConfig = options.getExternalStorage(); - this.externalStorage = - externalStorageConfig == null ? null : ExternalStorageRunner.create(externalStorageConfig); + ExternalStorage externalStorage = options.getExternalStorage(); + this.externalStorageRunner = + externalStorage == null ? null : ExternalStorageRunner.create(externalStorage); this.genericClient = new GenericWorkflowClientImpl(workflowServiceStubs, metricsScope); this.interceptors = options.getInterceptors(); this.workflowClientCallsInvoker = initializeClientInvoker(); @@ -823,8 +823,8 @@ public HeartbeatManager getHeartbeatManager() { @Override @Nullable - public ExternalStorageRunner getExternalStorage() { - return externalStorage; + public ExternalStorageRunner getExternalStorageRunner() { + return externalStorageRunner; } @Override diff --git a/temporal-sdk/src/main/java/io/temporal/internal/client/WorkflowClientInternal.java b/temporal-sdk/src/main/java/io/temporal/internal/client/WorkflowClientInternal.java index 7d351ae186..982ae56724 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/client/WorkflowClientInternal.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/client/WorkflowClientInternal.java @@ -28,5 +28,5 @@ public interface WorkflowClientInternal { HeartbeatManager getHeartbeatManager(); @Nullable - ExternalStorageRunner getExternalStorage(); + ExternalStorageRunner getExternalStorageRunner(); } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageDataConverter.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageDataConverter.java index d168d22f56..4c725752a2 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageDataConverter.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageDataConverter.java @@ -16,8 +16,8 @@ /** * A {@link DataConverter} that stores/retrieves payloads to/from external storage. * - *

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. + *

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 { @@ -80,6 +80,17 @@ public T fromPayloads( return delegate.fromPayload(resolved, parameterType, genericParameterType); } + @Override + public Object[] fromPayloads( + Optional content, Class[] parameterTypes, Type[] genericParameterTypes) + throws DataConverterException { + if (!content.isPresent()) { + return delegate.fromPayloads(content, parameterTypes, genericParameterTypes); + } + return delegate.fromPayloads( + Optional.of(retrieveAll(content.get())), parameterTypes, genericParameterTypes); + } + @Override @Nonnull public RuntimeException failureToException(@Nonnull Failure failure) { @@ -99,6 +110,15 @@ public DataConverter withContext(@Nonnull SerializationContext context) { delegate.withContext(context), externalStorage, storageTarget); } + private Payloads retrieveAll(Payloads payloads) { + for (Payload payload : payloads.getPayloadsList()) { + if (ExternalStorageReferences.isReference(payload)) { + return retrieveMessage(payloads); + } + } + return payloads; + } + private Payload retrieve(Payload payload) { if (!ExternalStorageReferences.isReference(payload)) { return payload; diff --git a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageReferences.java b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageReferences.java index 1a81e3b676..8d2695e995 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageReferences.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/payload/storage/ExternalStorageReferences.java @@ -9,7 +9,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -public final class ExternalStorageReferences { +final class ExternalStorageReferences { private static final String ENCODING_PROTOBUF_JSON = "json/protobuf"; private static final String REFERENCE_MESSAGE_TYPE = ExternalStorageReference.getDescriptor().getFullName(); @@ -79,7 +79,7 @@ static Payload toReferencePayload( } /** True if {@code payload} has an external storage reference encoding and message type. */ - public static boolean isReference(Payload payload) { + static boolean isReference(Payload payload) { return hasMetadata(payload, EncodingKeys.METADATA_ENCODING_KEY, ENCODING_PROTOBUF_JSON) && hasMetadata(payload, EncodingKeys.METADATA_MESSAGE_TYPE_KEY, REFERENCE_MESSAGE_TYPE); } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java b/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java index 21c9a5f60b..a34e55d904 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/worker/SingleWorkerOptions.java @@ -47,7 +47,7 @@ public static final class Builder { private boolean allowActivityHeartbeatDuringShutdown; private String workerControlTaskQueue; private PreferredVersionProvider preferredVersionProvider; - private @Nullable ExternalStorageRunner externalStorage; + private @Nullable ExternalStorageRunner externalStorageRunner; private Builder() {} @@ -76,7 +76,7 @@ private Builder(SingleWorkerOptions options) { this.allowActivityHeartbeatDuringShutdown = options.getAllowActivityHeartbeatDuringShutdown(); this.workerControlTaskQueue = options.getWorkerControlTaskQueue(); this.preferredVersionProvider = options.getPreferredVersionProvider(); - this.externalStorage = options.getExternalStorage(); + this.externalStorageRunner = options.getExternalStorageRunner(); } public Builder setIdentity(String identity) { @@ -189,8 +189,8 @@ public Builder setPreferredVersionProvider(PreferredVersionProvider preferredVer return this; } - public Builder setExternalStorage(@Nullable ExternalStorageRunner externalStorage) { - this.externalStorage = externalStorage; + public Builder setExternalStorageRunner(@Nullable ExternalStorageRunner externalStorageRunner) { + this.externalStorageRunner = externalStorageRunner; return this; } @@ -237,7 +237,7 @@ public SingleWorkerOptions build() { this.allowActivityHeartbeatDuringShutdown, this.workerControlTaskQueue, this.preferredVersionProvider, - this.externalStorage); + this.externalStorageRunner); } } @@ -262,7 +262,7 @@ public SingleWorkerOptions build() { private final boolean allowActivityHeartbeatDuringShutdown; private final String workerControlTaskQueue; private final PreferredVersionProvider preferredVersionProvider; - private final @Nullable ExternalStorageRunner externalStorage; + private final @Nullable ExternalStorageRunner externalStorageRunner; private SingleWorkerOptions( String identity, @@ -286,7 +286,7 @@ private SingleWorkerOptions( boolean allowActivityHeartbeatDuringShutdown, String workerControlTaskQueue, PreferredVersionProvider preferredVersionProvider, - @Nullable ExternalStorageRunner externalStorage) { + @Nullable ExternalStorageRunner externalStorageRunner) { this.identity = identity; this.binaryChecksum = binaryChecksum; this.buildId = buildId; @@ -308,7 +308,7 @@ private SingleWorkerOptions( this.allowActivityHeartbeatDuringShutdown = allowActivityHeartbeatDuringShutdown; this.workerControlTaskQueue = workerControlTaskQueue; this.preferredVersionProvider = preferredVersionProvider; - this.externalStorage = externalStorage; + this.externalStorageRunner = externalStorageRunner; } public String getIdentity() { @@ -407,8 +407,8 @@ public PreferredVersionProvider getPreferredVersionProvider() { } @Nullable - public ExternalStorageRunner getExternalStorage() { - return externalStorage; + public ExternalStorageRunner getExternalStorageRunner() { + return externalStorageRunner; } public WorkerVersioningOptions getWorkerVersioningOptions() { diff --git a/temporal-sdk/src/main/java/io/temporal/worker/Worker.java b/temporal-sdk/src/main/java/io/temporal/worker/Worker.java index 2e0040db10..818308aace 100644 --- a/temporal-sdk/src/main/java/io/temporal/worker/Worker.java +++ b/temporal-sdk/src/main/java/io/temporal/worker/Worker.java @@ -125,8 +125,8 @@ private static final class TaskSnapshot { this.options = WorkerOptions.newBuilder(options).validateAndBuildWithDefaults(); this.clientOptions = client.getOptions(); this.cache = cache; - ExternalStorageRunner externalStorage = - ((WorkflowClientInternal) client.getInternal()).getExternalStorage(); + ExternalStorageRunner externalStorageRunner = + ((WorkflowClientInternal) client.getInternal()).getExternalStorageRunner(); factoryOptions = WorkerFactoryOptions.newBuilder(factoryOptions).validateAndBuildWithDefaults(); WorkflowClientOptions clientOptions = client.getOptions(); String namespace = clientOptions.getNamespace(); @@ -154,7 +154,7 @@ private static final class TaskSnapshot { taggedScope, workerInstanceKey, workerControlTaskQueue, - externalStorage, + externalStorageRunner, activityTaskAutoEnrollEligible); if (this.options.isLocalActivityWorkerOnly()) { activityWorker = null; @@ -190,7 +190,7 @@ private static final class TaskSnapshot { taggedScope, workerInstanceKey, workerControlTaskQueue, - externalStorage, + externalStorageRunner, nexusTaskAutoEnrollEligible); SlotSupplier nexusSlotSupplier = this.options.getWorkerTuner() == null @@ -212,7 +212,7 @@ private static final class TaskSnapshot { taggedScope, workerInstanceKey, workerControlTaskQueue, - externalStorage, + externalStorageRunner, workflowTaskAutoEnrollEligible); SingleWorkerOptions localActivityOptions = toLocalActivityOptions( @@ -223,7 +223,7 @@ private static final class TaskSnapshot { taggedScope, workerInstanceKey, workerControlTaskQueue, - externalStorage); + externalStorageRunner); SlotSupplier workflowSlotSupplier = this.options.getWorkerTuner() == null @@ -923,7 +923,7 @@ private static SingleWorkerOptions toActivityOptions( Scope metricsScope, String workerInstanceKey, String workerControlTaskQueue, - @Nullable ExternalStorageRunner externalStorage, + @Nullable ExternalStorageRunner externalStorageRunner, boolean autoEnrollEligible) { return toSingleWorkerOptions( factoryOptions, @@ -932,7 +932,7 @@ private static SingleWorkerOptions toActivityOptions( contextPropagators, workerInstanceKey, workerControlTaskQueue, - externalStorage) + externalStorageRunner) .setUsingVirtualThreads(options.isUsingVirtualThreadsOnActivityWorker()) .setAllowActivityHeartbeatDuringShutdown(options.getAllowActivityHeartbeatDuringShutdown()) .setPollerOptions( @@ -958,7 +958,7 @@ private static SingleWorkerOptions toNexusOptions( Scope metricsScope, String workerInstanceKey, String workerControlTaskQueue, - @Nullable ExternalStorageRunner externalStorage, + @Nullable ExternalStorageRunner externalStorageRunner, boolean autoEnrollEligible) { return toSingleWorkerOptions( factoryOptions, @@ -967,7 +967,7 @@ private static SingleWorkerOptions toNexusOptions( contextPropagators, workerInstanceKey, workerControlTaskQueue, - externalStorage) + externalStorageRunner) .setPollerOptions( PollerOptions.newBuilder() .setPollerBehavior( @@ -992,7 +992,7 @@ private static SingleWorkerOptions toWorkflowWorkerOptions( Scope metricsScope, String workerInstanceKey, String workerControlTaskQueue, - @Nullable ExternalStorageRunner externalStorage, + @Nullable ExternalStorageRunner externalStorageRunner, boolean autoEnrollEligible) { Map tags = new ImmutableMap.Builder(1).put(MetricsTag.TASK_QUEUE, taskQueue).build(); @@ -1029,7 +1029,7 @@ private static SingleWorkerOptions toWorkflowWorkerOptions( contextPropagators, workerInstanceKey, workerControlTaskQueue, - externalStorage) + externalStorageRunner) .setPollerOptions( PollerOptions.newBuilder() .setPollerBehavior( @@ -1055,7 +1055,7 @@ private static SingleWorkerOptions toLocalActivityOptions( Scope metricsScope, String workerInstanceKey, String workerControlTaskQueue, - @Nullable ExternalStorageRunner externalStorage) { + @Nullable ExternalStorageRunner externalStorageRunner) { return toSingleWorkerOptions( factoryOptions, options, @@ -1063,7 +1063,7 @@ private static SingleWorkerOptions toLocalActivityOptions( contextPropagators, workerInstanceKey, workerControlTaskQueue, - externalStorage) + externalStorageRunner) .setPollerOptions( PollerOptions.newBuilder() .setPollerBehavior(new PollerBehaviorSimpleMaximum(1)) @@ -1083,7 +1083,7 @@ private static SingleWorkerOptions.Builder toSingleWorkerOptions( List contextPropagators, String workerInstanceKey, String workerControlTaskQueue, - @Nullable ExternalStorageRunner externalStorage) { + @Nullable ExternalStorageRunner externalStorageRunner) { String buildId = null; if (options.getBuildId() != null) { buildId = options.getBuildId(); @@ -1098,7 +1098,7 @@ private static SingleWorkerOptions.Builder toSingleWorkerOptions( return SingleWorkerOptions.newBuilder() .setDataConverter(clientOptions.getDataConverter()) - .setExternalStorage(externalStorage) + .setExternalStorageRunner(externalStorageRunner) .setIdentity(identity) .setBuildId(buildId) .setUseBuildIdForVersioning(options.isUsingBuildIdForVersioning()) diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageDataConverterTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageDataConverterTest.java index 03114aa9ef..b9a83d8920 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageDataConverterTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageDataConverterTest.java @@ -1,16 +1,20 @@ package io.temporal.internal.payload.storage; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import com.google.protobuf.ByteString; import io.temporal.api.common.v1.Payload; import io.temporal.api.common.v1.Payloads; import io.temporal.api.failure.v1.Failure; +import io.temporal.common.converter.CodecDataConverter; import io.temporal.common.converter.DataConverter; import io.temporal.common.converter.DefaultDataConverter; import io.temporal.failure.ApplicationFailure; +import io.temporal.payload.codec.PayloadCodec; import io.temporal.payload.storage.ExternalStorage; import io.temporal.payload.storage.StorageDriver; import io.temporal.payload.storage.StorageDriverClaim; @@ -18,6 +22,7 @@ import io.temporal.payload.storage.StorageDriverStoreContext; import io.temporal.payload.storage.StorageDriverTargetInfo; import io.temporal.payload.storage.StorageDriverWorkflowInfo; +import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -25,6 +30,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.Test; public class ExternalStorageDataConverterTest { @@ -116,6 +122,57 @@ public void withoutATargetTheDriverSeesNone() { assertNull(driver.lastTarget); } + @Test + public void arrayFromPayloadsRoundTrips() { + RecordingDriver driver = new RecordingDriver(); + DataConverter converter = resolving(driver, 0); + + Optional stored = converter.toPayloads("a", 42); + + Object[] values = + converter.fromPayloads( + stored, + new Class[] {String.class, Integer.class}, + new Type[] {String.class, Integer.class}); + + assertEquals("a", values[0]); + assertEquals(42, values[1]); + } + + @Test + public void arrayFromPayloadsWithAbsentContentUsesDefaults() { + DataConverter converter = resolving(new RecordingDriver(), 0); + + Object[] values = + converter.fromPayloads( + Optional.empty(), new Class[] {String.class}, new Type[] {String.class}); + + assertNull(values[0]); + } + + @Test + public void arrayFromPayloadsDecodesThroughTheCodecInOneBatch() { + RecordingDriver driver = new RecordingDriver(); + CountingCodec codec = new CountingCodec(); + DataConverter converter = + new ExternalStorageDataConverter( + new CodecDataConverter(plain, Collections.singletonList(codec)), runner(driver, 0)); + + Optional stored = converter.toPayloads("a", "b", "c"); + + assertEquals(1, codec.encodeCalls.get()); + assertTrue(driver.sawOnlyEncodedPayloads); + + Object[] values = + converter.fromPayloads( + stored, + new Class[] {String.class, String.class, String.class}, + new Type[] {String.class, String.class, String.class}); + + assertArrayEquals(new Object[] {"a", "b", "c"}, values); + assertEquals(1, codec.decodeCalls.get()); + } + private DataConverter resolving(StorageDriver driver, int threshold) { return new ExternalStorageDataConverter(plain, runner(driver, threshold)); } @@ -125,10 +182,43 @@ private static ExternalStorageRunner runner(StorageDriver driver, int threshold) ExternalStorage.newBuilder().setDriver(driver).setPayloadSizeThreshold(threshold).build()); } + /** Prefixes payload data so an unencoded payload reaching the driver is detectable. */ + private static final class CountingCodec implements PayloadCodec { + static final ByteString PREFIX = ByteString.copyFromUtf8("ENC:"); + + final AtomicInteger encodeCalls = new AtomicInteger(); + final AtomicInteger decodeCalls = new AtomicInteger(); + + @Override + public List encode(List payloads) { + encodeCalls.incrementAndGet(); + List out = new ArrayList<>(); + for (Payload payload : payloads) { + out.add(payload.toBuilder().setData(PREFIX.concat(payload.getData())).build()); + } + return out; + } + + @Override + public List decode(List payloads) { + decodeCalls.incrementAndGet(); + List out = new ArrayList<>(); + for (Payload payload : payloads) { + ByteString data = payload.getData(); + if (!data.startsWith(PREFIX)) { + throw new IllegalStateException("payload reached decode without the codec prefix"); + } + out.add(payload.toBuilder().setData(data.substring(PREFIX.size())).build()); + } + return out; + } + } + private static final class RecordingDriver implements StorageDriver { final Map objects = new HashMap<>(); final List retrievedKeys = new ArrayList<>(); volatile StorageDriverTargetInfo lastTarget; + volatile boolean sawOnlyEncodedPayloads = true; private int counter = 0; @Override @@ -145,6 +235,11 @@ public String getType() { public synchronized CompletableFuture> store( StorageDriverStoreContext context, List payloads) { lastTarget = context.getTarget(); + for (Payload payload : payloads) { + if (!payload.getData().startsWith(CountingCodec.PREFIX)) { + sawOnlyEncodedPayloads = false; + } + } List claims = new ArrayList<>(); for (Payload payload : payloads) { String key = "k-" + (counter++); diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java index d6cf71eafd..7cef800eb5 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java @@ -120,6 +120,26 @@ public void searchAttributesAreNotOffloaded() throws Exception { assertEquals(payload("indexed-value"), indexed); } + @Test + public void throwIfContainsReferenceThrowsOnANestedReference() throws Exception { + ExternalStorageRunner transformer = transformer(new InMemoryDriver("d1"), 0); + RespondWorkflowTaskCompletedRequest.Builder request = + RespondWorkflowTaskCompletedRequest.newBuilder() + .addCommands( + Command.newBuilder() + .setScheduleActivityTaskCommandAttributes( + ScheduleActivityTaskCommandAttributes.newBuilder() + .setActivityId("act-1") + .setInput( + Payloads.newBuilder().addPayloads(payload("activity-input"))))); + transformer.store(request, null, null, CancellationToken.none()); + RespondWorkflowTaskCompletedRequest stored = request.build(); + + assertThrows( + ExternalStorageNotConfiguredException.class, + () -> ExternalStorageRunner.throwIfContainsReference(stored)); + } + @Test public void throwIfContainsReferenceThrowsOnReference() throws Exception { InMemoryDriver driver = new InMemoryDriver("d1"); From 09701f9b89264d6c415a235c843e32b059f8f97f Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Fri, 28 Aug 2026 11:55:26 -0400 Subject: [PATCH 15/23] add comment to WorkflowClientOptions warning that external storage is a no-op until we've fully integrated it. --- .../src/main/java/io/temporal/client/WorkflowClientOptions.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java index bc3ffa2b79..401c976dd4 100644 --- a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java @@ -92,6 +92,8 @@ public Builder setDataConverter(DataConverter dataConverter) { /** * External storage configuration used to store/retrieve large payloads. + * + * n.b. This is currently a no-op. External storage has not been fully integrated yet. * *

Defaults to null. */ From 65f972457e54b1e0a11e068ae5d58b0f4214a242 Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Fri, 28 Aug 2026 12:07:20 -0400 Subject: [PATCH 16/23] javadoc update --- .../main/java/io/temporal/client/WorkflowClientOptions.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java index 401c976dd4..e0e6a5a7b6 100644 --- a/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java +++ b/temporal-sdk/src/main/java/io/temporal/client/WorkflowClientOptions.java @@ -92,8 +92,8 @@ public Builder setDataConverter(DataConverter dataConverter) { /** * External storage configuration used to store/retrieve large payloads. - * - * n.b. This is currently a no-op. External storage has not been fully integrated yet. + * + *

n.b. This is currently a no-op. External storage has not been fully integrated yet. * *

Defaults to null. */ From 2d2d90e979b265c31261e4ea05dbe81ddbe620cb Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Fri, 28 Aug 2026 12:08:23 -0400 Subject: [PATCH 17/23] update tests --- .../ExternalStorageDataConverterTest.java | 67 +++++++++++++------ 1 file changed, 45 insertions(+), 22 deletions(-) diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageDataConverterTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageDataConverterTest.java index b9a83d8920..b4fd9cc787 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageDataConverterTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageDataConverterTest.java @@ -154,14 +154,10 @@ public void arrayFromPayloadsWithAbsentContentUsesDefaults() { public void arrayFromPayloadsDecodesThroughTheCodecInOneBatch() { RecordingDriver driver = new RecordingDriver(); CountingCodec codec = new CountingCodec(); - DataConverter converter = - new ExternalStorageDataConverter( - new CodecDataConverter(plain, Collections.singletonList(codec)), runner(driver, 0)); + DataConverter converter = codecBacked(driver, codec); Optional stored = converter.toPayloads("a", "b", "c"); - assertEquals(1, codec.encodeCalls.get()); - assertTrue(driver.sawOnlyEncodedPayloads); Object[] values = converter.fromPayloads( @@ -173,6 +169,39 @@ public void arrayFromPayloadsDecodesThroughTheCodecInOneBatch() { assertEquals(1, codec.decodeCalls.get()); } + /** + * A codec encrypts payloads, so a driver must never see the plaintext: conversion has to run + * before the payload is handed to storage. + */ + @Test + public void driversOnlyEverSeeCodecEncodedPayloads() { + RecordingDriver driver = new RecordingDriver(); + CountingCodec codec = new CountingCodec(); + DataConverter converter = codecBacked(driver, codec); + + Optional stored = converter.toPayloads("a", "b", "c"); + + assertEquals(3, driver.objects.size()); + for (Payload payload : driver.objects.values()) { + String data = payload.getData().toStringUtf8(); + assertFalse(data.contains("\"a\"")); + assertFalse(data.contains("\"b\"")); + assertFalse(data.contains("\"c\"")); + } + + assertArrayEquals( + new Object[] {"a", "b", "c"}, + converter.fromPayloads( + stored, + new Class[] {String.class, String.class, String.class}, + new Type[] {String.class, String.class, String.class})); + } + + private DataConverter codecBacked(StorageDriver driver, PayloadCodec codec) { + return new ExternalStorageDataConverter( + new CodecDataConverter(plain, Collections.singletonList(codec)), runner(driver, 0)); + } + private DataConverter resolving(StorageDriver driver, int threshold) { return new ExternalStorageDataConverter(plain, runner(driver, threshold)); } @@ -182,9 +211,9 @@ private static ExternalStorageRunner runner(StorageDriver driver, int threshold) ExternalStorage.newBuilder().setDriver(driver).setPayloadSizeThreshold(threshold).build()); } - /** Prefixes payload data so an unencoded payload reaching the driver is detectable. */ + /** Obscures payload bytes so plaintext reaching a driver is detectable. */ private static final class CountingCodec implements PayloadCodec { - static final ByteString PREFIX = ByteString.copyFromUtf8("ENC:"); + private static final byte KEY = 0x5A; final AtomicInteger encodeCalls = new AtomicInteger(); final AtomicInteger decodeCalls = new AtomicInteger(); @@ -192,23 +221,23 @@ private static final class CountingCodec implements PayloadCodec { @Override public List encode(List payloads) { encodeCalls.incrementAndGet(); - List out = new ArrayList<>(); - for (Payload payload : payloads) { - out.add(payload.toBuilder().setData(PREFIX.concat(payload.getData())).build()); - } - return out; + return apply(payloads); } @Override public List decode(List payloads) { decodeCalls.incrementAndGet(); + return apply(payloads); + } + + private static List apply(List payloads) { List out = new ArrayList<>(); for (Payload payload : payloads) { - ByteString data = payload.getData(); - if (!data.startsWith(PREFIX)) { - throw new IllegalStateException("payload reached decode without the codec prefix"); + byte[] bytes = payload.getData().toByteArray(); + for (int i = 0; i < bytes.length; i++) { + bytes[i] ^= KEY; } - out.add(payload.toBuilder().setData(data.substring(PREFIX.size())).build()); + out.add(payload.toBuilder().setData(ByteString.copyFrom(bytes)).build()); } return out; } @@ -218,7 +247,6 @@ private static final class RecordingDriver implements StorageDriver { final Map objects = new HashMap<>(); final List retrievedKeys = new ArrayList<>(); volatile StorageDriverTargetInfo lastTarget; - volatile boolean sawOnlyEncodedPayloads = true; private int counter = 0; @Override @@ -235,11 +263,6 @@ public String getType() { public synchronized CompletableFuture> store( StorageDriverStoreContext context, List payloads) { lastTarget = context.getTarget(); - for (Payload payload : payloads) { - if (!payload.getData().startsWith(CountingCodec.PREFIX)) { - sawOnlyEncodedPayloads = false; - } - } List claims = new ArrayList<>(); for (Payload payload : payloads) { String key = "k-" + (counter++); From d40dbe584b5bff54ac583d07d055bc0e423856dc Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Tue, 18 Aug 2026 16:39:59 -0400 Subject: [PATCH 18/23] feature(extstore): integrate into workflow worker pipeline, including replay handler. --- .../replay/ReplayWorkflowTaskHandler.java | 10 +- .../ServiceWorkflowHistoryIterator.java | 21 +++- .../internal/worker/WorkflowWorker.java | 103 +++++++++++++-- .../ServiceWorkflowHistoryIteratorTest.java | 117 ++++++++++++++++++ .../internal/worker/WorkflowWorkerTest.java | 62 ++++++++++ 5 files changed, 300 insertions(+), 13 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java b/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java index f5b7cb0d29..479b08379d 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java @@ -23,6 +23,7 @@ import io.temporal.common.converter.DataConverter; import io.temporal.internal.common.ProtobufTimeUtils; import io.temporal.internal.common.WorkflowExecutionUtils; +import io.temporal.internal.payload.storage.ExternalStorageRunner; import io.temporal.internal.worker.*; import io.temporal.payload.context.WorkflowSerializationContext; import io.temporal.serviceclient.MetricsTag; @@ -77,6 +78,12 @@ public WorkflowTaskHandler.Result handleWorkflowTask(PollWorkflowTaskQueueRespon String workflowType = workflowTask.getWorkflowType().getName(); Scope metricsScope = options.getMetricsScope().tagged(ImmutableMap.of(MetricsTag.WORKFLOW_TYPE, workflowType)); + ExternalStorageRunner externalStorage = options.getExternalStorage(); + if (externalStorage == null) { + ExternalStorageRunner.throwIfContainsReference(workflowTask); + } else { + workflowTask = externalStorage.retrieve(workflowTask); + } return handleWorkflowTaskWithQuery(workflowTask.toBuilder(), metricsScope); } @@ -94,7 +101,8 @@ private Result handleWorkflowTaskWithQuery( logWorkflowTaskToBeProcessed(workflowTask, createdNew); ServiceWorkflowHistoryIterator historyIterator = - new ServiceWorkflowHistoryIterator(service, namespace, workflowTask, metricsScope); + new ServiceWorkflowHistoryIterator( + service, namespace, workflowTask, metricsScope, options.getExternalStorage()); boolean finalCommand; Result result; diff --git a/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java b/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java index 229b66186e..14598af207 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java @@ -12,12 +12,14 @@ import io.temporal.api.workflowservice.v1.GetWorkflowExecutionHistoryRequest; import io.temporal.api.workflowservice.v1.GetWorkflowExecutionHistoryResponse; import io.temporal.api.workflowservice.v1.PollWorkflowTaskQueueResponseOrBuilder; +import io.temporal.internal.payload.storage.ExternalStorageRunner; import io.temporal.internal.retryer.GrpcRetryer; import io.temporal.serviceclient.RpcRetryOptions; import io.temporal.serviceclient.WorkflowServiceStubs; import java.time.Duration; import java.util.Iterator; import java.util.NoSuchElementException; +import javax.annotation.Nullable; /** Supports iteration over history while loading new pages through calls to the service. */ class ServiceWorkflowHistoryIterator implements WorkflowHistoryIterator { @@ -29,6 +31,7 @@ class ServiceWorkflowHistoryIterator implements WorkflowHistoryIterator { private final Scope metricsScope; private final PollWorkflowTaskQueueResponseOrBuilder task; private final GrpcRetryer grpcRetryer; + private final @Nullable ExternalStorageRunner externalStorage; private Deadline deadline; private Iterator current; ByteString nextPageToken; @@ -38,10 +41,20 @@ class ServiceWorkflowHistoryIterator implements WorkflowHistoryIterator { String namespace, PollWorkflowTaskQueueResponseOrBuilder task, Scope metricsScope) { + this(service, namespace, task, metricsScope, null); + } + + ServiceWorkflowHistoryIterator( + WorkflowServiceStubs service, + String namespace, + PollWorkflowTaskQueueResponseOrBuilder task, + Scope metricsScope, + @Nullable ExternalStorageRunner externalStorage) { this.service = service; this.namespace = namespace; this.task = task; this.metricsScope = metricsScope; + this.externalStorage = externalStorage; // TODO Refactor WorkflowHistoryIteratorTest or WorkflowHistoryIterator to remove this check. // `service == null` shouldn't be allowed as it's needed for a normal functioning of this // class. @@ -64,7 +77,13 @@ public boolean hasNext() { // true. GetWorkflowExecutionHistoryResponse response = queryWorkflowExecutionHistory(); - current = response.getHistory().getEventsList().iterator(); + History history = response.getHistory(); + if (externalStorage == null) { + ExternalStorageRunner.throwIfContainsReference(history); + } else { + history = externalStorage.retrieve(history); + } + current = history.getEventsList().iterator(); nextPageToken = response.getNextPageToken(); // Server can return an empty page, but a valid nextPageToken that contains // more events. diff --git a/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java b/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java index 3eed1099d3..c23f4cade5 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java @@ -6,21 +6,31 @@ import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.protobuf.ByteString; +import com.google.protobuf.MessageOrBuilder; import com.uber.m3.tally.Scope; import com.uber.m3.tally.Stopwatch; import com.uber.m3.util.ImmutableMap; import io.grpc.StatusRuntimeException; +import io.temporal.api.command.v1.ScheduleActivityTaskCommandAttributesOrBuilder; +import io.temporal.api.command.v1.SignalExternalWorkflowExecutionCommandAttributesOrBuilder; +import io.temporal.api.command.v1.StartChildWorkflowExecutionCommandAttributesOrBuilder; import io.temporal.api.common.v1.WorkflowExecution; import io.temporal.api.enums.v1.QueryResultType; import io.temporal.api.enums.v1.TaskQueueKind; import io.temporal.api.enums.v1.WorkflowTaskFailedCause; import io.temporal.api.failure.v1.Failure; import io.temporal.api.workflowservice.v1.*; +import io.temporal.common.CancellationToken; import io.temporal.failure.ApplicationFailure; import io.temporal.internal.logging.LoggerTag; +import io.temporal.internal.payload.storage.ExternalStorageRunner; +import io.temporal.internal.payload.visitor.MessageVisitor; import io.temporal.internal.retryer.GrpcMessageTooLargeException; import io.temporal.internal.retryer.GrpcRetryer; import io.temporal.payload.context.WorkflowSerializationContext; +import io.temporal.payload.storage.StorageDriverActivityInfo; +import io.temporal.payload.storage.StorageDriverTargetInfo; +import io.temporal.payload.storage.StorageDriverWorkflowInfo; import io.temporal.serviceclient.MetricsTag; import io.temporal.serviceclient.RpcRetryOptions; import io.temporal.serviceclient.WorkflowServiceStubs; @@ -381,6 +391,57 @@ public String toString() { options.getIdentity(), namespace, taskQueue); } + private void storeOutboundPayloads( + com.google.protobuf.Message.Builder builder, @Nullable StorageDriverTargetInfo target) { + ExternalStorageRunner externalStorage = options.getExternalStorage(); + if (externalStorage != null) { + externalStorage.store(builder, target); + } + } + + private void storeOutboundPayloads( + com.google.protobuf.Message.Builder builder, + @Nullable StorageDriverTargetInfo target, + MessageVisitor targetVisitor) { + ExternalStorageRunner externalStorage = options.getExternalStorage(); + if (externalStorage != null) { + externalStorage.store(builder, target, targetVisitor, CancellationToken.none()); + } + } + + @Nullable + private StorageDriverTargetInfo workflowStorageTarget( + WorkflowExecution execution, String workflowType) { + if (options.getExternalStorage() == null) { + return null; + } + return new StorageDriverWorkflowInfo( + namespace, execution.getWorkflowId(), execution.getRunId(), workflowType); + } + + static StorageDriverTargetInfo refineStorageTarget( + String namespace, StorageDriverTargetInfo current, MessageOrBuilder message) { + if (message instanceof ScheduleActivityTaskCommandAttributesOrBuilder) { + ScheduleActivityTaskCommandAttributesOrBuilder attrs = + (ScheduleActivityTaskCommandAttributesOrBuilder) message; + return new StorageDriverActivityInfo( + namespace, attrs.getActivityId(), null, attrs.getActivityType().getName()); + } + if (message instanceof StartChildWorkflowExecutionCommandAttributesOrBuilder) { + StartChildWorkflowExecutionCommandAttributesOrBuilder attrs = + (StartChildWorkflowExecutionCommandAttributesOrBuilder) message; + return new StorageDriverWorkflowInfo( + namespace, attrs.getWorkflowId(), null, attrs.getWorkflowType().getName()); + } + if (message instanceof SignalExternalWorkflowExecutionCommandAttributesOrBuilder) { + WorkflowExecution execution = + ((SignalExternalWorkflowExecutionCommandAttributesOrBuilder) message).getExecution(); + return new StorageDriverWorkflowInfo( + namespace, execution.getWorkflowId(), execution.getRunId(), null); + } + return current; + } + private class TaskHandlerImpl implements PollTaskExecutor.TaskHandler { final WorkflowTaskHandler handler; @@ -453,7 +514,10 @@ public void handle(WorkflowTask task) throws Exception { if (queryCompleted != null) { try { sendDirectQueryCompletedResponse( - currentTask.getTaskToken(), queryCompleted.toBuilder(), workflowTypeScope); + currentTask.getTaskToken(), + queryCompleted.toBuilder(), + workflowTypeScope, + workflowStorageTarget(workflowExecution, workflowType)); } catch (StatusRuntimeException e) { GrpcMessageTooLargeException tooLargeException = GrpcMessageTooLargeException.tryWrap(e); @@ -473,7 +537,10 @@ public void handle(WorkflowTask task) throws Exception { .setErrorMessage(failure.getMessage()) .setFailure(failure); sendDirectQueryCompletedResponse( - currentTask.getTaskToken(), queryFailedBuilder, workflowTypeScope); + currentTask.getTaskToken(), + queryFailedBuilder, + workflowTypeScope, + workflowStorageTarget(workflowExecution, workflowType)); } } else { try { @@ -489,7 +556,8 @@ public void handle(WorkflowTask task) throws Exception { currentTask.getTaskToken(), requestBuilder, result.getRequestRetryOptions(), - workflowTypeScope); + workflowTypeScope, + workflowStorageTarget(workflowExecution, workflowType)); // If we were processing a speculative WFT the server may instruct us that the // task was dropped by resting out event ID. long resetEventId = response.getResetHistoryEventId(); @@ -509,7 +577,8 @@ public void handle(WorkflowTask task) throws Exception { currentTask.getTaskToken(), taskFailed.toBuilder(), result.getRequestRetryOptions(), - workflowTypeScope); + workflowTypeScope, + workflowStorageTarget(workflowExecution, workflowType)); } // Apply post-completion metrics only if runnable present and the above succeeded @@ -546,7 +615,8 @@ public void handle(WorkflowTask task) throws Exception { currentTask.getTaskToken(), taskFailedBuilder, result.getRequestRetryOptions(), - workflowTypeScope); + workflowTypeScope, + workflowStorageTarget(workflowExecution, workflowType)); } } } catch (Exception e) { @@ -651,7 +721,8 @@ private RespondWorkflowTaskCompletedResponse sendTaskCompleted( ByteString taskToken, RespondWorkflowTaskCompletedRequest.Builder taskCompleted, RpcRetryOptions retryOptions, - Scope workflowTypeMetricsScope) { + Scope workflowTypeMetricsScope, + @Nullable StorageDriverTargetInfo storageTarget) { GrpcRetryer.GrpcRetryerOptions grpcRetryOptions = new GrpcRetryer.GrpcRetryerOptions( RpcRetryOptions.newBuilder().buildWithDefaultsFrom(retryOptions), null); @@ -674,12 +745,16 @@ private RespondWorkflowTaskCompletedResponse sendTaskCompleted( taskCompleted.setBinaryChecksum(options.getBuildId()); } + MessageVisitor storageTargetVisitor = + (current, message) -> refineStorageTarget(namespace, current, message); + storeOutboundPayloads(taskCompleted, storageTarget, storageTargetVisitor); + RespondWorkflowTaskCompletedRequest request = taskCompleted.build(); return grpcRetryer.retryWithResult( () -> service .blockingStub() .withOption(METRICS_TAGS_CALL_OPTIONS_KEY, workflowTypeMetricsScope) - .respondWorkflowTaskCompleted(taskCompleted.build()), + .respondWorkflowTaskCompleted(request), grpcRetryOptions); } @@ -688,7 +763,8 @@ private void sendTaskFailed( ByteString taskToken, RespondWorkflowTaskFailedRequest.Builder taskFailed, RpcRetryOptions retryOptions, - Scope workflowTypeMetricsScope) { + Scope workflowTypeMetricsScope, + @Nullable StorageDriverTargetInfo storageTarget) { GrpcRetryer.GrpcRetryerOptions grpcRetryOptions = new GrpcRetryer.GrpcRetryerOptions( RpcRetryOptions.newBuilder().buildWithDefaultsFrom(retryOptions), null); @@ -702,25 +778,30 @@ private void sendTaskFailed( taskFailed.setWorkerVersion(options.workerVersionStamp()); } + storeOutboundPayloads(taskFailed, storageTarget); + RespondWorkflowTaskFailedRequest request = taskFailed.build(); grpcRetryer.retry( () -> service .blockingStub() .withOption(METRICS_TAGS_CALL_OPTIONS_KEY, workflowTypeMetricsScope) - .respondWorkflowTaskFailed(taskFailed.build()), + .respondWorkflowTaskFailed(request), grpcRetryOptions); } private void sendDirectQueryCompletedResponse( ByteString taskToken, RespondQueryTaskCompletedRequest.Builder queryCompleted, - Scope workflowTypeMetricsScope) { + Scope workflowTypeMetricsScope, + @Nullable StorageDriverTargetInfo storageTarget) { queryCompleted.setTaskToken(taskToken).setNamespace(namespace); + storeOutboundPayloads(queryCompleted, storageTarget); + RespondQueryTaskCompletedRequest request = queryCompleted.build(); // Do not retry query response service .blockingStub() .withOption(METRICS_TAGS_CALL_OPTIONS_KEY, workflowTypeMetricsScope) - .respondQueryTaskCompleted(queryCompleted.build()); + .respondQueryTaskCompleted(request); } private void logExceptionDuringResultReporting( diff --git a/temporal-sdk/src/test/java/io/temporal/internal/replay/ServiceWorkflowHistoryIteratorTest.java b/temporal-sdk/src/test/java/io/temporal/internal/replay/ServiceWorkflowHistoryIteratorTest.java index ad0c665800..d302facdca 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/replay/ServiceWorkflowHistoryIteratorTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/replay/ServiceWorkflowHistoryIteratorTest.java @@ -1,12 +1,29 @@ package io.temporal.internal.replay; import com.google.protobuf.ByteString; +import io.temporal.api.common.v1.Payload; +import io.temporal.api.common.v1.Payloads; import io.temporal.api.history.v1.History; +import io.temporal.api.history.v1.HistoryEvent; +import io.temporal.api.history.v1.WorkflowExecutionStartedEventAttributes; import io.temporal.api.workflowservice.v1.GetWorkflowExecutionHistoryResponse; import io.temporal.api.workflowservice.v1.PollWorkflowTaskQueueResponse; +import io.temporal.internal.payload.storage.ExternalStorageNotConfiguredException; +import io.temporal.internal.payload.storage.ExternalStorageRunner; +import io.temporal.payload.storage.ExternalStorage; +import io.temporal.payload.storage.StorageDriver; +import io.temporal.payload.storage.StorageDriverClaim; +import io.temporal.payload.storage.StorageDriverRetrieveContext; +import io.temporal.payload.storage.StorageDriverStoreContext; import io.temporal.testUtils.HistoryUtils; import java.nio.charset.Charset; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.NoSuchElementException; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicInteger; import org.junit.Assert; import org.junit.Test; @@ -84,4 +101,104 @@ GetWorkflowExecutionHistoryResponse queryWorkflowExecutionHistory() { Assert.assertThrows(NoSuchElementException.class, iterator::next); Assert.assertEquals(4, timesCalledServer.get()); } + + @Test + public void resolvesExternalStorageReferencesInFetchedPages() { + ExternalStorageRunner storage = inMemoryStorage(); + History inline = historyWithInput(payload("big-input")); + History.Builder builder = inline.toBuilder(); + storage.store(builder, null); + History stored = builder.build(); + Assert.assertNotEquals( + "stored history should hold a reference, not the inline payload", inline, stored); + + ServiceWorkflowHistoryIterator iterator = fetchingIterator(stored, storage); + + HistoryEvent event = iterator.next(); + Assert.assertEquals( + payload("big-input"), + event.getWorkflowExecutionStartedEventAttributes().getInput().getPayloads(0)); + } + + @Test + public void failsLoudWhenAFetchedPageHasAReferenceAndStorageIsNotConfigured() { + History.Builder builder = historyWithInput(payload("big-input")).toBuilder(); + inMemoryStorage().store(builder, null); + History stored = builder.build(); + + ServiceWorkflowHistoryIterator iterator = fetchingIterator(stored, null); + + Assert.assertThrows(ExternalStorageNotConfiguredException.class, iterator::hasNext); + } + + private static ServiceWorkflowHistoryIterator fetchingIterator( + History page, ExternalStorageRunner storage) { + PollWorkflowTaskQueueResponse workflowTask = + PollWorkflowTaskQueueResponse.newBuilder().setNextPageToken(NEXT_PAGE_TOKEN).build(); + return new ServiceWorkflowHistoryIterator(null, "default", workflowTask, null, storage) { + @Override + GetWorkflowExecutionHistoryResponse queryWorkflowExecutionHistory() { + return GetWorkflowExecutionHistoryResponse.newBuilder().setHistory(page).build(); + } + }; + } + + private static ExternalStorageRunner inMemoryStorage() { + return ExternalStorageRunner.create( + ExternalStorage.newBuilder() + .setDriver(new InMemoryDriver()) + .setPayloadSizeThreshold(0) + .build()); + } + + private static History historyWithInput(Payload payload) { + return History.newBuilder() + .addEvents( + HistoryEvent.newBuilder() + .setWorkflowExecutionStartedEventAttributes( + WorkflowExecutionStartedEventAttributes.newBuilder() + .setInput(Payloads.newBuilder().addPayloads(payload)))) + .build(); + } + + private static Payload payload(String data) { + return Payload.newBuilder().setData(ByteString.copyFromUtf8(data)).build(); + } + + private static final class InMemoryDriver implements StorageDriver { + private final Map objects = new HashMap<>(); + private int counter = 0; + + @Override + public String getName() { + return "test"; + } + + @Override + public String getType() { + return "test.inmemory"; + } + + @Override + public synchronized CompletableFuture> store( + StorageDriverStoreContext context, List payloads) { + List claims = new ArrayList<>(); + for (Payload payload : payloads) { + String key = "k-" + (counter++); + objects.put(key, payload); + claims.add(new StorageDriverClaim(Collections.singletonMap("key", key))); + } + return CompletableFuture.completedFuture(claims); + } + + @Override + public synchronized CompletableFuture> retrieve( + StorageDriverRetrieveContext context, List claims) { + List payloads = new ArrayList<>(); + for (StorageDriverClaim claim : claims) { + payloads.add(objects.get(claim.getClaimData().get("key"))); + } + return CompletableFuture.completedFuture(payloads); + } + } } diff --git a/temporal-sdk/src/test/java/io/temporal/internal/worker/WorkflowWorkerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/worker/WorkflowWorkerTest.java index 5cd1fc8d3e..c97d85a188 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/worker/WorkflowWorkerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/worker/WorkflowWorkerTest.java @@ -12,6 +12,11 @@ import com.uber.m3.tally.RootScopeBuilder; import com.uber.m3.tally.Scope; import com.uber.m3.util.ImmutableMap; +import io.temporal.api.command.v1.CompleteWorkflowExecutionCommandAttributes; +import io.temporal.api.command.v1.ScheduleActivityTaskCommandAttributes; +import io.temporal.api.command.v1.SignalExternalWorkflowExecutionCommandAttributes; +import io.temporal.api.command.v1.StartChildWorkflowExecutionCommandAttributes; +import io.temporal.api.common.v1.ActivityType; import io.temporal.api.common.v1.WorkflowExecution; import io.temporal.api.common.v1.WorkflowType; import io.temporal.api.workflowservice.v1.*; @@ -20,6 +25,9 @@ import io.temporal.internal.replay.ReplayWorkflow; import io.temporal.internal.replay.ReplayWorkflowFactory; import io.temporal.internal.replay.ReplayWorkflowTaskHandler; +import io.temporal.payload.storage.StorageDriverActivityInfo; +import io.temporal.payload.storage.StorageDriverTargetInfo; +import io.temporal.payload.storage.StorageDriverWorkflowInfo; import io.temporal.serviceclient.WorkflowServiceStubs; import io.temporal.testUtils.Eventually; import io.temporal.testUtils.HistoryUtils; @@ -448,4 +456,58 @@ private ReplayWorkflowFactory setUpMockWorkflowFactory() throws Throwable { when(mockWorkflow.eventLoop()).thenReturn(false); return mockFactory; } + + @Test + public void refineStorageTargetPointsActivityCommandsAtTheActivity() { + StorageDriverTargetInfo workflowDefault = + new StorageDriverWorkflowInfo("ns", "wf-1", "run-1", "MyWorkflow"); + ScheduleActivityTaskCommandAttributes command = + ScheduleActivityTaskCommandAttributes.newBuilder() + .setActivityId("act-1") + .setActivityType(ActivityType.newBuilder().setName("MyActivity")) + .build(); + + assertEquals( + new StorageDriverActivityInfo("ns", "act-1", null, "MyActivity"), + WorkflowWorker.refineStorageTarget("ns", workflowDefault, command)); + } + + @Test + public void refineStorageTargetPointsChildWorkflowCommandsAtTheChild() { + StorageDriverTargetInfo parent = + new StorageDriverWorkflowInfo("ns", "parent", "parent-run", "Parent"); + StartChildWorkflowExecutionCommandAttributes command = + StartChildWorkflowExecutionCommandAttributes.newBuilder() + .setWorkflowId("child-1") + .setWorkflowType(WorkflowType.newBuilder().setName("Child")) + .build(); + + assertEquals( + new StorageDriverWorkflowInfo("ns", "child-1", null, "Child"), + WorkflowWorker.refineStorageTarget("ns", parent, command)); + } + + @Test + public void refineStorageTargetPointsSignalCommandsAtTheTargetWorkflow() { + StorageDriverTargetInfo self = new StorageDriverWorkflowInfo("ns", "self", "self-run", "Self"); + SignalExternalWorkflowExecutionCommandAttributes command = + SignalExternalWorkflowExecutionCommandAttributes.newBuilder() + .setExecution( + WorkflowExecution.newBuilder().setWorkflowId("other").setRunId("other-run")) + .build(); + + assertEquals( + new StorageDriverWorkflowInfo("ns", "other", "other-run", null), + WorkflowWorker.refineStorageTarget("ns", self, command)); + } + + @Test + public void refineStorageTargetKeepsTheCurrentTargetForOtherCommands() { + StorageDriverTargetInfo current = + new StorageDriverWorkflowInfo("ns", "wf-1", "run-1", "MyWorkflow"); + CompleteWorkflowExecutionCommandAttributes command = + CompleteWorkflowExecutionCommandAttributes.newBuilder().build(); + + assertSame(current, WorkflowWorker.refineStorageTarget("ns", current, command)); + } } From 2a012c1a987ca93cbb4ba805e62634f9879cb341 Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Mon, 24 Aug 2026 13:19:06 -0400 Subject: [PATCH 19/23] feat(extstore): make sure sticky cache miss path also retrieves external payloads after fetching history. --- .../replay/ReplayWorkflowTaskHandler.java | 6 + ...orkflowRunTaskHandlerTaskHandlerTests.java | 112 ++++++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java b/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java index 479b08379d..351d70d9e2 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java @@ -403,6 +403,12 @@ private WorkflowRunTaskHandler createStatefulHandler( .blockingStub() .withOption(METRICS_TAGS_CALL_OPTIONS_KEY, metricsScope) .getWorkflowExecutionHistory(getHistoryRequest); + ExternalStorageRunner externalStorage = options.getExternalStorage(); + if (externalStorage == null) { + ExternalStorageRunner.throwIfContainsReference(getHistoryResponse); + } else { + getHistoryResponse = externalStorage.retrieve(getHistoryResponse); + } workflowTask .setHistory(getHistoryResponse.getHistory()) .setNextPageToken(getHistoryResponse.getNextPageToken()); diff --git a/temporal-sdk/src/test/java/io/temporal/internal/replay/ReplayWorkflowRunTaskHandlerTaskHandlerTests.java b/temporal-sdk/src/test/java/io/temporal/internal/replay/ReplayWorkflowRunTaskHandlerTaskHandlerTests.java index ed6446678a..cca0db9b02 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/replay/ReplayWorkflowRunTaskHandlerTaskHandlerTests.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/replay/ReplayWorkflowRunTaskHandlerTaskHandlerTests.java @@ -7,32 +7,46 @@ import static org.junit.Assume.assumeFalse; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.protobuf.ByteString; import com.google.protobuf.util.Durations; import com.uber.m3.tally.NoopScope; +import io.temporal.api.common.v1.Payload; +import io.temporal.api.common.v1.Payloads; import io.temporal.api.enums.v1.EventType; import io.temporal.api.history.v1.History; import io.temporal.api.history.v1.HistoryEvent; import io.temporal.api.taskqueue.v1.StickyExecutionAttributes; import io.temporal.api.workflowservice.v1.*; import io.temporal.internal.common.InternalUtils; +import io.temporal.internal.payload.storage.ExternalStorageRunner; import io.temporal.internal.statemachines.ExecuteLocalActivityParameters; import io.temporal.internal.worker.SingleWorkerOptions; import io.temporal.internal.worker.WorkflowExecutorCache; import io.temporal.internal.worker.WorkflowRunLockManager; import io.temporal.internal.worker.WorkflowTaskHandler; +import io.temporal.payload.storage.ExternalStorage; +import io.temporal.payload.storage.StorageDriver; +import io.temporal.payload.storage.StorageDriverClaim; +import io.temporal.payload.storage.StorageDriverRetrieveContext; +import io.temporal.payload.storage.StorageDriverStoreContext; import io.temporal.serviceclient.Version; import io.temporal.serviceclient.WorkflowServiceStubs; import io.temporal.testUtils.HistoryUtils; import io.temporal.testing.internal.SDKTestWorkflowRule; import java.time.Duration; +import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; +import java.util.concurrent.CompletableFuture; import org.junit.Rule; import org.junit.Test; +import org.mockito.ArgumentCaptor; public class ReplayWorkflowRunTaskHandlerTaskHandlerTests { @@ -121,6 +135,67 @@ public void workflowTaskFailOnIncompleteHistory() throws Throwable { result.getTaskFailed().getFailure().getMessage()); } + @Test + public void resolvesExternalStorageReferencesInFetchedFullHistory() throws Throwable { + ExternalStorageRunner externalStorage = + ExternalStorageRunner.create( + ExternalStorage.newBuilder() + .setDriver(new InMemoryStorageDriver()) + .setPayloadSizeThreshold(0) + .build()); + PollWorkflowTaskQueueResponse fullTask = HistoryUtils.generateWorkflowTaskWithInitialHistory(); + HistoryEvent startedEvent = fullTask.getHistory().getEvents(0); + Payload input = Payload.newBuilder().setData(ByteString.copyFromUtf8("input")).build(); + History.Builder storedHistory = + fullTask.getHistory().toBuilder() + .setEvents( + 0, + startedEvent.toBuilder() + .setWorkflowExecutionStartedEventAttributes( + startedEvent.getWorkflowExecutionStartedEventAttributes().toBuilder() + .setInput(Payloads.newBuilder().addPayloads(input)))); + externalStorage.store(storedHistory, null); + + WorkflowServiceStubs client = mock(WorkflowServiceStubs.class); + when(client.getServerCapabilities()) + .thenReturn(() -> GetSystemInfoResponse.Capabilities.newBuilder().build()); + WorkflowServiceGrpc.WorkflowServiceBlockingStub blockingStub = + mock(WorkflowServiceGrpc.WorkflowServiceBlockingStub.class); + when(client.blockingStub()).thenReturn(blockingStub); + when(blockingStub.withOption(any(), any())).thenReturn(blockingStub); + when(blockingStub.getWorkflowExecutionHistory(any())) + .thenReturn( + GetWorkflowExecutionHistoryResponse.newBuilder().setHistory(storedHistory).build()); + + ReplayWorkflow workflow = mock(ReplayWorkflow.class); + when(workflow.eventLoop()).thenReturn(true); + when(workflow.getOutput()).thenReturn(Optional.empty()); + WorkflowContext workflowContext = mock(WorkflowContext.class); + when(workflowContext.getRunningUpdateHandlers()).thenReturn(new HashMap<>()); + when(workflow.getWorkflowContext()).thenReturn(workflowContext); + ReplayWorkflowFactory workflowFactory = mock(ReplayWorkflowFactory.class); + when(workflowFactory.getWorkflow(any(), any())).thenReturn(workflow); + WorkflowTaskHandler taskHandler = + new ReplayWorkflowTaskHandler( + "namespace", + workflowFactory, + new WorkflowExecutorCache(10, new WorkflowRunLockManager(), new NoopScope()), + SingleWorkerOptions.newBuilder().setExternalStorage(externalStorage).build(), + null, + Duration.ofSeconds(5), + client, + null); + + taskHandler.handleWorkflowTask( + fullTask.toBuilder().setHistory(History.getDefaultInstance()).build()); + + ArgumentCaptor event = ArgumentCaptor.forClass(HistoryEvent.class); + verify(workflow).start(event.capture(), any()); + assertEquals( + input, + event.getValue().getWorkflowExecutionStartedEventAttributes().getInput().getPayloads(0)); + } + @Test public void localActivityMeteringHelper() { ReplayWorkflowRunTaskHandler.LocalActivityMeteringHelper laMeteringHelper = @@ -231,4 +306,41 @@ private ReplayWorkflowFactory setUpMockWorkflowFactory() throws Throwable { when(mockWorkflow.getWorkflowContext()).thenReturn(mockWorkflowContext); return mockFactory; } + + private static final class InMemoryStorageDriver implements StorageDriver { + private final Map payloads = new HashMap<>(); + private int nextKey; + + @Override + public String getName() { + return "test"; + } + + @Override + public String getType() { + return "test.in-memory"; + } + + @Override + public synchronized CompletableFuture> store( + StorageDriverStoreContext context, List payloads) { + List claims = new ArrayList<>(); + for (Payload payload : payloads) { + String key = Integer.toString(nextKey++); + this.payloads.put(key, payload); + claims.add(new StorageDriverClaim(Collections.singletonMap("key", key))); + } + return CompletableFuture.completedFuture(claims); + } + + @Override + public synchronized CompletableFuture> retrieve( + StorageDriverRetrieveContext context, List claims) { + List retrieved = new ArrayList<>(); + for (StorageDriverClaim claim : claims) { + retrieved.add(payloads.get(claim.getClaimData().get("key"))); + } + return CompletableFuture.completedFuture(retrieved); + } + } } From f6a25f9ac8a1d09fbe7bc699965a3b3b196f0c05 Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Mon, 24 Aug 2026 14:05:52 -0400 Subject: [PATCH 20/23] refactor(extstore): refactor the way we derive storage targets by using command.getAttributesCase() + switch for exhaustiveness checking. --- .../internal/worker/WorkflowWorker.java | 74 +++++++++++----- .../storage/ExternalStorageRunnerTest.java | 23 +++-- .../internal/worker/WorkflowWorkerTest.java | 88 ++++++++++++++----- 3 files changed, 135 insertions(+), 50 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java b/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java index c23f4cade5..4d68511bc7 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java @@ -11,9 +11,7 @@ import com.uber.m3.tally.Stopwatch; import com.uber.m3.util.ImmutableMap; import io.grpc.StatusRuntimeException; -import io.temporal.api.command.v1.ScheduleActivityTaskCommandAttributesOrBuilder; -import io.temporal.api.command.v1.SignalExternalWorkflowExecutionCommandAttributesOrBuilder; -import io.temporal.api.command.v1.StartChildWorkflowExecutionCommandAttributesOrBuilder; +import io.temporal.api.command.v1.*; import io.temporal.api.common.v1.WorkflowExecution; import io.temporal.api.enums.v1.QueryResultType; import io.temporal.api.enums.v1.TaskQueueKind; @@ -419,27 +417,59 @@ private StorageDriverTargetInfo workflowStorageTarget( namespace, execution.getWorkflowId(), execution.getRunId(), workflowType); } - static StorageDriverTargetInfo refineStorageTarget( + static StorageDriverTargetInfo deriveStorageTarget( String namespace, StorageDriverTargetInfo current, MessageOrBuilder message) { - if (message instanceof ScheduleActivityTaskCommandAttributesOrBuilder) { - ScheduleActivityTaskCommandAttributesOrBuilder attrs = - (ScheduleActivityTaskCommandAttributesOrBuilder) message; - return new StorageDriverActivityInfo( - namespace, attrs.getActivityId(), null, attrs.getActivityType().getName()); + if (!(message instanceof CommandOrBuilder)) { + return current; } - if (message instanceof StartChildWorkflowExecutionCommandAttributesOrBuilder) { - StartChildWorkflowExecutionCommandAttributesOrBuilder attrs = - (StartChildWorkflowExecutionCommandAttributesOrBuilder) message; - return new StorageDriverWorkflowInfo( - namespace, attrs.getWorkflowId(), null, attrs.getWorkflowType().getName()); - } - if (message instanceof SignalExternalWorkflowExecutionCommandAttributesOrBuilder) { - WorkflowExecution execution = - ((SignalExternalWorkflowExecutionCommandAttributesOrBuilder) message).getExecution(); - return new StorageDriverWorkflowInfo( - namespace, execution.getWorkflowId(), execution.getRunId(), null); + CommandOrBuilder command = (CommandOrBuilder) message; + // Keep this exhaustive so new command attributes require an explicit target decision. + switch (command.getAttributesCase()) { + case SCHEDULE_ACTIVITY_TASK_COMMAND_ATTRIBUTES: + ScheduleActivityTaskCommandAttributesOrBuilder activity = + command.getScheduleActivityTaskCommandAttributesOrBuilder(); + return new StorageDriverActivityInfo( + namespace, activity.getActivityId(), null, activity.getActivityType().getName()); + case START_CHILD_WORKFLOW_EXECUTION_COMMAND_ATTRIBUTES: + StartChildWorkflowExecutionCommandAttributesOrBuilder child = + command.getStartChildWorkflowExecutionCommandAttributesOrBuilder(); + return new StorageDriverWorkflowInfo( + namespace, child.getWorkflowId(), null, child.getWorkflowType().getName()); + case SIGNAL_EXTERNAL_WORKFLOW_EXECUTION_COMMAND_ATTRIBUTES: + WorkflowExecution execution = + command.getSignalExternalWorkflowExecutionCommandAttributes().getExecution(); + return new StorageDriverWorkflowInfo( + namespace, execution.getWorkflowId(), execution.getRunId(), null); + case CONTINUE_AS_NEW_WORKFLOW_EXECUTION_COMMAND_ATTRIBUTES: + if (current instanceof StorageDriverWorkflowInfo) { + ContinueAsNewWorkflowExecutionCommandAttributesOrBuilder continueAsNew = + command.getContinueAsNewWorkflowExecutionCommandAttributesOrBuilder(); + StorageDriverWorkflowInfo currentWorkflow = (StorageDriverWorkflowInfo) current; + String workflowType = continueAsNew.getWorkflowType().getName(); + return new StorageDriverWorkflowInfo( + namespace, + currentWorkflow.getId(), + null, + Strings.isNullOrEmpty(workflowType) ? currentWorkflow.getType() : workflowType); + } + return current; + case ATTRIBUTES_NOT_SET: + case START_TIMER_COMMAND_ATTRIBUTES: + case COMPLETE_WORKFLOW_EXECUTION_COMMAND_ATTRIBUTES: + case FAIL_WORKFLOW_EXECUTION_COMMAND_ATTRIBUTES: + case REQUEST_CANCEL_ACTIVITY_TASK_COMMAND_ATTRIBUTES: + case CANCEL_TIMER_COMMAND_ATTRIBUTES: + case CANCEL_WORKFLOW_EXECUTION_COMMAND_ATTRIBUTES: + case REQUEST_CANCEL_EXTERNAL_WORKFLOW_EXECUTION_COMMAND_ATTRIBUTES: + case RECORD_MARKER_COMMAND_ATTRIBUTES: + case UPSERT_WORKFLOW_SEARCH_ATTRIBUTES_COMMAND_ATTRIBUTES: + case PROTOCOL_MESSAGE_COMMAND_ATTRIBUTES: + case MODIFY_WORKFLOW_PROPERTIES_COMMAND_ATTRIBUTES: + case SCHEDULE_NEXUS_OPERATION_COMMAND_ATTRIBUTES: + case REQUEST_CANCEL_NEXUS_OPERATION_COMMAND_ATTRIBUTES: + return current; } - return current; + throw new IllegalStateException("Unhandled command attributes: " + command.getAttributesCase()); } private class TaskHandlerImpl implements PollTaskExecutor.TaskHandler { @@ -746,7 +776,7 @@ private RespondWorkflowTaskCompletedResponse sendTaskCompleted( } MessageVisitor storageTargetVisitor = - (current, message) -> refineStorageTarget(namespace, current, message); + (current, message) -> deriveStorageTarget(namespace, current, message); storeOutboundPayloads(taskCompleted, storageTarget, storageTargetVisitor); RespondWorkflowTaskCompletedRequest request = taskCompleted.build(); return grpcRetryer.retryWithResult( diff --git a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java index 7cef800eb5..ff23eeb55e 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/payload/storage/ExternalStorageRunnerTest.java @@ -8,6 +8,7 @@ import com.google.protobuf.ByteString; import io.temporal.api.command.v1.Command; +import io.temporal.api.command.v1.CommandOrBuilder; import io.temporal.api.command.v1.CompleteWorkflowExecutionCommandAttributes; import io.temporal.api.command.v1.ScheduleActivityTaskCommandAttributes; import io.temporal.api.command.v1.ScheduleActivityTaskCommandAttributesOrBuilder; @@ -16,6 +17,7 @@ import io.temporal.api.common.v1.Payload; import io.temporal.api.common.v1.Payloads; import io.temporal.api.common.v1.SearchAttributes; +import io.temporal.api.sdk.v1.UserMetadata; import io.temporal.api.workflowservice.v1.RespondWorkflowTaskCompletedRequest; import io.temporal.common.CancellationToken; import io.temporal.internal.concurrent.structured.CancelSource; @@ -160,7 +162,7 @@ public void throwIfContainsReferenceAllowsInlinePayloads() { } @Test - public void storeAppliesPerCommandTargetFromMessageVisitor() { + public void storeScopesCommandTargetOverAttributesAndMetadata() { TargetCapturingDriver driver = new TargetCapturingDriver("d1"); ExternalStorageRunner storage = transformer(driver, 0); @@ -168,6 +170,8 @@ public void storeAppliesPerCommandTargetFromMessageVisitor() { RespondWorkflowTaskCompletedRequest.newBuilder() .addCommands( Command.newBuilder() + .setUserMetadata( + UserMetadata.newBuilder().setSummary(payload("activity-summary"))) .setScheduleActivityTaskCommandAttributes( ScheduleActivityTaskCommandAttributes.newBuilder() .setActivityId("act-1") @@ -184,11 +188,15 @@ public void storeAppliesPerCommandTargetFromMessageVisitor() { new StorageDriverWorkflowInfo("ns", "wf-1", "run-1", "MyWorkflow"); MessageVisitor visitor = (current, message) -> { - if (message instanceof ScheduleActivityTaskCommandAttributesOrBuilder) { - ScheduleActivityTaskCommandAttributesOrBuilder attrs = - (ScheduleActivityTaskCommandAttributesOrBuilder) message; - return new StorageDriverActivityInfo( - "ns", attrs.getActivityId(), null, attrs.getActivityType().getName()); + if (message instanceof CommandOrBuilder) { + CommandOrBuilder command = (CommandOrBuilder) message; + if (command.getAttributesCase() + == Command.AttributesCase.SCHEDULE_ACTIVITY_TASK_COMMAND_ATTRIBUTES) { + ScheduleActivityTaskCommandAttributesOrBuilder attrs = + command.getScheduleActivityTaskCommandAttributesOrBuilder(); + return new StorageDriverActivityInfo( + "ns", attrs.getActivityId(), null, attrs.getActivityType().getName()); + } } return current; }; @@ -198,6 +206,9 @@ public void storeAppliesPerCommandTargetFromMessageVisitor() { assertEquals( new StorageDriverActivityInfo("ns", "act-1", null, "MyActivity"), driver.targetFor("activity-input")); + assertEquals( + new StorageDriverActivityInfo("ns", "act-1", null, "MyActivity"), + driver.targetFor("activity-summary")); assertEquals(workflowTarget, driver.targetFor("wf-result")); } diff --git a/temporal-sdk/src/test/java/io/temporal/internal/worker/WorkflowWorkerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/worker/WorkflowWorkerTest.java index c97d85a188..2554227fcb 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/worker/WorkflowWorkerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/worker/WorkflowWorkerTest.java @@ -12,7 +12,9 @@ import com.uber.m3.tally.RootScopeBuilder; import com.uber.m3.tally.Scope; import com.uber.m3.util.ImmutableMap; +import io.temporal.api.command.v1.Command; import io.temporal.api.command.v1.CompleteWorkflowExecutionCommandAttributes; +import io.temporal.api.command.v1.ContinueAsNewWorkflowExecutionCommandAttributes; import io.temporal.api.command.v1.ScheduleActivityTaskCommandAttributes; import io.temporal.api.command.v1.SignalExternalWorkflowExecutionCommandAttributes; import io.temporal.api.command.v1.StartChildWorkflowExecutionCommandAttributes; @@ -458,56 +460,98 @@ private ReplayWorkflowFactory setUpMockWorkflowFactory() throws Throwable { } @Test - public void refineStorageTargetPointsActivityCommandsAtTheActivity() { + public void deriveStorageTargetPointsActivityCommandsAtTheActivity() { StorageDriverTargetInfo workflowDefault = new StorageDriverWorkflowInfo("ns", "wf-1", "run-1", "MyWorkflow"); - ScheduleActivityTaskCommandAttributes command = - ScheduleActivityTaskCommandAttributes.newBuilder() - .setActivityId("act-1") - .setActivityType(ActivityType.newBuilder().setName("MyActivity")) + Command command = + Command.newBuilder() + .setScheduleActivityTaskCommandAttributes( + ScheduleActivityTaskCommandAttributes.newBuilder() + .setActivityId("act-1") + .setActivityType(ActivityType.newBuilder().setName("MyActivity"))) .build(); assertEquals( new StorageDriverActivityInfo("ns", "act-1", null, "MyActivity"), - WorkflowWorker.refineStorageTarget("ns", workflowDefault, command)); + WorkflowWorker.deriveStorageTarget("ns", workflowDefault, command)); } @Test - public void refineStorageTargetPointsChildWorkflowCommandsAtTheChild() { + public void deriveStorageTargetPointsChildWorkflowCommandsAtTheChild() { StorageDriverTargetInfo parent = new StorageDriverWorkflowInfo("ns", "parent", "parent-run", "Parent"); - StartChildWorkflowExecutionCommandAttributes command = - StartChildWorkflowExecutionCommandAttributes.newBuilder() - .setWorkflowId("child-1") - .setWorkflowType(WorkflowType.newBuilder().setName("Child")) + Command command = + Command.newBuilder() + .setStartChildWorkflowExecutionCommandAttributes( + StartChildWorkflowExecutionCommandAttributes.newBuilder() + .setWorkflowId("child-1") + .setWorkflowType(WorkflowType.newBuilder().setName("Child"))) .build(); assertEquals( new StorageDriverWorkflowInfo("ns", "child-1", null, "Child"), - WorkflowWorker.refineStorageTarget("ns", parent, command)); + WorkflowWorker.deriveStorageTarget("ns", parent, command)); } @Test - public void refineStorageTargetPointsSignalCommandsAtTheTargetWorkflow() { + public void deriveStorageTargetPointsSignalCommandsAtTheTargetWorkflow() { StorageDriverTargetInfo self = new StorageDriverWorkflowInfo("ns", "self", "self-run", "Self"); - SignalExternalWorkflowExecutionCommandAttributes command = - SignalExternalWorkflowExecutionCommandAttributes.newBuilder() - .setExecution( - WorkflowExecution.newBuilder().setWorkflowId("other").setRunId("other-run")) + Command command = + Command.newBuilder() + .setSignalExternalWorkflowExecutionCommandAttributes( + SignalExternalWorkflowExecutionCommandAttributes.newBuilder() + .setExecution( + WorkflowExecution.newBuilder() + .setWorkflowId("other") + .setRunId("other-run"))) .build(); assertEquals( new StorageDriverWorkflowInfo("ns", "other", "other-run", null), - WorkflowWorker.refineStorageTarget("ns", self, command)); + WorkflowWorker.deriveStorageTarget("ns", self, command)); } @Test - public void refineStorageTargetKeepsTheCurrentTargetForOtherCommands() { + public void deriveStorageTargetPointsContinueAsNewAtTheNewRun() { + StorageDriverTargetInfo current = + new StorageDriverWorkflowInfo("ns", "wf-1", "run-1", "CurrentWorkflow"); + Command command = + Command.newBuilder() + .setContinueAsNewWorkflowExecutionCommandAttributes( + ContinueAsNewWorkflowExecutionCommandAttributes.newBuilder() + .setWorkflowType(WorkflowType.newBuilder().setName("NextWorkflow"))) + .build(); + + assertEquals( + new StorageDriverWorkflowInfo("ns", "wf-1", null, "NextWorkflow"), + WorkflowWorker.deriveStorageTarget("ns", current, command)); + } + + @Test + public void deriveStorageTargetKeepsWorkflowTypeForContinueAsNewWithoutOverride() { + StorageDriverTargetInfo current = + new StorageDriverWorkflowInfo("ns", "wf-1", "run-1", "CurrentWorkflow"); + Command command = + Command.newBuilder() + .setContinueAsNewWorkflowExecutionCommandAttributes( + ContinueAsNewWorkflowExecutionCommandAttributes.newBuilder()) + .build(); + + assertEquals( + new StorageDriverWorkflowInfo("ns", "wf-1", null, "CurrentWorkflow"), + WorkflowWorker.deriveStorageTarget("ns", current, command)); + } + + @Test + public void deriveStorageTargetKeepsTheCurrentTargetForOtherCommands() { StorageDriverTargetInfo current = new StorageDriverWorkflowInfo("ns", "wf-1", "run-1", "MyWorkflow"); - CompleteWorkflowExecutionCommandAttributes command = - CompleteWorkflowExecutionCommandAttributes.newBuilder().build(); + Command command = + Command.newBuilder() + .setCompleteWorkflowExecutionCommandAttributes( + CompleteWorkflowExecutionCommandAttributes.newBuilder()) + .build(); - assertSame(current, WorkflowWorker.refineStorageTarget("ns", current, command)); + assertSame(current, WorkflowWorker.deriveStorageTarget("ns", current, command)); } } From 9597d56f14f3fe5f3967e3f91237825e605ed4ca Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Thu, 27 Aug 2026 14:30:23 -0400 Subject: [PATCH 21/23] Explicitly pass cancellation tokens for external storage methods. --- .../internal/replay/ReplayWorkflowTaskHandler.java | 5 +++-- .../internal/replay/ServiceWorkflowHistoryIterator.java | 3 ++- .../java/io/temporal/internal/worker/WorkflowWorker.java | 7 ++----- .../ReplayWorkflowRunTaskHandlerTaskHandlerTests.java | 3 ++- .../replay/ServiceWorkflowHistoryIteratorTest.java | 5 +++-- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java b/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java index 351d70d9e2..3a8d66a8e4 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java @@ -20,6 +20,7 @@ import io.temporal.api.taskqueue.v1.StickyExecutionAttributes; import io.temporal.api.taskqueue.v1.TaskQueue; import io.temporal.api.workflowservice.v1.*; +import io.temporal.common.CancellationToken; import io.temporal.common.converter.DataConverter; import io.temporal.internal.common.ProtobufTimeUtils; import io.temporal.internal.common.WorkflowExecutionUtils; @@ -82,7 +83,7 @@ public WorkflowTaskHandler.Result handleWorkflowTask(PollWorkflowTaskQueueRespon if (externalStorage == null) { ExternalStorageRunner.throwIfContainsReference(workflowTask); } else { - workflowTask = externalStorage.retrieve(workflowTask); + workflowTask = externalStorage.retrieve(workflowTask, CancellationToken.none()); } return handleWorkflowTaskWithQuery(workflowTask.toBuilder(), metricsScope); } @@ -407,7 +408,7 @@ private WorkflowRunTaskHandler createStatefulHandler( if (externalStorage == null) { ExternalStorageRunner.throwIfContainsReference(getHistoryResponse); } else { - getHistoryResponse = externalStorage.retrieve(getHistoryResponse); + getHistoryResponse = externalStorage.retrieve(getHistoryResponse, CancellationToken.none()); } workflowTask .setHistory(getHistoryResponse.getHistory()) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java b/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java index 14598af207..fbb33033ba 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java @@ -12,6 +12,7 @@ import io.temporal.api.workflowservice.v1.GetWorkflowExecutionHistoryRequest; import io.temporal.api.workflowservice.v1.GetWorkflowExecutionHistoryResponse; import io.temporal.api.workflowservice.v1.PollWorkflowTaskQueueResponseOrBuilder; +import io.temporal.common.CancellationToken; import io.temporal.internal.payload.storage.ExternalStorageRunner; import io.temporal.internal.retryer.GrpcRetryer; import io.temporal.serviceclient.RpcRetryOptions; @@ -81,7 +82,7 @@ public boolean hasNext() { if (externalStorage == null) { ExternalStorageRunner.throwIfContainsReference(history); } else { - history = externalStorage.retrieve(history); + history = externalStorage.retrieve(history, CancellationToken.none()); } current = history.getEventsList().iterator(); nextPageToken = response.getNextPageToken(); diff --git a/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java b/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java index 4d68511bc7..f16fa9644f 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java @@ -391,16 +391,13 @@ public String toString() { private void storeOutboundPayloads( com.google.protobuf.Message.Builder builder, @Nullable StorageDriverTargetInfo target) { - ExternalStorageRunner externalStorage = options.getExternalStorage(); - if (externalStorage != null) { - externalStorage.store(builder, target); - } + storeOutboundPayloads(builder, target, null); } private void storeOutboundPayloads( com.google.protobuf.Message.Builder builder, @Nullable StorageDriverTargetInfo target, - MessageVisitor targetVisitor) { + @Nullable MessageVisitor targetVisitor) { ExternalStorageRunner externalStorage = options.getExternalStorage(); if (externalStorage != null) { externalStorage.store(builder, target, targetVisitor, CancellationToken.none()); diff --git a/temporal-sdk/src/test/java/io/temporal/internal/replay/ReplayWorkflowRunTaskHandlerTaskHandlerTests.java b/temporal-sdk/src/test/java/io/temporal/internal/replay/ReplayWorkflowRunTaskHandlerTaskHandlerTests.java index cca0db9b02..afb933c3aa 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/replay/ReplayWorkflowRunTaskHandlerTaskHandlerTests.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/replay/ReplayWorkflowRunTaskHandlerTaskHandlerTests.java @@ -20,6 +20,7 @@ import io.temporal.api.history.v1.HistoryEvent; import io.temporal.api.taskqueue.v1.StickyExecutionAttributes; import io.temporal.api.workflowservice.v1.*; +import io.temporal.common.CancellationToken; import io.temporal.internal.common.InternalUtils; import io.temporal.internal.payload.storage.ExternalStorageRunner; import io.temporal.internal.statemachines.ExecuteLocalActivityParameters; @@ -154,7 +155,7 @@ public void resolvesExternalStorageReferencesInFetchedFullHistory() throws Throw .setWorkflowExecutionStartedEventAttributes( startedEvent.getWorkflowExecutionStartedEventAttributes().toBuilder() .setInput(Payloads.newBuilder().addPayloads(input)))); - externalStorage.store(storedHistory, null); + externalStorage.store(storedHistory, null, null, CancellationToken.none()); WorkflowServiceStubs client = mock(WorkflowServiceStubs.class); when(client.getServerCapabilities()) diff --git a/temporal-sdk/src/test/java/io/temporal/internal/replay/ServiceWorkflowHistoryIteratorTest.java b/temporal-sdk/src/test/java/io/temporal/internal/replay/ServiceWorkflowHistoryIteratorTest.java index d302facdca..3eb43c3467 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/replay/ServiceWorkflowHistoryIteratorTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/replay/ServiceWorkflowHistoryIteratorTest.java @@ -8,6 +8,7 @@ import io.temporal.api.history.v1.WorkflowExecutionStartedEventAttributes; import io.temporal.api.workflowservice.v1.GetWorkflowExecutionHistoryResponse; import io.temporal.api.workflowservice.v1.PollWorkflowTaskQueueResponse; +import io.temporal.common.CancellationToken; import io.temporal.internal.payload.storage.ExternalStorageNotConfiguredException; import io.temporal.internal.payload.storage.ExternalStorageRunner; import io.temporal.payload.storage.ExternalStorage; @@ -107,7 +108,7 @@ public void resolvesExternalStorageReferencesInFetchedPages() { ExternalStorageRunner storage = inMemoryStorage(); History inline = historyWithInput(payload("big-input")); History.Builder builder = inline.toBuilder(); - storage.store(builder, null); + storage.store(builder, null, null, CancellationToken.none()); History stored = builder.build(); Assert.assertNotEquals( "stored history should hold a reference, not the inline payload", inline, stored); @@ -123,7 +124,7 @@ public void resolvesExternalStorageReferencesInFetchedPages() { @Test public void failsLoudWhenAFetchedPageHasAReferenceAndStorageIsNotConfigured() { History.Builder builder = historyWithInput(payload("big-input")).toBuilder(); - inMemoryStorage().store(builder, null); + inMemoryStorage().store(builder, null, null, CancellationToken.none()); History stored = builder.build(); ServiceWorkflowHistoryIterator iterator = fetchingIterator(stored, null); From f2f9181e47f8a51c26693886553526da1c68573b Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Thu, 27 Aug 2026 16:23:43 -0400 Subject: [PATCH 22/23] more cancellation token threading --- .../replay/ReplayWorkflowTaskHandler.java | 36 +++++++++++++++++-- .../ServiceWorkflowHistoryIterator.java | 10 ++++-- .../internal/worker/SyncWorkflowWorker.java | 15 ++++++-- .../internal/worker/WorkflowWorker.java | 8 +++-- .../ServiceWorkflowHistoryIteratorTest.java | 28 ++++++++++++++- .../internal/worker/WorkflowWorkerTest.java | 10 ++++-- 6 files changed, 92 insertions(+), 15 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java b/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java index 3a8d66a8e4..a32e521d99 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java @@ -36,6 +36,7 @@ import java.time.Duration; import java.util.List; import java.util.Objects; +import java.util.concurrent.CancellationException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -53,6 +54,7 @@ public final class ReplayWorkflowTaskHandler implements WorkflowTaskHandler { private final WorkflowServiceStubs service; private final TaskQueue stickyTaskQueue; private final LocalActivityDispatcher localActivityDispatcher; + private final CancellationToken storageCancellation; public ReplayWorkflowTaskHandler( String namespace, @@ -63,6 +65,29 @@ public ReplayWorkflowTaskHandler( Duration stickyTaskQueueScheduleToStartTimeout, WorkflowServiceStubs service, LocalActivityDispatcher localActivityDispatcher) { + this( + namespace, + asyncWorkflowFactory, + cache, + options, + stickyTaskQueue, + stickyTaskQueueScheduleToStartTimeout, + service, + localActivityDispatcher, + CancellationToken.none()); + } + + public ReplayWorkflowTaskHandler( + String namespace, + ReplayWorkflowFactory asyncWorkflowFactory, + WorkflowExecutorCache cache, + SingleWorkerOptions options, + TaskQueue stickyTaskQueue, + Duration stickyTaskQueueScheduleToStartTimeout, + WorkflowServiceStubs service, + LocalActivityDispatcher localActivityDispatcher, + CancellationToken storageCancellation) { + this.storageCancellation = storageCancellation; this.namespace = namespace; this.workflowFactory = asyncWorkflowFactory; this.cache = cache; @@ -83,7 +108,7 @@ public WorkflowTaskHandler.Result handleWorkflowTask(PollWorkflowTaskQueueRespon if (externalStorage == null) { ExternalStorageRunner.throwIfContainsReference(workflowTask); } else { - workflowTask = externalStorage.retrieve(workflowTask, CancellationToken.none()); + workflowTask = externalStorage.retrieve(workflowTask, storageCancellation); } return handleWorkflowTaskWithQuery(workflowTask.toBuilder(), metricsScope); } @@ -103,7 +128,12 @@ private Result handleWorkflowTaskWithQuery( ServiceWorkflowHistoryIterator historyIterator = new ServiceWorkflowHistoryIterator( - service, namespace, workflowTask, metricsScope, options.getExternalStorage()); + service, + namespace, + workflowTask, + metricsScope, + options.getExternalStorage(), + storageCancellation); boolean finalCommand; Result result; @@ -408,7 +438,7 @@ private WorkflowRunTaskHandler createStatefulHandler( if (externalStorage == null) { ExternalStorageRunner.throwIfContainsReference(getHistoryResponse); } else { - getHistoryResponse = externalStorage.retrieve(getHistoryResponse, CancellationToken.none()); + getHistoryResponse = externalStorage.retrieve(getHistoryResponse, storageCancellation); } workflowTask .setHistory(getHistoryResponse.getHistory()) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java b/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java index fbb33033ba..6eccdace7a 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java @@ -20,6 +20,7 @@ import java.time.Duration; import java.util.Iterator; import java.util.NoSuchElementException; +import java.util.concurrent.CancellationException; import javax.annotation.Nullable; /** Supports iteration over history while loading new pages through calls to the service. */ @@ -33,6 +34,7 @@ class ServiceWorkflowHistoryIterator implements WorkflowHistoryIterator { private final PollWorkflowTaskQueueResponseOrBuilder task; private final GrpcRetryer grpcRetryer; private final @Nullable ExternalStorageRunner externalStorage; + private final CancellationToken storageCancellation; private Deadline deadline; private Iterator current; ByteString nextPageToken; @@ -42,7 +44,7 @@ class ServiceWorkflowHistoryIterator implements WorkflowHistoryIterator { String namespace, PollWorkflowTaskQueueResponseOrBuilder task, Scope metricsScope) { - this(service, namespace, task, metricsScope, null); + this(service, namespace, task, metricsScope, null, CancellationToken.none()); } ServiceWorkflowHistoryIterator( @@ -50,7 +52,9 @@ class ServiceWorkflowHistoryIterator implements WorkflowHistoryIterator { String namespace, PollWorkflowTaskQueueResponseOrBuilder task, Scope metricsScope, - @Nullable ExternalStorageRunner externalStorage) { + @Nullable ExternalStorageRunner externalStorage, + CancellationToken storageCancellation) { + this.storageCancellation = storageCancellation; this.service = service; this.namespace = namespace; this.task = task; @@ -82,7 +86,7 @@ public boolean hasNext() { if (externalStorage == null) { ExternalStorageRunner.throwIfContainsReference(history); } else { - history = externalStorage.retrieve(history, CancellationToken.none()); + history = externalStorage.retrieve(history, storageCancellation); } current = history.getEventsList().iterator(); nextPageToken = response.getNextPageToken(); diff --git a/temporal-sdk/src/main/java/io/temporal/internal/worker/SyncWorkflowWorker.java b/temporal-sdk/src/main/java/io/temporal/internal/worker/SyncWorkflowWorker.java index be128a5e62..b58db1c1ec 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/worker/SyncWorkflowWorker.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/worker/SyncWorkflowWorker.java @@ -10,6 +10,7 @@ import io.temporal.internal.activity.ActivityExecutionContextFactory; import io.temporal.internal.activity.ActivityTaskHandlerImpl; import io.temporal.internal.activity.LocalActivityExecutionContextFactoryImpl; +import io.temporal.internal.concurrent.structured.CancelSource; import io.temporal.internal.replay.ReplayWorkflowTaskHandler; import io.temporal.internal.sync.POJOWorkflowImplementationFactory; import io.temporal.internal.sync.WorkflowThreadExecutor; @@ -54,6 +55,8 @@ public class SyncWorkflowWorker implements SuspendableWorker { private final POJOWorkflowImplementationFactory factory; private final DataConverter dataConverter; private final ActivityTaskHandlerImpl laTaskHandler; + private final CancelSource storageCancellation = + new CancelSource<>(() -> new CancellationException("Worker shutdown")); private boolean runningLocalActivityWorker; public SyncWorkflowWorker( @@ -111,7 +114,8 @@ public SyncWorkflowWorker( stickyTaskQueue, singleWorkerOptions.getStickyQueueScheduleToStartTimeout(), client.getWorkflowServiceStubs(), - laWorker.getLocalActivityScheduler()); + laWorker.getLocalActivityScheduler(), + storageCancellation.token()); workflowWorker = new WorkflowWorker( @@ -126,7 +130,8 @@ public SyncWorkflowWorker( eagerActivityDispatcher, maxEagerActivityReservationsPerWorkflowTask, slotSupplier, - namespaceCapabilities); + namespaceCapabilities, + storageCancellation.token()); // Exists to support Worker#replayWorkflowExecution functionality. // This handler has to be non-sticky to avoid evicting actual executions from the cache @@ -139,7 +144,8 @@ public SyncWorkflowWorker( null, Duration.ZERO, client.getWorkflowServiceStubs(), - laWorker.getLocalActivityScheduler()); + laWorker.getLocalActivityScheduler(), + storageCancellation.token()); queryReplayHelper = new QueryReplayHelper(nonStickyReplayTaskHandler); } @@ -175,6 +181,9 @@ public boolean start() { @Override public CompletableFuture shutdown(ShutdownManager shutdownManager, boolean interruptTasks) { + if (interruptTasks) { + storageCancellation.cancel(); + } return workflowWorker .shutdown(shutdownManager, interruptTasks) .thenCompose(ignore -> laWorker.shutdown(shutdownManager, interruptTasks)) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java b/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java index f16fa9644f..7ff7f4a200 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java @@ -35,6 +35,7 @@ import io.temporal.worker.*; import io.temporal.worker.tuning.*; import java.util.*; +import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; @@ -67,6 +68,7 @@ final class WorkflowWorker implements SuspendableWorker { private final PollerTracker pollerTracker = new PollerTracker(); private final PollerTracker stickyPollerTracker = new PollerTracker(); private final NamespaceCapabilities namespaceCapabilities; + private final CancellationToken storageCancellation; private PollTaskExecutor pollTaskExecutor; @@ -88,7 +90,8 @@ public WorkflowWorker( @Nonnull EagerActivityDispatcher eagerActivityDispatcher, int maxEagerActivityReservationsPerWorkflowTask, @Nonnull SlotSupplier slotSupplier, - @Nonnull NamespaceCapabilities namespaceCapabilities) { + @Nonnull NamespaceCapabilities namespaceCapabilities, + CancellationToken storageCancellation) { this.service = Objects.requireNonNull(service); this.namespace = Objects.requireNonNull(namespace); this.taskQueue = Objects.requireNonNull(taskQueue); @@ -105,6 +108,7 @@ public WorkflowWorker( this.maxEagerActivityReservationsPerWorkflowTask = maxEagerActivityReservationsPerWorkflowTask; this.slotSupplier = new TrackingSlotSupplier<>(slotSupplier, this.workerMetricsScope); this.namespaceCapabilities = namespaceCapabilities; + this.storageCancellation = storageCancellation; } @Override @@ -400,7 +404,7 @@ private void storeOutboundPayloads( @Nullable MessageVisitor targetVisitor) { ExternalStorageRunner externalStorage = options.getExternalStorage(); if (externalStorage != null) { - externalStorage.store(builder, target, targetVisitor, CancellationToken.none()); + externalStorage.store(builder, target, targetVisitor, storageCancellation); } } diff --git a/temporal-sdk/src/test/java/io/temporal/internal/replay/ServiceWorkflowHistoryIteratorTest.java b/temporal-sdk/src/test/java/io/temporal/internal/replay/ServiceWorkflowHistoryIteratorTest.java index 3eb43c3467..43ebf51275 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/replay/ServiceWorkflowHistoryIteratorTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/replay/ServiceWorkflowHistoryIteratorTest.java @@ -9,6 +9,7 @@ import io.temporal.api.workflowservice.v1.GetWorkflowExecutionHistoryResponse; import io.temporal.api.workflowservice.v1.PollWorkflowTaskQueueResponse; import io.temporal.common.CancellationToken; +import io.temporal.internal.concurrent.structured.CancelSource; import io.temporal.internal.payload.storage.ExternalStorageNotConfiguredException; import io.temporal.internal.payload.storage.ExternalStorageRunner; import io.temporal.payload.storage.ExternalStorage; @@ -24,6 +25,7 @@ import java.util.List; import java.util.Map; import java.util.NoSuchElementException; +import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicInteger; import org.junit.Assert; @@ -132,11 +134,35 @@ public void failsLoudWhenAFetchedPageHasAReferenceAndStorageIsNotConfigured() { Assert.assertThrows(ExternalStorageNotConfiguredException.class, iterator::hasNext); } + @Test + public void aCancelledTokenAbortsRetrievalOfAFetchedPage() { + ExternalStorageRunner storage = inMemoryStorage(); + History.Builder builder = historyWithInput(payload("big-input")).toBuilder(); + storage.store(builder, null, null, CancellationToken.none()); + History stored = builder.build(); + + CancelSource source = + new CancelSource<>(() -> new CancellationException("Worker shutdown")); + source.cancel(); + + ServiceWorkflowHistoryIterator iterator = fetchingIterator(stored, storage, source.token()); + + Assert.assertThrows(CancellationException.class, iterator::hasNext); + } + private static ServiceWorkflowHistoryIterator fetchingIterator( History page, ExternalStorageRunner storage) { + return fetchingIterator(page, storage, CancellationToken.none()); + } + + private static ServiceWorkflowHistoryIterator fetchingIterator( + History page, + ExternalStorageRunner storage, + CancellationToken storageCancellation) { PollWorkflowTaskQueueResponse workflowTask = PollWorkflowTaskQueueResponse.newBuilder().setNextPageToken(NEXT_PAGE_TOKEN).build(); - return new ServiceWorkflowHistoryIterator(null, "default", workflowTask, null, storage) { + return new ServiceWorkflowHistoryIterator( + null, "default", workflowTask, null, storage, storageCancellation) { @Override GetWorkflowExecutionHistoryResponse queryWorkflowExecutionHistory() { return GetWorkflowExecutionHistoryResponse.newBuilder().setHistory(page).build(); diff --git a/temporal-sdk/src/test/java/io/temporal/internal/worker/WorkflowWorkerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/worker/WorkflowWorkerTest.java index 2554227fcb..687db40e8e 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/worker/WorkflowWorkerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/worker/WorkflowWorkerTest.java @@ -22,6 +22,7 @@ import io.temporal.api.common.v1.WorkflowExecution; import io.temporal.api.common.v1.WorkflowType; import io.temporal.api.workflowservice.v1.*; +import io.temporal.common.CancellationToken; import io.temporal.common.reporter.TestStatsReporter; import io.temporal.internal.common.InternalUtils; import io.temporal.internal.replay.ReplayWorkflow; @@ -96,7 +97,8 @@ public void concurrentPollRequestLockTest() throws Exception { eagerActivityDispatcher, 3, slotSupplier, - new NamespaceCapabilities()); + new NamespaceCapabilities(), + CancellationToken.none()); WorkflowServiceGrpc.WorkflowServiceFutureStub futureStub = mock(WorkflowServiceGrpc.WorkflowServiceFutureStub.class); @@ -268,7 +270,8 @@ public void respondWorkflowTaskFailureMetricTest() throws Exception { eagerActivityDispatcher, 3, slotSupplier, - new NamespaceCapabilities()); + new NamespaceCapabilities(), + CancellationToken.none()); WorkflowServiceGrpc.WorkflowServiceFutureStub futureStub = mock(WorkflowServiceGrpc.WorkflowServiceFutureStub.class); @@ -413,7 +416,8 @@ public boolean isAnyTypeSupported() { eagerActivityDispatcher, 3, slotSupplier, - new NamespaceCapabilities()); + new NamespaceCapabilities(), + CancellationToken.none()); WorkflowServiceGrpc.WorkflowServiceFutureStub futureStub = mock(WorkflowServiceGrpc.WorkflowServiceFutureStub.class); From d7b9a625ee784bac475eac87edc4a2bf719b1f39 Mon Sep 17 00:00:00 2001 From: Chris Constable Date: Fri, 28 Aug 2026 17:36:47 -0400 Subject: [PATCH 23/23] externalStorage -> externalStorageRunner --- .../replay/ReplayWorkflowTaskHandler.java | 15 ++++++++------- .../replay/ServiceWorkflowHistoryIterator.java | 10 +++++----- .../temporal/internal/worker/WorkflowWorker.java | 8 ++++---- ...layWorkflowRunTaskHandlerTaskHandlerTests.java | 2 +- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java b/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java index a32e521d99..72c32137b0 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/replay/ReplayWorkflowTaskHandler.java @@ -104,11 +104,11 @@ public WorkflowTaskHandler.Result handleWorkflowTask(PollWorkflowTaskQueueRespon String workflowType = workflowTask.getWorkflowType().getName(); Scope metricsScope = options.getMetricsScope().tagged(ImmutableMap.of(MetricsTag.WORKFLOW_TYPE, workflowType)); - ExternalStorageRunner externalStorage = options.getExternalStorage(); - if (externalStorage == null) { + ExternalStorageRunner externalStorageRunner = options.getExternalStorageRunner(); + if (externalStorageRunner == null) { ExternalStorageRunner.throwIfContainsReference(workflowTask); } else { - workflowTask = externalStorage.retrieve(workflowTask, storageCancellation); + workflowTask = externalStorageRunner.retrieve(workflowTask, storageCancellation); } return handleWorkflowTaskWithQuery(workflowTask.toBuilder(), metricsScope); } @@ -132,7 +132,7 @@ private Result handleWorkflowTaskWithQuery( namespace, workflowTask, metricsScope, - options.getExternalStorage(), + options.getExternalStorageRunner(), storageCancellation); boolean finalCommand; Result result; @@ -434,11 +434,12 @@ private WorkflowRunTaskHandler createStatefulHandler( .blockingStub() .withOption(METRICS_TAGS_CALL_OPTIONS_KEY, metricsScope) .getWorkflowExecutionHistory(getHistoryRequest); - ExternalStorageRunner externalStorage = options.getExternalStorage(); - if (externalStorage == null) { + ExternalStorageRunner externalStorageRunner = options.getExternalStorageRunner(); + if (externalStorageRunner == null) { ExternalStorageRunner.throwIfContainsReference(getHistoryResponse); } else { - getHistoryResponse = externalStorage.retrieve(getHistoryResponse, storageCancellation); + getHistoryResponse = + externalStorageRunner.retrieve(getHistoryResponse, storageCancellation); } workflowTask .setHistory(getHistoryResponse.getHistory()) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java b/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java index 6eccdace7a..8c9974a5ef 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/replay/ServiceWorkflowHistoryIterator.java @@ -33,7 +33,7 @@ class ServiceWorkflowHistoryIterator implements WorkflowHistoryIterator { private final Scope metricsScope; private final PollWorkflowTaskQueueResponseOrBuilder task; private final GrpcRetryer grpcRetryer; - private final @Nullable ExternalStorageRunner externalStorage; + private final @Nullable ExternalStorageRunner externalStorageRunner; private final CancellationToken storageCancellation; private Deadline deadline; private Iterator current; @@ -52,14 +52,14 @@ class ServiceWorkflowHistoryIterator implements WorkflowHistoryIterator { String namespace, PollWorkflowTaskQueueResponseOrBuilder task, Scope metricsScope, - @Nullable ExternalStorageRunner externalStorage, + @Nullable ExternalStorageRunner externalStorageRunner, CancellationToken storageCancellation) { this.storageCancellation = storageCancellation; this.service = service; this.namespace = namespace; this.task = task; this.metricsScope = metricsScope; - this.externalStorage = externalStorage; + this.externalStorageRunner = externalStorageRunner; // TODO Refactor WorkflowHistoryIteratorTest or WorkflowHistoryIterator to remove this check. // `service == null` shouldn't be allowed as it's needed for a normal functioning of this // class. @@ -83,10 +83,10 @@ public boolean hasNext() { GetWorkflowExecutionHistoryResponse response = queryWorkflowExecutionHistory(); History history = response.getHistory(); - if (externalStorage == null) { + if (externalStorageRunner == null) { ExternalStorageRunner.throwIfContainsReference(history); } else { - history = externalStorage.retrieve(history, storageCancellation); + history = externalStorageRunner.retrieve(history, storageCancellation); } current = history.getEventsList().iterator(); nextPageToken = response.getNextPageToken(); diff --git a/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java b/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java index 7ff7f4a200..9a5125fcfd 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java @@ -402,16 +402,16 @@ private void storeOutboundPayloads( com.google.protobuf.Message.Builder builder, @Nullable StorageDriverTargetInfo target, @Nullable MessageVisitor targetVisitor) { - ExternalStorageRunner externalStorage = options.getExternalStorage(); - if (externalStorage != null) { - externalStorage.store(builder, target, targetVisitor, storageCancellation); + ExternalStorageRunner externalStorageRunner = options.getExternalStorageRunner(); + if (externalStorageRunner != null) { + externalStorageRunner.store(builder, target, targetVisitor, storageCancellation); } } @Nullable private StorageDriverTargetInfo workflowStorageTarget( WorkflowExecution execution, String workflowType) { - if (options.getExternalStorage() == null) { + if (options.getExternalStorageRunner() == null) { return null; } return new StorageDriverWorkflowInfo( diff --git a/temporal-sdk/src/test/java/io/temporal/internal/replay/ReplayWorkflowRunTaskHandlerTaskHandlerTests.java b/temporal-sdk/src/test/java/io/temporal/internal/replay/ReplayWorkflowRunTaskHandlerTaskHandlerTests.java index afb933c3aa..e178f14c91 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/replay/ReplayWorkflowRunTaskHandlerTaskHandlerTests.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/replay/ReplayWorkflowRunTaskHandlerTaskHandlerTests.java @@ -181,7 +181,7 @@ public void resolvesExternalStorageReferencesInFetchedFullHistory() throws Throw "namespace", workflowFactory, new WorkflowExecutorCache(10, new WorkflowRunLockManager(), new NoopScope()), - SingleWorkerOptions.newBuilder().setExternalStorage(externalStorage).build(), + SingleWorkerOptions.newBuilder().setExternalStorageRunner(externalStorage).build(), null, Duration.ofSeconds(5), client,