-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix(proposals): Incremental proposal key for zero proposals #8005
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ahsanbarkati)
dgraph/cmd/zero/raft.go, line 89 at r1 (raw file):
func (n *node) initProposalKey(id uint64) { x.AssertTrue(id != 0) proposalKey = uint64(n.Id)<<48 | uint64(z.FastRand())<<16
You can use crypto.Rand instead of fast rand, which would provide more unique numbers.
Change the proposal's unique key to an atomic counter instead of using a randomly generated key. (cherry picked from commit a515d0d)
…8567) Change the proposal's unique key to an atomic counter instead of using a randomly generated key. Proposal key initialisation has a bug where we want to reserve first 2 bytes for node id but do not do because we read random bytes in all 8 bytes and do a logical OR. This PR adds a test case and fixes the logic of initialising the proposal key. (cherry picked from commit a515d0d) (cherry picked from commit 2aa3d3e)
Change proposal's unique key to an atomic counter instead of using randomly generated key. For a randomly generated key of 32bits, there is a considerable probability of getting the same key used again.
This change is