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/28 18:15:26 UTC

[GitHub] zookeeper pull request #610: [ZOOKEEPER-3124] Remove special logic to handle...

GitHub user lvfangmin opened a pull request:

    https://github.com/apache/zookeeper/pull/610

    [ZOOKEEPER-3124] Remove special logic to handle cversion and pzxid in DataTree.processTxn

    There is special logic in the DataTree.processTxn to handle the NODEEXISTS when createNode, which is used to handle the cversion and pzxid not being updated due to fuzzy snapshot: 
    
    https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/DataTree.java#L962-L994. 
    
    But seems this is not a real issue, in the current code, when serializing a parent node, we'll lock on it, and take a children snapshot at that time. If the child added after the parent is serialized to disk, then it won't be written out, so we shouldn't hit the issue where the child is in the snapshot but parent cversion and pzxid is not changed.
    
    But maybe I'm missing something, there is not much discussion in the Jira, so create a PR to have more attention.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/lvfangmin/zookeeper ZOOKEEPER-3124

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/610.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 #610
    
----
commit 53ab8f9773af6b763d246cd13dcd3ddd221e48d8
Author: Fangmin Lyu <al...@...>
Date:   2018-08-28T18:08:28Z

    Pzxid inconsistent issue when replaying a txn for a deleted node

----


---

[GitHub] zookeeper pull request #610: [ZOOKEEPER-3124] Remove special logic to handle...

Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin closed the pull request at:

    https://github.com/apache/zookeeper/pull/610


---

[GitHub] zookeeper issue #610: [ZOOKEEPER-3124] Add the correct comment to show why w...

Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on the issue:

    https://github.com/apache/zookeeper/pull/610
  
    With #622, we may actually not need the setCVersionPzxid logic anymore, any opinions on this?


---

[GitHub] zookeeper issue #610: [ZOOKEEPER-3124] Remove special logic to handle cversi...

Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on the issue:

    https://github.com/apache/zookeeper/pull/610
  
    Seems there is a Hudson environment problem on Jenkins, it's complaining about java.lang.NoClassDefFoundError: hudson.model.Computer


---

[GitHub] zookeeper issue #610: [ZOOKEEPER-3124] Remove special logic to handle cversi...

Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on the issue:

    https://github.com/apache/zookeeper/pull/610
  
    @nkalmar the child node deletion problem is still there, which should be solved in the other PR I sent out recently: https://github.com/apache/zookeeper/pull/605. 
    
    When revisit that, I found this code is actually won't be hit on prod, that's why I opened the PR to remove it.



---

[GitHub] zookeeper issue #610: [ZOOKEEPER-3124] Remove special logic to handle cversi...

Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on the issue:

    https://github.com/apache/zookeeper/pull/610
  
    I forgot but actually I think I've considered the children of children case when I created this diff, it actually won't cause the cversion and pzxid mismatch issue.
    
    The tricky scenario here is that node is being deleted and recreated when taking fuzzy snapshot, the node is there and will be serialized, but the cversion and pzxid of parent may not be changed since it might already serialized.
    
    I'll use this JIRA to update the comment to make it clearer, so people don't get confused as well in the future.


---

[GitHub] zookeeper issue #610: [ZOOKEEPER-3124] Remove special logic to handle cversi...

Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on the issue:

    https://github.com/apache/zookeeper/pull/610
  
    After revisiting the logic, I found it's not possible that the direct children is being creating and serialized after the parent is serialized, but the children of children might be added during fuzzy snapshot, so we do need to handle this.


---

[GitHub] zookeeper issue #610: [ZOOKEEPER-3124] Remove special logic to handle cversi...

Posted by nkalmar <gi...@git.apache.org>.
Github user nkalmar commented on the issue:

    https://github.com/apache/zookeeper/pull/610
  
    Looks like the C test failed, can you please re-run if Jenkins throws the same error? (just add an empty --amend commit).


---

[GitHub] zookeeper issue #610: [ZOOKEEPER-3124] Remove special logic to handle cversi...

Posted by nkalmar <gi...@git.apache.org>.
Github user nkalmar commented on the issue:

    https://github.com/apache/zookeeper/pull/610
  
    Oh, nice, thanks @lvfangmin !
    I will take a look on both then.


---

[GitHub] zookeeper pull request #610: [ZOOKEEPER-3124] Remove special logic to handle...

Posted by lvfangmin <gi...@git.apache.org>.
GitHub user lvfangmin reopened a pull request:

    https://github.com/apache/zookeeper/pull/610

    [ZOOKEEPER-3124] Remove special logic to handle cversion and pzxid in DataTree.processTxn

    There is special logic in the DataTree.processTxn to handle the NODEEXISTS when createNode, which is used to handle the cversion and pzxid not being updated due to fuzzy snapshot: 
    
    https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/DataTree.java#L962-L994. 
    
    But seems this is not a real issue, in the current code, when serializing a parent node, we'll lock on it, and take a children snapshot at that time. If the child added after the parent is serialized to disk, then it won't be written out, so we shouldn't hit the issue where the child is in the snapshot but parent cversion and pzxid is not changed.
    
    But maybe I'm missing something, there is not much discussion in the Jira, so create a PR to have more attention.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/lvfangmin/zookeeper ZOOKEEPER-3124

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/610.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 #610
    
----
commit 53ab8f9773af6b763d246cd13dcd3ddd221e48d8
Author: Fangmin Lyu <al...@...>
Date:   2018-08-28T18:08:28Z

    Pzxid inconsistent issue when replaying a txn for a deleted node

commit c6901493ee3958b9e418d8854022b776ebd7df90
Author: Fangmin Lyu <al...@...>
Date:   2018-08-28T20:39:34Z

    Also remove the useless non-sense test in LoadFromLogNoServerTest

commit 93fa440f74e86f94657acd6d93905ff6763c1251
Author: Fangmin Lyu <al...@...>
Date:   2018-08-31T23:41:18Z

    trigger jenkins

----


---