Skip to content

Bootstrap database before opening connection #2021

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 10, 2025

Conversation

avinassh
Copy link
Member

@avinassh avinassh commented Apr 9, 2025

This patch is a follow up on #2017 It changes the heuristics for bootstrapping the DB.

One more change is, it also does bootstrap before any connection to the local SQLite is created

Earlier, we bootstrapped whenever we noticed a change in the generation. However, this patch changes this behaviour and does bootstrap only if local files don't exist:

  1. if no db file or the metadata file exists, then user is starting from scratch
    and we will do the sync
  2. if the db file exists, but the metadata file does not exist (or other way around),
    then local db is in an incorrect state. we stop and return with an error
  3. if the db file exists and the metadata file exists, then we don't need to do the
    sync

@avinassh avinassh requested a review from penberg April 9, 2025 11:37
@avinassh avinassh force-pushed the local-sync branch 2 times, most recently from 7af7834 to 0558de3 Compare April 9, 2025 11:46
@avinassh avinassh marked this pull request as draft April 9, 2025 11:56
@avinassh avinassh force-pushed the local-sync branch 2 times, most recently from aef6b24 to 9f2b898 Compare April 9, 2025 13:06
@avinassh avinassh marked this pull request as ready for review April 9, 2025 15:29
let result: String = rows.next().await.unwrap().unwrap().get(0).unwrap();
assert_eq!(result, "ok", "integrity check failed after sync");
println!("Integrity check passed after sync");
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please drop this and let's instead work on #1998 to do this internally.

@penberg penberg changed the title Change the heuristics for bootstrapping the db Bootstrap database before opening connection Apr 10, 2025
@penberg
Copy link
Collaborator

penberg commented Apr 10, 2025

I think we should just make connect() bootstrap if needed to avoid people having to call sync(). If there's some problem with that, then connect() at minimum needs to return an error telling people to sync first for empty database.

@avinassh
Copy link
Member Author

I have opened an issue to track integrity check and bootstrapping on connect

#2024
#2023

we use the following heuristic to determine if we need to sync the db file
1. if no db file or the metadata file exists, then user is starting from scratch
and we will do the sync
2. if the metadata file exists, but the db file does not exist,
then local db is in an incorrect state. we stop and return with an error
3. if the db file exists and the metadata file exists, then we don't need to do the
sync
If we try to bootstrap the db after a connection is opened, then it is
incorrect, as it could create a local .db file and a successful sync
would replace this file
@penberg penberg enabled auto-merge April 10, 2025 15:20
@penberg penberg added this pull request to the merge queue Apr 10, 2025
Merged via the queue into tursodatabase:main with commit 361d082 Apr 10, 2025
19 checks passed
@avinassh avinassh deleted the local-sync branch April 10, 2025 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants