introduce a random jitter to region cache ttl #1148
Conversation
Signed-off-by: zyguan <zhongyangguan@gmail.com>
Signed-off-by: zyguan <zhongyangguan@gmail.com>
Signed-off-by: zyguan <zhongyangguan@gmail.com>
|
@crazycs520 PTAL |
| if regionCacheTTLJitterSec > 0 { | ||
| jitter = rand.Int63n(regionCacheTTLJitterSec) | ||
| } | ||
| return ts + regionCacheTTLSec + jitter |
There was a problem hiding this comment.
Is there a way to add some constraint checks to make sure the unit of the input ts is second, to avoid being used mistakenly?
There was a problem hiding this comment.
We pass ts here to avoid overhead of time.Now(). To validate it, maybe we can:
- record process start ts by a global var (eg. processStartTs)
- the ts might be invalid if ts - processStartTs > 100 years or ts < processStartTs
- if the ts is invalid, we can fallback to call
time.Now()
But do we really need to do that? There are many mechanisms to invalidate a stale region, it might be not a big problem even when we call nextTTL with epoch millis, I guess.
Or, we can just change the function signature (also many releated functions like checkRegionCacheTTL), let it (them) accept an arg of type time.Time. (I've tried this, but gave up, since it will introduce a lot of changes)
There was a problem hiding this comment.
Maybe we could just add some comments to explain the constraints of the input ts in the comment if it's not a big problem when used mistakenly.
Signed-off-by: zyguan <zhongyangguan@gmail.com>
Ref #1122 (comment) and #1104 .
This PR tries to address #1122 (comment) by introducing a random jitter to region cache TTL.
To observe the reason of region reloading, the methods for searching cached regions are refactored.