Skip to content

Commit 9f20edf

Browse files
committed
fixup! Implement tiered storage
Record the attempted write version even when a multi-store peration fails, as one store may already contain the newer state. This prevents a previously started write that completes later from overwriting a newer primary write whose corresponding backup write failed. Add a regression test covering a successful primary write paired with a failed backup write.
1 parent 9290e8b commit 9f20edf

1 file changed

Lines changed: 112 additions & 19 deletions

File tree

src/io/tier_store.rs

Lines changed: 112 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,9 @@ impl TierStoreInner {
367367
Ok(())
368368
} else {
369369
let res = callback().await;
370-
if res.is_ok() {
371-
*last_written_version = version;
372-
}
370+
// A failed multi-store operation may still have updated one of its stores. We record
371+
// the attempted version regardless so an older operation cannot overwrite newer state.
372+
*last_written_version = version;
373373
res
374374
}
375375
};
@@ -614,6 +614,7 @@ mod tests {
614614
use std::future::Future;
615615
use std::panic::RefUnwindSafe;
616616
use std::path::PathBuf;
617+
use std::sync::atomic::{AtomicUsize, Ordering};
617618
use std::sync::Arc;
618619

619620
use lightning::util::logger::Level;
@@ -647,20 +648,25 @@ mod tests {
647648
TierStore::new(primary_store, logger)
648649
}
649650

650-
/// A store whose `list`/`list_paginated` always fail while every other operation is delegated
651-
/// to an inner [`InMemoryStore`]. Used to prove that a failing ephemeral list does not sink a
652-
/// listing for a namespace that can never hold an ephemeral-cached key.
653-
struct FailingListStore {
651+
enum FailureMode {
652+
List,
653+
Write { attempts: Arc<AtomicUsize> },
654+
}
655+
656+
/// A store that injects a selected failure while delegating other operations to an inner
657+
/// [`InMemoryStore`].
658+
struct FailingStore {
654659
inner: InMemoryStore,
660+
failure_mode: FailureMode,
655661
}
656662

657-
impl FailingListStore {
658-
fn new() -> Self {
659-
Self { inner: InMemoryStore::new() }
663+
impl FailingStore {
664+
fn new(failure_mode: FailureMode) -> Self {
665+
Self { inner: InMemoryStore::new(), failure_mode }
660666
}
661667
}
662668

663-
impl KVStore for FailingListStore {
669+
impl KVStore for FailingStore {
664670
fn read(
665671
&self, primary_namespace: &str, secondary_namespace: &str, key: &str,
666672
) -> impl Future<Output = Result<Vec<u8>, io::Error>> + 'static + Send {
@@ -669,26 +675,62 @@ mod tests {
669675
fn write(
670676
&self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: Vec<u8>,
671677
) -> impl Future<Output = Result<(), io::Error>> + 'static + Send {
672-
KVStore::write(&self.inner, primary_namespace, secondary_namespace, key, buf)
678+
let write = if let FailureMode::Write { attempts } = &self.failure_mode {
679+
attempts.fetch_add(1, Ordering::Relaxed);
680+
None
681+
} else {
682+
Some(KVStore::write(&self.inner, primary_namespace, secondary_namespace, key, buf))
683+
};
684+
async move {
685+
match write {
686+
Some(write) => write.await,
687+
None => Err(io::Error::new(io::ErrorKind::Other, "write failed")),
688+
}
689+
}
673690
}
674691
fn remove(
675692
&self, primary_namespace: &str, secondary_namespace: &str, key: &str, lazy: bool,
676693
) -> impl Future<Output = Result<(), io::Error>> + 'static + Send {
677694
KVStore::remove(&self.inner, primary_namespace, secondary_namespace, key, lazy)
678695
}
679696
fn list(
680-
&self, _primary_namespace: &str, _secondary_namespace: &str,
697+
&self, primary_namespace: &str, secondary_namespace: &str,
681698
) -> impl Future<Output = Result<Vec<String>, io::Error>> + 'static + Send {
682-
async { Err(io::Error::new(io::ErrorKind::Other, "list failed")) }
699+
let list = match &self.failure_mode {
700+
FailureMode::List => None,
701+
FailureMode::Write { .. } => {
702+
Some(KVStore::list(&self.inner, primary_namespace, secondary_namespace))
703+
},
704+
};
705+
async move {
706+
match list {
707+
Some(list) => list.await,
708+
None => Err(io::Error::new(io::ErrorKind::Other, "list failed")),
709+
}
710+
}
683711
}
684712
}
685713

686-
impl PaginatedKVStore for FailingListStore {
714+
impl PaginatedKVStore for FailingStore {
687715
fn list_paginated(
688-
&self, _primary_namespace: &str, _secondary_namespace: &str,
689-
_page_token: Option<PageToken>,
716+
&self, primary_namespace: &str, secondary_namespace: &str,
717+
page_token: Option<PageToken>,
690718
) -> impl Future<Output = Result<PaginatedListResponse, io::Error>> + 'static + Send {
691-
async { Err(io::Error::new(io::ErrorKind::Other, "list_paginated failed")) }
719+
let list = match &self.failure_mode {
720+
FailureMode::List => None,
721+
FailureMode::Write { .. } => Some(PaginatedKVStore::list_paginated(
722+
&self.inner,
723+
primary_namespace,
724+
secondary_namespace,
725+
page_token,
726+
)),
727+
};
728+
async move {
729+
match list {
730+
Some(list) => list.await,
731+
None => Err(io::Error::new(io::ErrorKind::Other, "list_paginated failed")),
732+
}
733+
}
692734
}
693735
}
694736

@@ -1026,7 +1068,8 @@ mod tests {
10261068
let mut tier = setup_tier_store(Arc::clone(&primary_store), logger);
10271069

10281070
// An ephemeral store whose `list`/`list_paginated` always fail.
1029-
let ephemeral_store: Arc<DynStore> = Arc::new(DynStoreWrapper(FailingListStore::new()));
1071+
let ephemeral_store: Arc<DynStore> =
1072+
Arc::new(DynStoreWrapper(FailingStore::new(FailureMode::List)));
10301073
tier.set_ephemeral_store(Arc::clone(&ephemeral_store));
10311074

10321075
// A durable key in a namespace that can never hold an ephemeral-cached key.
@@ -1165,6 +1208,56 @@ mod tests {
11651208
assert_eq!(persisted, new_data);
11661209
}
11671210

1211+
#[tokio::test]
1212+
async fn failed_newer_backup_write_still_supersedes_older_write() {
1213+
let base_dir = random_storage_path();
1214+
let log_path = base_dir.join("tier_store_test.log").to_string_lossy().into_owned();
1215+
let logger = Arc::new(Logger::new_fs_writer(log_path, Level::Trace).unwrap());
1216+
1217+
let _cleanup = CleanupDir(base_dir);
1218+
1219+
let primary_store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
1220+
let mut tier = setup_tier_store(Arc::clone(&primary_store), logger);
1221+
1222+
let backup_write_attempts = Arc::new(AtomicUsize::new(0));
1223+
let backup_store: Arc<DynStore> =
1224+
Arc::new(DynStoreWrapper(FailingStore::new(FailureMode::Write {
1225+
attempts: Arc::clone(&backup_write_attempts),
1226+
})));
1227+
tier.set_backup_store(backup_store);
1228+
1229+
let old_data = vec![1u8; 32];
1230+
let new_data = vec![2u8; 32];
1231+
let old_write = tier.write(
1232+
CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE,
1233+
CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE,
1234+
CHANNEL_MANAGER_PERSISTENCE_KEY,
1235+
old_data,
1236+
);
1237+
let new_write = tier.write(
1238+
CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE,
1239+
CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE,
1240+
CHANNEL_MANAGER_PERSISTENCE_KEY,
1241+
new_data.clone(),
1242+
);
1243+
1244+
// The primary write succeeds, but the same newer write fails on the backup.
1245+
assert!(new_write.await.is_err());
1246+
// The older operation must be treated as stale even though the newer operation failed.
1247+
old_write.await.unwrap();
1248+
1249+
let persisted = primary_store
1250+
.read(
1251+
CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE,
1252+
CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE,
1253+
CHANNEL_MANAGER_PERSISTENCE_KEY,
1254+
)
1255+
.await
1256+
.unwrap();
1257+
assert_eq!(persisted, new_data);
1258+
assert_eq!(backup_write_attempts.load(Ordering::Relaxed), 1);
1259+
}
1260+
11681261
#[tokio::test]
11691262
async fn ephemeral_writes_preserve_latest_call_order() {
11701263
let base_dir = random_storage_path();

0 commit comments

Comments
 (0)