ColtenOuO opened a new pull request, #72572: URL: https://github.com/apache/airflow/pull/72572
### Sumarry `TestBundleVersionLock::test_that_shared_lock_doesnt_block_shared_lock` and `TestBundleVersionLock::test_that_shared_lock_blocks_ex_lock` started a worker thread, slept a fixed `0.1s`, and then asserted that the thread had already entered the `BundleVersionLock` context. That assumption only holds when the machine running the tests is idle. On a loaded CI worker the thread can take longer than `0.1s` just to get scheduled and open/flock the lock file, and the test fails for reasons that have nothing to do with the code under test. I measured this inside Breeze on an 8-core box by instrumenting the same helper the tests use and recording the delay between `Thread.start()` and the lock actually being held, over 200 iterations: | load | n | p50 (Median) | max | over 0.1s | | --- | --- | --- | --- | --- | | 2 x nproc busy loops | 200 | 0.014s | 0.038s | 0 | | 12 x nproc busy loops | 200 | 0.022s | 0.126s | 1 (0.5%) | So under CI-like contention the fixed sleep is genuinely too short, at roughly a 0.5% failure rate per assertion on this hardware -- and CI runners are typically smaller and noisier than this one. Running the two tests themselves in a loop under the same contention reproduces it directly. On `main`, 40 iterations produced a real failure (`assert lth2.did_lock is True` -> `AssertionError`), and after that failure both worker threads kept logging their `sleeping: idx=...` lines forever because `stop` was never set, which left the process unable to exit — exactly the hang described below rather than a clean test failure. With this change, 150 iterations (300 test executions) under identical load passed with zero failures and the whole loop finished in 23 seconds. There were two further problems in the same tests, both of which turn a timing miss into a bad failure rather than a clear one: - `self.locker` was only a bare annotation on `LockTestHelper` and was assigned inside the worker thread. If the thread had not run yet, `lth1.locker.lock_file_path` raised `AttributeError`, which gives no hint that the real cause is timing. - Both tests called `join()` with no timeout, and the `stop` flag was only set on the success path. If an assertion failed before `stop` was set, the worker thread stayed in its `while not self.stop` loop forever and the test hung until the CI job timed out instead of failing fast. ### Change - The helper signals `did_lock` through a `threading.Event` that the test waits on with a generous timeout - `stop` becomes an `Event` that the worker waits on (so shutdown is immediate rather than up to 200ms late) - The lock object is constructed in `__init__` so it is always available to the test - Teardown moves into `finally` with bounded `join()` calls plus an `is_alive()` assertion so a stuck thread fails loudly. --- ##### Was generative AI tooling used to co-author this PR? - [X] Yes — Claude Code (Opus 5) -- 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]
