[GitHub] zookeeper pull request #333: ZOOKEEPER-2872: Interrupted snapshot sync cause...

[asfgit]

Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #333: ZOOKEEPER-2872: Interrupted snapshot sync cause...

[hanm]

Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/333#discussion_r132832602
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/Learner.java ---
@@ -364,6 +364,7 @@ protected void syncWithLeader(long newLeaderZxid) 
throws Exception{
 readPacket(qp);
 LinkedList packetsCommitted = new LinkedList();
 LinkedList packetsNotCommitted = new 
LinkedList();
+boolean syncSnapshot = false;
--- End diff --

Another possibility as I just commented is to get rid of this variable and 
always Fsync snapshot serialization.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #333: ZOOKEEPER-2872: Interrupted snapshot sync cause...

[hanm]

Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/333#discussion_r132832594
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/Learner.java ---
@@ -364,6 +364,7 @@ protected void syncWithLeader(long newLeaderZxid) 
throws Exception{
 readPacket(qp);
 LinkedList packetsCommitted = new LinkedList();
 LinkedList packetsNotCommitted = new 
LinkedList();
+boolean syncSnapshot = false;
--- End diff --

We can level this variable definition up so it's clustered with 
`snapshotNeed` boolean.

Another possibility is to get ride of this variable and use existing 
`snapshotNeeded` - but that will do fysnc snapshot for TRUNC sync, which the 
existing patch will not. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #333: ZOOKEEPER-2872: Interrupted snapshot sync cause...

[enixon]

GitHub user enixon opened a pull request:

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

ZOOKEEPER-2872: Interrupted snapshot sync causes data loss



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

$ git pull https://github.com/enixon/zookeeper snap-sync

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

https://github.com/apache/zookeeper/pull/333.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 #333


commit 39bd1a3eb9171a014845fff97648341cbfb40a11
Author: Brian Nixon 
Date:   2017-08-01T20:25:51Z

ZOOKEEPER-2872: Interrupted snapshot sync causes data loss




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---