-
Notifications
You must be signed in to change notification settings - Fork 7.2k
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
ZOOKEEPER-2789 Reassign ZXID
for solving 32bit overflow problem
#2164
base: master
Are you sure you want to change the base?
Conversation
@kezhuw @eolivelli @li4wang @ztzg I've just come across this one and started to review for 3.9.3, but need more eyeballs. Could you please review? |
ZXID
for solving 32bit overflow problem
This pr will change the server side data format. I think it does not fit a patch release.
Given above, I think it is promising. It promotes rollover rate from Before finalizing this path, I may want to taste whether leadership inheritance is feasible. |
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. Let's submit it to master (3.10.0) as @kezhuw suggested.
* Here, a new file is created locally to store the current number of epoch bits. This file will be created locally after upgrading the version to ensure a smooth joining of the cluster upon startup. | ||
*/ | ||
try { | ||
currentEpochPosition = readLongFromFile(CURRENT_EPOCH_POSITION_FILENAME); |
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.
Why do you need to store this information in a file?
Leader will set it automatically, learners will use the new version based on leader's version.
} else { | ||
byte[] ver = new byte[4]; | ||
ByteBuffer.wrap(ver).putInt(0x10000); | ||
ByteBuffer.wrap(ver).putInt(0x11000); |
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.
The patch doesn't alter anything in the protocol, but you still have to increase protocol version number in order to reject old followers. Odd, but seams reasonable.
@zichen-gan You need to close / re-open PR or force push to trigger another CI run. |
Sure, I'll wait for your review. |
My fault! By "no protocol change", I mean we don't need to prove its correctness in ZAB. |
The original PR link: #262
Since the aforementioned PR does not support rolling hot updates, this PR aims to add rolling upgrade capabilities. The goal of this PR is to change the counter bit length from 32 to 40 and the epoch bit length from 32 to 20 through a rolling upgrade approach.