pd-ctl: keyspace-group support set show keyspaces by ystaticy · Pull Request #11208 · tikv/pd

ystaticy · 2026-09-09T03:08:46Z

What problem does this PR solve?

pd-ctl keyspace-group includes the keyspace ID list in its output by default. For keyspace groups with many keyspaces, this can make the output unnecessarily long. This PR adds an option to hide the keyspace ID list when a more concise output is preferred, while preserving the existing default behavior.

Issue Number: Close #11207

What is changed and how does it work?

Add the --hide-keyspaces flag to pd-ctl keyspace-group.
Show the keyspaces field by default for both single-keyspace-group and list output.
Support --hide-keyspaces=true to omit the keyspaces field from the output.
Use a dedicated output structure so the keyspaces field is included or omitted consistently without changing the source API response.
Add unit tests for single and list output conversion, including empty keyspace lists.
Add integration coverage for the default behavior and the explicit hide behavior, and update existing tests to rely on the default output.

Check List

Tests

Unit test
Integration test
manual test

pd-ctl keyspace-group <keyspace-group-id>
pd-ctl keyspace-group --hide-keyspaces <keyspace-group-id>

Release note

`pd-ctl keyspace-group` now shows the keyspaces field by default. Use `--hide-keyspaces` to hide it.

Summary by CodeRabbit

New Features
- Added a --hide-keyspaces option to omit keyspace lists from keyspace-group command output.
- Keyspace lists are now included in output by default.
- Output omits empty optional fields for cleaner results.
Bug Fixes
- Improved keyspace-group output validation and reliability in command-line checks.

Signed-off-by: ystaticy <y_static_y@sina.com> (cherry picked from commit 84d1fcb3a39aff3422c6729dd322d6bcf0d89d8d)

Signed-off-by: ystaticy <y_static_y@sina.com>

coderabbitai · 2026-09-09T03:09:16Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: c4ce78db-8ec6-4085-bbf9-251231d3447e

📥 Commits

Reviewing files that changed from the base of the PR and between 2d96361 and c6136af.

📒 Files selected for processing (3)

tools/pd-ctl/pdctl/command/keyspace_group_command.go
tools/pd-ctl/pdctl/command/keyspace_group_command_test.go
tools/pd-ctl/tests/keyspace/keyspace_group_test.go

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Walkthrough

The keyspace-group command now includes keyspaces by default. The --hide-keyspaces flag omits them. JSON conversion uses a dedicated output structure, with unit and integration test coverage.

Changes

Keyspace group output

Layer / File(s)	Summary
Flag and conditional output `tools/pd-ctl/pdctl/command/keyspace_group_command.go`	Adds the default-disabled `--hide-keyspaces` flag. The command passes its inverse to conversion helpers. JSON output conditionally includes `keyspaces` and omits empty split or merge state.
Unit and integration validation `tools/pd-ctl/pdctl/command/keyspace_group_command_test.go`, `tools/pd-ctl/tests/keyspace/keyspace_group_test.go`	Tests verify default inclusion, explicit hiding, converted group output, and keyspace membership checks.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Merge Risk: ⚪ Minimal · up to c6136

Keyspace-group output includes keyspaces by default and hides them only when requested, consistently across supported output modes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description explains the problem, implementation, tests, manual steps, issue number, and release note. It omits the optional Code changes, Side effects, and Related changes checklist sections, but…
Title check	✅ Passed	The title identifies the pd-ctl keyspace-group change and the show-keyspaces behavior. Its wording is awkward but remains related to the main change.
Linked Issues check	✅ Passed	The implementation satisfies issue `#11207`. `--hide-keyspaces` defaults to `false`. Single and list conversion paths pass the inverse flag to include `keyspaces` by default and omit it when the flag is…
Out of Scope Changes check	✅ Passed	The changes stay within issue `#11207`. The output structure, command flag, conversion helpers, unit tests, and integration test all support the requested keyspace output behavior. No unrelated product …

Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

ti-chi-bot · 2026-09-09T03:14:07Z

@coderabbitai[bot]: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

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.

ti-chi-bot · 2026-09-09T03:14:08Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: coderabbitai[bot]
Once this PR has been reviewed and has the lgtm label, please assign binshi-bing for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

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

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

codecov · 2026-09-09T03:20:12Z

Codecov Report

