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-2249: CRC check failed when preAllocSize smaller than node data #436
Conversation
fc03120
to
44ad057
Compare
@@ -211,7 +211,7 @@ public static long padLogFile(FileOutputStream f,long currentSize, | |||
long preAllocSize) throws IOException{ | |||
long position = f.getChannel().position(); | |||
if (position + 4096 >= currentSize) { | |||
currentSize = currentSize + preAllocSize; | |||
currentSize = position + preAllocSize; |
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.
Nice catch Abe! This looks like a reasonable fix, however it results in the doc'd invariant not being followed
"allocates space in the transaction log file in blocks of preAllocSize kilobytes"
I believe in this case you want something which will maintain the file as a multiple of this value.
Perhaps a while loop inside the conditional, replacing "currentSize = currentSize + preAllocSize;" with:
while (position + 4096 >= currentSize) {
currentSize = currentSize + preAllocSize
}
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.
Fixed
a97392b
to
e57676f
Compare
e57676f
to
f3bc06c
Compare
…data This bug is caused by an issue in our file padding logic. We calculate an incorrect position to add padding when appending to the transaction log, this often corrupts a transaction. When the log is read the CRC check will correctly fail. Author: Abraham Fine <afine@apache.org> Reviewers: phunt@apache.org Closes #436 from afine/ZOOKEEPER-2249 and squashes the following commits: f3bc06c [Abraham Fine] Improve testing and respond to phunt's comment 44ad057 [Abraham Fine] ZOOKEEPER-2249: CRC check failed when preAllocSize smaller than node data Change-Id: Id8ba9ad730760cb78672127b8c0e02db60b4e87d (cherry picked from commit 4d629d2) Signed-off-by: Patrick Hunt <phunt@apache.org>
…data This bug is caused by an issue in our file padding logic. We calculate an incorrect position to add padding when appending to the transaction log, this often corrupts a transaction. When the log is read the CRC check will correctly fail. Author: Abraham Fine <afine@apache.org> Reviewers: phunt@apache.org Closes #436 from afine/ZOOKEEPER-2249 and squashes the following commits: f3bc06c [Abraham Fine] Improve testing and respond to phunt's comment 44ad057 [Abraham Fine] ZOOKEEPER-2249: CRC check failed when preAllocSize smaller than node data Change-Id: Id8ba9ad730760cb78672127b8c0e02db60b4e87d (cherry picked from commit 4d629d2) Signed-off-by: Patrick Hunt <phunt@apache.org>
LGTM +1. Thanks @afine . Committed to 3.4/3.5/trunk. |
…data This bug is caused by an issue in our file padding logic. We calculate an incorrect position to add padding when appending to the transaction log, this often corrupts a transaction. When the log is read the CRC check will correctly fail. Author: Abraham Fine <afine@apache.org> Reviewers: phunt@apache.org Closes apache#436 from afine/ZOOKEEPER-2249 and squashes the following commits: f3bc06c [Abraham Fine] Improve testing and respond to phunt's comment 44ad057 [Abraham Fine] ZOOKEEPER-2249: CRC check failed when preAllocSize smaller than node data Change-Id: Id8ba9ad730760cb78672127b8c0e02db60b4e87d
…data This bug is caused by an issue in our file padding logic. We calculate an incorrect position to add padding when appending to the transaction log, this often corrupts a transaction. When the log is read the CRC check will correctly fail. Author: Abraham Fine <afine@apache.org> Reviewers: phunt@apache.org Closes apache#436 from afine/ZOOKEEPER-2249 and squashes the following commits: f3bc06c [Abraham Fine] Improve testing and respond to phunt's comment 44ad057 [Abraham Fine] ZOOKEEPER-2249: CRC check failed when preAllocSize smaller than node data Change-Id: Id8ba9ad730760cb78672127b8c0e02db60b4e87d
…data This bug is caused by an issue in our file padding logic. We calculate an incorrect position to add padding when appending to the transaction log, this often corrupts a transaction. When the log is read the CRC check will correctly fail. Author: Abraham Fine <afine@apache.org> Reviewers: phunt@apache.org Closes apache#436 from afine/ZOOKEEPER-2249 and squashes the following commits: f3bc06c [Abraham Fine] Improve testing and respond to phunt's comment 44ad057 [Abraham Fine] ZOOKEEPER-2249: CRC check failed when preAllocSize smaller than node data Change-Id: Id8ba9ad730760cb78672127b8c0e02db60b4e87d
This bug is caused by an issue in our file padding logic. We calculate an incorrect position to add padding when appending to the transaction log, this often corrupts a transaction. When the log is read the CRC check will correctly fail.