dhruvarya-db opened a new pull request, #2859: URL: https://github.com/apache/iceberg-rust/pull/2859
## Which issue does this PR close? <!-- No issue; concurrency bug found while reviewing the delete-file loading paths. --> N/A — a latent concurrency bug in the positional-delete loading path. ## What changes are included in this PR? Fixes a lost-wakeup hang on the positional-delete wait path (`PosDelLoadAction::WaitFor` in `arrow/delete_filter.rs`, awaited in `arrow/caching_delete_file_loader.rs`). `tokio::sync::Notify::notify_waiters()` stores no permit and only wakes `Notified` futures that already exist. Previously `WaitFor` carried a raw `Arc<Notify>`, and the waiter turned it into a `Notified` only later, at `.notified().await` — after releasing the state lock. The exact losing interleaving: 1. Loader calls `try_start_pos_del_load(path)` → `Load` (claims the load). 2. Waiter calls `try_start_pos_del_load(path)` → `WaitFor(Arc<Notify>)` — a raw notifier, **not yet a `Notified`**. The lock is released as the call returns. 3. Loader calls `finish_pos_del_load(path)` → sets `Loaded`, fires `notify_waiters()`. No `Notified` exists yet, so the wakeup is dropped. 4. Waiter calls `notify.notified().await`, creating its `Notified` **after** step 3. 5. That `Notified` waits for the next `notify_waiters()`, which never comes → hangs forever. Since positional-delete retrieval (`get_delete_vector`) is synchronous, this waiter must not proceed early, so the hang is on the critical path. **Fix:** create the `Notified` under the state lock in `try_start_pos_del_load` and carry it in `WaitFor(OwnedNotified)`. The loader cannot fire `notify_waiters()` until it takes the write lock, so the waiter's `Notified` is guaranteed to exist before the signal. This is latent today (the loader does real parquet I/O before signaling, so the window is rarely hit), but is one scheduling hiccup from a permanent hang. It is the same bug class as the `DeleteFileIndex` site in #2696. ## Are these changes tested? Yes — adds a unit test (`wait_for_completes_when_load_finishes_before_await`) that drives the exact interleaving above through the real API and asserts the waiter completes after `finish_pos_del_load`. It passes with this change and hangs (times out) on the old `WaitFor(Arc<Notify>)` contract. No test seam is needed because `WaitFor` hands the notifier to the caller. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
