[
https://issues.apache.org/jira/browse/ZOOKEEPER-1090?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13071458#comment-13071458
]
Camille Fournier commented on ZOOKEEPER-1090:
---------------------------------------------
bq. I don't have a test that that will fail without the patch.
Ok, I see. So the test verifies that a snapshot with a later zxid than the
named zxid processes correctly, and it does. That's good.
I think this is all fine. The only comment I would make is that for future
reference, you should use Assert.assertEquals instead of Assert.assertTrue when
asserting equality. The Assert will take care of the work of printing out
"expected foo but was bar" for you in that case, so you don't need it in the
message of the Assert statement. Just a bit easier for you to write and others
to read.
bq. After going through the code again, I think the following hypothetical
failure scenario could be a problem: For some reason, modification to
data tree fails for transactions x to x-n on one of the peer. The
lastProcessedZxid will still be set to x. If the peer restores after
snapshot, then it will not have transactions x-n to x in its data
tree. Now, if the modification had failed due to transient errors (say
out of memory), then we will not apply transactions x-n to x even if
we should have. The only other reason we could run into this scenario
(apart from runtime failures) is if we had a bug in DataTree
modification code path. This looks like a rare corner case.
Hmm. So if we had a bug in DataTree, we could lose transactions, yes? Well, I
think it's fair to say that in general, if we have bugs we might lose data.
Probably worth keeping in mind but not losing sleep over at this time.
> 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
