Introduce option to checkpoint database on Flush() #1104
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dawidk-msft
requested review from
kindbe,
maxgolov,
mkoscumb,
reyang,
bliptec,
lalitb and
sid-dahiya
as code owners
The way SQLite is initialized (synchronous=NORMAL config) introduces potential issues with durability of changes/updates in some corner cases. iOS Outlook logs telemetry in scenarios where some critical validation/check failed and cannot be handled and process self-terminates. In between logging the event and terminating we call Flush() to avoid lost events. If we checkpoint the DB before returning from the Flush() call app can safely assume no events will be lost. This change introduces an option that enables such behavior. Alternative solution would be to use the default configuration (with synchronous=FULL) which doesn't have that problem but has overhead of filesystem syncing for each transaction which makes it a bad idea from the perf point of view.
dawidk-msft
force-pushed
the
dawidk/db-checkpoint-on-flush
branch
from
84b6b5d
to
0ed7e39
Compare
|
While the changes are behind config flag, so should be good in general. Just wanted to check, if you have seen the DB corruption / data loss issue in power failure scenario. The synchronous=NORMAL mode should be in general resilient, with very small window of WAL logs not getting synced to physical disk since last checkpoint. Haven't seen this window getting hit, so just curious. |
lalitb
approved these changes
Mar 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The way SQLite is initialized (synchronous=NORMAL config) introduces potential issues with durability of changes/updates in some corner cases (power loss, ungraceful process termination/exit, etc).
iOS Outlook logs telemetry in scenarios where if some critical validation/check failed and cannot be handled process will self-terminate. In between logging the event and terminating we call Flush() to avoid lost events. If we checkpoint the DB before returning from the Flush() call app can safely assume no events will be lost. This change introduces an option that enables such behavior.
Alternative solution would be to use the default configuration (with synchronous=FULL) which doesn't have that problem but has overhead of filesystem syncing for each transaction which makes it a bad idea from the perf point of view.