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

[HOLD] Allocate all SQLite pool handles on init #2020

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

coleaeason
Copy link
Contributor

@coleaeason coleaeason commented Dec 12, 2024

Details

@tylerkaraszewski this causes us to allocate the entirety of a SQLitePool on creation, which for our largest pools is at start up time. We hope this prevents random slow PRAGMAs from starting new handles JIT.

Fixed Issues

https://expensify.slack.com/archives/C07PP7QV8P5/p1733855847062219

Fixes https://github.com/Expensify/Expensify/issues/453048

Tests

Existing. Ran tests, saw expected logs.


Internal Testing Reminder: when changing bedrock, please compile auth against your new changes

@coleaeason coleaeason self-assigned this Dec 12, 2024
Copy link
Contributor

@tylerkaraszewski tylerkaraszewski left a comment

Choose a reason for hiding this comment

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

Can we time this with the production number of DB handles before merging?

@coleaeason
Copy link
Contributor Author

On my mac:

2024-12-12T22:57:08.763985+00:00 expensidev2004 bedrock: xxxxxx (SQLitePool.cpp:16) SQLitePool [sync] [info] Allocating SQLitePool handles.
2024-12-12T22:57:18.182813+00:00 expensidev2004 bedrock: xxxxxx (SQLitePool.cpp:21) SQLitePool [sync] [info] Allocated 25000 handles for SQLitePool.

takes 10s

@coleaeason coleaeason changed the title Allocate all SQLite pool handles on init [HOLD] Allocate all SQLite pool handles on init Dec 12, 2024
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.

5 participants