feat: Broadcast min_commit_ts for pipelined transactions#1458
feat: Broadcast min_commit_ts for pipelined transactions#1458ti-chi-bot[bot] merged 24 commits intotikv:masterfrom
Conversation
Signed-off-by: ekexium <eke@fastmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
1789ae4 to
ad0302e
Compare
…-ts-for-large-txn
Co-authored-by: you06 <you1474600@gmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
Done.
It's possible. But I think we need more empirical data before implementing that. It's unclear now how this optimization would benefit normal large transactions. |
Co-authored-by: crazycs <crazycs520@gmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
Co-authored-by: crazycs <crazycs520@gmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
…lient-go into feat-resolved-ts-for-large-txn
| if store == nil || store.GetState() == metapb.StoreState_Tombstone { | ||
| s.setResolveState(tombstone) | ||
| continue | ||
| } | ||
| addr := store.GetAddress() | ||
| if addr == "" { | ||
| continue |
There was a problem hiding this comment.
Better to explain in the comments when the pdClient.GetAllStores(ctx) may return a store list containg nil store and emptry address and how should they be treated. For example, dose it mean the returned store list is invalid and pdClient.GetAllStores(ctx) should be retried when there invalid element in the list?
There was a problem hiding this comment.
I believe that GetAllStores is supposed to return only stores in Up and Offline states. This additional check serves as a defensive measure and ensures consistency with the store resolve procedure
Signed-off-by: ekexium <eke@fastmail.com>
| wg.Wait() | ||
| } | ||
|
|
||
| if err := txn.spawnWithStorePool(broadcastFunc); err != nil { |
There was a problem hiding this comment.
Since we've already used a goroutine pool, how about spawning broadcast tasks like the following:
stores := store.GetRegionCache().GetStoresByType(tikvrpc.TiKV)
rateLimit := make(chan struct{}, min(broadcastMaxConcurrency, len(stores)))
for _, s := range stores {
rateLimit <- struct{}{}
target := s
err := txn.spawnWithStorePool(func() {
defer func() {
<-rateLimit
}()
// send request to target
})
if err != nil {
<-rateLimit
}
}Signed-off-by: ekexium <eke@fastmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
afbcb64 to
4aada7a
Compare
…-ts-for-large-txn
3e9a4ec to
012425a
Compare
Signed-off-by: ekexium <eke@fastmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfzjywxk, zyguan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Ref: tikv/tikv#17459
Prerequires pingcap/kvproto#1265
Changes