[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers

2018-12-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/628 Merged into master, thanks @enixon! ---

[GitHub] zookeeper issue #703: [ZOOKEEPER-1818] Correctly handle potential inconsiste...

2018-12-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/703 Here is the PR for 3.5: #714, thanks @mkedwards. ---

[GitHub] zookeeper issue #714: Zookeeper 1818 (on branch-3.5)

2018-12-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/714 @mkedwards I just made a 3.5 patch as well and saw this when I tried to send out, thanks for porting this over, thee change seems identical with my 3.5 one except two comments change to follow

[GitHub] zookeeper issue #711: ZOOKEEPER-3177. Revert globalOutstandingLimit refactor...

2018-12-07 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/711 Sorry, just saw this, @anmolnar what's the findbugs issue caused by that refactor? ---

[GitHub] zookeeper issue #732: typo

2018-12-07 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/732 +1 Thanks @stanlyDoge for correcting this, can you create a JIRA to make it easier to track this: https://issues.apache.org/jira/projects/ZOOKEEPER/issues. ---

[GitHub] zookeeper pull request #628: ZOOKEEPER-3140: Allow Followers to host Observe...

2018-12-06 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/628#discussion_r239578960 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverMaster.java --- @@ -0,0 +1,514 @@ +/** + * Licensed

[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers

2018-12-06 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/628 Thanks @enixon for doing the update and rebase, went through this again, looks legit to me. I also compared with internal version and made sure this has included all the improvement and bug

[GitHub] zookeeper issue #703: [ZOOKEEPER-1818] Correctly handle potential inconsiste...

2018-12-06 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/703 Thanks @anmolnar, I'll send out the PR for 3.5. ---

[GitHub] zookeeper pull request #731: [ZOOKEEPER-3208] Remove the SSLTest.java.orig i...

2018-12-05 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/zookeeper/pull/731 [ZOOKEEPER-3208] Remove the SSLTest.java.orig introduced in ZOOKEEPER-3032 You can merge this pull request into a Git repository by running: $ git pull https://github.com/lvfangmin

[GitHub] zookeeper pull request #729: [ZOOKEEPER-3207] Removing the unexpected WatchM...

2018-12-04 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/zookeeper/pull/729 [ZOOKEEPER-3207] Removing the unexpected WatchManager.java introduced by mistake during maven migration You can merge this pull request into a Git repository by running: $ git pull

[GitHub] zookeeper pull request #684: ZOOKEEPER-3180: Add response cache to improve t...

2018-12-04 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/684#discussion_r238787735 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/DumbWatcher.java --- @@ -69,7 +69,7 @@ public void sendCloseSession

[GitHub] zookeeper pull request #684: ZOOKEEPER-3180: Add response cache to improve t...

2018-12-04 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/684#discussion_r238786172 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxn.java --- @@ -151,12 +148,17 @@ void sendBufferSync(ByteBuffer bb

[GitHub] zookeeper pull request #684: ZOOKEEPER-3180: Add response cache to improve t...

2018-12-04 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/684#discussion_r238794022 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/ResponseCache.java --- @@ -0,0 +1,84 @@ +/** + * Licensed to the Apache

[GitHub] zookeeper pull request #684: ZOOKEEPER-3180: Add response cache to improve t...

2018-12-04 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/684#discussion_r238788593 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/ResponseCache.java --- @@ -0,0 +1,84 @@ +/** + * Licensed to the Apache

[GitHub] zookeeper pull request #684: ZOOKEEPER-3180: Add response cache to improve t...

2018-12-04 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/684#discussion_r238788174 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxn.java --- @@ -235,10 +237,12 @@ void handleWrite(SelectionKey k

[GitHub] zookeeper pull request #684: ZOOKEEPER-3180: Add response cache to improve t...

2018-12-04 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/684#discussion_r238790553 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/ResponseCache.java --- @@ -0,0 +1,84 @@ +/** + * Licensed to the Apache

[GitHub] zookeeper pull request #684: ZOOKEEPER-3180: Add response cache to improve t...

2018-12-04 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/684#discussion_r238789004 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/ResponseCache.java --- @@ -0,0 +1,84 @@ +/** + * Licensed to the Apache

[GitHub] zookeeper issue #703: [ZOOKEEPER-1818] Correctly handle potential inconsiste...

2018-12-04 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/703 @hanm is the latest change looking good to you? ---

[GitHub] zookeeper pull request #703: [ZOOKEEPER-1818] Correctly handle potential inc...

2018-12-04 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/703#discussion_r238742520 --- Diff: zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/FLEOutOfElectionTest.java --- @@ -0,0 +1,136 @@ +/** + * Licensed

[GitHub] zookeeper pull request #703: [ZOOKEEPER-1818] Correctly handle potential inc...

2018-12-01 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/703#discussion_r238085247 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/FastLeaderElection.java --- @@ -1023,9 +1016,9 @@ else if (validVoter

[GitHub] zookeeper pull request #703: [ZOOKEEPER-1818] Correctly handle potential inc...

2018-12-01 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/703#discussion_r238085234 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java --- @@ -1989,6 +1989,38 @@ private boolean updateVote(long

[GitHub] zookeeper issue #726: ZOOKEEPER-3205: o.a.jute test cases

2018-11-28 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/726 Only a few nits, other LGTM. ---

[GitHub] zookeeper issue #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thread and...

2018-11-28 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/689 Merged, thanks @tumativ. Btw, what's your Jira user id, I tried to assign this JIRA to you but I cannot find you. ---

[GitHub] zookeeper pull request #722: [ZOOKEEPER-3203] Tracking the number of non vot...

2018-11-28 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/722#discussion_r23726 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LeaderBean.java --- @@ -51,6 +51,15 @@ public String followerInfo

[GitHub] zookeeper issue #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thread and...

2018-11-28 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/689 LGTM, +1 Thanks @tumativ for all your effort on this, I'll commit this today. ---

[GitHub] zookeeper issue #703: [ZOOKEEPER-1818] Correctly handle potential inconsiste...

2018-11-28 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/703 May need an extra +1 from another committer, @hanm do you have time to review this? ---

[GitHub] zookeeper pull request #722: [ZOOKEEPER-3203] Tracking the number of non vot...

2018-11-25 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/zookeeper/pull/722 [ZOOKEEPER-3203] Tracking the number of non voting followers in ZK You can merge this pull request into a Git repository by running: $ git pull https://github.com/lvfangmin/zookeeper

[GitHub] zookeeper pull request #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thr...

2018-11-25 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/689#discussion_r236098795 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatcherCleaner.java --- @@ -161,10 +163,10 @@ public void doWork() throws

[GitHub] zookeeper pull request #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thr...

2018-11-25 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/689#discussion_r236098818 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatcherCleaner.java --- @@ -102,12 +103,13 @@ public void addDeadWatcher

[GitHub] zookeeper issue #703: [ZOOKEEPER-1818] Correctly handle potential inconsiste...

2018-11-25 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/703 @anmolnar following are my understanding about the acceptedEpoch, currentEpoch and electionEpoch: * acceptedEpoch : the previous epoch we accepted so far, usually is the epoch

[GitHub] zookeeper issue #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thread and...

2018-11-24 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/689 @tumativ thanks for working on this, only a minor comment now, I'll merge this when you updated this. ---

[GitHub] zookeeper pull request #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thr...

2018-11-24 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/689#discussion_r236063171 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatcherCleaner.java --- @@ -102,12 +103,13 @@ public void addDeadWatcher

[GitHub] zookeeper pull request #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thr...

2018-11-24 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/689#discussion_r236063217 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatcherCleaner.java --- @@ -102,24 +104,24 @@ public void addDeadWatcher

[GitHub] zookeeper pull request #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thr...

2018-11-24 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/689#discussion_r236063177 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatcherCleaner.java --- @@ -163,8 +165,8 @@ public void doWork() throws

[GitHub] zookeeper pull request #692: ZOOKEEPER-3184: Use the same method to generate...

2018-11-24 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/692#discussion_r236063130 --- Diff: README.md --- @@ -1,49 +1,24 @@ ## Generating the static Apache ZooKeeper website -In this directory you will find text files

[GitHub] zookeeper issue #703: [ZOOKEEPER-1818] Correctly handle potential inconsiste...

2018-11-18 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/703 @anmolnar this is the fix for the DONTCARE on trunk, please take a look, I'll port it to 3.5 when it's being reviewed and merged. ---

[GitHub] zookeeper pull request #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thr...

2018-11-18 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/689#discussion_r234494972 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatcherCleaner.java --- @@ -102,24 +104,24 @@ public void addDeadWatcher

[GitHub] zookeeper issue #632: [ZOOKEEPER-3150] Add tree digest check and verify data...

2018-11-17 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/632 Rebase onto latest master. ---

[GitHub] zookeeper issue #673: [ZOOKEEPER-3177] Refactor request throttle logic in NI...

2018-11-16 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/673 This seems able to be merged now, @anmolnar can you help merge it? ---

[GitHub] zookeeper issue #676: ZOOKEEPER-3181: ZOOKEEPER-2355 broke Curator TestingQu...

2018-11-16 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/676 @Randgalt I'm not familiar with the Curator release here, do you think it's easy to make a new release for 2.x for this? ---

[GitHub] zookeeper pull request #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thr...

2018-11-16 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/689#discussion_r234399372 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatcherCleaner.java --- @@ -50,6 +50,8 @@ private volatile

[GitHub] zookeeper pull request #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thr...

2018-11-16 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/689#discussion_r234399479 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatcherCleaner.java --- @@ -177,6 +180,7 @@ public void shutdown

[GitHub] zookeeper pull request #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thr...

2018-11-16 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/689#discussion_r234399392 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatcherCleaner.java --- @@ -102,24 +104,25 @@ public void addDeadWatcher

[GitHub] zookeeper issue #690: ZOOKEEPER-3179: Add snapshot compression to reduce the...

2018-11-16 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/690 retest this please ---

[GitHub] zookeeper pull request #692: ZOOKEEPER-3184: Use the same method to generate...

2018-11-16 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/692#discussion_r234399026 --- Diff: README.md --- @@ -1,49 +1,24 @@ ## Generating the static Apache ZooKeeper website -In this directory you will find text files

[GitHub] zookeeper pull request #703: [ZOOKEEPER-1818] Correctly handle potential inc...

2018-11-16 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/zookeeper/pull/703 [ZOOKEEPER-1818] Correctly handle potential inconsistent zxid/electionEpoch and peerEpoch during leader election This is similar to the 3.4 implementation in Jira ZOOKEEPER-1817, main

[GitHub] zookeeper issue #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thread and...

2018-11-14 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/689 @anmolnar that's a good point, I think the reason we didn't use ZooKeeperThread or ZooKeeperCriticalThread is because we don't expect to exit abnormally from the WatcherCleaner thread, which

[GitHub] zookeeper pull request #701: [ZOOKEEPER-3125] Only patching the pzxid when i...

2018-11-14 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/zookeeper/pull/701 [ZOOKEEPER-3125] Only patching the pzxid when it's larger than the current pzxid This previous fix in #605 has a corner case which might revert the pzxid, it's being fixed when port to 3.5

[GitHub] zookeeper issue #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thread and...

2018-11-13 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/689 @tumativ There is a very short window that we'll still add the dead watcher to the cleaner thread, I don't expect that will add too much GC overhead for these small amount of dead watch bits

[GitHub] zookeeper pull request #647: [ZOOKEEPER-3125] Fixing pzxid consistent issue ...

2018-11-11 Thread lvfangmin
Github user lvfangmin closed the pull request at: https://github.com/apache/zookeeper/pull/647 ---

[GitHub] zookeeper issue #647: [ZOOKEEPER-3125] Fixing pzxid consistent issue when re...

2018-11-11 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/647 Merged, close this PR. ---

[GitHub] zookeeper issue #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thread and...

2018-11-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/689 @tumativ I understand the intention of this JIRA, but to me looks like adding this.interrupt() in shutdown() should solve those problems. We may add a dead watch to the cleaner after

[GitHub] zookeeper issue #689: ZOOKEEPER-3183:Notifying the WatcherCleaner thread and...

2018-11-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/689 Thanks @tumativ, I'm reviewing this now. ---

[GitHub] zookeeper issue #647: [ZOOKEEPER-3125] Fixing pzxid consistent issue when re...

2018-11-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/647 @anmolnar should we get this in? ---

[GitHub] zookeeper issue #697: ZOOKEEPER-3155: Remove Forrest XMLs and their build pr...

2018-11-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/697 retest this please ---

[GitHub] zookeeper issue #698: ZOOKEEPER-3155: Remove Forrest XMLs and their build pr...

2018-11-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/698 Do we need this on 3.4, I thought we only do bug fixes on 3.4. ---

[GitHub] zookeeper issue #685: [ZOOKEEPER-3104] Fix potential data inconsistency due ...

2018-11-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/685 Thanks Andor, I'll close this one. ---

[GitHub] zookeeper pull request #685: [ZOOKEEPER-3104] Fix potential data inconsisten...

2018-11-08 Thread lvfangmin
Github user lvfangmin closed the pull request at: https://github.com/apache/zookeeper/pull/685 ---

[GitHub] zookeeper issue #676: ZOOKEEPER-3181: ZOOKEEPER-2355 broke Curator TestingQu...

2018-11-04 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/676 @eolivelli I agree the naming is a bit confusing here, in this case we should rename the Jira and PR to make it more explicitly, we can change the name here, but it doesn't make sense to rename

[GitHub] zookeeper issue #682: ZOOKEEPER-2807. Flaky test: org.apache.zookeeper.test....

2018-11-04 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/682 Thanks @anmolnar, I've merged this onto master. ---

[GitHub] zookeeper pull request #685: [ZOOKEEPER-3104] Fix potential data inconsisten...

2018-11-02 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/zookeeper/pull/685 [ZOOKEEPER-3104] Fix potential data inconsistency due to NEWLEADER packet being sent too early during SNAP sync Port this fix from master to 3.5. You can merge this pull request into a Git

[GitHub] zookeeper pull request #684: ZOOKEEPER-3180: Add response cache to improve t...

2018-10-31 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/684#discussion_r229870463 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxn.java --- @@ -68,29 +70,74 @@ private volatile boolean

[GitHub] zookeeper pull request #684: ZOOKEEPER-3180: Add response cache to improve t...

2018-10-31 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/684#discussion_r229871456 --- Diff: zookeeper-server/src/test/java/org/apache/zookeeper/test/ResponseCacheTest.java --- @@ -0,0 +1,118 @@ +/** + * Licensed

[GitHub] zookeeper pull request #682: ZOOKEEPER-2807. Flaky test: org.apache.zookeepe...

2018-10-25 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/682#discussion_r228315225 --- Diff: zookeeper-server/src/test/java/org/apache/zookeeper/test/WatchEventWhenAutoResetTest.java --- @@ -95,6 +96,7 @@ public void setUp

[GitHub] zookeeper issue #629: ZOOKEEPER-2641:AvgRequestLatency metric improves to be...

2018-10-25 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/629 @maoling if I remember correctly, the 3.4 branch is the stable branch, which only accept bug fix, new features or improvement will only be applied onto 3.5 and 3.6. I think

[GitHub] zookeeper issue #676: ZOOKEEPER-3181: ZOOKEEPER-2355 broke Curator TestingQu...

2018-10-25 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/676 -1 Wait for confirming why we have to do this. ---

[GitHub] zookeeper issue #676: ZOOKEEPER-3181: ZOOKEEPER-2355 broke Curator TestingQu...

2018-10-25 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/676 @aajisaka we shouldn't change the code because of a failed test, and this is only for a test case which has overriding this function name in Curator test. Isn't the right thing

[GitHub] zookeeper issue #652: ZOOKEEPER-3156: Add in option to canonicalize host nam...

2018-10-25 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/652 +1 The new change looks good to me, please rebase to resolve the conflict, will merge this in after that. Sorry for lately reply, somehow lost tracking this session. ---

[GitHub] zookeeper pull request #673: [ZOOKEEPER-3177] Refactor request throttle logi...

2018-10-24 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/673#discussion_r227962259 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java --- @@ -1107,6 +1102,19 @@ public void processPacket

[GitHub] zookeeper pull request #632: [ZOOKEEPER-3150] Add tree digest check and veri...

2018-10-23 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/632#discussion_r227533265 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/util/AdHash.java --- @@ -0,0 +1,84 @@ +/** + * Licensed to the Apache

[GitHub] zookeeper pull request #632: [ZOOKEEPER-3150] Add tree digest check and veri...

2018-10-23 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/632#discussion_r227532976 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/command/HashCommand.java --- @@ -0,0 +1,49 @@ +/** + * Licensed

[GitHub] zookeeper pull request #673: [ZOOKEEPER-3177] Refactor request throttle logi...

2018-10-23 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/673#discussion_r227532671 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java --- @@ -1107,6 +1102,19 @@ public void processPacket

[GitHub] zookeeper issue #665: [ZOOKEEPER-3163] Use session map in the Netty to impro...

2018-10-23 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/665 @maoling, we can port this back to 3.4 in the same Jira, I'll send out a PR separately for that. ---

[GitHub] zookeeper issue #665: [ZOOKEEPER-3163] Use session map in the Netty to impro...

2018-10-22 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/665 Yes, it's similar to ZOOKEEPER-1669, which uses sessionMap to reduce the cost of close session, most of the code are identical as well. In 3.4, it has the removeCnxn method defined

[GitHub] zookeeper issue #647: [ZOOKEEPER-3125] Fixing pzxid consistent issue when re...

2018-10-22 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/647 @anmolnar what's your opinion with @hanm 's reply? ---

[GitHub] zookeeper pull request #673: [ZOOKEEPER-3177] Refactor request throttle logi...

2018-10-18 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/zookeeper/pull/673 [ZOOKEEPER-3177] Refactor request throttle logic in NIO and Netty to keep the same behavior and make the code easier to maintain You can merge this pull request into a Git repository

[GitHub] zookeeper issue #647: [ZOOKEEPER-3125] Fixing pzxid consistent issue when re...

2018-10-18 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/647 @anmolnar I can understand your concern, let's remove the RetryRule for now, we can add it when necessary. ---

[GitHub] zookeeper issue #665: [ZOOKEEPER-3163] Use session map in the Netty to impro...

2018-10-16 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/665 retest this please ---

[GitHub] zookeeper issue #665: [ZOOKEEPER-3163] Use session map in the Netty to impro...

2018-10-16 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/665 The failure seems not related to this code change, I'll trigger another round of test, this should be ready to get in. ---

[GitHub] zookeeper issue #647: [ZOOKEEPER-3125] Fixing pzxid consistent issue when re...

2018-10-16 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/647 retest this please ---

[GitHub] zookeeper pull request #632: [ZOOKEEPER-3150] Add tree digest check and veri...

2018-10-12 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/632#discussion_r224842221 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java --- @@ -154,6 +160,26 @@ private final

[GitHub] zookeeper pull request #632: [ZOOKEEPER-3150] Add tree digest check and veri...

2018-10-12 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/632#discussion_r224843197 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java --- @@ -1521,4 +1566,179 @@ public boolean removeWatch(String path

[GitHub] zookeeper pull request #632: [ZOOKEEPER-3150] Add tree digest check and veri...

2018-10-12 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/632#discussion_r224844559 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/util/AdHash.java --- @@ -0,0 +1,84 @@ +/** + * Licensed to the Apache

[GitHub] zookeeper pull request #632: [ZOOKEEPER-3150] Add tree digest check and veri...

2018-10-12 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/632#discussion_r224841261 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/DataNode.java --- @@ -37,6 +37,14 @@ * */ public class DataNode

[GitHub] zookeeper pull request #632: [ZOOKEEPER-3150] Add tree digest check and veri...

2018-10-12 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/632#discussion_r224843869 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/NodeHashMapImpl.java --- @@ -0,0 +1,116 @@ +/** + * Licensed

[GitHub] zookeeper pull request #667: ZOOKEEPER-3158:firstConnect.countDown() will no...

2018-10-12 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/667#discussion_r224818567 --- Diff: zookeeper-common/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java --- @@ -146,13 +146,13 @@ public void operationComplete

[GitHub] zookeeper issue #624: ZOOKEEPER-3108:use a new property server.id in the zoo...

2018-10-09 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/624 Previously, it was not part of the dynamic config, because it’s in a separate config file. But after this change it will move to the dynamic config, if I remember correctly it check and move

[GitHub] zookeeper pull request #663: ZOOKEEPER-3154: Update release process to use t...

2018-10-09 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/663#discussion_r223838835 --- Diff: pom.xml --- @@ -30,7 +30,7 @@ org.apache.zookeeper zookeeper pom - 2.6.0-SNAPSHOT + 3.5.5-beta-SNAPSHOT

[GitHub] zookeeper pull request #665: [ZOOKEEPER-3163] Use session map in the Netty t...

2018-10-09 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/zookeeper/pull/665 [ZOOKEEPER-3163] Use session map in the Netty to improve close session performance This is a refactor to make the Netty able to use the same closeSession logic in NIOServerCnxn, which is more

[GitHub] zookeeper issue #658: ZOOKEEPER-3032 - MAVEN MIGRATION - branch 3.4 move jav...

2018-10-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/658 retest this please ---

[GitHub] zookeeper issue #660: ZOOKEEPER-2320. C-client crashes when removing watcher...

2018-10-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/660 @anmolnar do you know why it failing the check here? From the latest Jenkins build link I cannot find any failure. ---

[GitHub] zookeeper pull request #659: ZOOKEEPER-3161. Refactor QuorumPeerMainTest.jav...

2018-10-08 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/659#discussion_r223564880 --- Diff: zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java --- @@ -402,4 +421,129 @@ public File getConfFile

[GitHub] zookeeper issue #659: ZOOKEEPER-3161. Refactor QuorumPeerMainTest.java: move...

2018-10-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/659 retest this please ---

[GitHub] zookeeper issue #647: [ZOOKEEPER-3125] Fixing pzxid consistent issue when re...

2018-10-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/647 I remember I commented in Jira ZOOKEEPER-3157, not sure why it didn't show up. I mentioned that we still need RetryRule, because there might be temporary quorum unstable issues like

[GitHub] zookeeper issue #632: [ZOOKEEPER-3150] Add tree digest check and verify data...

2018-10-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/632 retest this please ---

[GitHub] zookeeper pull request #632: [ZOOKEEPER-3150] Add tree digest check and veri...

2018-10-08 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/632#discussion_r223538616 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java --- @@ -1521,4 +1562,179 @@ public boolean removeWatch(String path

[GitHub] zookeeper issue #632: [ZOOKEEPER-3150] Add tree digest check and verify data...

2018-10-06 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/632 This is a really useful feature, which helps us find multiple data inconsistent issues, like ZOOKEEPER-3144, ZOOKEEPER-3127, ZOOKEEPER-3125. It can avoid introducing new inconsistent

[GitHub] zookeeper pull request #650: ZOOKEEPER-1908: setAcl should be have a recursi...

2018-10-03 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/650#discussion_r222432633 --- Diff: src/java/main/org/apache/zookeeper/cli/SetAclCommand.java --- @@ -19,12 +19,17 @@ import java.util.List; import

[GitHub] zookeeper issue #650: ZOOKEEPER-1908: setAcl should be have a recursive func...

2018-10-02 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/650 retest this please ---

[GitHub] zookeeper pull request #652: ZOOKEEPER-3156: Add in option to canonicalize h...

2018-10-02 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/652#discussion_r221836418 --- Diff: src/java/main/org/apache/zookeeper/ClientCnxn.java --- @@ -793,7 +794,87 @@ public RWServerFoundException(String msg) { super

  1   2   3   4   >