Glean Rust shutdown() doesn't have a way to await upload tasks
Categories
(Data Platform and Tools :: Glean: SDK, defect, P1)
Tracking
(firefox112 fixed)
Tracking | Status | |
---|---|---|
firefox112 | --- | fixed |
People
(Reporter: travis_, Assigned: janerik)
References
Details
Attachments
(1 file)
(deleted),
text/x-github-pull-request
|
Details |
When the Glean Rust SDK triggers an upload it doesn't keep track of the handle of the thread that it spawns. If it did track the thread handles, then when glean::shutdown()
is called these threads could be awaited. This isn't useful for desktop, so it might be necessary to have a separate shutdown()
function that awaits uploads or a passed parameter to the shutdown()
function to control this functionality.
For an example where I ran into this, I was adding Glean to the Nimbus CLI example to test the Rust bindings and telemetry collection and because it was a CLI app and exited quickly, Glean didn't have enough time to finish uploading pings without a sleep
, because shutdown
doesn't wait for uploads to complete.
Comment 1•3 years ago
|
||
This isn't useful for desktop
Really? I mean, sure, desktop doesn't care enough (at least not yet) to have asked to be able to await ping sends on shutdown...
FOG's Firefox Desktop shutdown comes after desktop net teardown meaning all these ping upload requests would quickly fail anyway, so join'ing on glean.upload
wouldn't be a big deal.
An additional wrinkle is the background update agent which has its own ideas of how it wants shutdown to proceed (see bug 1703572).
All in all, I'm not sure Desktop needs a specifically distinct behaviour here. We might be just fine.
Assignee | ||
Comment 2•3 years ago
|
||
(In reply to Chris H-C :chutten from comment #1)
This isn't useful for desktop
Really? I mean, sure, desktop doesn't care enough (at least not yet) to have asked to be able to await ping sends on shutdown...
FOG's Firefox Desktop shutdown comes after desktop net teardown meaning all these ping upload requests would quickly fail anyway, so join'ing on
glean.upload
wouldn't be a big deal.
See, I wasn't sure anymore and had a vague feeling we blocked something by mistake which we changed.
If this "just works" for Desktop: even better!
Updated•3 years ago
|
Comment 3•2 years ago
|
||
bug 1797884 is an instance of an intermittent test failure that we suspect is likely caused by shutdown having no way to await upload tasks in progress. In it the Glean SDK uses the Glean storage for internal uploader metrics (number of pending pings, size of the pending ping dir, that sort of thing.) which could very well be happening during shutdown (where we delete the storage as we're using it).
This one's happening in a test (with test_reset_glean
and all that) in much the same way as we worked around Swift test interkittens via this code... so even ignoring outside consumers like Nimbus, Firefox Desktop Background Update Agent, and Firefox Desktop Directory Cleaner, we have our own reasons for making this work (and for updating test_reset_glean
to take advantage of it).
Comment 4•2 years ago
|
||
Kicking back to triage now that the scope's changed.
Updated•2 years ago
|
Assignee | ||
Comment 5•2 years ago
|
||
I plan to propose a way to handle this in the next week.
Updated•2 years ago
|
Comment 6•2 years ago
|
||
Assignee | ||
Comment 7•2 years ago
|
||
badboy merged PR #2332: "Bug 1749510 - RLB: Wait for uploader on shutdown using new shutdown mechanism" in dbc1281.
This landed. I want to take this for a spin in m-c, see if this works as expected there.
Assignee | ||
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Comment 8•2 years ago
|
||
This landed a while ago in a release and subsequently in m-c. Nothing suspicious popped up so I'm closing this.
We have some "See Also" bugs and those can be tackled independently.
Comment 9•2 years ago
|
||
(In reply to Jan-Erik Rediger [:janerik] from comment #8)
This landed a while ago in a release and subsequently in m-c. Nothing suspicious popped up so I'm closing this.
We have some "See Also" bugs and those can be tackled independently.
Could you please set the tracking flags so it's clear when this hit Nightly, Beta, Release, etc? I had the same issue with https://bugzilla.mozilla.org/show_bug.cgi?id=1802094: it's not easy to track these out of tree changes and see when they make it to Firefox.
Assignee | ||
Comment 10•2 years ago
|
||
Absolutely. We indeed forgot to link the bug landing the update here as well.
Description
•