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.

Sign up for GitHub

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

Jump to bottom

Bug 1665041 - Don't backoff on wait and add count limit for pending pings #1217

Merged
merged 5 commits into from
Sep 17, 2020
Merged

Bug 1665041 - Don't backoff on wait and add count limit for pending pings #1217

merged 5 commits into from
Sep 17, 2020

Conversation

badboy
Copy link
Member

@badboy badboy commented Sep 17, 2020

No description provided.

brizental
brizental approved these changes Sep 17, 2020
View reviewed changes
Copy link
Contributor

@brizental brizental left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

glean-core/src/upload/policy.rs Show resolved Hide resolved
travis79
travis79 approved these changes Sep 17, 2020
View reviewed changes
Copy link
Member

@travis79 travis79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! Let's go ahead and file a bug to revisit the directory size limit after Bea's analysis. Thanks for doing this!

glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/PingUploadWorker.kt Outdated Show resolved Hide resolved
@badboy
Don't retry on wait, but sleep
1185f4a 
This way we don't use the backoff policy of WorkManager itself, which
will keep increase as we check and check again.

[skip ci]
@badboy
Implement a quota based on number of pending pnigs
6448c24 
We limit it to a maximum of 250 pending pings.

The average number of baseline pings per client (on Fenix) is at 15 pings a day.
The P99 value is ~110.
With a maximum of 250 we can store about 2 days worth of pings.
A baseline ping file averages about 600 bytes, so that's a total of just 144 kB we store.
With the default rate limit of 15 pings per 60s it would take roughly 16 minutes to send out all pending
pings.
@badboy
Document storage limitations based on number of pending pings
6f666a8 
[doc only]
@badboy
Track the total number of pending pings at startup
db0d4ad 
This introduces a new metric to do so.
@badboy badboy force-pushed the dont-backoff-on-wait branch 2 times, most recently from 0b296d9 to 5332059 Compare September 17, 2020 14:53
@badboy badboy merged commit bb35159 into main Sep 17, 2020
@badboy badboy deleted the dont-backoff-on-wait branch September 17, 2020 15:18
badboy added a commit to badboy/application-services that referenced this pull request Sep 18, 2020
@badboy
Upgrade Glean to v32.4.0
319da93 
Glean changelog:

* General
  * Allow using quantity metric type outside of Gecko ([mozilla#1198](mozilla/glean#1198))
  * Update `glean_parser` to 1.28.5
    * The `SUPERFLUOUS_NO_LINT` warning has been removed from the glinter. It likely did more harm than good, and makes it hard to make metrics.yaml files that pass across different versions of `glean_parser`.
    * Expired metrics will now produce a linter warning, `EXPIRED_METRIC`.
    * Expiry dates that are more than 730 days (~2 years) in the future will produce a linter warning, `EXPIRATION_DATE_TOO_FAR`.
    * Allow using the Quantity metric type outside of Gecko.
    * New parser configs `custom_is_expired` and `custom_validate_expires` added. These are both functions that take the expires value of the metric and return a bool. (See `Metric.is_expired` and `Metric.validate_expires`). These will allow FOG to provide custom validation for its version-based `expires` values.
  * Add a limit of 250 pending ping files. ([mozilla#1217](mozilla/glean#1217)).

Note: This also gets rid of the 2 workarounds (removed code) in
AppService thanks to upstream changes.
@badboy badboy mentioned this pull request Sep 18, 2020
Merged
4 tasks
badboy added a commit to badboy/application-services that referenced this pull request Sep 21, 2020
@badboy
Upgrade Glean to v32.4.0
5b3f4cf 
Glean changelog:

* General
  * Allow using quantity metric type outside of Gecko ([mozilla#1198](mozilla/glean#1198))
  * Update `glean_parser` to 1.28.5
    * The `SUPERFLUOUS_NO_LINT` warning has been removed from the glinter. It likely did more harm than good, and makes it hard to make metrics.yaml files that pass across different versions of `glean_parser`.
    * Expired metrics will now produce a linter warning, `EXPIRED_METRIC`.
    * Expiry dates that are more than 730 days (~2 years) in the future will produce a linter warning, `EXPIRATION_DATE_TOO_FAR`.
    * Allow using the Quantity metric type outside of Gecko.
    * New parser configs `custom_is_expired` and `custom_validate_expires` added. These are both functions that take the expires value of the metric and return a bool. (See `Metric.is_expired` and `Metric.validate_expires`). These will allow FOG to provide custom validation for its version-based `expires` values.
  * Add a limit of 250 pending ping files. ([mozilla#1217](mozilla/glean#1217)).

Note: This also gets rid of the 2 workarounds (removed code) in
AppService thanks to upstream changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@travis79 travis79 travis79 approved these changes

@brizental brizental brizental approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@badboy @travis79 @brizental