Skip to content
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

*: auto assign keyspace group #6268

Merged
merged 9 commits into from
Apr 11, 2023
Merged

*: auto assign keyspace group #6268

merged 9 commits into from
Apr 11, 2023

Conversation

rleungx
Copy link
Member

@rleungx rleungx commented Apr 4, 2023

What problem does this PR solve?

Issue Number: Close #6231.

What is changed and how does it work?

Check List

Tests

  • Unit test

Release note

None.

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Apr 4, 2023

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • JmPotato
  • binshi-bing

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot
Copy link
Member

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ti-chi-bot ti-chi-bot added do-not-merge/needs-linked-issue do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. labels Apr 4, 2023
@ti-chi-bot ti-chi-bot requested review from disksing and HunDunDM April 4, 2023 03:29
@rleungx rleungx requested review from binshi-bing and lhy1024 and removed request for disksing and HunDunDM April 4, 2023 04:02
@codecov
Copy link

codecov bot commented Apr 4, 2023

Codecov Report

Patch coverage: 69.94% and project coverage change: -0.12 ⚠️

Comparison is base (f80b215) 75.24% compared to head (c8c0221) 75.13%.

❗ Current head c8c0221 differs from pull request most recent head 58bf43c. Consider uploading reports for the commit 58bf43c to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6268      +/-   ##
==========================================
- Coverage   75.24%   75.13%   -0.12%     
==========================================
  Files         404      404              
  Lines       39417    39574     +157     
==========================================
+ Hits        29661    29734      +73     
- Misses       7202     7253      +51     
- Partials     2554     2587      +33     
Flag Coverage Δ
unittests 75.13% <69.94%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
server/apiv2/handlers/tso_keyspace_group.go 29.50% <0.00%> (ø)
pkg/keyspace/keyspace.go 70.42% <54.16%> (-1.72%) ⬇️
pkg/keyspace/tso_keyspace_group.go 69.76% <68.26%> (-14.02%) ⬇️
pkg/keyspace/util.go 90.12% <84.61%> (-5.12%) ⬇️
pkg/slice/slice.go 100.00% <100.00%> (ø)

... and 25 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rleungx rleungx force-pushed the group-cache branch 2 times, most recently from 4e41cec to e5ac19f Compare April 4, 2023 08:02
@rleungx rleungx removed the request for review from lhy1024 April 4, 2023 08:11
@nolouch
Copy link
Contributor

nolouch commented Apr 4, 2023

some comments from @zhangjinpeng1987 , It's better supports strategy like let the keyspace in a group within continuously keyrange, then the storage service easier to isolate the data.

Copy link
Contributor

@binshi-bing binshi-bing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some comments. Haven't finished my review yet.

pkg/keyspace/tso_keyspace_group.go Show resolved Hide resolved
UserKind string `json:"user-kind"`
ID uint32 `json:"id"`
UserKind string `json:"user-kind"`
Keyspaces []uint32 `json:"keyspaces"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used "Keyspaces []uint32" in #6247 too, but I'm thinking this isn't efficient to store huge number of keyspace ids, considering that keyspace ids belonging to one group has locality, probably we can define as compressed string. I mentioned this in [Keyspace Group Management Interaction and Storage Format (https://pingcap.feishu.cn/wiki/wikcnCvqydi8qgvRyRRT7j8hxL9)

// "keyspaces" contains Keyspace Group Membership Metadata
// TODO: support more compact format, e.g., 0-2, 4, 7-11, ...
"keyspaces": [ 0, 1, 2, 4, ...]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can handle it in a separate pr. It isn't a trivial change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we might do it later.

pkg/keyspace/tso_keyspace_group.go Show resolved Hide resolved
pkg/keyspace/util.go Show resolved Hide resolved
pkg/keyspace/util.go Show resolved Hide resolved
Copy link
Contributor

@binshi-bing binshi-bing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some comments

pkg/keyspace/tso_keyspace_group.go Outdated Show resolved Hide resolved
return nil
}

// CreateKeyspaceGroups creates keyspace groups.
func (m *GroupManager) CreateKeyspaceGroups(keyspaceGroups []*endpoint.KeyspaceGroup) error {
return m.saveKeyspaceGroups(keyspaceGroups)
if err := m.saveKeyspaceGroups(keyspaceGroups, false); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need read lock to protect keyspaceGroups from being concurrently accessed in m.saveKeyspaceGroups?

Copy link
Member Author

@rleungx rleungx Apr 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a lock in the group manager level, which can prevent saveKeyspaceGroups from calling concurrently.

@@ -72,42 +122,137 @@ func (m *GroupManager) GetKeyspaceGroupByID(id uint32) (*endpoint.KeyspaceGroup,
err error
)

m.store.RunInTxn(m.ctx, func(txn kv.Txn) error {
if err := m.store.RunInTxn(m.ctx, func(txn kv.Txn) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is single key-value pair read, isn't it? Do we still transaction for this case? Usually, single key-value get/put is atomic which is all we need. I'm not sure, so ask for confirmation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we use the storage interface, we will use the txn. Otherwise, we need to use etcd client to directly access etcd.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to support non-txn APIs in storage interface which is out of scope of this pr. please ignore.

)

if err := m.store.RunInTxn(m.ctx, func(txn kv.Txn) error {
kg, err = m.store.LoadKeyspaceGroup(txn, id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few questions:

  1. KV deletion usually returns the existing value -- does it apply to the case here? i.e., can we let m.store.DeleteKeyspaceGroup(txn, id) return the existing value while deleting the pair?
  2. If the pair doesn't exist, what does m.store.LoadKeyspaceGroup() return? Can we avoid invoking m.store.DeleteKeyspaceGroup(txn, id) if the pair doesn't exist. Please ignore my comment, if this is trivial case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, now we return the deleted value when it exists.

return nil
}

func (m *GroupManager) saveKeyspaceGroups(keyspaceGroups []*endpoint.KeyspaceGroup) error {
func (m *GroupManager) saveKeyspaceGroups(keyspaceGroups []*endpoint.KeyspaceGroup, overwrite bool) error {
return m.store.RunInTxn(m.ctx, func(txn kv.Txn) error {
for _, keyspaceGroup := range keyspaceGroups {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need read lock for keyspaceGroups here?

Copy link
Member Author

@rleungx rleungx Apr 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use lock in high level now

}

// UpdateKeyspaceForGroup updates the keyspace field for the keyspace group.
func (m *GroupManager) UpdateKeyspaceForGroup(userKind endpoint.UserKind, groupID string, keyspaceID uint32, mutation string) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Bootstrap, is it necessary to batching the operations? Collects all keyspaces to add/remove for every keyspace group from preAlloc, then apply saveKeyspaceGroups for each group just once.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slice.Remove() is an O(n), we might need a more efficient data structure in the future, considering we also need a more compact way to store ids, mentioned in another comment. We can change it in the separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just keep the original logic, which separates the preAlloc from the default keyspace. As for the second comment, I agree with it.

return errors.Errorf("keyspace group %s not found in %s group", newGroupID, newUserKind)
}

if !slice.Contains(newKG.Keyspaces, keyspaceID) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O(n). Another reason we might need a more efficient and compact data structure like tree map

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ignore it for now

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

UserKind: keyspaceGroup.UserKind,
ID: keyspaceGroup.ID,
UserKind: keyspaceGroup.UserKind,
Keyspaces: keyspaceGroup.Keyspaces,
}
// Check if keyspace group has already existed.
oldKG, err := m.store.LoadKeyspaceGroup(txn, keyspaceGroup.ID)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In UpdateKeyspaceGroup, we call m.saveKeyspaceGroups([]*endpoint.KeyspaceGroup{oldKG, newKG}, true). In this case, is it possible to skip m.store.LoadKeyspaceGroup and just call m.store.SaveKeyspaceGroup?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean when overwrite is true, we directly SaveKeyspaceGroup?

@rleungx rleungx marked this pull request as ready for review April 6, 2023 05:49
@ti-chi-bot ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 6, 2023
Copy link
Contributor

@binshi-bing binshi-bing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have time, could you please reply my left comments so that I can deepen my understanding for the code and etcd/endpoint utilities that we're using?

Sign off for now.

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 6, 2023
@@ -149,3 +150,98 @@ func makeLabelRule(id uint32) *labeler.LabelRule {
Data: makeKeyRanges(id),
}
}

// indexedHeap is a heap with index.
type indexedHeap struct {
Copy link
Contributor

@binshi-bing binshi-bing Apr 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rleungx it seems that we haven't accessed the data as a heap though it's stored in a heap. I expect to see somewhere, in this change, you call heap.Pop(&indexedHeap) to get keyspace group which has the least number of keyspaces of the kind then access this group to the given keyspace, but I didn't see it. In short word, where do we 'auto' assign keyspace group, as the PR title mentioned, for a given keyspace?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can check GetAvailableKeyspaceGroupIDByKind. Some functions of the indexed heap are indeed not used for now, if you think it's unnecessary, I can remove them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw you call indexedHeap.Top() instead of calling heap.Pop(&indexedHeap). That works too. Thanks for confirmation. Keep the unused APIs. It doesn't bother me.

@@ -149,3 +150,98 @@ func makeLabelRule(id uint32) *labeler.LabelRule {
Data: makeKeyRanges(id),
}
}

// indexedHeap is a heap with index.
type indexedHeap struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw you call indexedHeap.Top() instead of calling heap.Pop(&indexedHeap). That works too. Thanks for confirmation. Keep the unused APIs. It doesn't bother me.

@rleungx rleungx requested a review from JmPotato April 10, 2023 03:16
newUserKind := endpoint.StringUserKind(newConfig[UserKindKey])
oldID := oldConfig[TSOKeyspaceGroupIDKey]
newID := newConfig[TSOKeyspaceGroupIDKey]
if oldUserKind != newUserKind || oldID != newID {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if oldUserKind != newUserKind || oldID != newID {
needUpdate := oldUserKind != newUserKind || oldID != newID
if needUpdate {

// Save the updated keyspace meta.
return manager.store.SaveKeyspaceMeta(txn, meta)
if err := manager.store.SaveKeyspaceMeta(txn, meta); err != nil {
if oldUserKind != newUserKind || oldID != newID {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if oldUserKind != newUserKind || oldID != newID {
if needUpdate {


m.Lock()
defer m.Unlock()
kg := m.groups[userKind].Get(uint32(id))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that userKind does not exist in m.groups here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think no because we always update the group after saving the keyspace group. But I add a check here.

rleungx added 8 commits April 11, 2023 16:13
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 11, 2023
@JmPotato
Copy link
Member

/merge

@ti-chi-bot
Copy link
Member

@JmPotato: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: c8c0221

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 11, 2023
@ti-chi-bot ti-chi-bot merged commit 4493c08 into tikv:master Apr 11, 2023
@rleungx rleungx deleted the group-cache branch April 11, 2023 09:57
rleungx added a commit to rleungx/pd that referenced this pull request Jun 5, 2023
* client: refine serviceModeKeeper code (tikv#6201)

ref tikv#5895

Some code refinements for `serviceModeKeeper`.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>

* *: use revision for watch test (tikv#6205)

ref tikv#6071

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>

* *: remove unnecessary rand init (tikv#6207)

close tikv#6134

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>

* Refactor TSO forward/dispatcher to be shared by both PD and TSO (tikv#6175)

ref tikv#5895

Add general tso forward/dispatcher for independent pd(tso)/tso services and cross cluster forwarding.

Signed-off-by: Bin Shi <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* Add basic multi-keyspace-group management (tikv#6214)

ref tikv#5895

Support basic functions of multi-keyspace-group management

Signed-off-by: Bin Shi <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* *: support keyspace group RESTful API (tikv#6229)

ref tikv#6231

Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* mcs: add more tso tests (tikv#6184)

ref tikv#5836

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>

* client: fix compatibility problem of pd client (tikv#6244)

close tikv#6243

Signed-off-by: Ryan Leung <[email protected]>

* *: unify the key prefix (tikv#6248)

ref tikv#5836

Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* *: remove cluster dependency from keyspace (tikv#6249)

ref tikv#6231

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* *: make code clear by rename `isServing` to `isRunning` (tikv#6258)

ref tikv#4399

Signed-off-by: lhy1024 <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* cgroup: fix the path problem due to special container name (tikv#6267)

close tikv#6266

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>

* server: fix watch keyspace revision (tikv#6251)

ref tikv#5895

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>

* tso, server: refine the TSO allocator manager parameters (tikv#6269)

ref tikv#5895

- Refine the TSO allocator manager parameters.
- Always run `tsoAllocatorLoop` to advance the Global TSO.

Signed-off-by: JmPotato <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* tso: unify the TSO ServiceConfig and ConfigProvider interfaces (tikv#6272)

ref tikv#5895

Unify the TSO `ServiceConfig` and `ConfigProvider` interfaces.

Signed-off-by: JmPotato <[email protected]>

* Load initial assignment and dynamically watch/apply keyspace groups' membership/distribution change (tikv#6247)

ref tikv#6232

Load initial keyspace group assignment.
Dynamically watch/apply keyspace groups' membership/distribution change.

Signed-off-by: Bin Shi <[email protected]>

* *: define user kind for keyspace group (tikv#6241)

ref tikv#6231

Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* Add more failure tests when tso service loading initial keyspace groups assignment (tikv#6280)

ref tikv#6232

Add more failure tests when tso service loading initial keyspace groups assignment

Signed-off-by: Bin Shi <[email protected]>

* Apply multi-keyspace-group membership to tso service and handle inconsistency issue (tikv#6282)

ref tikv#6232

Apply multi-keyspace-group membership to tso service and handle inconsistency issue.

1. Add KeyspaceLookupTable to endpoint.KeyspaceGroup
type KeyspaceGroup struct {
        ...
        // KeyspaceLookupTable is for fast lookup if a given keyspace belongs to this keyspace group.
        // It's not persisted and will be built when loading from storage.
        KeyspaceLookupTable map[uint32]struct{} `json:"-"`
}

2. After loading keyspace groups, the Keyspace Group Manager builds KeyspaceLookupTable for every keyspace groups.

3. When Keyspace Group Manager handles tso requests, it uses the keyspaceLookupTable to check if the required keypsace still belongs to the required keyspace group. If not, returns the current keyspace group id in the tso response header.

Signed-off-by: Bin Shi <[email protected]>

* *: auto assign keyspace group (tikv#6268)

close tikv#6231

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* keyspace, api: support the keyspace group split (tikv#6293)

ref tikv#6232

Support the keyspace group split and add related tests.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* Improve lock mechanism in tso.KeyspaceGroupManager (tikv#6305)

ref tikv#6232

Use the RWMutex instead of individual atomic types to better protect the state of the keyspace group manager

Signed-off-by: Bin Shi <[email protected]>

* keyspace: add split-from field for endpoint.KeyspaceGroup (tikv#6309)

ref tikv#6232

Add `split-from` field for `endpoint.KeyspaceGroup`.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>

* Add read lock at one place for protection and better structure (tikv#6310)

ref tikv#6232, ref tikv#6305

follow-up tikv#6305 
Add read lock at one place for protection and better structure

Signed-off-by: Bin Shi <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>

* tso: optimize function signatures to reduce parameter passing (tikv#6315)

ref tikv#6232

Optimize function signatures to reduce parameter passing.

Signed-off-by: JmPotato <[email protected]>

* *: bootstrap keyspace group when server is in API mode (tikv#6308)

ref tikv#6231

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* keyspace: avoid keyspace being updated during the split (tikv#6316)

ref tikv#6232

Prevent keyspace from being updated during the split.

Signed-off-by: JmPotato <[email protected]>

* *: fix `TestConcurrentlyReset` (tikv#6318)

close tikv#6275

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>

* bootstrap default keyspace group in the tso service (tikv#6306)

ref tikv#6232

Changes:

1. Introduce the initialization logic of the default keyspace group.
- If the default keyspace group isn't configured in the etcd, every tso node/pod should initialize it and join the election for the primary of this group.
- If the default keyspace group is configured in the etcd, the tso nodes/pods which are assigned with this group will initialize it and join the election for the primary of this group.

2. Introduce the keyspace group membership restriction -- default keyspace always belongs to default keyspace group.

Signed-off-by: Bin Shi <[email protected]>

* *: change the log level (tikv#6324)

ref tikv#6232

Signed-off-by: Ryan Leung <[email protected]>

* *: fix the missing log panic (tikv#6325)

close tikv#6257

Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* mcs: fix watch primary address revision and update cache when meets not leader  (tikv#6279)

ref tikv#5895

Signed-off-by: lhy1024 <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* tso, member: support TSO split based on keyspace group split (tikv#6313)

ref tikv#6232

Support TSO split based on keyspace group split.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* mcs: support metrics HTTP interface for tso/resource manager server (tikv#6329)

ref tikv#5895

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* tso: put finishSplitKeyspaceGroup into the critical section (tikv#6331)

ref tikv#6232

Put `finishSplitKeyspaceGroup` into the critical section.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>

* *: make `TestServerRegister` stable (tikv#6337)

close tikv#6334

Signed-off-by: Ryan Leung <[email protected]>

* tests: divide all tests into the CI chunks including submodule tests (tikv#6198)

ref tikv#6181, ref tikv#6183

Divide all tests into the CI chunks including submodule tests.

Signed-off-by: JmPotato <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* tests: introduce TSO TestCluster in the test (tikv#6333)

ref tikv#6232

Introduce TSO `TestCluster` in the test.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* mcs: add balancer for keyspace group (tikv#6274)

close tikv#6233

Signed-off-by: lhy1024 <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* Fixed bugs in tso service registry watching loop. (tikv#6346)

ref tikv#6343

Fixed the following two bugs:
1. When re-watch a range, to continue from what left by the last watch, the revision is wresp.Header.Revision + 1 instead of wresp.Header.Revision, where wresp.Header.Revision is the revision indicated in the response of the last watch. Because of this bug, it was processing the same event endless as you can see from the log below.
2. In tso service watch loop in /Users/binshi/code/pingcap/my-pd/pkg/keyspace/tso_keyspace_group.go, If this is delete event, the json.Unmarshal(event.Kv.Value, s) will fail with the error "unexpected end of JSON input", so there is no way to get s.serviceAddr from the result of json.Unmarshal.

Signed-off-by: Bin Shi <[email protected]>

* mcs: fix double compression of prom handler (tikv#6339)

ref prometheus/client_golang#622, ref tikv#5895

Signed-off-by: Ryan Leung <[email protected]>

* tests, tso: add more TSO split tests (tikv#6338)

ref tikv#6232

Add more TSO split tests.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>

* keyspace, tso: fix next revision to watch after watch/Get/RangeScan (tikv#6353)

ref tikv#6232

The next revision to watch should always be Header.Revision + 1 where header is response header of watch/Get/RangeScan

Signed-off-by: Bin Shi <[email protected]>

* mcs, tests: use TSO cluster to do the failover test (tikv#6356)

ref tikv#5895

Use TSO cluster to do the failover test.

Signed-off-by: JmPotato <[email protected]>

* fix startWatchLoop leak (tikv#6352)

Signed-off-by: Ryan Leung <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* mcs: update client when meet transport is closing (tikv#6341)

* mcs: update client when meet transport is closing

Signed-off-by: lhy1024 <[email protected]>

* address comments

Signed-off-by: lhy1024 <[email protected]>

* add retry

Signed-off-by: lhy1024 <[email protected]>

---------

Signed-off-by: lhy1024 <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Signed-off-by: lhy1024 <[email protected]>

* add bootstrap test (tikv#6347)

Signed-off-by: Ryan Leung <[email protected]>
Co-authored-by: Ti Chi Robot <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Signed-off-by: lhy1024 <[email protected]>

* Fix flaky TestLoadKeyspaceGroupsAssignment test (tikv#6365)

Reduce the count of keyspace groups from 4096 to 512 in TestKeyspaceGroupManagerTestSuite/TestLoadKeyspaceGroupsAssignment to avoid timeout when test running slow.

Signed-off-by: Bin Shi <[email protected]>

* mcs, tso: fix ts fallback caused by multi-primary of the same keyspace group  (tikv#6362)

* Change participant election-prifix from listen-addr to advertise-listen-addr to gurantee uniqueness.

Signed-off-by: Bin Shi <[email protected]>

* Add TestPariticipantStartWithAdvertiseListenAddr

Signed-off-by: Bin Shi <[email protected]>

* Add comments to fix go fmt errors

Signed-off-by: Bin Shi <[email protected]>

---------

Signed-off-by: Bin Shi <[email protected]>
Co-authored-by: Ryan Leung <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* fix log output (tikv#6364)

Signed-off-by: Ryan Leung <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* mcs: fix duplicate start of RaftCluster. (tikv#6358)

* Using double-checked locking to avoid duplicate start of RaftCluster.

Signed-off-by: Bin Shi <[email protected]>

* Handle feedback

Signed-off-by: Bin Shi <[email protected]>

* improve locking

Signed-off-by: Bin Shi <[email protected]>

* handle feedback

Signed-off-by: Bin Shi <[email protected]>

---------

Signed-off-by: Bin Shi <[email protected]>
Co-authored-by: Ryan Leung <[email protected]>

* Add retry mechanism for updating keyspace group (tikv#6372)

Signed-off-by: JmPotato <[email protected]>

* mcs: add set handler for balancer and alloc node for default keyspace group (tikv#6342)

ref tikv#6233

Signed-off-by: lhy1024 <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* mcs, tso: fix Nil pointer deference when (*AllocatorManager).GetMember (tikv#6383)

close tikv#6381

If the desired keyspace group fall back to the default keyspace group and the AM isn't initialized, return not served error.

Signed-off-by: Bin Shi <[email protected]>

* mcs, tso: support multi-keyspace-group and its service discovery in E2E path (tikv#6321)

ref tikv#6232

Support multi-keyspace-group in PD(TSO) client

Signed-off-by: Bin Shi <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Signed-off-by: lhy1024 <[email protected]>

* client: add `NewClientWithKeyspaceName` for client (tikv#6380)

ref tikv#5895

Signed-off-by: Ryan Leung <[email protected]>

* keyspace, tso: check the replica count before the split (tikv#6382)

ref tikv#6233

Check the replica count before the split.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: lhy1024 <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* tso: fix bugs to make split test case to pass (tikv#6389)

ref tikv#6232

fix bugs to make split test case to pass

Signed-off-by: Bin Shi <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* keyspace: patrol keyspace assignment before the first split (tikv#6388)

ref tikv#6232

Patrol the keyspace assignment before the first split to make sure every keyspace has its group assignment.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* tests: fix Flaky TestMicroserviceTSOServer/TestConcurrentlyReset (tikv#6396)

close tikv#6385

Get a copy of now then call base.add, because now is shared by all goroutines and now.add() will add to itself which isn't atomic and multi-goroutine safe.

Signed-off-by: Bin Shi <[email protected]>

* keyspace, slice: improve code efficiency in membership ops (tikv#6392)

ref tikv#6231

Improve code efficiency in membership ops

Signed-off-by: Bin Shi <[email protected]>

* tests: enable TestTSOKeyspaceGroupSplitClient (tikv#6398)

ref tikv#6232

Enable `TestTSOKeyspaceGroupSplitClient`.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* tests: add more tests for multiple keyspace groups (tikv#6395)

ref tikv#5895

Add CheckMultiKeyspacesTSO() and WaitForMultiKeyspacesTSOAvailable in test utility. Add TestTSOKeyspaceGroupManager/TestKeyspacesServedByNonDefaultKeyspaceGroup. Cover TestGetTS, TestGetTSAsync, TestUpdateAfterResetTSO in TestMicroserviceTSOClient for multiple keyspace groups.

Signed-off-by: Bin Shi <[email protected]>

* tests: fix failpoint disable (tikv#6401)

ref tikv#4399

Signed-off-by: lhy1024 <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Signed-off-by: lhy1024 <[email protected]>

* client: retry load keyspace meta when creating a new client (tikv#6402)

ref tikv#5895

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* Fix test issue in TestRandomResignLeader. (tikv#6410)

close tikv#6404

We need to make sure the selected keyspaces are from different keyspace groups, otherwise multiple goroutines below could try to resign the primary of the same keyspace group and cause race condition.

Signed-off-by: Bin Shi <[email protected]>

* keyspace, api2: fix the keyspace assignment patrol consistency (tikv#6397)

ref tikv#6232

Fix the keyspace assignment patrol consistency.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: Ryan Leung <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* election, tso: fix data race in lease.go (tikv#6379)

close tikv#6378

fix data race in lease.go

Signed-off-by: Bin Shi <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* mcs: fix forward test with pd mode client (tikv#6290)

ref tikv#5895, ref tikv#6279, close tikv#6289

Signed-off-by: lhy1024 <[email protected]>

* keyspace: patrol the keyspace assignment in batch (tikv#6411)

ref tikv#6232

Patrol the keyspace assignment in batch.

Signed-off-by: JmPotato <[email protected]>

* etcdutil: add watch loop (tikv#6390)

close tikv#6391

Signed-off-by: lhy1024 <[email protected]>

* mcs, tso: add API interface to obtain the TSO keyspace group member info (tikv#6373)

ref tikv#6232

Add API interface to obtain the TSO keyspace group member info.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* pkg: move operator_check out from test_util tikv#6162

Signed-off-by: lhy1024 <[email protected]>

* keysapce: wait region split when creating keyspace (tikv#6414)

ref tikv#6231

Signed-off-by: zeminzhou <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

Co-authored-by: zzm <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Signed-off-by: lhy1024 <[email protected]>

* mcs: use getClusterInfo to check whether api service is ready (tikv#6422)

ref tikv#5836

Signed-off-by: lhy1024 <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Signed-off-by: lhy1024 <[email protected]>

* fix data race by replace clone (tikv#6242)

close tikv#6230

Signed-off-by: bufferflies <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* fix test and git mod tidy

Signed-off-by: lhy1024 <[email protected]>

* revert makefile

Signed-off-by: lhy1024 <[email protected]>

* fix ctx in watch loop

Signed-off-by: lhy1024 <[email protected]>

* delete pd-tests.yaml

Signed-off-by: lhy1024 <[email protected]>

* pd-ctl, tests: add the keyspace group commands (tikv#6423)

ref tikv#6232

Add the keyspace group commands to show and split keyspace groups.

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* Handle compatibility issue in GetClusterInfo RPC (tikv#6434)

ref tikv#5895, close tikv#6448

Handle the compatibility issue in the GetClusterInfo RPC

Signed-off-by: Bin Shi <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* Provide GetMinTS API to solve the compatibility issue brought by multi-timeline tso (tikv#6421)

ref tikv#6142

1. Import kvproto change to introduce GetMinTS rpc in the TSO service.
6. Add server side implementation for GetMinTS rpc.
7. Add client side implementation for GetMinTS rpc.
8. Add unit test

Signed-off-by: Bin Shi <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* tso: use less interval when waiting api service (tikv#6451)

close tikv#6449

Signed-off-by: lhy1024 <[email protected]>

* etcdutil: fix ctx in watch loop (tikv#6445)

close tikv#6439

Signed-off-by: lhy1024 <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Signed-off-by: lhy1024 <[email protected]>

* Fix "non-default keyspace groups use the same timestamp path by mistake" (tikv#6457)

close tikv#6453, close tikv#6465

The tso servers are loading keyspace groups asynchronously. Make sure all keyspace groups
are available for serving tso requests from corresponding keyspaces by querying
IsKeyspaceServing(keyspaceID, the Desired KeyspaceGroupID). if use default keyspace group id
in the query, it will always return true as the keyspace will be served by default keyspace group
before the keyspace groups are loaded.

Signed-off-by: Bin Shi <[email protected]>
Signed-off-by: lhy1024 <[email protected]>

* TSO microservice discovery fallback path shouldn't call FindGroupByKeyspaceID (tikv#6473)

close tikv#6472

TSO microservice discovery fallback path shouldn't call FindGroupByKeyspaceID

Signed-off-by: Bin Shi <[email protected]>

* Revert "cgroup: fix the path problem due to special container name (tikv#6267)"

This reverts commit 0c4cf7f947799e5c45d6e37448475b921044bdde.

* *: rm debug file (tikv#6458)

ref tikv#4399

Signed-off-by: lhy1024 <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* Revert "*: remove unnecessary rand init (tikv#6207)"

This reverts commit 7383ded7581c417a3866da271eb2ec0a27b5a6c8.

* mcs, tso: handle null keyspace (tikv#6476)

ref tikv#5895

For API V1 and legacy path (NewClientWithContext w/o keyspace id/name),
using Null Keypsace ID (uint32max) instead of default keyspace id and
make sure it can be served by the default keyspace group's timeline. Modifying test accordingly.

Signed-off-by: Bin Shi <[email protected]>

* mcs, tso: print TSO service discovery fallback log just once (tikv#6478)

ref tikv#5895

Print TSO service discovery fallback log just once

Signed-off-by: Bin Shi <[email protected]>

* client: return error if the keyspace meta cannot be found (tikv#6479)

ref tikv#6142

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

* client: support use API context to create client (tikv#6482)

ref tikv#6142

Signed-off-by: Ryan Leung <[email protected]>

---------

Signed-off-by: Bin Shi <[email protected]>
Signed-off-by: lhy1024 <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: JmPotato <[email protected]>
Co-authored-by: JmPotato <[email protected]>
Co-authored-by: Ti Chi Robot <[email protected]>
Co-authored-by: Ryan Leung <[email protected]>
Co-authored-by: Bin Shi <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Co-authored-by: zzm <[email protected]>
Co-authored-by: buffer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note-none Denotes a PR that doesn't merit a release note. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keyspace group membership management
6 participants