From 91ae8b87ca6d665e4c29023861b1708c9fc8fba9 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Mon, 9 Dec 2024 11:20:10 +0100 Subject: [PATCH 1/5] Define `MigratableKVStore` trait with `list_all_keys` method .. which will be used for generic `KVStore`-to-`KVStore` migration logic. --- lightning/src/util/persist.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs index 732b54955..325aebfed 100644 --- a/lightning/src/util/persist.rs +++ b/lightning/src/util/persist.rs @@ -163,6 +163,19 @@ pub trait KVStore { ) -> Result, io::Error>; } +/// Provides additional interface methods that are required for [`KVStore`]-to-[`KVStore`] +/// data migration. +pub trait MigratableKVStore: KVStore { + /// Returns *all* known keys as a list of `primary_namespace`, `secondary_namespace`, `key` tuples. + /// + /// This is useful for migrating data from [`KVStore`] implementation to [`KVStore`] + /// implementation. + /// + /// Must exhaustively return all entries known to the store to ensure no data is missed, but + /// may return the items in arbitrary order. + fn list_all_keys(&self) -> Result, io::Error>; +} + /// Trait that handles persisting a [`ChannelManager`], [`NetworkGraph`], and [`WriteableScore`] to disk. /// /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager From d69a5ee4600bda0858e6cf1dabcdf2fb47c9f672 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 11 Dec 2024 15:34:18 +0100 Subject: [PATCH 2/5] Prefactor: DRY up dir entry checks and key extraction to helper methods .. which we'll be reusing shortly. --- lightning-persister/src/fs_store.rs | 171 ++++++++++++++++------------ 1 file changed, 97 insertions(+), 74 deletions(-) diff --git a/lightning-persister/src/fs_store.rs b/lightning-persister/src/fs_store.rs index 3f9cdd156..9e42e935c 100644 --- a/lightning-persister/src/fs_store.rs +++ b/lightning-persister/src/fs_store.rs @@ -316,84 +316,13 @@ impl KVStore for FilesystemStore { let entry = entry?; let p = entry.path(); - if let Some(ext) = p.extension() { - #[cfg(target_os = "windows")] - { - // Clean up any trash files lying around. - if ext == "trash" { - fs::remove_file(p).ok(); - continue; - } - } - if ext == "tmp" { - continue; - } - } - - let metadata = p.metadata()?; - - // We allow the presence of directories in the empty primary namespace and just skip them. - if metadata.is_dir() { + if !dir_entry_is_key(&p)? { continue; } - // If we otherwise don't find a file at the given path something went wrong. - if !metadata.is_file() { - debug_assert!( - false, - "Failed to list keys of {}/{}: file couldn't be accessed.", - PrintableString(primary_namespace), - PrintableString(secondary_namespace) - ); - let msg = format!( - "Failed to list keys of {}/{}: file couldn't be accessed.", - PrintableString(primary_namespace), - PrintableString(secondary_namespace) - ); - return Err(lightning::io::Error::new(lightning::io::ErrorKind::Other, msg)); - } + let key = get_key_from_dir_entry(&p, &prefixed_dest)?; - match p.strip_prefix(&prefixed_dest) { - Ok(stripped_path) => { - if let Some(relative_path) = stripped_path.to_str() { - if is_valid_kvstore_str(relative_path) { - keys.push(relative_path.to_string()) - } - } else { - debug_assert!( - false, - "Failed to list keys of {}/{}: file path is not valid UTF-8", - PrintableString(primary_namespace), - PrintableString(secondary_namespace) - ); - let msg = format!( - "Failed to list keys of {}/{}: file path is not valid UTF-8", - PrintableString(primary_namespace), - PrintableString(secondary_namespace) - ); - return Err(lightning::io::Error::new( - lightning::io::ErrorKind::Other, - msg, - )); - } - }, - Err(e) => { - debug_assert!( - false, - "Failed to list keys of {}/{}: {}", - PrintableString(primary_namespace), - PrintableString(secondary_namespace), - e - ); - let msg = format!( - "Failed to list keys of {}/{}: {}", - PrintableString(primary_namespace), - PrintableString(secondary_namespace), - e - ); - return Err(lightning::io::Error::new(lightning::io::ErrorKind::Other, msg)); - }, - } + keys.push(key); } self.garbage_collect_locks(); @@ -402,6 +331,100 @@ impl KVStore for FilesystemStore { } } +fn dir_entry_is_key(p: &Path) -> Result { + if let Some(ext) = p.extension() { + #[cfg(target_os = "windows")] + { + // Clean up any trash files lying around. + if ext == "trash" { + fs::remove_file(p).ok(); + return Ok(false); + } + } + if ext == "tmp" { + return Ok(false); + } + } + + let metadata = p.metadata().map_err(|e| { + let msg = format!( + "Failed to list keys at path {}: {}", + PrintableString(p.to_str().unwrap_or_default()), + e + ); + lightning::io::Error::new(lightning::io::ErrorKind::Other, msg) + })?; + + // We allow the presence of directories in the empty primary namespace and just skip them. + if metadata.is_dir() { + return Ok(false); + } + + // If we otherwise don't find a file at the given path something went wrong. + if !metadata.is_file() { + debug_assert!( + false, + "Failed to list keys at path {}: file couldn't be accessed.", + PrintableString(p.to_str().unwrap_or_default()) + ); + let msg = format!( + "Failed to list keys at path {}: file couldn't be accessed.", + PrintableString(p.to_str().unwrap_or_default()) + ); + return Err(lightning::io::Error::new(lightning::io::ErrorKind::Other, msg)); + } + + Ok(true) +} + +fn get_key_from_dir_entry(p: &Path, base_path: &Path) -> Result { + match p.strip_prefix(&base_path) { + Ok(stripped_path) => { + if let Some(relative_path) = stripped_path.to_str() { + if is_valid_kvstore_str(relative_path) { + return Ok(relative_path.to_string()); + } else { + debug_assert!( + false, + "Failed to list keys of path {}: file path is not valid key", + PrintableString(p.to_str().unwrap_or_default()) + ); + let msg = format!( + "Failed to list keys of path {}: file path is not valid key", + PrintableString(p.to_str().unwrap_or_default()) + ); + return Err(lightning::io::Error::new(lightning::io::ErrorKind::Other, msg)); + } + } else { + debug_assert!( + false, + "Failed to list keys of path {}: file path is not valid UTF-8", + PrintableString(p.to_str().unwrap_or_default()) + ); + let msg = format!( + "Failed to list keys of path {}: file path is not valid UTF-8", + PrintableString(p.to_str().unwrap_or_default()) + ); + return Err(lightning::io::Error::new(lightning::io::ErrorKind::Other, msg)); + } + }, + Err(e) => { + debug_assert!( + false, + "Failed to list keys of path {}: {}", + PrintableString(p.to_str().unwrap_or_default()), + e + ); + let msg = format!( + "Failed to list keys of path {}: {}", + PrintableString(p.to_str().unwrap_or_default()), + e + ); + return Err(lightning::io::Error::new(lightning::io::ErrorKind::Other, msg)); + }, + } +} + #[cfg(test)] mod tests { use super::*; From 96f7bfda8cee486b47549119007558493a95f178 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 11 Dec 2024 16:12:49 +0100 Subject: [PATCH 3/5] Implement `MigratableKVStore` for `FilesystemStore` We implement the new interface on `FilesystemStore`, in particular `list_all_keys`. --- lightning-persister/src/fs_store.rs | 68 ++++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/lightning-persister/src/fs_store.rs b/lightning-persister/src/fs_store.rs index 9e42e935c..44417a963 100644 --- a/lightning-persister/src/fs_store.rs +++ b/lightning-persister/src/fs_store.rs @@ -1,7 +1,7 @@ //! Objects related to [`FilesystemStore`] live here. use crate::utils::{check_namespace_key_validity, is_valid_kvstore_str}; -use lightning::util::persist::KVStore; +use lightning::util::persist::{KVStore, MigratableKVStore}; use lightning::util::string::PrintableString; use std::collections::HashMap; @@ -425,6 +425,72 @@ fn get_key_from_dir_entry(p: &Path, base_path: &Path) -> Result Result, lightning::io::Error> { + let prefixed_dest = &self.data_dir; + if !prefixed_dest.exists() { + return Ok(Vec::new()); + } + + let mut keys = Vec::new(); + + 'primary_loop: for primary_entry in fs::read_dir(prefixed_dest)? { + let primary_path = primary_entry?.path(); + + if dir_entry_is_key(&primary_path)? { + let primary_namespace = String::new(); + let secondary_namespace = String::new(); + let key = get_key_from_dir_entry(&primary_path, prefixed_dest)?; + keys.push((primary_namespace, secondary_namespace, key)); + continue 'primary_loop; + } + + // The primary_entry is actually also a directory. + 'secondary_loop: for secondary_entry in fs::read_dir(&primary_path)? { + let secondary_path = secondary_entry?.path(); + + if dir_entry_is_key(&secondary_path)? { + let primary_namespace = get_key_from_dir_entry(&primary_path, prefixed_dest)?; + let secondary_namespace = String::new(); + let key = get_key_from_dir_entry(&secondary_path, &primary_path)?; + keys.push((primary_namespace, secondary_namespace, key)); + continue 'secondary_loop; + } + + // The secondary_entry is actually also a directory. + for tertiary_entry in fs::read_dir(&secondary_path)? { + let tertiary_entry = tertiary_entry?; + let tertiary_path = tertiary_entry.path(); + + if dir_entry_is_key(&tertiary_path)? { + let primary_namespace = + get_key_from_dir_entry(&primary_path, prefixed_dest)?; + let secondary_namespace = + get_key_from_dir_entry(&secondary_path, &primary_path)?; + let key = get_key_from_dir_entry(&tertiary_path, &secondary_path)?; + keys.push((primary_namespace, secondary_namespace, key)); + } else { + debug_assert!( + false, + "Failed to list keys of path {}: only two levels of namespaces are supported", + PrintableString(tertiary_path.to_str().unwrap_or_default()) + ); + let msg = format!( + "Failed to list keys of path {}: only two levels of namespaces are supported", + PrintableString(tertiary_path.to_str().unwrap_or_default()) + ); + return Err(lightning::io::Error::new( + lightning::io::ErrorKind::Other, + msg, + )); + } + } + } + } + Ok(keys) + } +} + #[cfg(test)] mod tests { use super::*; From 4c1f6bdf34fa752058c0163f4e66bc529122bd26 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 12 Dec 2024 13:51:50 +0100 Subject: [PATCH 4/5] Add `migrate_kv_store_data` util method .. allowing to migrate data from one store to another. --- lightning/src/util/persist.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs index 325aebfed..d0e9e01a4 100644 --- a/lightning/src/util/persist.rs +++ b/lightning/src/util/persist.rs @@ -176,6 +176,27 @@ pub trait MigratableKVStore: KVStore { fn list_all_keys(&self) -> Result, io::Error>; } +/// Migrates all data from one store to another. +/// +/// This operation assumes that `target_store` is empty, i.e., any data present under copied keys +/// might get overriden. User must ensure `source_store` is not modified during operation, +/// otherwise no consistency guarantees can be given. +/// +/// Will abort and return an error if any IO operation fails. Note that in this case the +/// `target_store` might get left in an intermediate state. +pub fn migrate_kv_store_data( + source_store: &mut S, target_store: &mut T, +) -> Result<(), io::Error> { + let keys_to_migrate = source_store.list_all_keys()?; + + for (primary_namespace, secondary_namespace, key) in &keys_to_migrate { + let data = source_store.read(primary_namespace, secondary_namespace, key)?; + target_store.write(primary_namespace, secondary_namespace, key, &data)?; + } + + Ok(()) +} + /// Trait that handles persisting a [`ChannelManager`], [`NetworkGraph`], and [`WriteableScore`] to disk. /// /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager From b0af39faafb661b7fb772867aa41d558a53e17f4 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 12 Dec 2024 15:39:39 +0100 Subject: [PATCH 5/5] Add test for `FilesystemStore`-to-`FilesystemStore` migration --- lightning-persister/src/fs_store.rs | 17 +++++++++++- lightning-persister/src/test_utils.rs | 40 ++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/lightning-persister/src/fs_store.rs b/lightning-persister/src/fs_store.rs index 44417a963..487e1a7ac 100644 --- a/lightning-persister/src/fs_store.rs +++ b/lightning-persister/src/fs_store.rs @@ -494,7 +494,9 @@ impl MigratableKVStore for FilesystemStore { #[cfg(test)] mod tests { use super::*; - use crate::test_utils::{do_read_write_remove_list_persist, do_test_store}; + use crate::test_utils::{ + do_read_write_remove_list_persist, do_test_data_migration, do_test_store, + }; use bitcoin::Txid; @@ -527,6 +529,19 @@ mod tests { do_read_write_remove_list_persist(&fs_store); } + #[test] + fn test_data_migration() { + let mut source_temp_path = std::env::temp_dir(); + source_temp_path.push("test_data_migration_source"); + let mut source_store = FilesystemStore::new(source_temp_path); + + let mut target_temp_path = std::env::temp_dir(); + target_temp_path.push("test_data_migration_target"); + let mut target_store = FilesystemStore::new(target_temp_path); + + do_test_data_migration(&mut source_store, &mut target_store); + } + #[test] fn test_if_monitors_is_not_dir() { let store = FilesystemStore::new("test_monitors_is_not_dir".into()); diff --git a/lightning-persister/src/test_utils.rs b/lightning-persister/src/test_utils.rs index 735870e49..18492336d 100644 --- a/lightning-persister/src/test_utils.rs +++ b/lightning-persister/src/test_utils.rs @@ -3,7 +3,10 @@ use lightning::ln::functional_test_utils::{ connect_block, create_announced_chan_between_nodes, create_chanmon_cfgs, create_dummy_block, create_network, create_node_cfgs, create_node_chanmgrs, send_payment, }; -use lightning::util::persist::{read_channel_monitors, KVStore, KVSTORE_NAMESPACE_KEY_MAX_LEN}; +use lightning::util::persist::{ + migrate_kv_store_data, read_channel_monitors, KVStore, MigratableKVStore, + KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN, +}; use lightning::util::test_utils; use lightning::{check_added_monitors, check_closed_broadcast, check_closed_event}; @@ -59,6 +62,41 @@ pub(crate) fn do_read_write_remove_list_persist(kv_s assert_eq!(listed_keys.len(), 0); } +pub(crate) fn do_test_data_migration( + source_store: &mut S, target_store: &mut T, +) { + // We fill the source with some bogus keys. + let dummy_data = [42u8; 32]; + let num_primary_namespaces = 2; + let num_secondary_namespaces = 2; + let num_keys = 3; + for i in 0..num_primary_namespaces { + let primary_namespace = + format!("testspace{}", KVSTORE_NAMESPACE_KEY_ALPHABET.chars().nth(i).unwrap()); + for j in 0..num_secondary_namespaces { + let secondary_namespace = + format!("testsubspace{}", KVSTORE_NAMESPACE_KEY_ALPHABET.chars().nth(j).unwrap()); + for k in 0..num_keys { + let key = + format!("testkey{}", KVSTORE_NAMESPACE_KEY_ALPHABET.chars().nth(k).unwrap()); + source_store + .write(&primary_namespace, &secondary_namespace, &key, &dummy_data) + .unwrap(); + } + } + } + let total_num_entries = num_primary_namespaces * num_secondary_namespaces * num_keys; + let all_keys = source_store.list_all_keys().unwrap(); + assert_eq!(all_keys.len(), total_num_entries); + + migrate_kv_store_data(source_store, target_store).unwrap(); + + assert_eq!(target_store.list_all_keys().unwrap().len(), total_num_entries); + for (p, s, k) in &all_keys { + assert_eq!(target_store.read(p, s, k).unwrap(), dummy_data); + } +} + // Integration-test the given KVStore implementation. Test relaying a few payments and check that // the persisted data is updated the appropriate number of times. pub(crate) fn do_test_store(store_0: &K, store_1: &K) {