Skip to content
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

Add API and command to fetch information of about a Raft node #3544

Merged
merged 12 commits into from
Sep 7, 2024

Conversation

bryanhuhta
Copy link
Contributor

closes https://github.com/grafana/pyroscope-squad/issues/230

Adds a CLI command (and corresponding API) to fetch info about a raft node. The command provides a variety of information regarding the node's role, log state, peers, election state, and leader. It has two output modes, the default mode is to provide the data in JSON format, which is a direct copy from what the underlying API returns. The other mode (-H) prints the information in a slightly more tabular, human-readable format.

usage: profilecli admin raft info [<flags>]

Print info about a Raft node.

Flags:
  -h, --help              Show context-sensitive help (also try --help-long and --help-man).
      --version           Show application version.
  -v, --verbose           Enable verbose logging.
      --url="http://localhost:4040"  
                          URL of the profile store.
      --tenant-id=""      The tenant ID to be used for the X-Scope-OrgID header.
      --username=""       The username to be used for basic auth.
      --password=""       The password to be used for basic auth.
      --protocol=connect  The protocol to be used for communicating with the server.
  -H, --human             Human readable output

Sample output:

{
  "id": "pyroscope-metastore-1.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099",
  "state": "Follower",
  "leaderId": "pyroscope-metastore-0.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099",
  "lastLeaderContact": "1725564476270",
  "term": "40",
  "suffrage": "Voter",
  "log": {
    "commitIndex": "357591",
    "appliedIndex": "357591",
    "lastIndex": "357591",
    "fsmPendingLength": "0"
  },
  "snapshot": {
    "lastIndex": "0",
    "lastTerm": "36"
  },
  "protocol": {
    "version": "3",
    "minVersion": "0",
    "maxVersion": "0",
    "minSnapshotVersion": "1",
    "maxSnapshotVersion": "0"
  },
  "peers": [
    {
      "id": "pyroscope-metastore-0.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099",
      "address": "pyroscope-metastore-0.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099",
      "suffrage": "Voter"
    },
    {
      "id": "pyroscope-metastore-2.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.",
      "address": "pyroscope-metastore-2.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099",
      "suffrage": "Voter"
    }
  ]
}

Sample output with -H:

ID:                  pyroscope-metastore-1.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099
State:               Follower
Leader ID:           pyroscope-metastore-0.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099
Last leader contact: 2024-09-05T12:28:32-07:00
Term:                40
Suffrage:            Voter
Log:
  Commit index:       357613
  Applied index:      357613
  Last index:         357613
  FSM pending length: 0
Snapshot:
  Last index: 0
  Last term:  36
Protocol:
  Version:              3
  Min version:          0
  Max version:          0
  Min snapshot version: 1
  Max snapshot version: 0
Peers:
  ID:       pyroscope-metastore-0.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099
  Address:  pyroscope-metastore-0.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099
  Suffrage: Voter

  ID:       pyroscope-metastore-2.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.
  Address:  pyroscope-metastore-2.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099
  Suffrage: Voter

@bryanhuhta bryanhuhta self-assigned this Sep 5, 2024
@bryanhuhta bryanhuhta marked this pull request as ready for review September 5, 2024 21:05
@bryanhuhta bryanhuhta requested a review from a team as a code owner September 5, 2024 21:05
Copy link
Contributor

@aleks-p aleks-p left a comment

Choose a reason for hiding this comment

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

Looks good to me. I left one comment about the leadership check.

Comment on lines +48 to +49
switch m.raft.State() {
case raft.Leader:
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably does not make a big difference here based on the context of how we use it, but this check can return stale information (a non-leader could "think" they are the leader when they are not).

There is a VerifyLeader() method that we can call which will run a sequence of commands through the raft log that will make it a stronger guarantee.

We use it in

if err := m.raft.VerifyLeader().Error(); err != nil {
where we return an error if we are not the leader but it can be repurposed here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In 270171c I incorporated this check. It's reflected in the IsLeaderVerified field of the response. Here's an example output:

ID:                  pyroscope-metastore-0.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099
State:               Follower
State verified:      true
Leader ID:           pyroscope-metastore-1.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099
Last leader contact: 2024-09-06T17:15:32-07:00
Term:                8
Suffrage:            Voter
Log:
  Commit index:       988
  Applied index:      988
  Last index:         988
  FSM pending length: 0
Snapshot:
  Last index: 0
  Last term:  0
Protocol:
  Version:              3
  Min version:          0
  Max version:          0
  Min snapshot version: 1
  Max snapshot version: 0
Peers:
  ID:       pyroscope-metastore-1.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099
  Address:  pyroscope-metastore-1.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099
  Suffrage: Voter

  ID:       pyroscope-metastore-2.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099
  Address:  pyroscope-metastore-2.pyroscope-metastore-headless.pyroscope-test.svc.cluster.local.:9099
  Suffrage: Voter

I have observed this change causes a noticeable increase in latency of the command. I'm not sure if that's an artifact of my local setup or a more systemic one. Probably nothing to worry about now though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The State verified field will be true if the state reported by the node matches the state the cluster thinks the node is. I.e. if the node thinks it's a leader and the cluster agrees, then it is set to true. If the node thinks it's not a leader, and the cluster agress, then it is set to true, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the latency increase is expected as the call should result in network requests between peers but I am not sure what is the expected order of magnitude for this latency. Probably fine for a command line tool for now.

@bryanhuhta bryanhuhta enabled auto-merge (squash) September 7, 2024 00:19
@bryanhuhta bryanhuhta merged commit e08ff84 into main Sep 7, 2024
18 checks passed
@bryanhuhta bryanhuhta deleted the metastore branch September 7, 2024 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants