Skip to content

Commit 5139759

Browse files
committed
fix: shorten checkpoint wait for blocked caller
The checkpoint collector waited on an empty queue after it had already collected a synchronous checkpoint. A synchronous checkpoint has a caller blocked until the batch is persisted, and a blocked caller cannot enqueue more work. So that wait could not enlarge the batch. It only delayed the caller by the shutdown-signal poll interval of 100 ms. Sequential steps paid that cost once per step. Measured in us-east-1 on python3.13 at 1024 MB over 1000 steps, warm, comparing this change against d61985e: median interval between StepSucceeded 142 ms -> 41 ms p95 interval 163 ms -> 62 ms invocation duration 144.4 s -> 43.1 s steps per second 6.92 -> 23.22 The step body performs no I/O, so nearly all of the removed time was the idle wait. Changes - Track whether a batch holds a synchronous checkpoint. Set the flag on all three insertion paths: the overflow drain, the first blocking read, and the additional read. - Once set, wait only _BLOCKED_CALLER_WAIT_SECONDS on an empty queue instead of the full poll interval. Work already queued still joins the batch, so an asynchronous step START still travels in the same request as its synchronous SUCCEED. Sequential steps keep one checkpoint call of two updates per step, unchanged. - Wait 1 ms rather than not at all. Branch threads start one at a time, so gaps in checkpoint arrivals occur while a parallel or map thread pool starts. Not waiting cut extra small batches during that startup. On Lambda, waiting 1 ms produced the same checkpoint call count as d61985e at both 200 and 500 branches, and ran 1.21x to 1.28x faster. - Include empty synchronous checkpoints. ConcurrentExecutor collects every due branch, calls create_checkpoint() once for the whole resume wave, then resubmits. So a wide map resume enqueues one empty checkpoint, not one per branch, and the full window would delay that single caller with nothing to coalesce. - Leave asynchronous-only batching unchanged. With no blocked caller the wait costs no latency and still reduces the API call count. - Name the existing 0.1 second literal _STOP_SIGNAL_POLL_SECONDS. Both waits in the collector are now named, so the min() compares two named intervals rather than one named value and one literal. - Correct the max_batch_time_seconds docstring. It bounds the total collection window, not the idle wait, which the poll interval caps regardless of the configured value. Neither constant is reachable by an end user, so this changes no public signature. CheckpointBatcherConfig is absent from the package exports, no caller outside state.py passes batcher_config, and config.py has no batcher settings. Tests Add six collector tests that assert the timeout values passed to the queue read rather than elapsed time. The collector's waiting policy is fully described by those timeouts, so the tests need no sleeps and no wall-clock thresholds. Five of the six fail when the shortened wait is removed. The sixth pins the asynchronous-only path this change leaves alone. Rewrite test_create_checkpoint_multiple_sync_calls_all_block. Its old assertion required each caller to block for at least 0.15 s, but each caller started its clock after the processor began its 0.15 s sleep, so its own elapsed time was always under that. The assertion held only because the collector added 100 ms on top, which is the defect. The test now proves the invariant by observation order and uses no delay at all. It waits until all three operations are queued, asserts no caller has returned, collects the batch, asserts no caller has returned, then sets the completion events and asserts every caller returned. Settling the waiters before collection fails it. Correct the description in map_with_concurrent_waits_int_test. It said the test mirrors the resubmitter issuing one empty checkpoint per branch. ConcurrentExecutor issues one per resume wave, so the test exercises a synthetic burst. It still guards the batch-limit behaviour that #325 changed. Fixes #710
1 parent 8742ad9 commit 5139759

3 files changed

Lines changed: 264 additions & 72 deletions

File tree

packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/state.py

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,25 @@
4747

4848
logger = logging.getLogger(__name__)
4949

