[GitHub] zookeeper issue #441: ZOOKEEPER-2960. The dataDir and dataLogDir are used op...

2018-01-07 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/441 +1 gonna attempt to commit this ---

[GitHub] zookeeper issue #307: ZOOKEEPER-2770 ZooKeeper slow operation log

2017-11-21 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/307 I appreciate the work that you are trying to do here, and this still feels like an incomplete approach to a problem that would be worth investing further into. It seems like we all agree

[GitHub] zookeeper pull request #136: [ZOOKEEPER-1416] Persistent Recursive Watch

2017-09-01 Thread skamille
Github user skamille commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/136#discussion_r136637514 --- Diff: src/java/main/org/apache/zookeeper/server/WatchManager.java --- @@ -40,50 +40,110 @@ class WatchManager { private static final

[GitHub] zookeeper issue #336: ZOOKEEPER-2836: fix SocketTimeoutException

2017-08-23 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/336 @bitgaoshu if you could make this patch apply to the 3.5 branch I will commit it there as well. Unfortunately that branch has a significant divergence from master and I suspect we want this fix

[GitHub] zookeeper issue #336: ZOOKEEPER-2836: fix SocketTimeoutException

2017-08-22 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/336 +1 merging --- 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

[GitHub] zookeeper pull request #336: ZOOKEEPER-2836: fix SocketTimeoutException

2017-08-18 Thread skamille
Github user skamille commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/336#discussion_r133994057 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java --- @@ -638,13 +639,22 @@ public void run

[GitHub] zookeeper pull request #336: ZOOKEEPER-2836: fix SocketTimeoutException

2017-08-18 Thread skamille
Github user skamille commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/336#discussion_r133989279 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java --- @@ -638,13 +639,22 @@ public void run

[GitHub] zookeeper issue #136: [ZOOKEEPER-1416] Persistent Recursive Watch

2017-08-18 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/136 > Per 3. This PR does not guarantee that you will see all events. I'll double check the doc to make sure that that's clear. These watches behave exactly as other watches in ZK other t

[GitHub] zookeeper issue #136: [ZOOKEEPER-1416] Persistent Recursive Watch

2017-08-18 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/136 I want to make it clear that I'm not arguing against the feature, I just want us to make sure that we're implementing it in a way that makes sense absent the TreeSet use case, and that we

[GitHub] zookeeper issue #136: [ZOOKEEPER-1416] Persistent Recursive Watch

2017-08-17 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/136 We have to remember that people who don't use TreeCache will still use this feature. Not to say that we shouldn't keep it in mind as an important user, but presumably people who don't actually

[GitHub] zookeeper issue #136: [ZOOKEEPER-1416] Persistent Recursive Watch

2017-08-16 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/136 Questions I have about this from a high level design perspective: 1. As I asked on the mailing list, have we done load/performance testing or addressed what that might look like in the design

[GitHub] zookeeper pull request #307: ZOOKEEPER-2770 ZooKeeper slow operation log

2017-07-25 Thread skamille
Github user skamille commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/307#discussion_r129444784 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java --- @@ -61,6 +61,7 @@ private static boolean

[GitHub] zookeeper pull request #307: ZOOKEEPER-2770 ZooKeeper slow operation log

2017-07-25 Thread skamille
Github user skamille commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/307#discussion_r129424302 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java --- @@ -61,6 +61,7 @@ private static boolean

[GitHub] zookeeper issue #191: ZOOKEEPER-2722: fix flaky testSessionEstablishment tes...

2017-03-16 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/191 I think your idea to create a connector that waits for specific types of connection (RO or R/W) is a good idea to try to fix this issue. --- If your project is set up for it, you can reply

[GitHub] zookeeper issue #191: ZOOKEEPER-2722: fix flaky testSessionEstablishment tes...

2017-03-16 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/191 Are we running multiple tests in parallel on Jenkins? I think the logs support this thesis but they seem to have many different tests logging into the same lines as the testSessionEstablishment

[GitHub] zookeeper issue #191: ZOOKEEPER-2722: fix flaky testSessionEstablishment tes...

2017-03-16 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/191 Is it possible that what happens is the client connects again in read-only mode, and then is bumped when quorum happens to be achieved? It seems like that shouldn't happen but it seems like

[GitHub] zookeeper issue #191: ZOOKEEPER-2722: fix flaky testSessionEstablishment tes...

2017-03-16 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/191 FWIW I can repro this on my desktop so let me see if I can figure out what's happening --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] zookeeper issue #191: ZOOKEEPER-2722: fix flaky testSessionEstablishment tes...

2017-03-16 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/191 > ConnectionLossException can happen after a connection between ZooKeeper client and server has been established, right? So having the check only in watcher is not enough. A pass in watcher d

[GitHub] zookeeper issue #191: ZOOKEEPER-2722: fix flaky testSessionEstablishment tes...

2017-03-16 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/191 Basically, I think it's worth understanding why we are getting a connection event in the watcher that should be waiting for connection, but still failing by not connecting, instead of fixing

[GitHub] zookeeper issue #191: ZOOKEEPER-2722: fix flaky testSessionEstablishment tes...

2017-03-16 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/191 Right but my point is, if that creation is failing due to connection loss, shouldn't the places that check the watcher connection fail there instead of in your check? The first place you added

[GitHub] zookeeper issue #194: ZOOKEEPER-2724: Skip cert files for releaseaudit targe...

2017-03-16 Thread skamille
Github user skamille commented on the issue: https://github.com/apache/zookeeper/pull/194 The lack of this appears to be breaking builds so this looks good to me. I will commit. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub