snapshot: sqlite: use a subdirectory for consistency with LMDB #932

Merged
lx merged 1 commit from baptiste/garage:snapshot_consistency_sqlite into main 2025-01-27 18:50:11 +00:00
Owner

Currently, taking a snapshot of the metadata database with sqlite creates
a sqlite file without extension with the following format:

snapshots/2025-01-26T15:29:17Z

This makes it hard to understand what kind of data this is, and is not
consistent with LMDB:

snapshots/2025-01-26T15:29:17Z/data.mdb

With this change, we now get a directory with a single db.sqlite file:

snapshots/2025-01-26T15:29:17Z/db.sqlite
Currently, taking a snapshot of the metadata database with sqlite creates a sqlite file without extension with the following format: snapshots/2025-01-26T15:29:17Z This makes it hard to understand what kind of data this is, and is not consistent with LMDB: snapshots/2025-01-26T15:29:17Z/data.mdb With this change, we now get a directory with a single db.sqlite file: snapshots/2025-01-26T15:29:17Z/db.sqlite
baptiste added 1 commit 2025-01-26 15:39:04 +00:00
snapshot: sqlite: use a subdirectory for consistency with LMDB
Some checks are pending
ci/woodpecker/pr/debug Pipeline is running
70db03373b
Currently, taking a snapshot of the metadata database with sqlite creates
a sqlite file without extension with the following format:

    snapshots/2025-01-26T15:29:17Z

This makes it hard to understand what kind of data this is, and is not
consistent with LMDB:

    snapshots/2025-01-26T15:29:17Z/data.mdb

With this change, we now get a directory with a single sqlite file:

    snapshots/2025-01-26T15:29:17Z/data.sqlite
Owner

@baptiste Could you rebase this PR on the current main to allow the updated CI to go through? Thanks, and sorry for the inconvenience.

@baptiste Could you rebase this PR on the current main to allow the updated CI to go through? Thanks, and sorry for the inconvenience.
baptiste force-pushed snapshot_consistency_sqlite from 70db03373b to 17b76a7ee0 2025-01-27 17:34:19 +00:00 Compare
lx requested changes 2025-01-27 17:52:03 +00:00
lx left a comment
Owner

Just a small change, otherwise LGTM

Just a small change, otherwise LGTM
@ -146,1 +146,4 @@
}
std::fs::create_dir_all(to)?;
let mut path = to.clone();
path.push("data.sqlite");
Owner

I think we should call this file db.sqlite, which is the same name as the sqlite database file in the metadata directory. This makes it clear which file to replace when trying to restore a snapshot.

I think we should call this file `db.sqlite`, which is the same name as the sqlite database file in the metadata directory. This makes it clear which file to replace when trying to restore a snapshot.
Author
Owner

Good catch, thanks, updated!

Good catch, thanks, updated!
baptiste marked this conversation as resolved
baptiste force-pushed snapshot_consistency_sqlite from 17b76a7ee0 to 43402c9619 2025-01-27 18:07:33 +00:00 Compare
lx merged commit beedc9fd11 into main 2025-01-27 18:50:11 +00:00
Sign in to join this conversation.
No reviewers
lx
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Deuxfleurs/garage#932
No description provided.