[jira] [Commented] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15311816#comment-15311816 ] Ed Rowe commented on ZOOKEEPER-2420: My original description was not 100% accurate - I've updated it to point out that loading the DB from logs actually only looks at a prior log if there isn't a log with the same zxid as the snapshot. However, whether there is equivalency between log zxid and snapshot zxid is timing dependent so the == case at best reduces occurrences of the bug. I'm not sure why this issue hasn't been found before now. I discovered it by reading the code and then empirically reproduced it. The patch adds new test case testSnapFilesEqualsToRetainWithPrecedingLog to test this specific case in isolation, and updates existing test cases testSnapFilesGreaterThanToRetain and testSnapFilesLessThanToRetain (all in PurgeTxnTest.java) to test the corrected behavior. If these new/updated tests are run against the original code (modulo the refactoring I did in the patch) they will all fail. Re: patch naming convention for updated patches - I hadn't noticed that. Will follow it in the future. > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe >Assignee: Ed Rowe > Attachments: ZOOKEEPER-2420.patch, ZOOKEEPER-2420.patch_v2, > ZOOKEEPER-2420.patch_v3 > > > Autopurge retains all log files whose zxid are >= the zxid of the oldest > snapshot file that it is going to retain (in PurgeTxnLog > retainNRecentSnapshots()). However, unless there is a log file with the same > zxid as the oldest snapshot file being retained (and whether log file and > snapshot file zxids are equal is timing dependent), loading the database from > snapshots/logs will start with the log file _prior_ to the snapshot's zxid. > Thus, to avoid data loss autopurge should retain the log file prior to the > oldest retained snapshot as well, unless it verifies that it contains no > zxids beyond what the snapshot contains or there is a log file whose zxid == > snapshot zxid. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ed Rowe updated ZOOKEEPER-2420: --- Description: Autopurge retains all log files whose zxid are >= the zxid of the oldest snapshot file that it is going to retain (in PurgeTxnLog retainNRecentSnapshots()). However, unless there is a log file with the same zxid as the oldest snapshot file being retained (and whether log file and snapshot file zxids are equal is timing dependent), loading the database from snapshots/logs will start with the log file _prior_ to the snapshot's zxid. Thus, to avoid data loss autopurge should retain the log file prior to the oldest retained snapshot as well, unless it verifies that it contains no zxids beyond what the snapshot contains or there is a log file whose zxid == snapshot zxid. was:Autopurge retains all log files whose zxid are >= the zxid of the oldest snapshot file that it is going to retain (in PurgeTxnLog retainNRecentSnapshots()). Given that loading the database from snapshots/logs will start with the log file _prior_ to the snapshot's zxid, autopurge should retain the log file prior to the oldest retained snapshot as well, unless it verifies that it contains no zxids beyond what the snapshot contains. > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe >Assignee: Ed Rowe > Attachments: ZOOKEEPER-2420.patch, ZOOKEEPER-2420.patch_v2, > ZOOKEEPER-2420.patch_v3 > > > Autopurge retains all log files whose zxid are >= the zxid of the oldest > snapshot file that it is going to retain (in PurgeTxnLog > retainNRecentSnapshots()). However, unless there is a log file with the same > zxid as the oldest snapshot file being retained (and whether log file and > snapshot file zxids are equal is timing dependent), loading the database from > snapshots/logs will start with the log file _prior_ to the snapshot's zxid. > Thus, to avoid data loss autopurge should retain the log file prior to the > oldest retained snapshot as well, unless it verifies that it contains no > zxids beyond what the snapshot contains or there is a log file whose zxid == > snapshot zxid. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-1549) Data inconsistency when follower is receiving a DIFF with a dirty snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15306472#comment-15306472 ] Ed Rowe commented on ZOOKEEPER-1549: [~fpj] I think the approach of deleting snapshots is a good one. Further, I think you hit the nail on the head when you say "Snapshots are simply compacted versions of the txn log history." The previous idea (further up the comment history) in which the log, but not snapshots, would be allowed to contain uncommitted transactions seems fragile. A pedantic update to your definition would be "Snapshots are simply compacted versions of the txn log history, as applied to the DataTree." to recognize that snapshots only ever contain information that has been in a DataTree (though not necessarily a DataTree that was ever visible outside a given Node) while logs can contain information that has never been in a DataTree. One issue to account for in the fix is the case where there is no earlier snapshot to rebuild from. This could occur if an operator has deleted older snapshots. I think we'd still want to delete the snapshot being truncated and arrange for the learner node to start over with a blank database. Another consideration is that, as I've written in ZOOKEEPER-2436, the learner might receive from the leader a SNAP rather than a TRUNC. In this situation, the snapshot on the learner node that a TRUNC would have deleted will still be present on the learner node, but it will no longer be the newest snapshot. I don't think this will cause any problems but I did want to bring it up. Finally, there were some concerns raised early in the comment thread that deleting snapshots might be too aggressive. If this is still an issue, we could (perhaps optionally) rename logs and snapshots that we are truncating rather than delete them. The snapshot/log purge code might then clean these up if they are old enough. I'm not convinced that this is worth implementing now. > Data inconsistency when follower is receiving a DIFF with a dirty snapshot > -- > > Key: ZOOKEEPER-1549 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1549 > Project: ZooKeeper > Issue Type: Bug > Components: quorum >Affects Versions: 3.4.3 >Reporter: Jacky007 >Assignee: Flavio Junqueira >Priority: Blocker > Fix For: 3.5.2, 3.6.0 > > Attachments: ZOOKEEPER-1549-3.4.patch, ZOOKEEPER-1549-learner.patch, > case.patch > > > the trunc code (from ZOOKEEPER-1154?) cannot work correct if the snapshot is > not correct. > here is scenario(similar to 1154): > Initial Condition > 1.Lets say there are three nodes in the ensemble A,B,C with A being the > leader > 2.The current epoch is 7. > 3.For simplicity of the example, lets say zxid is a two digit number, > with epoch being the first digit. > 4.The zxid is 73 > 5.All the nodes have seen the change 73 and have persistently logged it. > Step 1 > Request with zxid 74 is issued. The leader A writes it to the log but there > is a crash of the entire ensemble and B,C never write the change 74 to their > log. > Step 2 > A,B restart, A is elected as the new leader, and A will load data and take a > clean snapshot(change 74 is in it), then send diff to B, but B died before > sync with A. A died later. > Step 3 > B,C restart, A is still down > B,C form the quorum > B is the new leader. Lets say B minCommitLog is 71 and maxCommitLog is 73 > epoch is now 8, zxid is 80 > Request with zxid 81 is successful. On B, minCommitLog is now 71, > maxCommitLog is 81 > Step 4 > A starts up. It applies the change in request with zxid 74 to its in-memory > data tree > A contacts B to registerAsFollower and provides 74 as its ZxId > Since 71<=74<=81, B decides to send A the diff. > Problem: > The problem with the above sequence is that after truncate the log, A will > load the snapshot again which is not correct. > In 3.3 branch, FileTxnSnapLog.restore does not call listener(ZOOKEEPER-874), > the leader will send a snapshot to follower, it will not be a problem. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (ZOOKEEPER-2436) Inconsistent truncation logic and out of sync logging and comments in recovery code
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2436?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ed Rowe updated ZOOKEEPER-2436: --- Description: Consider this scenario: # Ensemble of nodes A, B, C, D, E with A as the leader # Nodes A, B get partitioned from C, D, E # Leader A receives a write _before it detects_ that it has lost its quorum so it logs the write # Nodes C, D, E elect node C as the leader # Partition resolves, and nodes A, B rejoin the C, D, E ensemble with C continuing to lead Depending on whether any updates have occurred in the C, D, E ensemble between steps 4 and 5, the re-joining nodes A,B will either receive a TRUNC or a SNAP. *The problems:* # If updates have occurred in the C,D,E ensemble, SNAP is sent to the re-joining nodes. This occurs because the code in LearnerHandler.queueCommittedProposals() notices that truncation would cross epochs and bails out, leading to a SNAP being sent. A comment in the code says "We cannot send TRUNC that cross epoch boundary. The learner will crash if it is asked to do so. We will send snapshot this those cases." LearnerHandler.syncFollower() then logs an ERROR saying "Unhandled scenario for peer sid: # fall back to use snapshot" and a comment with this code says "This should never happen, but we should fall back to sending snapshot just in case." Presumably since queueCommittedProposals() is intentionally triggering the snapshot logic, this is not an "unhandled scenario" that warrants logging an ERROR nor is it a case that "should never happen". This inconsistency should be cleaned up. It might also be the case that a TRUNC would work fine in this scenario - see #2 below. # If no updates have occurred in the C,D,E ensemble, when nodes A,B rejoin LearnerHandler.syncFollower() goes into the "Newer than commitedLog, send trunc and done" clause and sends them a TRUNC. This seems to work fine. However, this would also seem to be a cross-epoch TRUNC, which per the comment discussed above in #1, is expected to cause a crash in the learner. I haven't found anything special about a TRUNC that crosses epochs that would cause a crash in the learner, and I believe that at the time of the TRUNC (or SNAP), the learner is in the same state in both scenarios. It is certainly the case (pending resolution of ZOOKEEPER-1549) that TRUNC is not able to remove data that has been snapshotted, so perhaps detecting “cross-epoch” is a shortcut for trying to detect that scenario? If the resolution of ZOOKEEPER-1549 does not allow TRUNC through a snapshot (or alternately does not allow a benign TRUNC through a snapshot that may not contain uncommitted data), then this case should probably also be a SNAP. If TRUNC is allowed in this case, then perhaps it should also be allowed for case #1, which would be more performant. *While I certainly could have missed something, it would seem that either both cases should be SNAP or both should be a TRUNC given that the learner is in the same state in both cases*. was: Consider this scenario: Ensemble of nodes A, B, C, D, E with A as the leader Nodes A, B get partitioned from C, D, E Leader A receives a write _before it detects_ that it has lost its quorum so it logs the write Nodes C, D, E elect node C as the leader Partition resolves, and nodes A, B rejoin the C, D, E ensemble with C continuing to lead Depending on whether any updates have occurred in the C, D, E ensemble between steps 4 and 5, the re-joining nodes A,B will either receive a TRUNC or a SNAP. The problems: # If updates have occurred in the C,D,E ensemble, SNAP is sent to the re-joining nodes. This occurs because the code in LearnerHandler.queueCommittedProposals() notices that truncation would cross epochs and bails out, leading to a SNAP being sent. A comment in the code says "We cannot send TRUNC that cross epoch boundary. The learner will crash if it is asked to do so. We will send snapshot this those cases." LearnerHandler.syncFollower() then logs an ERROR saying "Unhandled scenario for peer sid: # fall back to use snapshot" and a comment with this code says "This should never happen, but we should fall back to sending snapshot just in case." Presumably since queueCommittedProposals() is intentionally triggering the snapshot logic, this is not an "unhandled scenario" that warrants logging an ERROR nor is it a case that "should never happen". This inconsistency should be cleaned up. It might also be the case that a TRUNC would work fine in this scenario - see #2 below. # If no updates have occurred in the C,D,E ensemble, when nodes A,B rejoin LearnerHandler.syncFollower() goes into the "Newer than commitedLog, send trunc and done" clause and sends them a TRUNC. This seems to work fine. However, this would also seem to be a cross-epoch TRUNC, which per the comment discussed above in #1, is expected to cause a crash in the learner. I hav
[jira] [Created] (ZOOKEEPER-2436) Inconsistent truncation logic and out of sync logging and comments in recovery code
Ed Rowe created ZOOKEEPER-2436: -- Summary: Inconsistent truncation logic and out of sync logging and comments in recovery code Key: ZOOKEEPER-2436 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2436 Project: ZooKeeper Issue Type: Bug Reporter: Ed Rowe Priority: Minor Consider this scenario: Ensemble of nodes A, B, C, D, E with A as the leader Nodes A, B get partitioned from C, D, E Leader A receives a write _before it detects_ that it has lost its quorum so it logs the write Nodes C, D, E elect node C as the leader Partition resolves, and nodes A, B rejoin the C, D, E ensemble with C continuing to lead Depending on whether any updates have occurred in the C, D, E ensemble between steps 4 and 5, the re-joining nodes A,B will either receive a TRUNC or a SNAP. The problems: # If updates have occurred in the C,D,E ensemble, SNAP is sent to the re-joining nodes. This occurs because the code in LearnerHandler.queueCommittedProposals() notices that truncation would cross epochs and bails out, leading to a SNAP being sent. A comment in the code says "We cannot send TRUNC that cross epoch boundary. The learner will crash if it is asked to do so. We will send snapshot this those cases." LearnerHandler.syncFollower() then logs an ERROR saying "Unhandled scenario for peer sid: # fall back to use snapshot" and a comment with this code says "This should never happen, but we should fall back to sending snapshot just in case." Presumably since queueCommittedProposals() is intentionally triggering the snapshot logic, this is not an "unhandled scenario" that warrants logging an ERROR nor is it a case that "should never happen". This inconsistency should be cleaned up. It might also be the case that a TRUNC would work fine in this scenario - see #2 below. # If no updates have occurred in the C,D,E ensemble, when nodes A,B rejoin LearnerHandler.syncFollower() goes into the "Newer than commitedLog, send trunc and done" clause and sends them a TRUNC. This seems to work fine. However, this would also seem to be a cross-epoch TRUNC, which per the comment discussed above in #1, is expected to cause a crash in the learner. I haven't found anything special about a TRUNC that crosses epochs that would cause a crash in the learner, and I believe that at the time of the TRUNC (or SNAP), the learner is in the same state in both scenarios. It is certainly the case (pending resolution of ZOOKEEPER-1549) that TRUNC is not able to remove data that has been snapshotted, so perhaps detecting “cross-epoch” is a shortcut for trying to detect that scenario? If the resolution of ZOOKEEPER-1549 does not allow TRUNC through a snapshot (or alternately does not allow a benign TRUNC through a snapshot that may not contain uncommitted data), then this case should probably also be a SNAP. If TRUNC is allowed in this case, then perhaps it should also be allowed for case #1, which would be more performant. While I certainly could have missed something, it would seem that either both cases should be SNAP or both should be a TRUNC. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (ZOOKEEPER-2434) Error handling issues in truncation code
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2434?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ed Rowe updated ZOOKEEPER-2434: --- Description: # If FileTxnLog.truncate() is unable to delete a log file, it calls LOG.warn() but otherwise does nothing. I think this should be a fatal error not a logged warning. Otherwise those log files are going be be read later when the DB is reloaded and data that should have been removed will still be present. # Learner.syncWithLeader() expects ZKDatabase.truncateLog() to return false on failure, and if this occurs it calls System.exit(). However, this will never happen because ZKDatabase.truncateLog() never returns false - instead an exception is thrown on failure. ZKDatabase.truncateLog() calls FileTxnSnapLog.truncateLog() which calls FileTxnLog.truncate(), each of which is documented to return false on failure but none of which ever does in practice. TruncateTest.testTruncationNullLog() clearly expects an exception on error in ZKDatabase.truncateLog() so there are conflicting expectations in the codebase. It appears that if Learner.syncWithLeader() encounters an exception, System.exit() will _not_ be called and instead we land in the main run loop where we'll start the whole thing again. So there are two things to deal with: a) whether we want to do system.exit or go back to the main run loop if truncation fails, and b) sort out the return false vs. throw exception discrepancy and make it consistent (and change the docs as needed). I'm happy to propose a patch, but I'd need people with more experience in the codebase to weigh in on the questions above. was: # If FileTxnLog.truncate() is unable to delete a log file, it calls LOG.warn() but otherwise ignores does nothing. I think this should be a fatal error not a logged warning. Otherwise those log files are going be be read later when the DB is reloaded and data that should have been removed will still be present. # Learner.syncWithLeader() expects ZKDatabase.truncateLog() to return false on failure, and if this occurs it calls System.exit(). However, this will never happen because ZKDatabase.truncateLog() never returns false - instead an exception is thrown on failure. ZKDatabase.truncateLog() calls FileTxnSnapLog.truncateLog() which calls FileTxnLog.truncate(), each of which is documented to return false on failure but none of which ever does in practice. TruncateTest.testTruncationNullLog() clearly expects an exception on error in ZKDatabase.truncateLog() so there are conflicting expectations in the codebase. It appears that if Learner.syncWithLeader() encounters an exception, System.exit() will _not_ be called and instead we land in the main run loop where we'll start the whole thing again. So there are two things to deal with: a) whether we want to do system.exit or go back to the main run loop if truncation fails, and b) sort out the return false vs. throw exception discrepancy and make it consistent (and change the docs as needed). I'm happy to propose a patch, but I'd need people with more experience in the codebase to weigh in on the questions above. > Error handling issues in truncation code > > > Key: ZOOKEEPER-2434 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2434 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe >Priority: Minor > > # If FileTxnLog.truncate() is unable to delete a log file, it calls > LOG.warn() but otherwise does nothing. I think this should be a fatal error > not a logged warning. Otherwise those log files are going be be read later > when the DB is reloaded and data that should have been removed will still be > present. > # Learner.syncWithLeader() expects ZKDatabase.truncateLog() to return false > on failure, and if this occurs it calls System.exit(). However, this will > never happen because ZKDatabase.truncateLog() never returns false - instead > an exception is thrown on failure. ZKDatabase.truncateLog() calls > FileTxnSnapLog.truncateLog() which calls FileTxnLog.truncate(), each of which > is documented to return false on failure but none of which ever does in > practice. TruncateTest.testTruncationNullLog() clearly expects an exception > on error in ZKDatabase.truncateLog() so there are conflicting expectations in > the codebase. It appears that if Learner.syncWithLeader() encounters an > exception, System.exit() will _not_ be called and instead we land in the main > run loop where we'll start the whole thing again. So there are two things to > deal with: a) whether we want to do system.exit or go back to the main run > loop if truncation fails, and b) sort out the return false vs. throw > exception discrepancy and make it consistent (and change the docs as needed). > > I'm hap
[jira] [Updated] (ZOOKEEPER-2434) Error handling issues in truncation code
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2434?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ed Rowe updated ZOOKEEPER-2434: --- Description: # If FileTxnLog.truncate() is unable to delete a log file, it calls LOG.warn() but otherwise ignores does nothing. I think this should be a fatal error not a logged warning. Otherwise those log files are going be be read later when the DB is reloaded and data that should have been removed will still be present. # Learner.syncWithLeader() expects ZKDatabase.truncateLog() to return false on failure, and if this occurs it calls System.exit(). However, this will never happen because ZKDatabase.truncateLog() never returns false - instead an exception is thrown on failure. ZKDatabase.truncateLog() calls FileTxnSnapLog.truncateLog() which calls FileTxnLog.truncate(), each of which is documented to return false on failure but none of which ever does in practice. TruncateTest.testTruncationNullLog() clearly expects an exception on error in ZKDatabase.truncateLog() so there are conflicting expectations in the codebase. It appears that if Learner.syncWithLeader() encounters an exception, System.exit() will _not_ be called and instead we land in the main run loop where we'll start the whole thing again. So there are two things to deal with: a) whether we want to do system.exit or go back to the main run loop if truncation fails, and b) sort out the return false vs. throw exception discrepancy and make it consistent (and change the docs as needed). I'm happy to propose a patch, but I'd need people with more experience in the codebase to weigh in on the questions above. was: # If FileTxnLog.truncate() is unable to delete a log file, it calls LOG.warn() but otherwise ignores does nothing. I think this should be a fatal error not a logged warning. Otherwise those log files are going be be read later when the DB is reloaded and data that should have been removed will still be present. # Learner.syncWithLeader() expects ZKDatabase.truncateLog() to return false on failure, and if this occurs it calls System.exit(). However, this will never happen because ZKDatabase.truncateLog() never returns false - instead an exception is thrown on failure. ZKDatabase.truncateLog() calls FileTxnSnapLog.truncateLog() which calls FileTxnLog.truncate(), each of which is documented to return false on failure but none of which ever does in practice. TruncateTest.testTruncationNullLog() clearly expects an exception on error in ZKDatabase.truncateLog() so there are conflicting expectations in the codebase. It appears that if Learner.syncWithLeader() encounters an exception, System.exit() will _not_ be called and instead we land in the main run loop where we'll start the whole thing again. So there are two things to deal with: a) whether we want to do system.exit or go back to the main run loop if truncation fails, and b) sort out the return false vs. throw exception discrepancy and make it consistent (and change the docs as needed). I'm happy to propose a patch, but I'd need people with more experience in the codebase to weigh in on the questions above. > Error handling issues in truncation code > > > Key: ZOOKEEPER-2434 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2434 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe >Priority: Minor > > # If FileTxnLog.truncate() is unable to delete a log file, it calls > LOG.warn() but otherwise ignores does nothing. I think this should be a fatal > error not a logged warning. Otherwise those log files are going be be read > later when the DB is reloaded and data that should have been removed will > still be present. > # Learner.syncWithLeader() expects ZKDatabase.truncateLog() to return false > on failure, and if this occurs it calls System.exit(). However, this will > never happen because ZKDatabase.truncateLog() never returns false - instead > an exception is thrown on failure. ZKDatabase.truncateLog() calls > FileTxnSnapLog.truncateLog() which calls FileTxnLog.truncate(), each of which > is documented to return false on failure but none of which ever does in > practice. TruncateTest.testTruncationNullLog() clearly expects an exception > on error in ZKDatabase.truncateLog() so there are conflicting expectations in > the codebase. It appears that if Learner.syncWithLeader() encounters an > exception, System.exit() will _not_ be called and instead we land in the main > run loop where we'll start the whole thing again. So there are two things to > deal with: a) whether we want to do system.exit or go back to the main run > loop if truncation fails, and b) sort out the return false vs. throw > exception discrepancy and make it consistent (and change the docs as need
[jira] [Created] (ZOOKEEPER-2434) Error handling issues in truncation code
Ed Rowe created ZOOKEEPER-2434: -- Summary: Error handling issues in truncation code Key: ZOOKEEPER-2434 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2434 Project: ZooKeeper Issue Type: Bug Components: server Reporter: Ed Rowe Priority: Minor # If FileTxnLog.truncate() is unable to delete a log file, it calls LOG.warn() but otherwise ignores does nothing. I think this should be a fatal error not a logged warning. Otherwise those log files are going be be read later when the DB is reloaded and data that should have been removed will still be present. # Learner.syncWithLeader() expects ZKDatabase.truncateLog() to return false on failure, and if this occurs it calls System.exit(). However, this will never happen because ZKDatabase.truncateLog() never returns false - instead an exception is thrown on failure. ZKDatabase.truncateLog() calls FileTxnSnapLog.truncateLog() which calls FileTxnLog.truncate(), each of which is documented to return false on failure but none of which ever does in practice. TruncateTest.testTruncationNullLog() clearly expects an exception on error in ZKDatabase.truncateLog() so there are conflicting expectations in the codebase. It appears that if Learner.syncWithLeader() encounters an exception, System.exit() will _not_ be called and instead we land in the main run loop where we'll start the whole thing again. So there are two things to deal with: a) whether we want to do system.exit or go back to the main run loop if truncation fails, and b) sort out the return false vs. throw exception discrepancy and make it consistent (and change the docs as needed). I'm happy to propose a patch, but I'd need people with more experience in the codebase to weigh in on the questions above. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15301716#comment-15301716 ] Ed Rowe commented on ZOOKEEPER-2420: v3 patch uploaded. [~hanm] it adds the test (augments existing test for findNRecentSnapshots), fixes the == and revises the comment as you suggested. > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe > Attachments: ZOOKEEPER-2420.patch, ZOOKEEPER-2420.patch_v2, > ZOOKEEPER-2420.patch_v3 > > > Autopurge retains all log files whose zxid are >= the zxid of the oldest > snapshot file that it is going to retain (in PurgeTxnLog > retainNRecentSnapshots()). Given that loading the database from > snapshots/logs will start with the log file _prior_ to the snapshot's zxid, > autopurge should retain the log file prior to the oldest retained snapshot as > well, unless it verifies that it contains no zxids beyond what the snapshot > contains. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ed Rowe updated ZOOKEEPER-2420: --- Attachment: ZOOKEEPER-2420.patch_v3 > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe > Attachments: ZOOKEEPER-2420.patch, ZOOKEEPER-2420.patch_v2, > ZOOKEEPER-2420.patch_v3 > > > Autopurge retains all log files whose zxid are >= the zxid of the oldest > snapshot file that it is going to retain (in PurgeTxnLog > retainNRecentSnapshots()). Given that loading the database from > snapshots/logs will start with the log file _prior_ to the snapshot's zxid, > autopurge should retain the log file prior to the oldest retained snapshot as > well, unless it verifies that it contains no zxids beyond what the snapshot > contains. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15299714#comment-15299714 ] Ed Rowe commented on ZOOKEEPER-2420: New patch uploaded. [~hanm] as you suggested I am now using FileTxnLog.getLogFiles() to find the zxid of the oldest log file we need to retain. This change keeps the logic about "prior logs" confined to getLogFiles() which is definitely cleaner. I did not remove the filter or the comparison by zxid because it is required to prevent incorrectly deleting new snapshots/logs in a race condition - I added a comment in the new patch to discuss this case. I also refactored/renamed retainNRecentSnapshots() -> purgeOlderSnapshots() and changed it to take a single snapshot file rather than a list because it makes the interface cleaner and more obvious (the list was solely used to provide the oldest snapshot). As part of this patch I also fixed a bug in FileSnap.findNRecentSnapshots() - if there were fewer than n snapshots available then it would also return log files. > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe > Attachments: ZOOKEEPER-2420.patch, ZOOKEEPER-2420.patch_v2 > > > Autopurge retains all log files whose zxid are >= the zxid of the oldest > snapshot file that it is going to retain (in PurgeTxnLog > retainNRecentSnapshots()). Given that loading the database from > snapshots/logs will start with the log file _prior_ to the snapshot's zxid, > autopurge should retain the log file prior to the oldest retained snapshot as > well, unless it verifies that it contains no zxids beyond what the snapshot > contains. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ed Rowe updated ZOOKEEPER-2420: --- Attachment: ZOOKEEPER-2420.patch_v2 > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe > Attachments: ZOOKEEPER-2420.patch, ZOOKEEPER-2420.patch_v2 > > > Autopurge retains all log files whose zxid are >= the zxid of the oldest > snapshot file that it is going to retain (in PurgeTxnLog > retainNRecentSnapshots()). Given that loading the database from > snapshots/logs will start with the log file _prior_ to the snapshot's zxid, > autopurge should retain the log file prior to the oldest retained snapshot as > well, unless it verifies that it contains no zxids beyond what the snapshot > contains. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15290613#comment-15290613 ] Ed Rowe commented on ZOOKEEPER-2420: Thanks Michael. I'm looking into your suggestion. In the process I think I've discovered another bug. Stay tuned. > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe > Attachments: ZOOKEEPER-2420.patch > > > Autopurge retains all log files whose zxid are >= the zxid of the oldest > snapshot file that it is going to retain (in PurgeTxnLog > retainNRecentSnapshots()). Given that loading the database from > snapshots/logs will start with the log file _prior_ to the snapshot's zxid, > autopurge should retain the log file prior to the oldest retained snapshot as > well, unless it verifies that it contains no zxids beyond what the snapshot > contains. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15283034#comment-15283034 ] Ed Rowe commented on ZOOKEEPER-2420: I've posted a patch for review. Please have a look. > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe > Attachments: ZOOKEEPER-2420.patch > > > Autopurge retains all log files whose zxid are >= the zxid of the oldest > snapshot file that it is going to retain (in PurgeTxnLog > retainNRecentSnapshots()). Given that loading the database from > snapshots/logs will start with the log file _prior_ to the snapshot's zxid, > autopurge should retain the log file prior to the oldest retained snapshot as > well, unless it verifies that it contains no zxids beyond what the snapshot > contains. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ed Rowe updated ZOOKEEPER-2420: --- Attachment: ZOOKEEPER-2420.patch > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe > Attachments: ZOOKEEPER-2420.patch > > > Autopurge retains all log files whose zxid are >= the zxid of the oldest > snapshot file that it is going to retain (in PurgeTxnLog > retainNRecentSnapshots()). Given that loading the database from > snapshots/logs will start with the log file _prior_ to the snapshot's zxid, > autopurge should retain the log file prior to the oldest retained snapshot as > well, unless it verifies that it contains no zxids beyond what the snapshot > contains. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15270317#comment-15270317 ] Ed Rowe commented on ZOOKEEPER-2420: Sure. I won't be able to get to it for a few days but I can put something together. > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe > > Autopurge retains all log files whose zxid are >= the zxid of the oldest > snapshot file that it is going to retain (in PurgeTxnLog > retainNRecentSnapshots()). Given that loading the database from > snapshots/logs will start with the log file _prior_ to the snapshot's zxid, > autopurge should retain the log file prior to the oldest retained snapshot as > well, unless it verifies that it contains no zxids beyond what the snapshot > contains. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ed Rowe updated ZOOKEEPER-2420: --- Description: Autopurge retains all log files whose zxid are >= the zxid of the oldest snapshot file that it is going to retain (in PurgeTxnLog retainNRecentSnapshots()). Given that loading the database from snapshots/logs will start with the log file _prior_ to the snapshot's zxid, autopurge should retain the log file prior to the oldest retained snapshot as well, unless it verifies that it contains no zxids beyond what the snapshot contains. (was: Autopurge retains all log files whose zxid are >= the zxid of the oldest log file that it is going to retain (in PurgeTxnLog retainNRecentSnapshots()). Given that loading the database from snapshots/logs will start with the log file _prior_ to the snapshot's zxid, autopurge should retain the log file prior to the first snapshot as well unless it verifies that it contains no zxids beyond what the snapshot contains. ) > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe > > Autopurge retains all log files whose zxid are >= the zxid of the oldest > snapshot file that it is going to retain (in PurgeTxnLog > retainNRecentSnapshots()). Given that loading the database from > snapshots/logs will start with the log file _prior_ to the snapshot's zxid, > autopurge should retain the log file prior to the oldest retained snapshot as > well, unless it verifies that it contains no zxids beyond what the snapshot > contains. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ed Rowe updated ZOOKEEPER-2420: --- Description: Autopurge retains all log files whose zxid are >= the zxid of the oldest log file that it is going to retain (in PurgeTxnLog retainNRecentSnapshots()). Given that loading the database from snapshots/logs will start with the log file _prior_ to the snapshot's zxid, autopurge should retain the log file prior to the first snapshot as well unless it verifies that it contains no zxids beyond what the snapshot contains. (was: Autopurge retains all log files whose zxid are >= the zxid of the oldest log file that it is going to retain (in PurgeTxnLog retainNRecentSnapshots()). Given that loading the database from snapshots/logs will start with the log file _prior_ to the snapshot's zxid, autopurge should retain this log file as well unless it verifies that it contains no zxids beyond what the snapshot contains. ) > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe > > Autopurge retains all log files whose zxid are >= the zxid of the oldest log > file that it is going to retain (in PurgeTxnLog retainNRecentSnapshots()). > Given that loading the database from snapshots/logs will start with the log > file _prior_ to the snapshot's zxid, autopurge should retain the log file > prior to the first snapshot as well unless it verifies that it contains no > zxids beyond what the snapshot contains. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore may need it
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ed Rowe updated ZOOKEEPER-2420: --- Summary: Autopurge deletes log file prior to oldest retained snapshot even though restore may need it (was: Autopurge deletes log file prior to oldest retained snapshot even though restore would need it) > Autopurge deletes log file prior to oldest retained snapshot even though > restore may need it > > > Key: ZOOKEEPER-2420 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Ed Rowe > > Autopurge retains all log files whose zxid are >= the zxid of the oldest log > file that it is going to retain (in PurgeTxnLog retainNRecentSnapshots()). > Given that loading the database from snapshots/logs will start with the log > file _prior_ to the snapshot's zxid, autopurge should retain this log file as > well unless it verifies that it contains no zxids beyond what the snapshot > contains. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Created] (ZOOKEEPER-2420) Autopurge deletes log file prior to oldest retained snapshot even though restore would need it
Ed Rowe created ZOOKEEPER-2420: -- Summary: Autopurge deletes log file prior to oldest retained snapshot even though restore would need it Key: ZOOKEEPER-2420 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2420 Project: ZooKeeper Issue Type: Bug Components: server Reporter: Ed Rowe Autopurge retains all log files whose zxid are >= the zxid of the oldest log file that it is going to retain (in PurgeTxnLog retainNRecentSnapshots()). Given that loading the database from snapshots/logs will start with the log file _prior_ to the snapshot's zxid, autopurge should retain this log file as well unless it verifies that it contains no zxids beyond what the snapshot contains. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
