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

REAPI remote cache no longer retries store failures #19732

Closed
huonw opened this issue Aug 31, 2023 · 0 comments · Fixed by #19737
Closed

REAPI remote cache no longer retries store failures #19732

huonw opened this issue Aug 31, 2023 · 0 comments · Fixed by #19737
Assignees
@huonw
Labels
bug remote
Milestone
2.18.x

Comments

@huonw
Copy link
Contributor

huonw commented Aug 31, 2023 •
edited
Loading

Describe the bug

In #19050, I moved code from src/rust/engine/fs/store/src/remote.rs to src/rust/engine/fs/store/src/remote/reapi.rs, but didn't move retry_calls for the stores (previously in store_buffered and store_bytes in remote.rs) into reapi.rs. Oops!

The reapi.rs store methods should call retry_call to retry transient failures too.

Pants version
#19050 landed in 2.18.0.dev0

OS
N/A

Additional info
N/A
@huonw huonw added this to the 2.18.x milestone Aug 31, 2023
@huonw huonw self-assigned this Aug 31, 2023
This was referenced Aug 31, 2023
Merged
Merged
stuhood pushed a commit that referenced this issue Sep 7, 2023
@huonw
Restore retries for stores to remote REAPI byte stores (#19737)
b1d9e14 
This fixes #19732 by restoring the retries when storing hits retryable
server failures from the REAPI remote cache server, which were lost in
the #19050 refactoring.

This also explicitly tests for retries, refactoring `StubCAS` to
generalise `read_request_count` to expose the counts of more requests
than just reads, and also consistently return a `Status::internal(...)`
for the simulated errors.

I think #19050 fortunately landed just after 2.17 was cut, so this
regression only affects the 2.18 pre-releases.
WorkerPants pushed a commit that referenced this issue Sep 7, 2023
@huonw @WorkerPants
Restore retries for stores to remote REAPI byte stores (#19737)
9f32da8 
This fixes #19732 by restoring the retries when storing hits retryable
server failures from the REAPI remote cache server, which were lost in
the #19050 refactoring.

This also explicitly tests for retries, refactoring `StubCAS` to
generalise `read_request_count` to expose the counts of more requests
than just reads, and also consistently return a `Status::internal(...)`
for the simulated errors.

I think #19050 fortunately landed just after 2.17 was cut, so this
regression only affects the 2.18 pre-releases.
@WorkerPants WorkerPants mentioned this issue Sep 7, 2023
Merged
huonw added a commit that referenced this issue Sep 7, 2023
@WorkerPants @huonw
Restore retries for stores to remote REAPI byte stores (Cherry-pick of …
dab1027 
…#19737) (#19798)

This fixes #19732 by restoring the retries when storing hits retryable
server failures from the REAPI remote cache server, which were lost in
the #19050 refactoring.

This also explicitly tests for retries, refactoring `StubCAS` to
generalise `read_request_count` to expose the counts of more requests
than just reads, and also consistently return a `Status::internal(...)`
for the simulated errors.

I think #19050 fortunately landed just after 2.17 was cut, so this
regression only affects the 2.18 pre-releases.

Co-authored-by: Huon Wilson <huon@exoflare.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@huonw huonw

Labels
bug remote
Projects
None yet
Milestone
2.18.x
Development

Successfully merging a pull request may close this issue.

Restore retries for stores to remote REAPI byte stores pantsbuild/pants
1 participant
@huonw