fix: deflake test_read_consistency_interval (#3713)

`test_read_consistency_interval` asserted that a table opened with a
100ms `read_consistency_interval` still read stale data immediately
after a concurrent write. The cache timestamp is set when the table is
opened and reads within the interval do not refresh it, so that
assertion only held if the intervening open/count/commit/count sequence
finished within 100ms of real wall-clock time. On a loaded CI runner it
did not: the TTL expired, `count_rows` refreshed synchronously, and the
test failed with `left: 1, right: 0`. This broke the Rust workflow on
`main` at 0bc08160 (a Python-only commit).

This pins the `background_cache` mock clock once `table2` has seeded its
cache, and advances it explicitly in place of `tokio::time::sleep`, so
the test controls when the interval elapses. Same approach as #3547.
With the clock pinned there is no real sleep left to be imprecise, so
the `cfg(not(target_os = "windows"))` guard is dropped and the test now
runs on Windows too.

Verified by inserting a stall before the write: 120ms reproduces the
original failure deterministically, and with this change the test still
passes with a 500ms stall.

Fixes #3712
This commit is contained in:
Will Jones
2026-07-29 13:06:41 -07:00
committed by GitHub
parent f7feed48c3
commit 03b26d585b
+9 -3
View File
@@ -4136,10 +4136,10 @@ mod tests {
Box::new(RecordBatchIterator::new(vec![Ok(batch)], schema))
}
// Windows does not support precise sleep durations due to timer resolution limitations.
#[cfg(not(target_os = "windows"))]
#[tokio::test]
async fn test_read_consistency_interval() {
use crate::utils::background_cache::clock;
let intervals = vec![
None,
Some(0),
@@ -4166,6 +4166,12 @@ mod tests {
let conn2 = conn2.execute().await.unwrap();
let table2 = conn2.open_table("my_table").execute().await.unwrap();
// Freeze the consistency clock now that `table2` has seeded its cache, so the
// interval only elapses when this test advances it. Otherwise the write and
// count_rows calls below race the real 100ms interval, which a loaded CI
// runner loses. Must come after open_table: creating the cache clears the mock.
clock::pin();
assert_eq!(table1.count_rows(None).await.unwrap(), 0);
assert_eq!(table2.count_rows(None).await.unwrap(), 0);
@@ -4183,7 +4189,7 @@ mod tests {
}
Some(100) => {
assert_eq!(table2.count_rows(None).await.unwrap(), 0);
tokio::time::sleep(Duration::from_millis(100)).await;
clock::advance_by(Duration::from_millis(100));
assert_eq!(table2.count_rows(None).await.unwrap(), 1);
}
_ => unreachable!(),