Skip to content

Commit bd0052e

Browse files
libsql-server: make the fenced snapshot stream test independent of compaction timing
`snapshot_stream_ends_typed` polled `get_snapshot_file(1)` and unwrapped its result while the compactor was still writing the first snapshot and the snapshot merger could be replacing merged files. The lookup lists the snapshot directory and then opens the chosen file, so under load it could fail with `NotFound` (directory not created yet, or a file removed by a merge between the listing and the open) and the test panicked. Open the stream through the `snapshot` RPC itself in a bounded loop that treats only "snapshot not found" and the vanished-file error as "not yet", and asserts that no read lease is held after a failed attempt. An opened stream holds its file open, so a later merge cannot affect it. Co-authored-by: Tomasz Szymczyszyn <tomasz.szymczyszyn@shopify.com>
1 parent 1a21f11 commit bd0052e

1 file changed

Lines changed: 37 additions & 9 deletions

File tree

‎libsql-server/src/namespace/fence/stream.rs‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ pub fn fence_status(error: FenceError) -> tonic::Status {
221221
#[cfg(test)]
222222
mod tests {
223223
use bytes::Bytes;
224+
use futures::stream::BoxStream;
224225
use futures::{Stream, StreamExt};
225226
use libsql_replication::rpc::replication::replication_log_server::ReplicationLog;
226227
use libsql_replication::rpc::replication::{
@@ -513,19 +514,11 @@ mod tests {
513514
.await
514515
.unwrap()
515516
.unwrap();
516-
tokio::time::timeout(PROMPT, async {
517-
// The snapshot is written by the compactor's own thread.
518-
while s.logger.get_snapshot_file(1).await.unwrap().is_none() {
519-
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
520-
}
521-
})
522-
.await
523-
.expect("the snapshot was never written");
524517
let acquired = s.execute(s.acquire(OP, 1, LONG)).await.unwrap();
525518
assert_eq!(fence_outcome(&acquired), FenceOutcome::Applied);
526519

527520
let r = Replication::new(&s).await;
528-
let stream = r.service.snapshot(r.offset(1)).await.unwrap().into_inner();
521+
let stream = open_snapshot_stream(&s, &r).await;
529522
assert_eq!(s.fence.read_lease_counts().replication, 1);
530523

531524
let fenced = tokio::time::timeout(PROMPT, s.execute(read_fence(&s, 2, LONG)))
@@ -540,6 +533,41 @@ mod tests {
540533
assert!(next_frame(&mut stream).await.is_none());
541534
}
542535

536+
/// Opens a `snapshot` stream from frame 1 once the compactor has written a snapshot.
537+
///
538+
/// The snapshot is written, and possibly merged with the previous one, by the compactor's
539+
/// own tasks, concurrently with this call. The server looks a snapshot up by listing the
540+
/// snapshot directory and then opening the file it chose, so while the directory does not
541+
/// exist yet, or while a merge removes the files it replaced, the call can fail with
542+
/// "snapshot not found" or with the `NotFound` of the vanished file. Those two failures,
543+
/// and only those, mean "not yet": any other status (a fence refusal in particular) fails
544+
/// the test. A stream that was opened holds its file open, so a later merge cannot affect it.
545+
async fn open_snapshot_stream(
546+
s: &Source,
547+
r: &Replication,
548+
) -> BoxStream<'static, Result<Frame, tonic::Status>> {
549+
tokio::time::timeout(PROMPT, async {
550+
loop {
551+
match r.service.snapshot(r.offset(1)).await {
552+
Ok(stream) => break stream.into_inner(),
553+
Err(status) => {
554+
let not_yet = match status.code() {
555+
tonic::Code::Unavailable => status.message() == "snapshot not found",
556+
tonic::Code::Internal => status.message().contains("os error 2"),
557+
_ => false,
558+
};
559+
assert!(not_yet, "unexpected snapshot status: {status:?}");
560+
assert_eq!(s.fence.read_lease_counts().total(), 0);
561+
// A polling interval, not evidence: the loop ends on the condition.
562+
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
563+
}
564+
}
565+
}
566+
})
567+
.await
568+
.expect("the snapshot was never written")
569+
}
570+
543571
/// While reads are fenced every replication call is refused at its start with the typed
544572
/// status (never `UNAVAILABLE`), and no lease is taken.
545573
#[tokio::test]

0 commit comments

Comments
 (0)