[jira] Issue Comment Edited: (ZOOKEEPER-38) headers (version+) in log/snap files

["Mahadev konar (JIRA)" <ji...@apache.org>]

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

mahadev edited comment on ZOOKEEPER-38 at 9/25/08 2:00 PM:
-----------------------------------------------------------------

here is an updated patch. I cleared the patch with nice javadocs and more testing. Fixed a few bugs i found during the testing.     

  bq.  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
i am not sure abt this. did i remove something?
   
 bq. In FileSnapLog.deserialize() if the snapshot is bad, you should try an earlier snapshot. (We should add a test case for this)
 you are right... i would rather have it as another jira since this would be an improved over our earilier versions and would help getting this in without more delay :). 
    
bq. SnapLog should really be called Snapshot since it does not have a log component
done.

bq. 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.
done.
    
bq. 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.
done... thats is true... I will open another jira for this as well. 
   
 bq. 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
done.
  
 bq. TxnLogCompletion is not used and should be removed
done. 
  
bq. The Util class has some methods that are not used that should be removed.
done.


      was (Author: mahadev):
    here is an updated patch. I cleared the patch with nice javadocs and more testing. Fixed a few bugs i found during the testing.     

  bq.  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
i am not sure abt this. did i remove something?
   
 bq. In FileSnapLog.deserialize() if the snapshot is bad, you should try an earlier snapshot. (We should add a test case for this)
 you are right... i would rather have it as another jira since this would be an improved over our earilier versions and would help getting this in without more delay :). 
    
bq. SnapLog should really be called Snapshot since it does not have a log component
done.

bq. 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.
done.
    
bq. 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.
done... thats is true... I will open another jira for this as well. 
   
 bq. 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
done.
  
 bq. TxnLogCompletion is not used and should be removed
done. 
  
bq. The Util class has some methods that are not used that should be removed.
done.
[ Show » ]
Benjamin Reed - 24/Sep/08 12:50 PM 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 component
    * 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, ZOOKEEPER-38_8.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.