Skip to content

Conversation

mahanteshwar-git
Copy link

What this PR does

This PR adds support for retrieving additional fields in StatusResponse

  • raftAppliedIndex
  • errorsList
  • dbSizeInUse
  • isLearner

Why this is useful

AppliedRaftIndex in comparison with RaftIndex can be used to determine if the PromoteMember operation could be retried
isLearner call in StatusResponse removes the need for another call to MemberList to determine if the member is a learner

Implementation Details

  • Added a fields to the class: StatusResponse
  • Updated testMemberStatus to verify that the new fields are indeed present and populated

Related Issues / Discussions

This change was based on discussion in #1499


Checklist

  • Code compiles successfully
  • Tests added / updated
  • Documentation updated (if needed)
  • Ready for review

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mahanteshwar-git
Once this PR has been reviewed and has the lgtm label, please assign vorburger for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

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

@mahanteshwar-git
Copy link
Author

Hi @lburgazzoli , @fanminshi , @vorburger ,

Please review the pull request

@vorburger vorburger requested a review from Copilot August 3, 2025 19:22
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for retrieving additional metadata fields in the etcd StatusResponse including raft applied index, error list, database size in use, and learner status. These fields enable better monitoring and decision-making for etcd cluster operations.

Key changes:

  • Added four new fields to the StatusResponse protobuf definition
  • Exposed these fields through getter methods in the Java StatusResponse class
  • Added comprehensive test coverage to verify the new fields are populated correctly

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
jetcd-grpc/src/main/proto/rpc.proto Added four new fields to StatusResponse message definition
jetcd-core/src/main/proto/rpc.proto Duplicate addition of the same four fields to StatusResponse
jetcd-core/src/main/java/io/etcd/jetcd/maintenance/StatusResponse.java Added getter methods for the new fields with proper documentation
jetcd-core/src/test/java/io/etcd/jetcd/impl/MaintenanceTest.java Enhanced test to verify new fields are present and have expected values
Comments suppressed due to low confidence (2)

jetcd-core/src/main/java/io/etcd/jetcd/maintenance/StatusResponse.java:111

  • The method name 'getIsLearner()' is not conventional for a boolean getter. It should be 'isLearner()' to follow standard Java naming conventions for boolean getters.
    public boolean getIsLearner() {

jetcd-core/src/test/java/io/etcd/jetcd/impl/MaintenanceTest.java:83

  • The test only verifies that the error list is empty, but doesn't test the case where errors might be present. Consider adding a test case that verifies the behavior when errors are actually returned.
        assertThat(statusResponse.getErrorList().size()).isEqualTo(0);

@mahanteshwar-git mahanteshwar-git force-pushed the feat/add-islearner-field-in-statusresponse branch from 089bc4b to 03d7461 Compare August 4, 2025 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants