[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15301216#comment-15301216
 ] 

Michael Han commented on ZOOKEEPER-2420:
----------------------------------------

Hi [~EdRowe]. The v2 patch LGTM. A few more comments:

L184: Good finding and fix on the bug in findNRecentSnapshots. Do you think if 
it's easy to add a test that explicitly cover the bug you fixed?

L189: (nit) missing spaces around '=='. This is not introduced in your patch, 
but since you are here we may just fix it so it's consistent with rest of code 
base :)

L355-356 : Should we revise the comment by adding one case, so it looks like: 
"get the snapshot logs that are greater than, or equal to, or just before the 
given zxid' ?

Also FYI, after you uploaded a new patch, you can click the "Submit Patch" 
patch in the JIRA issue, this will change the JIRA status to "Patch Available" 
(some folks may monitoring that list to pick items to review), also it will 
trigger pre-commit builds that will apply your patch and run tests. 




> 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)

Reply via email to