1fanwang commented on PR #70573:
URL: https://github.com/apache/airflow/pull/70573#issuecomment-5101344307
Thanks for tracking this down, @jason810496. landed on the same RC while
chasing this:
The detached job keeps the launcher's terminal on fd 0, so the fresh setsid
session leader re-acquires it as its controlling terminal. When the pty hangs
up, the job gets SIGHUP and dies. Redirecting stdin from /dev/null is what
fully detaches it, exactly as you have it on both the setsid and nohup
paths.
I reproduced it in a Linux container. The controlling-terminal timing makes
it intermittent on CI but deterministic in a slower Docker VM, which gives a
clean before/after.
1. Baseline (current main): the real test fails with the CI assertion, all 5
reruns exhausted
# Linux 6.10.14-linuxkit aarch64
$ pytest
test_remote_job.py::TestPosixKillBehaviour::test_kill_terminates_whole_job_tree_under_job_control
--noconftest
job_pids = subprocess.run(
["pgrep", "-f", marker], capture_output=True, text=True,
check=False
).stdout.split()
> assert job_pids, "job never started"
E AssertionError: job never started
E assert []
test_remote_job.py:426: AssertionError
=========================== short test summary info
============================
FAILED
test_remote_job.py::TestPosixKillBehaviour::test_kill_terminates_whole_job_tree_under_job_control
========================== 1 failed, 5 rerun in 0.32s
==========================
2. Mechanism: the detached job holds the pty as its controlling terminal
ps on the job right before the pty hangup. pid == sid == pgid confirms
setsid(2) gave it a new session; tty = pts/0 shows it still holds the
launcher's pty as its controlling terminal, which is the path the SIGHUP
travels down:
recorded pid: 31
ps (pid sid pgid tty) BEFORE hangup:
31 31 31 pts/0
job alive AFTER hangup: False
3. With this PR's change: the test passes deterministically, no regression
### flaky test, 20 consecutive runs with the fix:
RESULT: pass=20 fail=0 / 20
### full TestPosixKillBehaviour + wrapper-content tests:
........
[100%]
8 passed, 19 deselected in 0.08s
Reproduce
# From an apache/airflow checkout. The test skips on macOS, so run it under
Linux/Docker:
docker run --rm -v "$PWD:/repo" -w /tmp python:3.10 bash -c '
apt-get update -qq && apt-get install -y -qq util-linux procps >/dev/null
pip install -q pytest pytest-rerunfailures
cp /repo/providers/ssh/src/airflow/providers/ssh/utils/remote_job.py .
cp /repo/providers/ssh/tests/unit/ssh/utils/test_remote_job.py .
sed -i "s/from airflow.providers.ssh.utils.remote_job import (/from
remote_job import (/" test_remote_job.py
pytest
test_remote_job.py::TestPosixKillBehaviour::test_kill_terminates_whole_job_tree_under_job_control
--noconftest'
Dropping the @pytest.mark.flaky(reruns=5) marker is the right call too:
with stdin detached the test passed 20/20 for me, so it no longer needs reruns.
+1 from me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
