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

Prevent runaway error reporting #921

Merged
merged 4 commits into from
Dec 28, 2023
Merged

Prevent runaway error reporting #921

merged 4 commits into from
Dec 28, 2023

Conversation

abyrd
Copy link
Member

@abyrd abyrd commented Dec 20, 2023

This PR addresses problems discussed in #919 and #887. It makes two small changes, each of which alone would prevent the runaway error reporting observed on 2023-12-18 (EST). In addition it reduces the volume of errors sent and recorded.

Comment on lines +401 to +405
// Limit the number of errors recorded to one.
// Still using a Set<String> instead of just String since the set of errors is exposed in a UI-facing API.
if (job.errors.isEmpty()) {
job.errors.add(error);
}
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a sensible safeguard -- in most cases, error messages coming in from workers will be identical. And if they vary slightly (e.g. including a different origin id), we probably don't want to record all messages separately.

@abyrd abyrd merged commit f349656 into dev Dec 28, 2023
3 checks passed
@abyrd abyrd deleted the job-error-limiting branch December 28, 2023 01:55
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