modify current limit parameters dynamically (volume dimension) #3026
Conversation
pkg/chunk/cached_store.go
Outdated
| @@ -42,6 +42,8 @@ const SlowRequest = time.Second * time.Duration(10) | |||
|
|
|||
| var ( | |||
| logger = utils.GetLogger("juicefs") | |||
| UseMountUploadLimitConf = false | |||
There was a problem hiding this comment.
It's better to add a Reload() interface for chunk.Config and hide these details inside chunk/
Because we may want to reload other fields of chunk.Config, and we don't add these for them.
|
hide set conf logic inside chunk module. |
| err = s.store.downLimit.WaitN(context.TODO(), len(p)) | ||
| if err != nil { | ||
| s.store.getTokenFromBucketErrors.WithLabelValues("download").Add(1) | ||
| return 0, fmt.Errorf("method: ReadAt, key = %s, get token from tokenBucket, need token count (page) = %d, error : %s", key, len(p), err) |
There was a problem hiding this comment.
we should not fail the read() if waitN fail
| var err = store.upLimit.WaitN(context.TODO(), len(p.Data)) | ||
| if err != nil { | ||
| store.getTokenFromBucketErrors.WithLabelValues("upload").Add(1) | ||
| return fmt.Errorf("method: put, key = %s, get token from tokenBucket, need token count (page.Data) = %d, error : %s", key, len(p.Data), err) |
| objectReqsHistogram *prometheus.HistogramVec | ||
| objectReqErrors prometheus.Counter | ||
| objectDataBytes *prometheus.CounterVec | ||
| getTokenFromBucketErrors *prometheus.CounterVec |
There was a problem hiding this comment.
it's better to have metric for the wait period, other than errors
| @@ -613,7 +634,11 @@ func (store *cachedStore) load(key string, page *Page, cache bool, forceCache bo | |||
| compressed := needed > len(page.Data) | |||
| // we don't know the actual size for compressed block | |||
| if store.downLimit != nil && !compressed { | |||
| store.downLimit.Wait(int64(len(page.Data))) | |||
| err = store.downLimit.WaitN(context.TODO(), len(page.Data)) | |||
There was a problem hiding this comment.
Why we change to use golang.org/x/time/rate? When it will return error?
| logger.Error("get %s: %s", key, err) | ||
| } | ||
| var err1 error | ||
| if store.downLimit != nil && compressed { |
There was a problem hiding this comment.
there is a race between UpdateCachedStoreRateLimit and here.
There was a problem hiding this comment.
- fixbug rate
- golang.org/x/time/rate allows update limit or burst, can ensure concurrency.
- It returns an error if n exceeds the Limiter's burst size, the Context is canceled, or the expected wait time exceeds the Context's Deadline.
cancel and timeout are not set here, so only n(token count) is larger than burst size, can cause error. To ensure the effect of rate.limiter, this case should not be executed normally. - about metrics, make tokens at a constant speed(rate.NewLimiter), we may prefer to know whether there are config exception. if it is normal, block until get the needed tokens.
|
This PR is took over by #3574 , thanks! |
Modify juicefs client upload/download limit parameters dynamically.
Add parameters(flags) in format settings, and modify the value through the 'juicefs config' to dynamically modify the traffiic(object storage) limiting bucket setting, which acts on the volume level.
The priority of new configuration is lower than the mount parameter.
The commit use official rate package(golang) , And allow control of timeout, but it seems that there is no high necessity., in order to make the use easier, all case set to no timeout.
Test Result:
default 0 in format setting
./juicefs bench /tmp/zmc-1112 -p 4./juicefs config tikv://xxx:2379/zmc-1112 --default-upload-limit 480 --default-download-limit 960./juicefs config tikv://1xxx:2379/zmc-1112 --default-upload-limit 480 --default-download-limit 480./juicefs config tikv://xxx:2379/zmc-1112 --default-upload-limit 0 --default-download-limit 0