You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by lvfangmin <gi...@git.apache.org> on 2018/08/23 06:50:10 UTC
[GitHub] zookeeper pull request #606: [ZOOKEEPER-3127] Fixing potential data inconsis...
GitHub user lvfangmin opened a pull request:
https://github.com/apache/zookeeper/pull/606
[ZOOKEEPER-3127] Fixing potential data inconsistency due to update last processed zxid with partial multi-op txn
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/lvfangmin/zookeeper ZOOKEEPER-3127
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/606.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #606
----
commit 750e14f5edf6f0244662bb3224dcf8dfab2df91b
Author: Fangmin Lyu <al...@...>
Date: 2018-08-23T06:49:00Z
Fixing potential data inconsistency due to update last processed zxid with partial multi-op txn
----
---
[GitHub] zookeeper issue #606: [ZOOKEEPER-3127] Fixing potential data inconsistency d...
Posted by breed <gi...@git.apache.org>.
Github user breed commented on the issue:
https://github.com/apache/zookeeper/pull/606
+1 thank you @lvfangmin
---
[GitHub] zookeeper pull request #606: [ZOOKEEPER-3127] Fixing potential data inconsis...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zookeeper/pull/606
---
[GitHub] zookeeper issue #606: [ZOOKEEPER-3127] Fixing potential data inconsistency d...
Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on the issue:
https://github.com/apache/zookeeper/pull/606
The ReconfigRecoveryTest.testCurrentServersAreObserversInNextConfig is failing, I don't think it's related with this diff.
@breed I think this is ready to get in now.
---
[GitHub] zookeeper pull request #606: [ZOOKEEPER-3127] Fixing potential data inconsis...
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/606#discussion_r213189883
--- Diff: src/java/main/org/apache/zookeeper/server/DataTree.java ---
@@ -941,22 +945,41 @@ public ProcessTxnResult processTxn(TxnHeader header, Record txn)
LOG.debug("Failed: " + header + ":" + txn, e);
}
}
+
+
/*
- * A snapshot might be in progress while we are modifying the data
- * tree. If we set lastProcessedZxid prior to making corresponding
- * change to the tree, then the zxid associated with the snapshot
- * file will be ahead of its contents. Thus, while restoring from
- * the snapshot, the restore method will not apply the transaction
- * for zxid associated with the snapshot file, since the restore
- * method assumes that transaction to be present in the snapshot.
+ * Things we can only update after the whole txn is applied to data
+ * tree.
*
- * To avoid this, we first apply the transaction and then modify
- * lastProcessedZxid. During restore, we correctly handle the
- * case where the snapshot contains data ahead of the zxid associated
- * with the file.
+ * If we update the lastProcessedZxid with the first sub txn in multi
+ * and there is a snapshot in progress, it's possible that the zxid
+ * associated with the snapshot only include partial of the multi op.
+ *
+ * When loading snapshot, it will only load the txns after the zxid
+ * associated with snapshot file, which could data inconsistency due
--- End diff --
nit: which could **cause** data inconsistency
---
[GitHub] zookeeper issue #606: [ZOOKEEPER-3127] Fixing potential data inconsistency d...
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/606
committed to master @lvfangmin @breed
---
[GitHub] zookeeper issue #606: [ZOOKEEPER-3127] Fixing potential data inconsistency d...
Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on the issue:
https://github.com/apache/zookeeper/pull/606
Corrected the typo, and pushed to my branch, not sure why this is not got updated, maybe there is a delay.
---