Kotlin: Allow configuring delayPingLifetimeIo in Kotlin #2851
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
badboy
force-pushed
the
kotlin/delay-ping-lifetime-flushing
branch
from
b2f47b0
to
b8ac855
Compare
badboy
force-pushed
the
kotlin/delay-ping-lifetime-flushing
branch
from
b8ac855
to
bf06ed3
Compare
travis79
approved these changes
May 30, 2024
There was a problem hiding this comment.
Please file a bug to document this in the Glean Book, let's not let that cause us to not be able to move fast here, though. Maybe also a follow-up bug to add the PING_LIFETIME_THRESHOLD value to Nimbus so as to be able to tune it without needing to ship code-changes?
chutten
approved these changes
May 30, 2024
| // We need to flush the ping lifetime data before a full shutdown. | ||
| glean | ||
| .storage_opt() | ||
| .map(|storage| storage.persist_ping_lifetime_data()) |
There was a problem hiding this comment.
Huh, wonder if this'll uncover any bugs in Desktop tests.
There was a problem hiding this comment.
only one way to figure that out!
badboy
force-pushed
the
kotlin/delay-ping-lifetime-flushing
branch
from
bf06ed3
to
bcc1733
Compare
We now automatically flush data to disk after 1000 writes into the in-memory ping-lifetime map. If `delay_ping_lifetime_io` is set to true, the data is now flushed to disk in these cases: * When `persist_ping_lifetime_data` is called by the application. * At shutdown (that calls `persist_ping_lifetime_data`) automatically. * Every 1000 writes * Calling `persist_ping_lifetime_data` resets the counter! The downside with `delay_ping_lifetime_io` is the increased risk of small windows of data loss: Data is only persisted so often, so if the process crashes or is killed in between the data is not persisted to disk and we will never know about. We do our best to still persist frequently (after enough writes, on ping submission, on background) and the application could decide to call `persist_ping_lifetime_data` on their own as well, e.g. on idle.
badboy
force-pushed
the
kotlin/delay-ping-lifetime-flushing
branch
from
bcc1733
to
8f3ccdd
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2851 +/- ##
=======================================
Coverage 18.60% 18.60%
=======================================
Files 1 1
Lines 43 43
=======================================
Hits 8 8
Misses 35 35 ☔ View full report in Codecov by Sentry. |
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.
For some documentation (also in the commit message):
Persist ping-lifetime data after 1000 writes
We now automatically flush data to disk after 1000 writes into the
in-memory ping-lifetime map.
If
delay_ping_lifetime_iois set to true, the data is now flushed todisk in these cases:
persist_ping_lifetime_datais called by the application.persist_ping_lifetime_data) automatically.persist_ping_lifetime_dataresets the counter!The downside with
delay_ping_lifetime_iois the increased risk ofsmall windows of data loss:
Data is only persisted so often, so if the process crashes or is killed
in between the data is not persisted to disk and we will never know about.
We do our best to still persist frequently (after enough writes, on ping
submission, on background) and the application could decide to call
persist_ping_lifetime_dataon their own as well, e.g. on idle.