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

Hadoop QA commented on ZOOKEEPER-1090:
--------------------------------------

+1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12486671/ZOOKEEPER-1090
  against trunk revision 1146961.

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 6 new or modified tests.

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 javac.  The applied patch does not increase the total number of javac 
compiler warnings.

    +1 findbugs.  The patch does not introduce any new Findbugs (version 1.3.9) 
warnings.

    +1 release audit.  The applied patch does not increase the total number of 
release audit warnings.

    +1 core tests.  The patch passed core unit tests.

    +1 contrib tests.  The patch passed contrib unit tests.

Test results: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/397//testReport/
Findbugs warnings: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/397//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/397//console

This message is automatically generated.

> Race condition while taking snapshot can lead to not restoring data tree 
> correctly
> ----------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-1090
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1090
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: server
>    Affects Versions: 3.3.3
>            Reporter: Vishal K
>            Assignee: Vishal K
>            Priority: Critical
>              Labels: persistence, server, snapshot
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-1090
>
>
> I think I have found a bug in the snapshot mechanism.
> The problem occurs because dt.lastProcessedZxid is not synchronized (or 
> rather set before the data tree is modified):
> FileTxnSnapLog:
> {code}
>     public void save(DataTree dataTree,
>             ConcurrentHashMap<Long, Integer> sessionsWithTimeouts)
>         throws IOException {
>         long lastZxid = dataTree.lastProcessedZxid;
>         LOG.info("Snapshotting: " + Long.toHexString(lastZxid));
>         File snapshot=new File(
>                 snapDir, Util.makeSnapshotName(lastZxid));
>         snapLog.serialize(dataTree, sessionsWithTimeouts, snapshot);   <=== 
> the Datatree may not have the modification for lastProcessedZxid
>     }
> {code}
> DataTree:
> {code}
>     public ProcessTxnResult processTxn(TxnHeader header, Record txn) {
>         ProcessTxnResult rc = new ProcessTxnResult();
>         String debug = "";
>         try {
>             rc.clientId = header.getClientId();
>             rc.cxid = header.getCxid();
>             rc.zxid = header.getZxid();
>             rc.type = header.getType();
>             rc.err = 0;
>             if (rc.zxid > lastProcessedZxid) {
>                 lastProcessedZxid = rc.zxid;
>             }
>             [...modify data tree...]           
>  }
> {code}
> The lastProcessedZxid must be set after the modification is done.
> As a result, if server crashes after taking the snapshot (and the snapshot 
> does not contain change corresponding to lastProcessedZxid) restore will not 
> restore the data tree correctly:
> {code}
> public long restore(DataTree dt, Map<Long, Integer> sessions,
>             PlayBackListener listener) throws IOException {
>         snapLog.deserialize(dt, sessions);
>         FileTxnLog txnLog = new FileTxnLog(dataDir);
>         TxnIterator itr = txnLog.read(dt.lastProcessedZxid+1); <=== Assumes 
> lastProcessedZxid is deserialized
>  }
> {code}
> I have had offline discussion with Ben and Camille on this. I will be posting 
> the discussion shortly.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to