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

Bug 1679949: Don't block on running ping uploading process #1350

Merged
merged 4 commits into from
Dec 2, 2020

Conversation

mdboom
Copy link
Contributor

@mdboom mdboom commented Dec 1, 2020

We only want to run one ping uploading subprocess at a time -- for one, it might
delete ping files, and we can't have contention for that. This is currently
handled by waiting on an existing uploading subprocess before firing off a new
one. However, this can cause the main dispatcher thread to block, causing work
to go uncompleted and subsequent pings to go unsent, on shutdown of the main process.

Instead, this just does nothing if an uploading subprocess is already running,
since the existing uploader will also pick up any new pings added to the
outgoing directory and send them.

We only want to run one ping uploading subprocess at a time -- for one, it might
delete ping files, and we can't have contention for that. This is currently
handled by waiting on an existing uploading subprocess before firing off a new
one. However, this can cause the main dispatcher thread to block, causing work
to go uncompleted and subsequent pings to go unsent, on shutdown of the main process.

Instead, this just does nothing if an uploading subprocess is already running,
since the existing uploader will also pick up any new pings added to the
outgoing directory and send them.
@mdboom mdboom requested a review from badboy December 1, 2020 17:41
@auto-assign auto-assign bot requested a review from Dexterp37 December 1, 2020 17:41
@Dexterp37 Dexterp37 removed their request for review December 1, 2020 18:01
glean-core/python/tests/test_network.py Outdated Show resolved Hide resolved
@badboy badboy merged commit c9da1b7 into mozilla:main Dec 2, 2020
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.

2 participants