-
Notifications
You must be signed in to change notification settings - Fork 840
Add hedged request to Store Gateway #6388
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
Add hedged request to Store Gateway #6388
Conversation
b5378b2 to
3994384
Compare
yeya24
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Just a small nit
c595c0f to
cbd4232
Compare
friedrichg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Only a minor typo!
Signed-off-by: SungJin1212 <[email protected]>
cbd4232 to
dca3507
Compare
| hrt.mu.Lock() | ||
| err = hrt.TDigest.Add(duration) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential deadlock here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I made a PR here thanos-io/thanos#7962.
cc. @yeya24
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the catch! @damnever
This PR adds hedged requests to Store Gateway. It can reduce the tail latency between Store Gateway and Object store.
Which issue(s) this PR fixes:
Fixes #6343
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]