Skip to content

Commit f5a571e

Browse files
committed
💥 Enforce payload and memo size limits
1 parent 63a34cc commit f5a571e

38 files changed

Lines changed: 2412 additions & 41 deletions

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ to docs, or any other relevant information.
2323
- `WorkflowOutboundCallsInterceptor` has new `sleep(Duration, TimerOptions)` and
2424
`await(Duration, TimerOptions, String, Supplier)` methods. Implementations that don't extend
2525
`WorkflowOutboundCallsInterceptorBase` need to add them, and Base subclasses override them to see calls with options.
26+
- Workers now retryably fail a workflow or activity task whose payloads exceed the namespace's size
27+
limit, instead of sending it to the server. Disable with
28+
`WorkerOptions.Builder.setDisablePayloadErrorLimit`; tune warnings with the experimental
29+
`WorkflowServiceStubsOptions.Builder.setPayloadLimits`.
2630

2731
### Added
2832
- Added experimental `ChildWorkflowOptions.Builder.setVersioningOverride` and

‎temporal-sdk/src/main/java/io/temporal/internal/activity/ActivityExecutionContextFactoryImpl.java‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import io.temporal.client.WorkflowClient;
55
import io.temporal.common.converter.DataConverter;
66
import io.temporal.internal.client.external.ManualActivityCompletionClientFactory;
7+
import io.temporal.internal.payload.limits.PayloadErrorLimits;
78
import io.temporal.internal.payload.storage.ExternalStorageRunner;
89
import java.nio.ByteBuffer;
910
import java.time.Duration;
@@ -12,6 +13,7 @@
1213
import java.util.concurrent.ConcurrentHashMap;
1314
import java.util.concurrent.ConcurrentMap;
1415
import java.util.concurrent.ScheduledExecutorService;
16+
import java.util.function.Supplier;
1517
import javax.annotation.Nullable;
1618

1719
public class ActivityExecutionContextFactoryImpl implements ActivityExecutionContextFactory {
@@ -24,6 +26,7 @@ public class ActivityExecutionContextFactoryImpl implements ActivityExecutionCon
2426
private final ScheduledExecutorService heartbeatExecutor;
2527
private final ManualActivityCompletionClientFactory manualCompletionClientFactory;
2628
private final @Nullable ExternalStorageRunner externalStorage;
29+
private final Supplier<PayloadErrorLimits> payloadErrorLimits;
2730
private final ConcurrentMap<ByteBuffer, ActivityExecutionContextImpl> activeContexts =
2831
new ConcurrentHashMap<>();
2932

@@ -36,6 +39,33 @@ public ActivityExecutionContextFactoryImpl(
3639
DataConverter dataConverter,
3740
ScheduledExecutorService heartbeatExecutor,
3841
@Nullable ExternalStorageRunner externalStorage) {
42+
this(
43+
client,
44+
identity,
45+
namespace,
46+
maxHeartbeatThrottleInterval,
47+
defaultHeartbeatThrottleInterval,
48+
dataConverter,
49+
heartbeatExecutor,
50+
externalStorage,
51+
() -> null);
52+
}
53+
54+
/**
55+
* @param payloadErrorLimits read when each activity starts, because the namespace's limits are
56+
* only known once the worker factory has started.
57+
*/
58+
public ActivityExecutionContextFactoryImpl(
59+
WorkflowClient client,
60+
String identity,
61+
String namespace,
62+
Duration maxHeartbeatThrottleInterval,
63+
Duration defaultHeartbeatThrottleInterval,
64+
DataConverter dataConverter,
65+
ScheduledExecutorService heartbeatExecutor,
66+
@Nullable ExternalStorageRunner externalStorage,
67+
Supplier<PayloadErrorLimits> payloadErrorLimits) {
68+
this.payloadErrorLimits = Objects.requireNonNull(payloadErrorLimits);
3969
this.client = Objects.requireNonNull(client);
4070
this.identity = identity;
4171
this.namespace = Objects.requireNonNull(namespace);
@@ -69,7 +99,8 @@ public InternalActivityExecutionContext createContext(
6999
maxHeartbeatThrottleInterval,
70100
defaultHeartbeatThrottleInterval,
71101
() -> cleanupContext(info.getTaskToken(), false),
72-
externalStorage);
102+
externalStorage,
103+
payloadErrorLimits.get());
73104
activeContexts.put(taskToken, context);
74105
return context;
75106
}

‎temporal-sdk/src/main/java/io/temporal/internal/activity/ActivityExecutionContextImpl.java‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
import io.temporal.common.CancellationToken;
1111
import io.temporal.common.converter.DataConverter;
1212
import io.temporal.internal.client.external.ManualActivityCompletionClientFactory;
13+
import io.temporal.internal.payload.limits.PayloadErrorLimits;
14+
import io.temporal.internal.payload.limits.PayloadLimitViolation;
1315
import io.temporal.internal.payload.storage.ExternalStorageRunner;
1416
import io.temporal.payload.context.ActivitySerializationContext;
1517
import io.temporal.workflow.Functions;
@@ -58,7 +60,8 @@ class ActivityExecutionContextImpl implements InternalActivityExecutionContext {
5860
Duration maxHeartbeatThrottleInterval,
5961
Duration defaultHeartbeatThrottleInterval,
6062
Functions.Proc closeCallback,
61-
@Nullable ExternalStorageRunner externalStorage) {
63+
@Nullable ExternalStorageRunner externalStorage,
64+
@Nullable PayloadErrorLimits payloadErrorLimits) {
6265
this.client = client;
6366
this.activity = activity;
6467
this.metricsScope = metricsScope;
@@ -77,7 +80,8 @@ class ActivityExecutionContextImpl implements InternalActivityExecutionContext {
7780
identity,
7881
maxHeartbeatThrottleInterval,
7982
defaultHeartbeatThrottleInterval,
80-
externalStorage);
83+
externalStorage,
84+
payloadErrorLimits);
8185
}
8286

8387
/**
@@ -88,6 +92,17 @@ public <V> void heartbeat(V details) throws ActivityCompletionException {
8892
heartbeatContext.heartbeat(details);
8993
}
9094

95+
@Override
96+
public boolean isTaskReported() {
97+
return heartbeatContext.isTaskReported();
98+
}
99+
100+
@Override
101+
@Nullable
102+
public PayloadLimitViolation getPayloadLimitViolation() {
103+
return heartbeatContext.getPayloadLimitViolation();
104+
}
105+
91106
@Override
92107
public <V> Optional<V> getHeartbeatDetails(Class<V> detailsClass) {
93108
return getHeartbeatDetails(detailsClass, detailsClass);

‎temporal-sdk/src/main/java/io/temporal/internal/activity/ActivityTaskExecutors.java‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import io.temporal.api.common.v1.Payload;
99
import io.temporal.api.common.v1.Payloads;
1010
import io.temporal.api.workflowservice.v1.RespondActivityTaskCompletedRequest;
11+
import io.temporal.api.workflowservice.v1.RespondActivityTaskFailedRequest;
1112
import io.temporal.client.ActivityCanceledException;
1213
import io.temporal.common.context.ContextPropagator;
1314
import io.temporal.common.converter.DataConverter;
@@ -16,7 +17,10 @@
1617
import io.temporal.common.interceptors.ActivityInboundCallsInterceptor.ActivityOutput;
1718
import io.temporal.common.interceptors.Header;
1819
import io.temporal.common.interceptors.WorkerInterceptor;
20+
import io.temporal.failure.ApplicationFailure;
1921
import io.temporal.internal.common.FailureUtils;
22+
import io.temporal.internal.common.PayloadsTooLargeFailure;
23+
import io.temporal.internal.payload.limits.PayloadLimitViolation;
2024
import io.temporal.internal.worker.ActivityTaskHandler;
2125
import io.temporal.payload.context.ActivitySerializationContext;
2226
import io.temporal.serviceclient.CheckedExceptionWrapper;
@@ -91,18 +95,27 @@ public ActivityTaskHandler.Result execute(ActivityInfoInternal info, Scope metri
9195
return new ActivityTaskHandler.Result(
9296
info.getActivityId(), null, null, null, context.isUseLocalManualCompletion());
9397
}
98+
ActivityTaskHandler.Result heartbeatOutcome =
99+
heartbeatOutcome(context, info, dataConverterWithActivityContext);
100+
if (heartbeatOutcome != null) {
101+
return heartbeatOutcome;
102+
}
94103

95104
return this.constructSuccessfulResultValue(info, result, dataConverterWithActivityContext);
96105
} catch (Throwable e) {
97106
Throwable ex = CheckedExceptionWrapper.unwrap(e);
98107
boolean local = info.isLocal();
99108
if (ex instanceof ActivityCanceledException) {
100-
log.info(
101-
"{} canceled. ActivityId={}, activityType={}, attempt={}",
102-
local ? "Local activity" : "Activity",
103-
info.getActivityId(),
104-
info.getActivityType(),
105-
info.getAttempt());
109+
// An oversized heartbeat cancels the activity only to fail it, and the payload limit
110+
// validator already logged why.
111+
if (context.getPayloadLimitViolation() == null) {
112+
log.info(
113+
"{} canceled. ActivityId={}, activityType={}, attempt={}",
114+
local ? "Local activity" : "Activity",
115+
info.getActivityId(),
116+
info.getActivityType(),
117+
info.getAttempt());
118+
}
106119
} else if (FailureUtils.isBenignApplicationFailure(ex)) {
107120
log.debug(
108121
"{} failure. ActivityId={}, activityType={}, attempt={}",
@@ -121,6 +134,11 @@ public ActivityTaskHandler.Result execute(ActivityInfoInternal info, Scope metri
121134
ex);
122135
}
123136

137+
ActivityTaskHandler.Result heartbeatOutcome =
138+
heartbeatOutcome(context, info, dataConverterWithActivityContext);
139+
if (heartbeatOutcome != null) {
140+
return heartbeatOutcome;
141+
}
124142
return mapToActivityFailure(
125143
ex,
126144
info.getActivityId(),
@@ -142,6 +160,36 @@ public ActivityTaskHandler.Result execute(ActivityInfoInternal info, Scope metri
142160
}
143161
}
144162

163+
/**
164+
* Returns the result to report when an oversized heartbeat decided the task's outcome, or null.
165+
* The result sends nothing if the heartbeat already reported the failure, and otherwise reports
166+
* that failure in place of whatever the activity did.
167+
*/
168+
@Nullable
169+
private static ActivityTaskHandler.Result heartbeatOutcome(
170+
InternalActivityExecutionContext context,
171+
ActivityInfoInternal info,
172+
DataConverter dataConverterWithActivityContext) {
173+
if (context.isTaskReported()) {
174+
return new ActivityTaskHandler.Result(info.getActivityId(), null, null, null, false);
175+
}
176+
PayloadLimitViolation violation = context.getPayloadLimitViolation();
177+
if (violation == null) {
178+
return null;
179+
}
180+
ApplicationFailure failure = PayloadsTooLargeFailure.of(violation);
181+
RespondActivityTaskFailedRequest request =
182+
RespondActivityTaskFailedRequest.newBuilder()
183+
.setFailure(dataConverterWithActivityContext.exceptionToFailure(failure))
184+
.build();
185+
return new ActivityTaskHandler.Result(
186+
info.getActivityId(),
187+
null,
188+
new ActivityTaskHandler.Result.TaskFailedResult(request, failure),
189+
null,
190+
false);
191+
}
192+
145193
abstract ActivityInboundCallsInterceptor createRootInboundInterceptor();
146194

147195
abstract Object getActivity();

‎temporal-sdk/src/main/java/io/temporal/internal/activity/HeartbeatContext.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
import io.temporal.client.ActivityCanceledException;
44
import io.temporal.client.ActivityCompletionException;
55
import io.temporal.common.CancellationToken;
6+
import io.temporal.internal.payload.limits.PayloadLimitViolation;
67
import java.lang.reflect.Type;
78
import java.util.Optional;
9+
import javax.annotation.Nullable;
810

911
interface HeartbeatContext {
1012

@@ -35,4 +37,17 @@ interface HeartbeatContext {
3537

3638
/** Cancel any pending heartbeat and discard cached heartbeat details. */
3739
void cancelOutstandingHeartbeat();
40+
41+
/**
42+
* Returns whether this context already reported the activity task's outcome to the server, so the
43+
* activity's own result must not be sent.
44+
*/
45+
boolean isTaskReported();
46+
47+
/**
48+
* Returns the payload limit violation that failed a heartbeat, or null. Unless this context
49+
* already reported it, the activity must report the violation instead of its own outcome.
50+
*/
51+
@Nullable
52+
PayloadLimitViolation getPayloadLimitViolation();
3853
}

0 commit comments

Comments
 (0)