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

Track RequestedRangeNotSatisfiedException separately in S3 Metrics #109657

Merged
merged 19 commits into from
Jun 20, 2024

Conversation

arteam
Copy link
Contributor

@arteam arteam commented Jun 13, 2024

Due to RCO changes, we started getting a lot of RequestedRangeNotSatisfiedExceptions which are expected. We would like track them separately.

This change a new metrics es.repositories.exceptions.request_range_not_satisfied.total to track all client errors analogous to other S3 errors.

In the future, we can add the error code as an attribute to the metrics, so we can adapt it to all client errors.

Due to RCO changes, we started getting a lot of `RequestedRangeNotSatisfiedException`s which
are expected. We would like to not count them as failures and track them separately.
@arteam arteam added >enhancement :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs labels Jun 13, 2024
@elasticsearchmachine
Copy link
Collaborator

Hi @arteam, I've created a changelog YAML for you.

@arteam arteam marked this pull request as ready for review June 13, 2024 10:18
@arteam arteam requested a review from kingherc June 13, 2024 10:18
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Meta label for distributed team label Jun 13, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@arteam arteam requested a review from ywangd June 13, 2024 10:18
@arteam arteam requested a review from tlrx June 13, 2024 13:47
Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. I left a question.

@arteam arteam requested a review from idegtiarenko June 17, 2024 10:02
Copy link
Contributor

@kingherc kingherc left a comment

Choose a reason for hiding this comment

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

Nice. Some questions/comments.

@arteam arteam requested review from kingherc and ywangd June 18, 2024 08:38
@arteam arteam requested a review from kingherc June 18, 2024 23:51
@arteam
Copy link
Contributor Author

arteam commented Jun 19, 2024

@elasticmachine update branch

Copy link
Contributor

@kingherc kingherc left a comment

Choose a reason for hiding this comment

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

LGTM. However, I'd suggest to get one more approval, ideally @ywangd 's, before merging.

@arteam
Copy link
Contributor Author

arteam commented Jun 19, 2024

Thank you very much @kingherc for a through review! Will wait for @ywangd to give the PR another look 👍

@arteam arteam requested review from ywangd and removed request for ywangd June 19, 2024 15:44
Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

I have some minor comments for your consideration. Thanks!

It's not useful since request range requests are not retryable and we should have mostly one error
@arteam
Copy link
Contributor Author

arteam commented Jun 20, 2024

@elasticmachine update branch

@arteam arteam requested a review from ywangd June 20, 2024 10:22
@arteam
Copy link
Contributor Author

arteam commented Jun 20, 2024

@elasticmachine update branch

@arteam arteam merged commit 3566ee9 into main Jun 20, 2024
4 of 16 checks passed
@arteam arteam deleted the track-requested-range-not-satisfied-exceptions branch June 20, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement Team:Distributed Meta label for distributed team v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants