[amunra]

Problem

DatasetConsistencyWrapper (the cached manifest dataset behind the directory namespace) guards its Dataset with a tokio::sync::RwLock and exposes get() / get_refreshed() for readers and get_mut() for writers. get_mut() called reload() and only then requested the write lock. reload() itself takes a read lock to probe for a successor version and releases it before anything else happens, so from the lock's point of view the writer was just another reader until the moment it finally called .write().await.

tokio's RwLock is write-preferring: once a writer is queued, no new readers are admitted. That protection never engaged for get_mut() while it was reloading, because during that phase it held only a read lock and no write request was queued yet. Every reader that arrived while get_mut() was still reloading was admitted alongside it, and the writer then had to wait for all of them before its write request was served; the fairness guarantee only covered the tail of the operation. get_mut() is a public method of the wrapper; the namespace's own write operations currently go through get_refreshed() and the copy-on-write manifest rewrite path, so this affects external callers of get_mut() rather than any in-tree operation.

Change

get_mut() now acquires the write lock first and reloads the dataset while holding it, so write-fairness covers the whole operation: as soon as a writer calls get_mut(), new readers queue behind it.

The reload body is factored into reload_under_write_lock(), which takes the already-held RwLockWriteGuard, probes has_successor_version() and calls checkout_latest() if needed. The read path (get() / get_refreshed() -> reload()) is unchanged in behaviour: it still checks under a read lock and only upgrades to a write lock when a successor version exists, then reuses the same helper instead of duplicating the probe-and-checkout logic.

The cost is that every get_mut() now performs the successor probe while holding the write lock rather than under a read lock. Writers already had to take the write lock immediately afterwards, and has_successor_version() is a single object-store existence check on the manifest table, so the extra exclusive hold is short and only on the write path.

Testing

New unit test test_get_mut_is_served_before_later_readers in rust/lance-namespace-impls/src/dir/manifest.rs. It holds the wrapper's lock exclusively, starts get_mut() on one task and then, on a second task, a plain read-lock acquisition that keeps its permit until told to release it; it then releases the exclusive lock and asserts under a bounded timeout that get_mut() completes before the late reader is served, and that the reader completes once released. On a current-thread runtime the two tasks park on the lock in spawn order and the late reader holds a single permit from admission to release, so the outcome depends only on which lock get_mut() requests first: with the old ordering it queued as a reader, the late reader was admitted alongside it, and the writer's subsequent write request blocked on that reader until the timeout; with the new ordering it is queued as a writer and served first. Verified both ways, five runs each, with the fix locally reverted and restored.

The rest of the lance-namespace-impls suite (which exercises get() / get_refreshed() through the directory namespace operations and get_mut() in one existing unit test) passes unchanged, along with cargo fmt --all -- --check and cargo clippy --all --tests --benches -- -D warnings.

Compatibility

Internal locking change only. No public API, wire, or file-format changes; the manifest dataset layout is untouched.

Prior submission

The same fix was previously submitted as #6323 and went stale unreviewed.

---

Tracking: Ported from rerun-io#70.

[lance-gatekeeper]

✅ Gate recommendation: approve.

Acquiring the write lock before the successor probe makes Tokio’s FIFO writer preference cover the full get_mut() operation, while the read path keeps its existing optimistic probe. The shared reload helper preserves freshness and error handling, and the regression test covers the reader/writer ordering.

[zhangyue19921010]

+1 nice catch