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

Fix infinite retry bug #2125

Merged
merged 3 commits into from
Oct 18, 2023
Merged

Fix infinite retry bug #2125

merged 3 commits into from
Oct 18, 2023

Conversation

inahga
Copy link
Contributor

@inahga inahga commented Oct 13, 2023

An infinite retry could be triggered if a report with the same ID as an expired report was submitted.

Supports #2124.

I didn't add a separate method for get_client_reports_including_expired_ones because naming things is hard, and I think this is the only situation where we care about expired reports.

I altered the error message to provide more detail as to what's happening in this branch of code. The error here does matter because it is logged by tracing.

@inahga inahga requested a review from a team as a code owner October 13, 2023 20:49
aggregator_core/src/datastore.rs Outdated Show resolved Hide resolved
aggregator_core/src/datastore/tests.rs Outdated Show resolved Hide resolved
An infinite retry could be triggered if a report with the same ID as an expired report was ssubmitted.
It does somewhat matter what error we return here, because errors returned from this function are
logged.
@inahga inahga force-pushed the inahga/fix-runaway-retry-bug branch from 251193a to f43b2ab Compare October 18, 2023 21:29
@inahga inahga enabled auto-merge (squash) October 18, 2023 21:30
@inahga inahga mentioned this pull request Oct 18, 2023
@inahga inahga merged commit e4435c6 into main Oct 18, 2023
8 checks passed
@inahga inahga deleted the inahga/fix-runaway-retry-bug branch October 18, 2023 22:45
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