Skip to content
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

Set journal mode on create rather than open. #3091

Merged
merged 3 commits into from Jun 2, 2022
Merged

Conversation

ChrisPenner
Copy link
Contributor

Overview

We previously set the journal mode to WAL every time we opened a connection, this likely because we would switch the journal mode to DELETE when pushing to ensure the database was fully synced from the .wal file.

Since then, we've done a lot of work to ensure our codebase connections are better managed, and no longer need to set DELETE mode on push because we close connections correctly before pushing.

Now, since we never switch away from WAL mode there's no reason to explicitly set WAL mode on every connection, and it's causing some issues in the Share server since we open the codebase for every request and setting the journal mode is requires locking the whole DB.

So, now we can just set WAL on every new codebase and assume all existing codebases are already in WAL mode.

Implementation notes

  • Set WAL mode when creating a new sqlite DB rather than on every new connection.
  • Also set WAL mode when pulling from git repos, since it's possible some old codebases still exist which were pushed in DELETE mode.

@ChrisPenner ChrisPenner marked this pull request as ready for review June 2, 2022 20:34
Co-authored-by: Mitchell Rosen <mitchellwrosen@gmail.com>
@ChrisPenner ChrisPenner added the ready-to-merge Apply this to a PR and it will get merged automatically once CI passes and 1 reviewer has approved label Jun 2, 2022
@ChrisPenner ChrisPenner self-assigned this Jun 2, 2022
@mergify mergify bot merged commit 3234b5f into trunk Jun 2, 2022
@mergify mergify bot deleted the cp/wal-on-create branch June 2, 2022 22:00
@mergify mergify bot removed the ready-to-merge Apply this to a PR and it will get merged automatically once CI passes and 1 reviewer has approved label Jun 2, 2022
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.

None yet

2 participants