precompute min safe ts #622
Conversation
|
PTAL @Yisaer |
| @@ -571,6 +556,18 @@ func (s *KVStore) updateSafeTS(ctx context.Context) { | |||
| metrics.TiKVMinSafeTSGapSeconds.WithLabelValues(storeIDStr).Set(time.Since(safeTSTime).Seconds()) | |||
| }(ctx, wg, storeID, storeAddr) | |||
| } | |||
|
|
|||
| txnScopeMap := make(map[string][]uint64) | |||
| for _, store := range stores { | |||
There was a problem hiding this comment.
why not keep the original way
There was a problem hiding this comment.
sorry, what do you mean by original here?
There was a problem hiding this comment.
if store.IsLabelsMatch([]*metapb.StoreLabel{
There was a problem hiding this comment.
IsLabelsMatch requires a specific value for the label, where here tries to precompute the min safe ts for all the labelled value
Signed-off-by: hihihuhu <[email protected]>
Signed-off-by: hihihuhu <[email protected]>
|
the failed test seems related to tikv/tikv#13866, @Connor1996 do you know how often the tikv nightly image is updated? |
|
It should be updated every day, but not a promise |
|
Thanks for your contribution! |
|
hi @sticnarf, qq: is there a chance that we could port this to upcoming v6.5? if possible, do you know how is the process? thanks! |
|
Update tikv-client in TiDB. And v6.5 branch has already been check-out, you need to cherry-pick it to v6.5 branch. But not sure if the cherry-pick would be approved as usually only bugfix is allowed to be merged after code freeze。 |
|
i c, thanks for the info |
compute the min safe ts on the query path is expensive because it has to allocate thousands of small objects with lock, when there are hundreds of tikv node. this makes stale read using session variable
tidb_read_stalenessnoticeable slower than normal read.the safe ts from tikv are only updated every 2 seconds, thus the min safe ts could be precomputed on the update path to avoid the expensive computation on query path.
result on our benchmark
normal read:
stale read before the change:
zoom in:
stale read after the change:
zoom in:
to further reduce the cpu usage, could remove the warning logging from https://github.com/pingcap/tidb/blob/v6.5.0-alpha/expression/builtin_time.go#L6576