Bug 1749510 - RLB: Wait for uploader on shutdown using new shutdown mechanism #2332
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
chutten
reviewed
Jan 10, 2023
badboy
force-pushed
the
1749510-upload-shutdown
branch
from
280ccab
to
865791b
Compare
chutten
approved these changes
Feb 1, 2023
I would prefer to do the instrumentation in a followup. Because of how we upload this is ... not as easy as I would like and so to keep review on point doing that in a followup seems easier. |
Very well, r+ conditional on adding a CHANGELOG entry and filing a follow-up for instrumentation |
travis79
approved these changes
Feb 1, 2023
badboy
force-pushed
the
1749510-upload-shutdown
branch
3 times, most recently
from
e564ae3
to
649ff48
Compare
badboy
changed the title
On shutdown we now wait up to 30s for the upload thread to finish. This gets done just after we drain and shut down the dispatcher, but before Glean is considered fully shutdown. The callback MUST NOT put any new tasks on the dispatcher. The callback SHOULD NOT block arbitrarily long. The RLB implementation of that callback instructs a potential uploader thread to end operation after it finished one upload. It also blocks any new upload thread from getting started. This means an in-flight upload can finish as usual, but no new one is launched. If the upload thread is currently sleeping, it gets one more upload. This also adds a new test. The only way to reliably test this is from the outside, so the test invokes an example implementation and checks that the `pending_pings` directory is empty after the program ends. The uploader inside sleeps for a short period of time. Without waiting for this uploader the main thread would end and kill the uploader thread before it wakes up. With waiting the uploader finishes correctly before the main thread exits.
badboy
force-pushed
the
1749510-upload-shutdown
branch
from
649ff48
to
f6a1aeb
Compare
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.
RLB: Wait for uploader on shutdown using new
shutdownmechanismOn shutdown we now wait up to 30s for the upload thread to finish.
This gets done just after we drain and shut down the dispatcher,
but before Glean is considered fully shutdown.
The callback MUST NOT put any new tasks on the dispatcher.
The callback SHOULD NOT block arbitrarily long.
The RLB implementation of that callback instructs a potential uploader
thread to end operation after it finished one upload.
It also blocks any new upload thread from getting started.
This means an in-flight upload can finish as usual, but no new one
is launched.
If the upload thread is currently sleeping, it gets one more upload.
This also adds a new test.
The only way to reliably test this is from the outside,
so the test invokes an example implementation and checks that the
pending_pingsdirectory is empty after the program ends.The uploader inside sleeps for a short period of time.
Without waiting for this uploader the main thread would end and kill the
uploader thread before it wakes up.
With waiting the uploader finishes correctly before the main thread
exits.