keyspace: set GCManagementType default value to KeyspaceLevelGC for NextGen #9399
Conversation
…extGen Signed-off-by: tiancaiamao <[email protected]>
ti-chi-bot
bot
added
release-note-none
|
Hi @tiancaiamao. Thanks for your PR. I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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 kubernetes-sigs/prow repository. |
ti-chi-bot
bot
added
the
size/M
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #9399 +/- ##
==========================================
+ Coverage 76.19% 76.23% +0.03%
==========================================
Files 478 478
Lines 74684 74692 +8
==========================================
+ Hits 56908 56941 +33
+ Misses 14245 14231 -14
+ Partials 3531 3520 -11
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Signed-off-by: tiancaiamao <[email protected]>
|
/retest |
|
@tiancaiamao: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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 kubernetes-sigs/prow repository. |
pkg/keyspace/keyspace_test.go
Outdated
| re.NoError(err) | ||
| loaded, err := manager.LoadKeyspaceByID(created.Id) | ||
| re.NoError(err) | ||
| re.Equal(loaded.Config[GCManagementType], tc.expect) |
There was a problem hiding this comment.
| re.Equal(loaded.Config[GCManagementType], tc.expect) | |
| re.Equal(tc.expect, loaded.Config[GCManagementType]) |
pkg/keyspace/keyspace.go
Outdated
| @@ -233,6 +234,12 @@ func (manager *Manager) CreateKeyspace(request *CreateKeyspaceRequest) (*keyspac | |||
| request.Config[UserKindKey] = config[UserKindKey] | |||
| } | |||
| } | |||
| // Set default value of GCManagementType to KeyspaceLevelGC for NextGen | |||
| if kerneltype.IsNextGen() { | |||
| if _, ok := request.Config[GCManagementType]; !ok { | |||
There was a problem hiding this comment.
Maybe it's better to do this:
| if _, ok := request.Config[GCManagementType]; !ok { | |
| if v, ok := request.Config[GCManagementType]; !ok || len(v) == 0 { |
There was a problem hiding this comment.
BTW maybe it's better to comment that default keyspace is not affected as it's created during bootstrap
Signed-off-by: tiancaiamao <[email protected]>
ti-chi-bot
bot
added
needs-1-more-lgtm
|
/ok-to-test |
ti-chi-bot
bot
added
ok-to-test
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JmPotato, rleungx The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
|
/hold |
ti-chi-bot
bot
added
the
do-not-merge/hold
|
/unhold |
ti-chi-bot
bot
removed
the
do-not-merge/hold
|
/retest |
What problem does this PR solve?
Issue Number: Close #9398
What is changed and how does it work?
Check List
Tests
Code changes
Side effects
Related changes
pingcap/docs/pingcap/docs-cn:pingcap/tiup:Release note