Skip to content

Commit af8768c

Browse files
fix(dagster-async-executor): bound event receive so delayed retries don't hang (#327)
* chore(dagster-async-executor): add pytest-timeout dev dep, de-dupe pytest Remove the duplicate `pytest` entry from `[dependency-groups].dev` and add `pytest-timeout` (needed by upcoming regression test for the delayed retry hang). Regenerated `uv.lock` — resolves to pytest-timeout v2.4.0. Generated with AI Co-Authored-By: Claude Code * test(dagster-async-executor): add regression test for delayed-retry hang Add test_retry_policy.py with a 30-second pytest-timeout guard that reproduces the hang caused by recv_stream.receive() blocking forever when the last step is parked in _waiting_to_retry under a delayed RetryPolicy. Pre-fix: test times out via pytest-timeout (RED). Generated with AI Co-Authored-By: Claude Code * fix(dagster-async-executor): resolve delayed-retry hang via bounded receive() When the last in-flight step emits STEP_UP_FOR_RETRY under a RetryPolicy with delay > 0, the step is parked in ActiveExecution._waiting_to_retry. No worker is alive to emit an event, so recv_stream.receive() blocks forever while active.is_complete stays False — the run wedges in STARTED. Fix: wrap receive() in anyio.move_on_after(sleep_interval()) so the wait is bounded by the retry backoff. When the timeout fires (scope.cancelled_caught), continue to re-poll, which runs get_steps_to_execute() → _update() to promote the now-ready retry step. When sleep_interval() == 0 (hot path), we pass None to move_on_after, preserving exact current behavior. The removed try/except anyio.EndOfStream: raise was a no-op; EndOfStream is still handled by _drain_events after the loop. Mirroring stock in_process/multiprocess executors' sleep_til_ready() pattern. Generated with AI Co-Authored-By: Claude Code * test(dagster-async-executor): make retry test xdist-safe via config_schema Replace fixed global temp filename with per-test TemporaryDirectory and thread the attempts-file path into the op through config_schema + run_config, eliminating shared-state races under pytest-xdist parallel runs. Generated with AI Co-Authored-By: Claude Code
1 parent fe4d102 commit af8768c

4 files changed

Lines changed: 85 additions & 5 deletions

File tree

libraries/dagster-async-executor/dagster_async_executor/executor.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,20 @@ async def _async_execution_iterator(
238238
send_stream.clone(),
239239
)
240240

241-
try:
241+
# When the only remaining work is parked in ActiveExecution
242+
# (a step in _waiting_to_retry under a delayed RetryPolicy, or
243+
# a pending concurrency claim), no worker is alive to emit an
244+
# event, so receive() would block forever while is_complete
245+
# stays False. Bound the wait by sleep_interval() and re-poll,
246+
# mirroring stock executors' sleep_til_ready() behavior, so the
247+
# retry-ready step gets promoted by get_steps_to_execute().
248+
sleep_interval = active.sleep_interval()
249+
with anyio.move_on_after(
250+
sleep_interval if sleep_interval > 0 else None
251+
) as scope:
242252
event = await recv_stream.receive()
243-
except anyio.EndOfStream:
244-
raise
253+
if scope.cancelled_caught:
254+
continue
245255

246256
yield event
247257
active.handle_event(event)
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import tempfile
2+
3+
import dagster as dg
4+
import pytest
5+
6+
from dagster_async_executor import async_executor
7+
8+
9+
def fail_once_then_succeed_job() -> dg.JobDefinition:
10+
@dg.op(
11+
retry_policy=dg.RetryPolicy(max_retries=1, delay=0.5),
12+
config_schema={"attempts_file": str},
13+
)
14+
def fail_once_then_succeed(context: dg.OpExecutionContext) -> int:
15+
attempts_file: str = context.op_config["attempts_file"]
16+
with open(attempts_file) as f:
17+
attempts = int(f.read() or "0")
18+
with open(attempts_file, "w") as f:
19+
f.write(str(attempts + 1))
20+
if attempts == 0:
21+
raise RuntimeError("intentional first-attempt failure")
22+
return attempts
23+
24+
@dg.job(executor_def=async_executor)
25+
def retry_job() -> None:
26+
fail_once_then_succeed()
27+
28+
return retry_job
29+
30+
31+
@pytest.mark.timeout(30)
32+
def test_delayed_retry_policy_does_not_hang() -> None:
33+
with tempfile.TemporaryDirectory() as tmpdir:
34+
attempts_file = f"{tmpdir}/attempts"
35+
with open(attempts_file, "w") as f:
36+
f.write("0")
37+
with (
38+
dg.instance_for_test() as instance,
39+
dg.execute_job(
40+
dg.reconstructable(fail_once_then_succeed_job),
41+
instance=instance,
42+
run_config={
43+
"ops": {
44+
"fail_once_then_succeed": {
45+
"config": {"attempts_file": attempts_file}
46+
}
47+
}
48+
},
49+
) as result,
50+
):
51+
assert result.success
52+
# Prove the delayed retry actually ran (not a first-attempt success):
53+
# the op returns `attempts`, which is 1 only on the retried attempt.
54+
assert result.output_for_node("fail_once_then_succeed") == 1
55+
with open(attempts_file) as f:
56+
assert f.read() == "2" # op compute ran exactly twice

libraries/dagster-async-executor/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ dynamic = ["version"]
1313
dev = [
1414
"ruff",
1515
"pytest",
16+
"pytest-timeout",
1617
"ty==0.0.37",
17-
"pytest",
1818
]
1919

2020
[build-system]

libraries/dagster-async-executor/uv.lock

Lines changed: 15 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)