tests: Skip SQLite fsyncs while testing#21634
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
maflcko
left a comment
maflcko
left a comment
There was a problem hiding this comment.
Left a linter comment, didn't review
achow101
force-pushed
the
optimize-sqlite
branch
from
89a0728 to
7140ed3
Compare
vasild
left a comment
vasild
left a comment
There was a problem hiding this comment.
How much performance boost does this bring?
achow101
force-pushed
the
optimize-sqlite
branch
from
7140ed3 to
5c72335
Compare
vasild
left a comment
vasild
left a comment
There was a problem hiding this comment.
ACK 5c72335f0f848895079913e58534ac9488c18ed7
Locally,wallet_tests/CreateWallet test time [seconds]:
master (cb79cabd) sqlite: 63, 62, 60
master (cb79cabd) bdb: 2, 3, 3
PR (5c72335f) sqlite: 3, 3, 3
Maybe add -unsafesqlitesync=1 to self.args[] if there is no reason to use a separate call to append_config().
|
Concept ACK. Syncing is a robustness measure, and unless you're testing the syncing itself, there is no use to have it enabled in tests (at the expense if extra testing time). Seperately it would make sense to look into the sqlite performance issues, but this is a win. |
Since we want tests to run quickly, and since tests do a lot more db operations than expected we expect to see in actual usage, we disable sqlite's syncing behavior to make db operations run much faster. This syncing behavior is necessary for normal operation as it helps guarantee that data won't become lost or corrupted, but in tests, we don't care about that.
achow101
force-pushed
the
optimize-sqlite
branch
from
5c72335 to
41f891d
Compare
41f891d tests: Skip SQLite fsyncs while testing (Andrew Chow) Pull request description: Since we want tests to run quickly, and since tests do a lot more db operations than expected we expect to see in actual usage, we disable sqlite's syncing behavior to make db operations run much faster. This syncing behavior is necessary for normal operation as it helps guarantee that data won't become lost or corrupted, but in tests, we don't care about that. Fixes bitcoin#21628 ACKs for top commit: vasild: ACK 41f891d Tree-SHA512: f36f969a182c622691ae5113573a3250e8d367437e83a1a9d3d2b55dd3a9cdf3c6474169a7bd271007bb9ce47f585aa7a6aeae6eebbaeb02d79409b02f47fd8b
6 participants
Since we want tests to run quickly, and since tests do a lot more db operations than expected we expect to see in actual usage, we disable sqlite's syncing behavior to make db operations run much faster. This syncing behavior is necessary for normal operation as it helps guarantee that data won't become lost or corrupted, but in tests, we don't care about that.
Fixes #21628