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
Sidecar Flag: Random delay before upload blocks on sidecar #7239
base: main
Are you sure you want to change the base?
Changes from 9 commits
fd63531
6d18845
c9c9076
e09f3f8
15eceec
cced736
cfca6cb
24eafbe
3566f56
ab01ce9
da63e08
386dfee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -228,6 +228,7 @@ Flags: | |
configuration. See format details: | ||
https://thanos.io/tip/thanos/tracing.md/#configuration | ||
--tsdb.path="./data" Data directory of TSDB. | ||
--upload-jitter=0s Maximum random delay before uploading blocks. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to fix docs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also it looks like this flag is never consumed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also added nowhere There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yess will remove it |
||
--version Show application version. | ||
|
||
``` | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,10 @@ | |
// err := runutil.Repeat(10*time.Second, stopc, func() error { | ||
// // ... | ||
// }) | ||
// | ||
//err := runutil.RepeatWithJitter(30*time.Second, ctx, 0.05, func() error { | ||
// Your code here | ||
//}) | ||
|
||
// Retry starts executing closure function f until no error is returned from f: | ||
// | ||
// err := runutil.Retry(10*time.Second, stopc, func() error { | ||
|
@@ -50,8 +53,10 @@ | |
package runutil | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"io" | ||
"math/rand" | ||
"os" | ||
"path/filepath" | ||
"strings" | ||
|
@@ -99,6 +104,32 @@ func Repeat(interval time.Duration, stopc <-chan struct{}, f func() error) error | |
} | ||
} | ||
|
||
// RepeatWithJitter executes f with a random jitter added to the interval between each execution. | ||
// It continues until ctx is done or f returns an error. | ||
// The jitter factor should be between 0 and 1, where 0 means no jitter and 1 means the interval can vary from 0 to 2 times the original interval. | ||
func RepeatWithJitter(interval time.Duration, ctx context.Context, jitterFactor float64, f func() error) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The ctx should be always first parameter. https://pkg.go.dev/context#pkg-overview |
||
for { | ||
select { | ||
case <-ctx.Done(): | ||
return nil | ||
default: | ||
if err := f(); err != nil { | ||
return err | ||
} | ||
|
||
jitter := time.Duration(float64(interval) * jitterFactor) | ||
|
||
jitteredInterval := interval + time.Duration(rand.Float64()*float64(jitter)) | ||
|
||
select { | ||
case <-ctx.Done(): | ||
return nil | ||
case <-time.After(jitteredInterval): | ||
} | ||
} | ||
} | ||
} | ||
|
||
// Retry executes f every interval seconds until timeout or no error is returned from f. | ||
func Retry(interval time.Duration, stopc <-chan struct{}, f func() error) error { | ||
return RetryWithLog(log.NewNopLogger(), interval, stopc, f) | ||
|
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.
Does this really help though if we are jittering only 6s; at least for us block uploads can take far longer
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.
We can increase the jitter. What value do you think that makes sense?
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.
I don't know, the whole method feels kinda weird. It was about in house blob storage and blocks being uploaded at same time right?