Skip to content

Commit aeea6ff

Browse files
committed
docs(insight): correct two rationales, note drain latency
The log filter's bind_invocation took a process-global lock on every operation-start and user-function-start hook although the work is idempotent after the first call on a thread. It now returns early when the thread already carries this provider's claim. Two comments described an ordering the SDK prevents. Insight's lock said a checkpoint-path operation-change "genuinely races" the invocation-end hook; the SDK joins the checkpoint thread and the branch pools before that hook is dispatched. The lock stays -- it is what makes reentrancy from customer code inside a build safe, and a guard resting on the SDK's join ordering is one refactor from being wrong -- but the comment now says which of the two it is. The Insight README documents what the invocation-end drain costs under concurrency: it waits for every record any execution had pending, and one worker serializes every export and flush, so ends are released together at the slowest. Measured with a 30 ms exporter: ~72 ms at one execution, ~1.8 s each at 48.
1 parent 0559917 commit aeea6ff

3 files changed

Lines changed: 31 additions & 5 deletions

File tree

  • packages
    • aws-durable-execution-sdk-python-insight
    • aws-durable-execution-sdk-python-otel/src/aws_durable_execution_sdk_python_otel

packages/aws-durable-execution-sdk-python-insight/README.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,19 @@ Behavior is validated cross-SDK by the `insight` conformance suite
230230
> cadence the JS and Java plugins have. Only a sampled-out execution neither
231231
> exports nor flushes.
232232
233+
> **Note (invocation-end latency under concurrency).** The drain an invocation
234+
> end performs waits for every record any execution had pending when it was
235+
> called, and one worker serializes all exports and all flushes, so every
236+
> concurrently ending invocation is released together at the slowest one. The wait
237+
> therefore grows with the number of executions the environment is running, not
238+
> just with this execution's own work: measured with a 30 ms exporter, one
239+
> execution ended in ~72 ms and 48 concurrent executions in ~1.8 s each. That is
240+
> the deliberate trade against the alternative — releasing an end before its
241+
> record reached the exporters, which is what silently lost terminal records
242+
> before. It matters for an exporter that makes a network call per record: budget
243+
> invocation-end time against the environment's concurrency, not against one
244+
> execution.
245+
233246
## Requirements
234247

235248
- `aws-durable-execution-sdk-python` with the plugin invocation hooks that

packages/aws-durable-execution-sdk-python-insight/src/aws_durable_execution_sdk_python_insight/plugin.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -281,11 +281,16 @@ def __init__(
281281
self._build_revision = 0
282282
# Guards `_closed`, `_build_revision`, the operations rebind and record
283283
# emission, so a late hook can never slip a RUNNING record in after the
284-
# terminal one. Still earns its place with one instance per invocation: the
285-
# SDK dispatches every hook synchronously on the thread that produced the
286-
# event, so an operation-change raised off the checkpointing path runs
287-
# concurrently with the invocation thread's on_invocation_end -- two hooks,
288-
# one instance, genuinely racing.
284+
# terminal one.
285+
#
286+
# What it protects against is reentrancy, not two threads. The SDK
287+
# dispatches every hook synchronously on the thread that produced the
288+
# event, and it joins the checkpoint thread and the branch pools before
289+
# the invocation-end hook is dispatched, so a checkpoint-path
290+
# operation-change cannot overlap `on_invocation_end` -- an earlier
291+
# version of this comment claimed it could. The lock still earns its place
292+
# for the reason below, and it stays because a guard whose correctness
293+
# rests on the SDK's join ordering is one refactor away from being wrong.
289294
#
290295
# Reentrant on purpose: `_emit` runs the scheduler's `schedule()` inside
291296
# this hold, and `schedule()` releases the record it displaces, which can

packages/aws-durable-execution-sdk-python-otel/src/aws_durable_execution_sdk_python_otel/log_filter.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,17 @@ def bind_invocation(provider: _SpanContextProvider) -> None:
132132
executing user code. Idempotent, so a plugin can call it from every such
133133
hook without tracking which threads it has already claimed.
134134
135+
A thread that already carries this provider's claim returns before taking the
136+
registry lock. Every operation-start and user-function-start hook calls this,
137+
and after the first call on a thread there is nothing to add: membership in
138+
the open-invocation set is idempotent, and the claim is already in place.
139+
135140
Args:
136141
provider: The plugin serving the invocation that owns this thread.
137142
"""
143+
claim = _current_invocation.get()
144+
if claim is not None and claim() is provider:
145+
return
138146
with _registry_lock:
139147
_open_invocations.add(provider)
140148
_current_invocation.set(weakref.ref(provider))

0 commit comments

Comments
 (0)