❌ Patch coverage is 82.75862% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.73%. Comparing base (e000290) to head (c6136af).
⚠️ Report is 35 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11208      +/-   ##
==========================================
+ Coverage   79.35%   79.73%   +0.37%     
==========================================
  Files         542      545       +3     
  Lines       76993    78913    +1920     
==========================================
+ Hits        61097    62918    +1821     
- Misses      11594    11620      +26     
- Partials     4302     4375      +73

Flag	Coverage Δ
unittests	`79.73% <82.75%> (+0.37%)`	⬆️

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ystaticy · 2026-09-09T04:09:56Z

/test pull-unit-test-next-gen-2
/test pull-unit-test-next-gen-3

rleungx · 2026-09-10T03:45:44Z

+	testutil.Eventually(re, func() bool {
+		output, err := tests.ExecuteCommand(cmd, append(args, defaultKeyspaceGroupID)...)
+		re.NoError(err)
+		re.Contains(string(output), "\"keyspaces\"")


testutil.Eventually runs this callback in a polling goroutine, while require.Contains and require.NoError call FailNow. If the keyspace-group manager is still initializing and this request temporarily returns a non-JSON error—a state the helper below explicitly retries—the callback exits before returning false; the test is marked failed and then waits for the Eventually timeout instead of retrying, making the new integration test flaky.

Signed-off-by: ystaticy <y_static_y@sina.com>

ystaticy · 2026-09-11T08:16:40Z

/retest-required

ystaticy · 2026-09-11T08:35:50Z

/test pull-unit-test-next-gen-3

bufferflies · 2026-09-11T09:02:25Z

 }

-func convertToKeyspaceGroups(content string) string {
+func convertToKeyspaceGroups(content string, showKeyspaces bool) string {


How about implementing it on the server side? The response body may be very large if the keyspace count is too large.

ystaticy added 2 commits August 12, 2026 13:16

pd-ctl no ks

b054910

Signed-off-by: ystaticy <y_static_y@sina.com> (cherry picked from commit 84d1fcb3a39aff3422c6729dd322d6bcf0d89d8d)

pd-ctl: show keyspaces by default

2d96361

Signed-off-by: ystaticy <y_static_y@sina.com>

ti-chi-bot Bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has signed the dco. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 9, 2026

ystaticy changed the title ~~pd-ctl: show keyspaces by default~~ pd-ctl keyspace-group: show keyspaces by default Sep 9, 2026

coderabbitai Bot approved these changes Sep 9, 2026

View reviewed changes

ystaticy changed the title ~~pd-ctl keyspace-group: show keyspaces by default~~ pd-ctl: keyspace-group support set show keyspaces Sep 9, 2026

rleungx reviewed Sep 10, 2026

View reviewed changes

rleungx reviewed Sep 11, 2026

View reviewed changes

Comment thread tools/pd-ctl/pdctl/command/keyspace_group_command.go Outdated

pd-ctl: use hide-keyspaces flag

c6136af

Signed-off-by: ystaticy <y_static_y@sina.com>

bufferflies reviewed Sep 11, 2026

View reviewed changes

Navigation Menu

Uh oh!

pd-ctl: keyspace-group support set show keyspaces - #11208

pd-ctl: keyspace-group support set show keyspaces #11208
ystaticy wants to merge 3 commits into
tikv:masterfrom
ystaticy:tso_kg_list

ystaticy commented Sep 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 9, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

ti-chi-bot Bot commented Sep 9, 2026

Uh oh!

ti-chi-bot Bot commented Sep 9, 2026

Uh oh!

codecov Bot commented Sep 9, 2026 •

edited

Loading

Uh oh!

ystaticy commented Sep 9, 2026

Uh oh!

rleungx Sep 10, 2026

Uh oh!

Uh oh!

ystaticy commented Sep 11, 2026

Uh oh!

ystaticy commented Sep 11, 2026

Uh oh!

bufferflies Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ystaticy commented Sep 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

ti-chi-bot Bot commented Sep 9, 2026

Uh oh!

ti-chi-bot Bot commented Sep 9, 2026

Uh oh!

codecov Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ystaticy commented Sep 9, 2026

Uh oh!

rleungx Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ystaticy commented Sep 11, 2026

Uh oh!

ystaticy commented Sep 11, 2026

Uh oh!

bufferflies Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ystaticy commented Sep 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 9, 2026 •

edited

Loading

codecov Bot commented Sep 9, 2026 •

edited

Loading