server: Add initial `MetaStore` by LucioFranco · Pull Request #616 · tursodatabase/libsql

LucioFranco · 2023-11-13T20:49:44Z

This PR adds a new internal metadata store for namespace configuration.
This internally uses a normal namespace so that the replica also get's
synchronized. This is just initial work there are a few more issues to
solve regarding replica's getting updated configs and some internal
hardening that needs to be done. Tracking issue for follow up work #768.

Closes #501

psarna · 2023-11-15T11:41:02Z

what's MetaStore?

haaawk · 2023-11-15T11:50:55Z

what's MetaStore?

Storage for metadata (aka namespace configs)

LucioFranco · 2023-12-07T22:06:17Z

libsql-server/src/namespace/meta_store.rs

+    store: Namespace<T>,
+}
+
+async fn execute<B: QueryResultBuilder>(


These next few functions I would like to refactor into a proper struct but for now this will do. Marked as follow up item 👍

LucioFranco · 2023-12-07T22:09:33Z

libsql-server/src/namespace/meta_store.rs

+        let sender = lock.entry(namespace.clone()).or_insert_with(|| {
+            // TODO(lucio): if no entry exists we need to ensure we send the update to
+            // the bg channel.
+            let (tx, _) = watch::channel(Arc::new(DatabaseConfig::default()));


There is a follow up item for this in #768

LucioFranco · 2023-12-07T22:13:40Z

libsql-server/tests/namespaces/meta.rs

@@ -0,0 +1 @@
+


reserved for some tests that I will do as follow up but this will require getting bottomless backups also working in our test suite which may be a lot of work.

I think this will create more confusion than value. I don't think anyone will still the meta.rs file so I would just drop this from the PR

I've added a test here, these will be expanded upon in follow up PRs

MarinPostma · 2023-12-08T16:24:05Z

As discussed in our call, I don't think this is the right approach. Instead, I propose that we do the following:

The meta store becomes per-sqld instance. The instance has sole r/w permission to it.
This meta store db file is not stored in the /dbs directory, but at the root, of the directory structure.
The backups are provided by bottomless, by creating a BottomlessWalManager in a similar fashion as it is done for the primary: https://github.com/tursodatabase/libsql/blob/main/libsql-server/src/namespace/mod.rs#L923-L942. The configuration for the backup of the meta-store can be passed as a CLI option.
a connection to the meta-store is created using https://github.com/tursodatabase/libsql/blob/main/libsql-server/src/connection/libsql.rs#L216. we need the active_checkpoint variant to let the connection checkpoint.
The meta-store is initialized prior to openning the NamespaceStore, and it is passed to it. A handle to the namespace store id is passed to the admin API.

Configuration changes regarding a replica are propagated by mean of the replication protocol. The primary sends the relevant configuration to the replica in the handshake response. When the config for a replica changes, the primary requests a new handshake, and the replica updates its config.

libsql/tests/integration_tests.rs

libsql-server/tests/standalone/mod.rs

libsql-server/tests/cluster/mod.rs

libsql-server/src/rpc/mod.rs

libsql-server/src/rpc/replica_proxy.rs

libsql-server/src/namespace/mod.rs

libsql-server/src/namespace/meta_store.rs

haaawk · 2023-12-11T09:08:08Z

Member

@LucioFranco please make sure you split that work into two parts: on instance storage + primary/replica synchronization. The first part is blocking the per namespace jwts so it would be great to get it done quick.

LucioFranco · 2023-12-11T20:44:17Z

@haaawk ive extracted all the debug out into #777

LucioFranco · 2023-12-11T20:45:56Z

@LucioFranco please make sure you split that work into two parts: on instance storage + primary/replica synchronization. The first part is blocking the per namespace jwts so it would be great to get it done quick.

Added a TODO to #768 @MarinPostma and I have a good idea how to do this and yes we can merge this now and unblock your work.

LucioFranco · 2023-12-11T21:43:00Z

@haaawk @MarinPostma this is ready for review and I've added a test as well.

libsql-server/tests/cluster/mod.rs

libsql-server/src/namespace/meta_store.rs

MarinPostma · 2023-12-12T10:01:55Z

libsql-server/src/namespace/meta_store.rs

+
+    let rows = stmt.query(())?.mapped(|r| {
+        let ns = r.get::<_, String>(0)?;
+        let config = r.get::<_, Vec<u8>>(1)?;


There's no need to allocate here, you can get a &[u8] and parse json straight from it

I tried to change this to a borrowed version but since the row borrow is only for this closure we can't return it. I think since this is just a restore a string/vec is fine here, we can change if we find its really causing issues. Configs should be quite small anyways.

MarinPostma · 2023-12-12T10:09:11Z

libsql-server/src/namespace/meta_store.rs

+    Internal(Arc<Mutex<Arc<DatabaseConfig>>>),
+    External(mpsc::Sender<ChangeMsg>, Receiver<Arc<DatabaseConfig>>),


what's the purpose of those?

Some unit tests use the internal so its a legacy thing that we can eventually remove but it was done in place to allow those tests to not have to update how they work with the metastore.

MarinPostma · 2023-12-12T10:31:34Z

libsql-server/src/namespace/meta_store.rs

+    // when we are updating the config. The config si already synced via the watch
+    // channel.
+    configs: HashMap<NamespaceName, Sender<Arc<DatabaseConfig>>>,
+    db: Connection,


you should use a libsql_sys::Connection here instead

Also, I get it that you want to add durability in a subsequent PR?

Added as follow up item in #768 (related to bottomless backup).

MarinPostma · 2023-12-12T12:22:04Z

I left a few comments; feel free to dismiss, merge, and address them in a follow-up.

haaawk · 2023-12-12T12:38:39Z

libsql-server/src/namespace/meta_store.rs

+
+#[derive(Debug, Clone)]
+enum HandleState {
+    Internal(Arc<Mutex<Arc<DatabaseConfig>>>),


Could we have Arc<Mutex<DatabaseConfig>>?

Returned type expects an Arc<DatabaseConfig> so we unfortunately need to do this but that said this code should be test only and internal unit test so let me see if I can wrap in a cfg and we can forget about it until we refactor those tests.

haaawk · 2023-12-12T12:54:16Z

libsql-server/src/namespace/meta_store.rs

+
+    let mut configs = HashMap::new();
+
+    for row in rows {


Why two loops instead of looping over query results and do what this loop is doing straight away?

Conceptually and api wise, 1) being I like having the part that extracts out of the row be its own piece of code (but we rows doesn't implement iterator so we can't just loop) and 2) then putting the actual de serialization where we want some special logic in a separate block. Goal being that people should focus on the big for loop rather than the mapped rows section.

haaawk

LGTM

This PR adds a new internal metadata store for namespace configuration. This internally uses a normal namespace so that the replica also get's synchronized. This is just initial work there are a few more issues to solve regarding replica's getting updated configs and some internal hardening that needs to be done. Tracking issue for follow up work #768. Closes #501

LucioFranco force-pushed the lucio/auth-namespace2 branch from 92d0c97 to d26f101 Compare November 13, 2023 20:54

LucioFranco mentioned this pull request Nov 13, 2023

Allow custom bottomless db id for namespace #596

Merged

LucioFranco force-pushed the lucio/auth-namespace2 branch from d26f101 to 7cf6268 Compare November 14, 2023 21:12

LucioFranco force-pushed the lucio/auth-namespace2 branch 3 times, most recently from d86cc92 to 1e9ec0b Compare December 7, 2023 22:04

LucioFranco requested review from MarinPostma and haaawk December 7, 2023 22:05

LucioFranco marked this pull request as ready for review December 7, 2023 22:05

LucioFranco commented Dec 7, 2023

View reviewed changes

LucioFranco force-pushed the lucio/auth-namespace2 branch 4 times, most recently from 5ebc56a to c0d349d Compare December 8, 2023 02:01

haaawk reviewed Dec 11, 2023

View reviewed changes

libsql/tests/integration_tests.rs Outdated Show resolved Hide resolved

haaawk reviewed Dec 11, 2023

View reviewed changes

libsql-server/tests/standalone/mod.rs Show resolved Hide resolved

haaawk reviewed Dec 11, 2023

View reviewed changes

libsql-server/tests/cluster/mod.rs Show resolved Hide resolved

haaawk reviewed Dec 11, 2023

View reviewed changes

libsql-server/src/rpc/mod.rs Show resolved Hide resolved

haaawk reviewed Dec 11, 2023

View reviewed changes

libsql-server/src/rpc/replica_proxy.rs Outdated Show resolved Hide resolved

haaawk reviewed Dec 11, 2023

View reviewed changes

libsql-server/src/namespace/mod.rs Outdated Show resolved Hide resolved

haaawk reviewed Dec 11, 2023

View reviewed changes

libsql-server/src/namespace/meta_store.rs Outdated Show resolved Hide resolved

LucioFranco force-pushed the lucio/auth-namespace2 branch 3 times, most recently from c9ffaad to a545db6 Compare December 11, 2023 20:38

LucioFranco force-pushed the lucio/auth-namespace2 branch from a545db6 to 6668911 Compare December 11, 2023 20:42

LucioFranco force-pushed the lucio/auth-namespace2 branch from 6668911 to fdcb4a8 Compare December 11, 2023 21:41

LucioFranco requested a review from haaawk December 11, 2023 21:43

LucioFranco force-pushed the lucio/auth-namespace2 branch from fdcb4a8 to fce0516 Compare December 11, 2023 23:39

MarinPostma suggested changes Dec 12, 2023

View reviewed changes

haaawk reviewed Dec 12, 2023

View reviewed changes

LucioFranco force-pushed the lucio/auth-namespace2 branch from fce0516 to b1da52c Compare December 12, 2023 15:38

LucioFranco requested a review from MarinPostma December 12, 2023 15:50

MarinPostma approved these changes Dec 12, 2023

View reviewed changes

LucioFranco added this pull request to the merge queue Dec 12, 2023

Merged via the queue into main with commit 5e28284 Dec 12, 2023

LucioFranco deleted the lucio/auth-namespace2 branch December 12, 2023 18:48

psarna mentioned this pull request Dec 19, 2023

libsql-server: use LRU cache to store active namespaces #744

Merged

		Internal(Arc<Mutex<Arc<DatabaseConfig>>>),
		External(mpsc::Sender<ChangeMsg>, Receiver<Arc<DatabaseConfig>>),

Conversation

LucioFranco commented Nov 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

psarna commented Nov 15, 2023

Uh oh!

haaawk commented Nov 15, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LucioFranco Dec 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarinPostma commented Dec 8, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

haaawk commented Dec 11, 2023

Uh oh!

LucioFranco commented Dec 11, 2023

Uh oh!

LucioFranco commented Dec 11, 2023

Uh oh!

LucioFranco commented Dec 11, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarinPostma commented Dec 12, 2023

Uh oh!

haaawk Dec 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haaawk left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

LucioFranco commented Nov 13, 2023 •

edited

Loading

LucioFranco Dec 7, 2023 •

edited

Loading

haaawk Dec 12, 2023 •

edited

Loading