fix(pd): populate memberSize in GET / endpoint response#3003
Open
caglareker wants to merge 3 commits intoapache:masterfrom
Open
fix(pd): populate memberSize in GET / endpoint response#3003caglareker wants to merge 3 commits intoapache:masterfrom
caglareker wants to merge 3 commits intoapache:masterfrom
Conversation
dosubot
bot
added
size:XS
bitflicker64
approved these changes
Apr 20, 2026
Contributor
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Tested locally with Docker dev cluster , memberSize now returns 1 (was 0). LGTM
Author
|
Thanks for testing it out, glad it works. |
- Use pdService.getMembers() for memberSize (consistent with cluster()) instead of RaftEngine directly, as suggested in issue apache#3002 discussion - Add dataState field to BriefStatistics: exposes worst partition health state across all graphs, the most useful missing operational indicator - Align graphSize to count only user-facing graphs (endsWith("/g")), matching the semantics of cluster() to avoid silent count discrepancy - Add testQueryIndexInfo() to both RestApiTest classes to assert state, leader, memberSize > 0, and storeSize > 0 — catches this class of bug
dosubot
bot
added
size:M
Member
maybe we need test again for the new logic |
imbajin
reviewed
Apr 20, 2026
| statistics.memberSize = membersResp.get().get(0).getMembersList().size(); | ||
|
|
||
| statistics.storeSize = pdService.getStoreNodeService().getActiveStores().size(); | ||
| statistics.graphSize = pdService.getPartitionService().getGraphs().size(); |
Member
There was a problem hiding this comment.
The previous fix adds:
statistics.memberSize = RaftEngine.getInstance().getMembers().size();
But the cluster() method (the "correct" counterpart) uses the service layer:
pdService.getMembers(Pdpb.GetMembersRequest.newBuilder().build(), response);
statistics.memberSize = pdList.size(); // from pdService, not RaftEngineThe issue discussion (comments 3–4 from bitflicker64 and dosubot) explicitly recommends using pdService.getMembers().size() for consistency with cluster() and to stay in the service layer. The merged fix goes against this recommendation.
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose of the PR
Main Changes
The
memberSizefield inBriefStatisticswas never being set, so GET / always returned 0. Added the missing assignment fromRaftEngine.getInstance().getMembers().size(), same pattern as the other fields right next to it.Verifying these changes