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

DBP: Implement exponential backoff for optout retries #2815

Merged
merged 4 commits into from
Jun 5, 2024

Conversation

jotaemepereira
Copy link
Collaborator

Task/Issue URL: https://app.asana.com/0/1204006570077678/1207370913988488/f
Tech Design URL:
CC:

Description:
Implement exponential backoff for opt-out retries

Steps to test this PR:

  1. Run opt-out operations that you know will fail; you can add some manual code to make them fail if you want to. Or just throw in the CaptchaService endpoints for example.
  2. Then run queued operations. After finishing running, check that every time the preferredRunDate is set after the opt-out fails, it respects the exponential calculation depending on the number of failures. It should never pass the retryError limit on the broker JSON file (at the moment is 48 hours for all brokers)

@jotaemepereira jotaemepereira requested a review from Bunn May 23, 2024 19:23
Copy link
Contributor

@aataraxiaa aataraxiaa left a comment

Choose a reason for hiding this comment

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

I tested this using the following steps:

  1. Added a log statement to calculateNextRunDate
  2. Ran a profile scan
  3. Ran opt-outs via debug menu

I didn’t run queued operations, as I think running the opt-outs confirms the expected behavior, but please correct me if I am mistaken @jotaemepereira. The log output was the following, which seems to indicate correctness:

PAST TRIES: 1, NEXT DATE: 7200.0
PAST TRIES: 1, NEXT DATE: 7200.0
PAST TRIES: 1, NEXT DATE: 7200.0
PAST TRIES: 1, NEXT DATE: 7200.0
PAST TRIES: 2, NEXT DATE: 14400.0
PAST TRIES: 2, NEXT DATE: 14400.0
PAST TRIES: 1, NEXT DATE: 7200.0
PAST TRIES: 1, NEXT DATE: 7200.0
PAST TRIES: 3, NEXT DATE: 28800.0
PAST TRIES: 3, NEXT DATE: 28800.0
PAST TRIES: 4, NEXT DATE: 57600.0
PAST TRIES: 4, NEXT DATE: 57600.0
PAST TRIES: 5, NEXT DATE: 115200.0
PAST TRIES: 5, NEXT DATE: 115200.0
PAST TRIES: 6, NEXT DATE: 172800.0
PAST TRIES: 6, NEXT DATE: 172800.0
PAST TRIES: 2, NEXT DATE: 14400.0
PAST TRIES: 2, NEXT DATE: 14400.0
PAST TRIES: 7, NEXT DATE: 172800.0
PAST TRIES: 7, NEXT DATE: 172800.0
PAST TRIES: 8, NEXT DATE: 172800.0
PAST TRIES: 8, NEXT DATE: 172800.0
PAST TRIES: 9, NEXT DATE: 172800.0
PAST TRIES: 9, NEXT DATE: 172800.0
PAST TRIES: 10, NEXT DATE: 172800.0
PAST TRIES: 10, NEXT DATE: 172800.0
PAST TRIES: 3, NEXT DATE: 28800.0
PAST TRIES: 3, NEXT DATE: 28800.0
PAST TRIES: 11, NEXT DATE: 172800.0
PAST TRIES: 11, NEXT DATE: 172800.0
PAST TRIES: 12, NEXT DATE: 172800.0
PAST TRIES: 12, NEXT DATE: 172800.0
PAST TRIES: 13, NEXT DATE: 172800.0
PAST TRIES: 13, NEXT DATE: 172800.0
PAST TRIES: 14, NEXT DATE: 172800.0
PAST TRIES: 14, NEXT DATE: 172800.0
PAST TRIES: 4, NEXT DATE: 57600.0
PAST TRIES: 4, NEXT DATE: 57600.0
PAST TRIES: 15, NEXT DATE: 172800.0
PAST TRIES: 15, NEXT DATE: 172800.0
PAST TRIES: 1, NEXT DATE: 7200.0
PAST TRIES: 1, NEXT DATE: 7200.0
PAST TRIES: 2, NEXT DATE: 14400.0
PAST TRIES: 2, NEXT DATE: 14400.0
PAST TRIES: 16, NEXT DATE: 172800.0
PAST TRIES: 16, NEXT DATE: 172800.0
PAST TRIES: 3, NEXT DATE: 28800.0
PAST TRIES: 3, NEXT DATE: 28800.0
PAST TRIES: 17, NEXT DATE: 172800.0
PAST TRIES: 17, NEXT DATE: 172800.0
PAST TRIES: 4, NEXT DATE: 57600.0
PAST TRIES: 4, NEXT DATE: 57600.0
PAST TRIES: 5, NEXT DATE: 115200.0
PAST TRIES: 5, NEXT DATE: 115200.0
PAST TRIES: 6, NEXT DATE: 172800.0
...

So, based on my understanding of expected behavior and these results, approving. However, as I said @jotaemepereira, please correct me if I missed something.

Copy link
Collaborator

@Bunn Bunn left a comment

Choose a reason for hiding this comment

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

LGTM, small rename comment

Copy link
Contributor

github-actions bot commented Jun 5, 2024

This PR has been inactive for more than 7 days and will be automatically closed 7 days from now.

@jotaemepereira jotaemepereira force-pushed the juan/exponential-backoff-for-optout-retries branch from 52fd81b to a847bab Compare June 5, 2024 00:36
@jotaemepereira jotaemepereira merged commit 05bc6ef into main Jun 5, 2024
20 checks passed
@jotaemepereira jotaemepereira deleted the juan/exponential-backoff-for-optout-retries branch June 5, 2024 01:13
samsymons added a commit that referenced this pull request Jun 5, 2024
* main:
  DBP: Implement exponential backoff for optout retries (#2815)
  Update UI Tests CI workflows for macOS 13/14 (#2835)
  Display the addresses in the Debugger UI (#2828)
  Removing temporary password manager survey code (#2834)
  Surface specific XPC & login item errors (#2773)
  DuckPlayer PiP settings (#2830)
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.

3 participants