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

Update ignored_error logic for circuit_breaker #18613

Closed
wants to merge 4 commits into from

Conversation

amishra-u
Copy link
Contributor

@amishra-u amishra-u commented Jun 8, 2023

When the digest size exceeds the max configured digest size by remote-cache, an "out_of_range" error is returned. These errors should not be considered as API failures for the circuit breaker logic, as they do not indicate any issues with the remote-cache service.
Similarly there are other non-retriable errors that should not be treated as server failure such as ALREADY_EXISTS.

This change considers non-retriable errors as user/client error and logs them as success. While retriable errors such DEADLINE_EXCEEDED, UNKNOWN etc are logged as failure.

Related PRs
#18359
#18539

@amishra-u amishra-u changed the title Update ignored_error logic for circuit_breake Update ignored_error logic for circuit_breaker Jun 8, 2023
@amishra-u amishra-u marked this pull request as ready for review June 8, 2023 02:01
@amishra-u amishra-u requested a review from a team as a code owner June 8, 2023 02:01
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Remote-Exec Issues and PRs for the Execution (Remote) team labels Jun 8, 2023
@amishra-u
Copy link
Contributor Author

@coeuvre please review it. Updated code as per you suggestion in #18583.

Also related PR #18539 got merged to master and once I tried to merges the changes from master, it was not clean and added some unrelated commits to pr.

Copy link
Member

@coeuvre coeuvre left a comment

Choose a reason for hiding this comment

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

Thanks!

@coeuvre coeuvre added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jun 13, 2023
@coeuvre
Copy link
Member

coeuvre commented Jun 13, 2023

The tests are marked manual for now. b63b4b6

amishra-u added a commit to amishra-u/bazel that referenced this pull request Jun 13, 2023
When the digest size exceeds the max configured digest size by remote-cache, an "out_of_range" error is returned. These errors should not be considered as API failures for the circuit breaker logic, as they do not indicate any issues with the remote-cache service.
Similarly there are other non-retriable errors that should not be treated as server failure such as ALREADY_EXISTS.

This change considers non-retriable errors as user/client error and logs them as success. While retriable errors such `DEADLINE_EXCEEDED`, `UNKNOWN` etc are logged as failure.

Related PRs

Closes bazelbuild#18613.

PiperOrigin-RevId: 539948823
Change-Id: I5b51f6a3aecab7c17d73f78b8234d9a6da49fe6c
@iancha1992 iancha1992 removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jun 13, 2023
iancha1992 pushed a commit that referenced this pull request Jun 14, 2023
When the digest size exceeds the max configured digest size by remote-cache, an "out_of_range" error is returned. These errors should not be considered as API failures for the circuit breaker logic, as they do not indicate any issues with the remote-cache service.
Similarly there are other non-retriable errors that should not be treated as server failure such as ALREADY_EXISTS.

This change considers non-retriable errors as user/client error and logs them as success. While retriable errors such `DEADLINE_EXCEEDED`, `UNKNOWN` etc are logged as failure.

Related PRs

Closes #18613.

PiperOrigin-RevId: 539948823
Change-Id: I5b51f6a3aecab7c17d73f78b8234d9a6da49fe6c
traversaro pushed a commit to traversaro/bazel that referenced this pull request Jun 24, 2023
When the digest size exceeds the max configured digest size by remote-cache, an "out_of_range" error is returned. These errors should not be considered as API failures for the circuit breaker logic, as they do not indicate any issues with the remote-cache service.
Similarly there are other non-retriable errors that should not be treated as server failure such as ALREADY_EXISTS.

This change considers non-retriable errors as user/client error and logs them as success. While retriable errors such `DEADLINE_EXCEEDED`, `UNKNOWN` etc are logged as failure.

Related PRs
bazelbuild#18359
bazelbuild#18539

Closes bazelbuild#18613.

PiperOrigin-RevId: 539948823
Change-Id: I5b51f6a3aecab7c17d73f78b8234d9a6da49fe6c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants