fix: test failure due to global static (#25287)

This commit changes the write_buffer tests to acquire a lock so that
in tests where we need to have access to NEXT_FILE_ID that it won't
be overwritten since rust tests run as one process and share the
same statics.

While this isn't a problem for Edge as a singular process it is for
our tests. It's a bit unfortunate, but this solution is the easiest
and the locks are not held for long so there's no real big impact
on running these tests.

Closes #25286
pull/25304/head
Michael Gattozzi 2024-09-05 13:53:38 -04:00 committed by GitHub
parent 4e664d3da5
commit fb9d7d02f3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 26 additions and 0 deletions

View File

@ -592,6 +592,7 @@ impl LastCacheManager for WriteBufferImpl {
impl WriteBuffer for WriteBufferImpl {}
#[cfg(test)]
#[allow(clippy::await_holding_lock)]
mod tests {
use super::*;
use crate::paths::{CatalogFilePath, SnapshotInfoFilePath};
@ -609,6 +610,7 @@ mod tests {
use object_store::local::LocalFileSystem;
use object_store::memory::InMemory;
use object_store::{ObjectStore, PutPayload};
use parking_lot::{Mutex, MutexGuard};
#[test]
fn parse_lp_into_buffer() {
@ -733,6 +735,7 @@ mod tests {
#[tokio::test]
async fn last_cache_create_and_delete_is_durable() {
let lock = lock();
let (wbuf, _ctx) = setup(
Time::from_timestamp_nanos(0),
Arc::new(InMemory::new()),
@ -744,6 +747,7 @@ mod tests {
},
)
.await;
drop(lock);
let db_name = "db";
let tbl_name = "table";
let cache_name = "cache";
@ -872,6 +876,7 @@ mod tests {
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn returns_chunks_across_parquet_and_buffered_data() {
let lock = lock();
let (write_buffer, session_context) = setup(
Time::from_timestamp_nanos(0),
Arc::new(InMemory::new()),
@ -883,6 +888,7 @@ mod tests {
},
)
.await;
drop(lock);
let _ = write_buffer
.write_lp(
@ -1070,6 +1076,7 @@ mod tests {
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn catalog_snapshots_only_if_updated() {
let lock = lock();
let (write_buffer, _ctx) = setup(
Time::from_timestamp_nanos(0),
Arc::new(InMemory::new()),
@ -1081,6 +1088,7 @@ mod tests {
},
)
.await;
drop(lock);
let db_name = "foo";
// do three writes to force a snapshot
@ -1158,6 +1166,7 @@ mod tests {
let object_store: Arc<dyn ObjectStore> =
Arc::new(LocalFileSystem::new_with_prefix(test_helpers::tmp_dir().unwrap()).unwrap());
let lock = lock();
// create a snapshot file that will be loaded on initialization of the write buffer:
// Set NEXT_FILE_ID to a non zero number for the snapshot
NEXT_FILE_ID.store(500, Ordering::SeqCst);
@ -1196,6 +1205,7 @@ mod tests {
// Assert that loading the snapshots sets NEXT_FILE_ID to the correct id number
assert_eq!(NEXT_FILE_ID.load(Ordering::SeqCst), 500);
drop(lock);
// there should be one snapshot already, i.e., the one we created above:
verify_snapshot_count(1, &wbuf.persister).await;
@ -1619,4 +1629,20 @@ mod tests {
}
batches
}
/// Lock for the NEXT_FILE_ID data which is set during some of these tests.
/// We need to have exclusive access to it to test that it works when loading
/// from a snapshot. We lock in most of the calls to setup in this test suite
/// where it would cause problems. If running under `cargo-nextest`, return a
/// different mutex guard as it does not have this problem due to running
/// each test in it's own process
fn lock() -> MutexGuard<'static, ()> {
static FILE_ID_LOCK: Mutex<()> = Mutex::new(());
static DUMMY_LOCK: Mutex<()> = Mutex::new(());
if std::env::var("NEXTEST").unwrap_or("0".into()) == "1" {
DUMMY_LOCK.lock()
} else {
FILE_ID_LOCK.lock()
}
}
}