Benjamin Reed commented on ZOOKEEPER-38:

This looks great Mahadev. Just  small comments:

- In ZooKeeperServerMain you changed from using the configuration methods to 
putting everything in the constructor. I think we should stick with the 
configuration methods. (This same comment applies to Leader and QuorumPeer
- In FileSnapLog.deserialize() if the snapshot is bad, you should try an 
earlier snapshot. (We should add a test case for this)
- SnapLog should really be called Snapshot since it does not have a log 
- In FileTxnLog.getLogFiles() you should add a comment to the first (fzxid > 
logZxid) check that you are taking advantage of the fact that the files are 
sorted, so the last time this condition is true will be for the log file that 
has snapshotZxid.
- In FileTxnLog.getLastLoggedZxid() the comment is a bit unclear. I think this 
method will also have a problem if the last logfile consists of a single 
corrupt entry.
- In FileTxnLog.truncate() you may need to delete files as well if the zxid you 
are truncing to is in a log file before the last file
- TxnLogCompletion is not used and should be removed
- The Util class has some methods that are not used that should be removed.

> headers (version+) in log/snap files
> ------------------------------------
>                 Key: ZOOKEEPER-38
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-38
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: server
>            Reporter: Patrick Hunt
>            Assignee: Mahadev konar
>             Fix For: 3.0.0
>         Attachments: ZOOKEEPER-38.patch, ZOOKEEPER-38.patch, 
> ZOOKEEPER-38.patch, ZOOKEEPER-38_1.patch, ZOOKEEPER-38_2.patch, 
> ZOOKEEPER-38_3.patch, ZOOKEEPER-38_5.patch, ZOOKEEPER-38_6.patch, 
> ZOOKEEPER-38_7.patch
> Moved from SourceForge to Apache.
> http://sourceforge.net/tracker/index.php?func=detail&aid=1961767&group_id=209147&atid=1008547

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

Reply via email to