Implement MigratableKVStore for all KV Stores#945
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
benthecarman
force-pushed
the
migrate-dbs
branch
from
653afa3 to
0708bfe
Compare
This was unimplemented for the sqlite kv store. Useful if the user wants to migrate to a different database and also in tests so we don't have to re-init and setup a node. AI-assisted-by: OpenAI Codex
This was unimplemented for the postgres kv store. Useful if the user wants to migrate to a different database and also in tests so we don't have to re-init and setup a node. AI-assisted-by: OpenAI Codex
Extract the existing obfuscated key selection so later VSS listing changes can reuse it without changing the parsing behavior. AI-assisted-by: OpenAI Codex
This was unimplemented for the VSS kv store. Useful if the user wants to migrate to a different database and keeps VSS aligned with the other persistent stores. AI-assisted-by: OpenAI Codex
This was unimplemented for the in-memory kv store. Useful in tests so we can migrate data across all supported store implementations. AI-assisted-by: OpenAI Codex
benthecarman
force-pushed
the
migrate-dbs
branch
from
0708bfe to
3953744
Compare
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Thanks, first pass.
Would be cool to add a commit that does a full end-to-end test of migration. I.e., pick a random backend, setup a node with it, open a channel to another node, send some payments, then drop everything and migrate to a random new backend, then ensure the node is still fully functional.
| io::Error::new(io::ErrorKind::Other, msg) | ||
| })?; | ||
|
|
||
| let mut keys = Vec::new(); |
There was a problem hiding this comment.
Let's preallocate via with_capacity here.
| store_id: self.store_id.clone(), | ||
| key_prefix: None, | ||
| page_token, | ||
| page_size: None, |
There was a problem hiding this comment.
Do we need to use pagination in case we're migrating with many keys?
| && secondary_namespace.is_empty() | ||
| && key == VSS_SCHEMA_VERSION_KEY | ||
| { | ||
| continue; |
There was a problem hiding this comment.
Hmm, maybe we should attempt to always migrate to the latest schema version, allowing nodes to upgrade?
|
|
||
| fn list_all_keys_internal(&self) -> io::Result<Vec<(String, String, String)>> { | ||
| let persisted_lock = self.persisted_bytes.lock().unwrap(); | ||
| let mut keys = Vec::new(); |
There was a problem hiding this comment.
nit: Use with_capacity here.
3 participants
Pulled out of #915
These were unimplemented for our kv stores. Useful if the user wants to migrate to a different database and also in tests so we don't have to re-init and setup a node.