-
Notifications
You must be signed in to change notification settings - Fork 728
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
pdctl: support show keyspace meta with refresh group id #6751
Changes from 1 commit
c331765
ddd43a1
129adc2
0f92101
752067f
bf9852a
3d5ad44
85c7da6
0e4d25d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
// Copyright 2023 TiKV Project Authors. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package keyspace_test | ||
|
||
import ( | ||
"context" | ||
"encoding/json" | ||
"fmt" | ||
"strings" | ||
"testing" | ||
|
||
"github.com/pingcap/failpoint" | ||
"github.com/stretchr/testify/require" | ||
"github.com/tikv/pd/pkg/keyspace" | ||
"github.com/tikv/pd/pkg/mcs/utils" | ||
"github.com/tikv/pd/pkg/utils/tempurl" | ||
"github.com/tikv/pd/pkg/utils/testutil" | ||
api "github.com/tikv/pd/server/apiv2/handlers" | ||
"github.com/tikv/pd/server/config" | ||
"github.com/tikv/pd/tests" | ||
"github.com/tikv/pd/tests/pdctl" | ||
pdctlCmd "github.com/tikv/pd/tools/pd-ctl/pdctl" | ||
) | ||
|
||
func TestKeyspace(t *testing.T) { | ||
re := require.New(t) | ||
ctx, cancel := context.WithCancel(context.Background()) | ||
defer cancel() | ||
re.NoError(failpoint.Enable("github.com/tikv/pd/pkg/keyspace/acceleratedAllocNodes", `return(true)`)) | ||
re.NoError(failpoint.Enable("github.com/tikv/pd/pkg/tso/fastGroupSplitPatroller", `return(true)`)) | ||
re.NoError(failpoint.Enable("github.com/tikv/pd/server/delayStartServerLoop", `return(true)`)) | ||
keyspaces := make([]string, 0) | ||
for i := 1; i < 10; i++ { | ||
keyspaces = append(keyspaces, fmt.Sprintf("keyspace_%d", i)) | ||
} | ||
tc, err := tests.NewTestAPICluster(ctx, 3, func(conf *config.Config, serverName string) { | ||
conf.Keyspace.PreAlloc = keyspaces | ||
}) | ||
re.NoError(err) | ||
err = tc.RunInitialServers() | ||
re.NoError(err) | ||
pdAddr := tc.GetConfig().GetClientURL() | ||
|
||
_, tsoServerCleanup1, err := tests.StartSingleTSOTestServer(ctx, re, pdAddr, tempurl.Alloc()) | ||
defer tsoServerCleanup1() | ||
re.NoError(err) | ||
_, tsoServerCleanup2, err := tests.StartSingleTSOTestServer(ctx, re, pdAddr, tempurl.Alloc()) | ||
defer tsoServerCleanup2() | ||
re.NoError(err) | ||
cmd := pdctlCmd.GetRootCmd() | ||
|
||
tc.WaitLeader() | ||
leaderServer := tc.GetServer(tc.GetLeader()) | ||
re.NoError(leaderServer.BootstrapCluster()) | ||
defaultKeyspaceGroupID := fmt.Sprintf("%d", utils.DefaultKeyspaceGroupID) | ||
|
||
var k api.KeyspaceMeta | ||
keyspaceName := "keyspace_1" | ||
testutil.Eventually(re, func() bool { | ||
args := []string{"-u", pdAddr, "keyspace", keyspaceName} | ||
output, err := pdctl.ExecuteCommand(cmd, args...) | ||
re.NoError(err) | ||
re.NoError(json.Unmarshal(output, &k)) | ||
return k.GetName() == keyspaceName | ||
}) | ||
re.Equal(uint32(1), k.GetId()) | ||
re.Equal(defaultKeyspaceGroupID, k.Config[keyspace.TSOKeyspaceGroupIDKey]) | ||
|
||
// split keyspace group. | ||
newGroupID := "2" | ||
testutil.Eventually(re, func() bool { | ||
args := []string{"-u", pdAddr, "keyspace-group", "split", "0", newGroupID, "1"} | ||
output, err := pdctl.ExecuteCommand(cmd, args...) | ||
re.NoError(err) | ||
return strings.Contains(string(output), "Success") | ||
}) | ||
|
||
// check keyspace group in keyspace whether changed. | ||
testutil.Eventually(re, func() bool { | ||
args := []string{"-u", pdAddr, "keyspace", keyspaceName} | ||
output, err := pdctl.ExecuteCommand(cmd, args...) | ||
re.NoError(err) | ||
re.NoError(json.Unmarshal(output, &k)) | ||
return newGroupID == k.Config[keyspace.TSOKeyspaceGroupIDKey] | ||
}) | ||
|
||
// test error name | ||
args := []string{"-u", pdAddr, "keyspace", "error_name"} | ||
output, err := pdctl.ExecuteCommand(cmd, args...) | ||
re.NoError(err) | ||
re.Contains(string(output), "Fail") | ||
re.NoError(failpoint.Disable("github.com/tikv/pd/pkg/keyspace/acceleratedAllocNodes")) | ||
re.NoError(failpoint.Disable("github.com/tikv/pd/pkg/tso/fastGroupSplitPatroller")) | ||
re.NoError(failpoint.Disable("github.com/tikv/pd/server/delayStartServerLoop")) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
// Copyright 2023 TiKV Project Authors. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package command | ||
|
||
import ( | ||
"fmt" | ||
"net/http" | ||
|
||
"github.com/spf13/cobra" | ||
) | ||
|
||
const keyspacePrefix = "pd/api/v2/keyspaces" | ||
|
||
// NewKeyspaceCommand return a keyspace subcommand of rootCmd | ||
lhy1024 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func NewKeyspaceCommand() *cobra.Command { | ||
cmd := &cobra.Command{ | ||
Use: "keyspace [command] [flags]", | ||
Short: "show keyspace information", | ||
Run: showKeyspaceCommandFunc, | ||
} | ||
return cmd | ||
} | ||
|
||
func showKeyspaceCommandFunc(cmd *cobra.Command, args []string) { | ||
if len(args) != 1 { | ||
cmd.Usage() | ||
return | ||
} | ||
|
||
resp, err := doRequest(cmd, fmt.Sprintf("%s/%s?refresh_group_id=", keyspacePrefix, args[0]), http.MethodGet, http.Header{}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. /pd/api/v2/keyspaces/name?refresh_group_id= There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it the best practice? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it is not a best practice, the best practice should be to return the group id of the keyspace meta as the latest, not to add other fields to force it to be refreshed. But it is not suitable for our case. And renaming |
||
if err != nil { | ||
cmd.Printf("Failed to get the keyspace information: %s\n", err) | ||
return | ||
} | ||
cmd.Println(resp) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we maintain a
keyspaceLookupTable
as we did inpkg/tso/keyspace_group_manager.go
? It would be more efficient than the pure iteration to check which a keyspace ID belongs to.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, but I don't think it's necessary at the moment. We're only accessing this via pd-ctl right now, and it's a low-frequency operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @JmPotato @rleungx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some discussion, we decided to not change it until we encounter performance issues.