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

Support retrieving current term #620

Merged
merged 2 commits into from
Oct 10, 2024
Merged

Support retrieving current term #620

merged 2 commits into from
Oct 10, 2024

Conversation

otoolep
Copy link
Contributor

@otoolep otoolep commented Oct 9, 2024

Simple addition to make this internal variable accessible (without relying on stats object, which is a brittle approach).

Term is a fundamental concept in Raft consensus, so making it easily available to clients of this library seems like an obvious thing to do. There are also specific use cases supported by knowing the Term:

  • Allow clients of this library to retrieve and display diagnostic information about the state of the Raft system.
  • Support certain types of reads of a Raft-managed store. For example, if a client can check before and after a Read that an election has not taken place during the read, it allows those clients to make certain guarantees about the data read from the Raft-managed Store. One way to do this is to simply check that the Term has not changed (see this discussion for more details).

@otoolep otoolep requested review from a team as code owners October 9, 2024 15:54
@otoolep otoolep requested review from rboyer and removed request for a team October 9, 2024 15:54
@otoolep
Copy link
Contributor Author

otoolep commented Oct 9, 2024

I need this for my work to add Linearizable reads to rqlite, version 8.32.0. See https://github.com/rqlite/rqlite/blob/master/CHANGELOG.md

This seems like something the library should expose, perhaps no one needed it yet.

Copy link
Member

@schmichael schmichael 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, but I'm going to hold off slapping merge until @rboyer or somebody else gets a chance to weigh in.

I'd personally love a Why in the PR description. Skimming the linked mailing list thread gave me enough context to 👍 this, but a nice succinct summary here would be nice for any future generations hoping to figure out when Term is useful. Not a blocker though!

@otoolep
Copy link
Contributor Author

otoolep commented Oct 9, 2024

Description updated -- thanks.

@schmichael schmichael merged commit 2b0032e into hashicorp:main Oct 10, 2024
12 checks passed
@schmichael
Copy link
Member

Merged, thanks for your contribution and patience @otoolep !

@otoolep otoolep deleted the patch-3 branch October 10, 2024 17:05
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.

3 participants