50+
# Longest wait on an empty queue before re-checking the shutdown signal.
51+
_STOP_SIGNAL_POLL_SECONDS = 0.1
52+
53+
# Longest wait on an empty queue when a caller is already blocked on the batch.
54+
# Long enough for sibling branch threads to arrive. Short enough that the
55+
# blocked caller stays fast.
56+
_BLOCKED_CALLER_WAIT_SECONDS = 0.001
57+
5058

5159
@dataclass(frozen=True)
5260
class CheckpointBatcherConfig:
5361
"""Configuration for checkpoint batching behavior.
5462
5563
Attributes:
5664
max_batch_size_bytes: Maximum batch size in bytes (default: 750KB)
57-
max_batch_time_seconds: Maximum time to wait before flushing batch (default: 1.0 second)
65+
max_batch_time_seconds: Longest a batch keeps accumulating (default:
66+
1.0 second). This caps total collection, not the idle wait. The
67+
collector flushes as soon as one poll slice finds the queue empty.
68+
So a larger value only matters while operations keep arriving.
5869
max_batch_operations: Maximum number of operations per batch (default: 250)
5970
"""
6071

@@ -1025,6 +1036,14 @@ def _collect_checkpoint_batch(self) -> list[QueuedOperation]:
10251036
operation if queues are empty, then collects additional operations within the time
10261037
window.
10271038
1039+
How long the collector waits on an empty queue depends on the batch. A batch
1040+
holding a sync checkpoint waits _BLOCKED_CALLER_WAIT_SECONDS, because a caller is
1041+
blocked on it. An async-only batch waits the full window, because no caller is
1042+
blocked and a larger batch means fewer API calls.
1043+
1044+
Both cases drain the queue first. So a step's async START and its sync SUCCEED go
1045+
in one request.
1046+
10281047
Empty checkpoints (operation_update=None) are coalesced: the first empty checkpoint
10291048
counts toward the batch operation limit, but subsequent empty checkpoints do not.
10301049
All empty checkpoints remain in the batch so their completion events are signaled.
@@ -1037,13 +1056,18 @@ def _collect_checkpoint_batch(self) -> list[QueuedOperation]:
10371056
"""
10381057
batch: list[QueuedOperation] = []
10391058
has_empty_checkpoint = False
1059+
# Set once the batch holds any sync checkpoint. That caller is blocked until
1060+
# the batch persists, so it cannot queue more work.
1061+
has_blocked_caller = False
10401062
total_size = 0
10411063
effective_operation_count = 0 # Operations that count toward batch limit
10421064

10431065
# First, drain overflow queue (FIFO order preserved)
10441066
try:
10451067
while effective_operation_count < self._batcher_config.max_batch_operations:
10461068
overflow_op = self._overflow_queue.get_nowait()
1069+
if overflow_op.completion_event is not None:
1070+
has_blocked_caller = True
10471071

10481072
if overflow_op.operation_update is None: # Empty checkpoint
10491073
batch.append(overflow_op)
@@ -1071,10 +1095,12 @@ def _collect_checkpoint_batch(self) -> list[QueuedOperation]:
10711095
while not self._checkpointing_stopped.is_set():
10721096
try:
10731097
first_op = self._checkpoint_queue.get(
1074-
timeout=0.1
1075-
) # Check stop signal every 100ms
1098+
timeout=_STOP_SIGNAL_POLL_SECONDS
1099+
)
10761100
self._checkpoint_queue.task_done()
10771101
batch.append(first_op)
1102+
if first_op.completion_event is not None:
1103+
has_blocked_caller = True
10781104

10791105
if first_op.operation_update is None:
10801106
has_empty_checkpoint = True
@@ -1101,15 +1127,22 @@ def _collect_checkpoint_batch(self) -> list[QueuedOperation]:
11011127
):
11021128
remaining_time = min(
11031129
batch_deadline - time.time(),
1104-
0.1, # Check stop signal every 100ms
1130+
_STOP_SIGNAL_POLL_SECONDS,
11051131
)
11061132

11071133
if remaining_time <= 0:
11081134
break
11091135

11101136
try:
1111-
additional_op = self._checkpoint_queue.get(timeout=remaining_time)
1137+
if has_blocked_caller:
1138+
additional_op = self._checkpoint_queue.get(
1139+
timeout=min(_BLOCKED_CALLER_WAIT_SECONDS, remaining_time)
1140+
)
1141+
else:
1142+
additional_op = self._checkpoint_queue.get(timeout=remaining_time)
11121143
self._checkpoint_queue.task_done()
1144+
if additional_op.completion_event is not None:
1145+
has_blocked_caller = True
11131146

11141147
if additional_op.operation_update is None: # Empty checkpoint
11151148
batch.append(additional_op)

packages/aws-durable-execution-sdk-python/tests/e2e/map_with_concurrent_waits_int_test.py

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,23 @@
88
99
Background
1010
----------
11-
When a map branch suspends via TimedSuspendExecution and later resumes, the
12-
ConcurrentExecutor resubmitter calls::
11+
NOTE ON PRODUCTION FIDELITY. ConcurrentExecutor no longer issues one empty
12+
checkpoint per branch. It collects every due branch, calls
13+
execution_state.create_checkpoint() once for the whole resume wave, then
14+
resubmits (executor.py, "One checkpoint refresh serves the whole due wave").
15+
So a wide map resume enqueues ONE empty checkpoint, not hundreds.
1316
14-
execution_state.create_checkpoint() # empty checkpoint
15-
16-
before resubmitting the branch. In high-concurrency scenarios (300+ branches)
17-
all resuming at the same time, 300+ empty checkpoints flood the checkpoint
18-
queue.
17+
These tests therefore exercise a synthetic burst of concurrent empty
18+
checkpoints rather than the current resubmitter. They still guard the batch
19+
limit behaviour below, which is what issue #325 changed.
1920
2021
Without the coalescing optimization (issue #325), the 250-operation batch limit
2122
causes these to be split across multiple batches → multiple API calls.
2223
With the optimization, all subsequent empty checkpoints beyond the first do
2324
NOT count toward the batch limit, so they are coalesced into a single batch
2425
and a single API call.
2526
26-
These tests directly simulate that concurrent-checkpoint pattern by launching
27-
many threads that each call ``create_checkpoint()`` simultaneously, mirroring
28-
what the map resubmitter does when all branches resume at once.
27+
They launch many threads that each call ``create_checkpoint()`` at once.
2928
"""
3029

3130
from __future__ import annotations
@@ -92,16 +91,15 @@ def _checkpoint(
9291

9392

9493
def test_map_with_concurrent_waits_coalesces_empty_checkpoints():
95-
"""300 concurrent branches all create empty checkpoints simultaneously.
94+
"""300 concurrent empty checkpoints must collect into one batch.
9695
97-
Simulates the Java MapWithConditionAndCallbackExample scenario: 300 map
98-
branches all resuming from a wait operation at the same time, each calling
99-
the resubmitter which enqueues an empty checkpoint.
96+
A synthetic burst, not the current resubmitter, which issues one empty
97+
checkpoint per resume wave. See the module docstring.
10098
101-
Without the coalescing optimization, the 250-op batch limit splits 300
102-
empty checkpoints into 2 batches (250 + 50) → 2 API calls.
103-
With the optimization (effective_operation_count stays 1 for empties),
104-
all 300 are collected in a single batch → 1 API call.
99+
Without the coalescing optimization the 250-op batch limit splits 300 empty
100+
checkpoints into 2 batches of 250 and 50, so 2 API calls. With it,
101+
effective_operation_count stays 1 for empties, so all 300 collect into one
102+
batch and one API call.
105103
"""
106104
mock_client, calls = _make_tracking_client()
107105
state = _make_state(mock_client, batch_time=5.0, max_ops=250)

0 commit comments

Comments
 (0)