[GitHub] zookeeper issue #513: ZOOKEEPER-2982: Re-try DNS hostname -> IP resolution i...

2018-05-08 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/513 The reviews and approvals for this change are in pull request #468 . The intention of this PR was to rebase it to master, while PR #468 targeted branch-3.5. ---

[GitHub] zookeeper issue #468: ZOOKEEPER-2982: Re-try DNS hostname -> IP resolution i...

2018-05-08 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/468 Merged to branch 3.5 as part of merging pull request #513, which targeted master. ---

[GitHub] zookeeper pull request #451: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2018-05-08 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/451#discussion_r186759876 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -111,9 +154,18 @@ public InetSocketAddress next(long spinDelay

[GitHub] zookeeper pull request #451: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2018-05-08 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/451#discussion_r186690327 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -111,9 +154,18 @@ public InetSocketAddress next(long spinDelay

[GitHub] zookeeper pull request #451: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2018-05-08 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/451#discussion_r186690242 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -111,9 +154,18 @@ public InetSocketAddress next(long spinDelay

[GitHub] zookeeper pull request #451: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2018-05-08 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/451#discussion_r186689865 --- Diff: src/java/main/org/apache/zookeeper/client/HostProvider.java --- @@ -53,7 +54,7 @@ * @param spinDelay *Milliseconds

[GitHub] zookeeper pull request #451: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2018-05-07 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/451#discussion_r186553219 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -111,9 +154,18 @@ public InetSocketAddress next(long spinDelay

[GitHub] zookeeper pull request #451: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2018-05-07 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/451#discussion_r186546855 --- Diff: src/java/main/org/apache/zookeeper/client/HostProvider.java --- @@ -53,7 +54,7 @@ * @param spinDelay *Milliseconds

[GitHub] zookeeper pull request #451: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2018-05-07 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/451#discussion_r186553004 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -111,9 +154,18 @@ public InetSocketAddress next(long spinDelay

[GitHub] zookeeper issue #456: ZOOKEEPER-2930: Leader cannot be elected due to networ...

2018-02-15 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/456 I have also tested this PR a bit locally. ---

[GitHub] zookeeper pull request #456: ZOOKEEPER-2930: Leader cannot be elected due to...

2018-02-15 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/456#discussion_r168620297 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java --- @@ -318,76 +318,167 @@ public Thread newThread(Runnable r

[GitHub] zookeeper pull request #167: ZOOKEEPER-2684 commitProcessor does not crash w...

2017-06-08 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/167#discussion_r121006265 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java --- @@ -254,24 +254,23 @@ public void run

[GitHub] zookeeper pull request #127: ZOOKEEPER-2646: Java target in branch 3.4 doesn...

2017-02-02 Thread fpj
Github user fpj closed the pull request at: https://github.com/apache/zookeeper/pull/127 --- 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

[GitHub] zookeeper issue #150: ZOOKEEPER-2184: Zookeeper Client should re-resolve hos...

2017-01-30 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/150 @hanm I believe we do have the same issue with the C client, I don't see it re-resolving addresses there, I need to have a closer look, though. --- If your project is set up for it, you can reply

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-29 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/150#discussion_r98385538 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -87,15 +75,104 @@ public StaticHostProvider(Collection serverAddresses

[GitHub] zookeeper issue #157: ZOOKEEPER-2678: Discovery and Sync can take a very lon...

2017-01-28 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/157 Thanks for the patch @revans2 . It makes sense to port this change to both 3.5 and master. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] zookeeper pull request #157: ZOOKEEPER-2678: Discovery and Sync can take a v...

2017-01-28 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/157#discussion_r98337403 --- Diff: src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java --- @@ -839,6 +839,13 @@ public void converseWithFollower(InputArchive ia

[GitHub] zookeeper pull request #157: ZOOKEEPER-2678: Discovery and Sync can take a v...

2017-01-28 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/157#discussion_r98336253 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/Learner.java --- @@ -364,10 +367,12 @@ else if (qp.getType() == Leader.SNAP

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-23 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/150#discussion_r97446760 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -87,15 +73,69 @@ public StaticHostProvider(Collection serverAddresses

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-23 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/150#discussion_r97437693 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -87,15 +86,69 @@ public StaticHostProvider(Collection serverAddresses

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-23 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/150#discussion_r97436370 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -87,15 +73,69 @@ public StaticHostProvider(Collection serverAddresses

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-23 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/150#discussion_r97436195 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -57,26 +62,20 @@ public StaticHostProvider(Collection

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-23 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/150#discussion_r97435427 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -57,26 +62,20 @@ public StaticHostProvider(Collection

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-18 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/150#discussion_r96642443 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -57,26 +62,20 @@ public StaticHostProvider(Collection

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-17 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/150#discussion_r96442215 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -87,15 +73,69 @@ public StaticHostProvider(Collection serverAddresses

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-17 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/150#discussion_r96439377 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -87,15 +73,69 @@ public StaticHostProvider(Collection serverAddresses

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-17 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/150#discussion_r96433850 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -87,15 +73,69 @@ public StaticHostProvider(Collection serverAddresses

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-17 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/150#discussion_r96431605 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -87,12 +73,69 @@ public StaticHostProvider(Collection serverAddresses

[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...

2017-01-17 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/122 @hanm @Randgalt sounds like a plan, let's follow the steps that Michael lined up above. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...

2017-01-16 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/122 @Randgalt >> when are we going to be removing these deprecated methods, in trunk > Maybe when we get a stable release of 3.5? Are you OK with us removing the deprecated met

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-14 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/150#discussion_r96125997 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -45,6 +45,9 @@ private int currentIndex = -1

[GitHub] zookeeper issue #150: ZOOKEEPER-2184: Zookeeper Client should re-resolve hos...

2017-01-14 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/150 The error is expected because we haven't setup QA to build out of the 3.4 branch. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] zookeeper pull request #150: ZOOKEEPER-2184: Zookeeper Client should re-reso...

2017-01-14 Thread fpj
GitHub user fpj opened a pull request: https://github.com/apache/zookeeper/pull/150 ZOOKEEPER-2184: Zookeeper Client should re-resolve hosts when connection attempts fail This is a version of the patch for ZK-2184 for the 3.4 branch, compatible with Java 6. You can merge

[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...

2017-01-11 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/122 Just so that I understand, when are we going to be removing this methods, in trunk? I'm asking this for two reasons: 1. So that users know in which version these methods are going away 2

[GitHub] zookeeper pull request #122: [ZOOKEEPER-2642] Resurrect the reconfig() metho...

2017-01-11 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/122#discussion_r95591971 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperReconfig.xml --- @@ -300,6 +300,11 @@ server.3=125.23.63.25:2782:2785:participant

[GitHub] zookeeper pull request #128: ZOOKEEPER-2647: Fix TestReconfigServer.cc

2016-12-16 Thread fpj
GitHub user fpj opened a pull request: https://github.com/apache/zookeeper/pull/128 ZOOKEEPER-2647: Fix TestReconfigServer.cc You can merge this pull request into a Git repository by running: $ git pull https://github.com/fpj/zookeeper ZK-2647 Alternatively you can review

[GitHub] zookeeper issue #101: ZOOKEEPER-2383

2016-12-16 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/101 This is including the commit I just did for ZOOKEEPER-761, could you rebase your branch, please, @rakeshadr ? --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] zookeeper issue #127: ZOOKEEPER-2646: Java target in branch 3.4 doesn't matc...

2016-12-15 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/127 The test failure is simply because the pr test script doesn't exist in the 3.4 branch. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] zookeeper pull request #127: ZOOKEEPER-2646: Java target in branch 3.4 doesn...

2016-12-15 Thread fpj
GitHub user fpj opened a pull request: https://github.com/apache/zookeeper/pull/127 ZOOKEEPER-2646: Java target in branch 3.4 doesn't match documentation You can merge this pull request into a Git repository by running: $ git pull https://github.com/fpj/zookeeper ZK-2646

[GitHub] zookeeper pull request #101: ZOOKEEPER-2383

2016-12-15 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/101#discussion_r92712695 --- Diff: src/java/test/org/apache/zookeeper/server/ZooKeeperServerStartupTest.java --- @@ -0,0 +1,266 @@ +/** + * Licensed to the Apache Software

[GitHub] zookeeper issue #90: ZOOKEEPER-761: Remove *synchronous* calls from the *sin...

2016-12-09 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/90 It sounds like there is a conflict in `TestReconfigServer`, could you resolve it, please @breed ? --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] zookeeper issue #109: Zookeeper 2542

2016-11-24 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/109 agreed, @rakeshadr 's branch seems to be out of sync. --- 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

[GitHub] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.

2016-11-23 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/102#discussion_r89283922 --- Diff: src/java/main/org/apache/zookeeper/server/command/FourLetterCommands.java --- @@ -151,9 +153,13 @@ */ public final static int

[GitHub] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.

2016-11-23 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/102#discussion_r89283699 --- Diff: src/java/main/org/apache/zookeeper/cli/DeleteCommand.java --- @@ -56,14 +56,7 @@ public CliCommand parse(String[] cmdArgs) throws CliParseException

[GitHub] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.

2016-11-23 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/102#discussion_r89285632 --- Diff: src/java/main/org/apache/jute/compiler/JRecord.java --- @@ -576,174 +580,174 @@ public void genCsharpCode(File outputDirectory) throws IOException

[GitHub] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.

2016-11-23 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/102#discussion_r89284431 --- Diff: src/java/test/config/findbugsExcludeFile.xml --- @@ -144,4 +144,10 @@ +

[GitHub] zookeeper issue #73: practise git

2016-11-20 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/73 @eribeiro I'm not sure I'm able to close it, is it only the creator who can close it? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] zookeeper issue #90: ZOOKEEPER-761: Remove *synchronous* calls from the *sin...

2016-11-17 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/90 It looks good, I just left a comment about creating a jira before we forget. Could you do it before we close this issue, please @breed ? --- If your project is set up for it, you can reply

[GitHub] zookeeper issue #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object into auth...

2016-11-14 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/84 @Randgalt thanks for the recent changes, they look fine for me, except that we didn't get QA to run on it. @lvfangmin there are a few levels of indirection here, which doesn't make it look

[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...

2016-11-14 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r87957495 --- Diff: src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java --- @@ -543,7 +546,7 @@ protected void pRequest2Txn(int type, long zxid, Request

[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...

2016-11-14 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r87957002 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml --- @@ -1193,6 +1193,55 @@ authProvider.2=com.f.MyAuth2 only one

[GitHub] zookeeper issue #98: ZOOKEEPER-2479

2016-11-14 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/98 +1, LGTM. @eribeiro is this ready according to you? --- 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

[GitHub] zookeeper pull request #104: ZOOKEEPER-2631: Make issue extraction in the gi...

2016-11-11 Thread fpj
GitHub user fpj opened a pull request: https://github.com/apache/zookeeper/pull/104 ZOOKEEPER-2631: Make issue extraction in the git pull request script more robust You can merge this pull request into a Git repository by running: $ git pull https://github.com/fpj/zookeeper

[GitHub] zookeeper pull request #96: ZOOKEEPER-2014: Only admin should be allowed to ...

2016-11-10 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/96#discussion_r87536973 --- Diff: src/java/test/org/apache/zookeeper/server/DataTreeTest.java --- @@ -200,29 +198,34 @@ public void testSerializeDoesntLockDataNodeWhileWriting

[GitHub] zookeeper issue #73: practise git

2016-11-10 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/73 @eribeiro yeah, this should be closed. --- 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

[GitHub] zookeeper issue #97: ZOOKEEPER-2624: Add test script for pull requests

2016-11-06 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/97 The failures are the same as before: `@author` string in the script and new findbugs warnings due to jenkins upgrade. --- If your project is set up for it, you can reply to this email and have your

[GitHub] zookeeper issue #97: ZOOKEEPER-2624: Add test script for pull requests

2016-11-06 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/97 @breed @rgs1 let's get this in, I'll create a jira to cleanup the script. Note that a lot of the issues you pointed out are legacy from the other script, so we will probably want to address in both

[GitHub] zookeeper issue #97: ZOOKEEPER-2624: Add test script for pull requests

2016-11-04 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/97 @hanm The `DEVELOPER` mode is for running locally, so initially I left it mostly unchanged and the original script takes a patch file to test. Because of your feedback, I thought that it might

[GitHub] zookeeper issue #97: ZOOKEEPER-2624: Add test script for pull requests

2016-11-03 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/97 The failure is actually expected because the script contains the `@author` string. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] zookeeper pull request #97: ZOOKEEPER-2624: Add test script for pull request...

2016-11-03 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/97#discussion_r86386241 --- Diff: src/java/test/bin/test-github-pr.sh --- @@ -0,0 +1,607 @@ +#!/usr/bin/env bash +# Licensed under the Apache License, Version 2.0

[GitHub] zookeeper pull request #97: ZOOKEEPER-2624: Add test script for pull request...

2016-11-03 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/97#discussion_r86386008 --- Diff: build.xml --- @@ -1622,6 +1623,26 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyl

[GitHub] zookeeper issue #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object into auth...

2016-11-02 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/84 @Randgalt I'm sorry that QA isn't working for pull requests yet, could please create a diff patch and upload to the jira so that we can get QA to run on it? --- If your project is set up for it, you

[GitHub] zookeeper issue #94: ZOOKEEPER-2014: Only admin should be allowed to reconfi...

2016-10-30 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/94 @hanm since there is no comment, would you mind closing this PR and resubmitting it to see if QA picks it up? --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...

2016-10-29 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r85642051 --- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java --- @@ -0,0 +1,131 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...

2016-10-29 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r85642282 --- Diff: src/java/main/org/apache/zookeeper/server/auth/ServerAuthenticationProvider.java --- @@ -0,0 +1,79 @@ +/** + * Licensed to the Apache

[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...

2016-10-29 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r85642202 --- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java --- @@ -0,0 +1,131 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] zookeeper issue #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object into auth...

2016-10-29 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/84 @Randgalt My comments are pretty minor, but it will give it a chance to QA to test this PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...

2016-10-29 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r85642270 --- Diff: src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java --- @@ -31,7 +31,15 @@ private static boolean initialized = false

[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...

2016-10-29 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r85641947 --- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml --- @@ -1193,6 +1193,55 @@ authProvider.2=com.f.MyAuth2 only one

[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...

2016-10-29 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r85642186 --- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java --- @@ -0,0 +1,131 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...

2016-10-29 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r85641968 --- Diff: src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java --- @@ -543,7 +546,7 @@ protected void pRequest2Txn(int type, long zxid, Request

[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...

2016-10-29 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r85641975 --- Diff: src/java/main/org/apache/zookeeper/server/ServerCnxn.java --- @@ -51,7 +47,7 @@ final public static Object me = new Object

[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...

2016-10-29 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r85642389 --- Diff: src/java/main/org/apache/zookeeper/server/auth/WrappedAuthenticationProvider.java --- @@ -0,0 +1,74 @@ +/** + * Licensed to the Apache

[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...

2016-10-29 Thread fpj
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r85642046 --- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java --- @@ -0,0 +1,131 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] zookeeper pull request #95: ZOOKEEPER-2622: ZooTrace.logQuorumPacket does no...

2016-10-28 Thread fpj
GitHub user fpj opened a pull request: https://github.com/apache/zookeeper/pull/95 ZOOKEEPER-2622: ZooTrace.logQuorumPacket does nothing DO NOT MERGE You can merge this pull request into a Git repository by running: $ git pull https://github.com/fpj/zookeeper ZK-2622

[GitHub] zookeeper issue #90: ZOOKEEPER-761: Remove *synchronous* calls from the *sin...

2016-10-27 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/90 @breed `make check` does not compile for me: ``` g++ -DHAVE_CONFIG_H -I. -I./include -I./tests -I./generated -DUSE_STATIC_LIB -DZKSERVER_CMD="\"./tests/zkServer.sh\""

Re: Review Request 51546: ZOOKEEPER-2014: Only admin should be allowed to reconfig a cluster

2016-10-02 Thread fpj
va/main/org/apache/zookeeper/admin/ZooKeeperAdmin.java (line 177) <https://reviews.apache.org/r/51546/#comment219292> Is access to ClientCnxn the main reason for extending `ZooKeeper`? - fpj On Sept. 29, 2016, 1:41 a.m., Michael Han wr

Re: Review Request 51546: ZOOKEEPER-2014: Only admin should be allowed to reconfig a cluster

2016-09-24 Thread fpj
ent218207> This isn't really a case of bad argument, it is more like an unauthorized operation. does it make sense to create a new keeper exception for this? src/java/test/org/apache/zookeeper/test/ReconfigExceptionTest.java (line 161) <https://reviews.apache.org/r/51546/#comment218

Re: Review Request 25160: Major throughput improvement with mixed workloads

2016-03-27 Thread fpj
mment188435> Long line. ./src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorConcurrencyTest.java (line 165) <https://reviews.apache.org/r/25160/#comment188434> Long line. - fpj On March 26, 2016, 7:06 p.m., Kfir Lev-Ari wrote: > >

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-13 Thread fpj
On March 12, 2015, 9:47 p.m., fpj wrote: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java, line 486 https://reviews.apache.org/r/31277/diff/16/?file=890432#file890432line486 I don't understand why we have this for loop here iterating over cnxns, why can't

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-12 Thread fpj
/31277/#comment123811 It might be a good idea to do a CLIENT_COUNT for clarity. src/java/test/org/apache/zookeeper/test/SSLTest.java https://reviews.apache.org/r/31277/#comment123812 Should we run an operation just to make sure the wiring on the server side is fine? - fpj On March

Re: Review Request 27244: ZOOKEEPER-2069

2014-12-12 Thread fpj
On Dec. 11, 2014, 11:16 p.m., fpj wrote: src/java/main/org/apache/zookeeper/ClientCnxn.java, line 1472 https://reviews.apache.org/r/27244/diff/28/?file=789547#file789547line1472 I'm now confused by this change. Are you getting rid of enableWrite? saslCompleted is fairly different

Re: Review Request 27244: ZOOKEEPER-2069

2014-12-12 Thread fpj
On Dec. 11, 2014, 11:16 p.m., fpj wrote: src/java/main/org/apache/zookeeper/ClientCnxn.java, line 134 https://reviews.apache.org/r/27244/diff/28/?file=789547#file789547line134 It sounds like making pendingQueue and outgoingQueue are optimizations that are independent from

Re: Review Request 27244: ZOOKEEPER-2069

2014-12-11 Thread fpj
sasl changes are necessary here so that we can proceed only if authentication suceeds. Please confirm. - fpj On Dec. 11, 2014, 6:11 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 27244: ZOOKEEPER-2069

2014-12-11 Thread fpj
Good catch. - fpj On Dec. 11, 2014, 6:11 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27244

Re: Review Request 27244: ZOOKEEPER-2069

2014-12-11 Thread fpj
On Dec. 11, 2014, 10:53 p.m., fpj wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 70 https://reviews.apache.org/r/27244/diff/28/?file=789550#file789550line70 I see, the previous sasl changes are necessary here so that we can proceed only

Re: Review Request 27244: ZOOKEEPER-2069

2014-12-04 Thread fpj
On Dec. 2, 2014, 11:43 p.m., fpj wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 66 https://reviews.apache.org/r/27244/diff/22/?file=774526#file774526line66 Is the reason to have workSemaphore to block in doTransport when there isn't anything

Re: Review Request 27244: ZOOKEEPER-2069

2014-12-04 Thread fpj
On Dec. 2, 2014, 11:43 p.m., fpj wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 188 https://reviews.apache.org/r/27244/diff/22/?file=774526#file774526line188 Could you elaborate on what you're trying to do with this NIOLock? I'm not sure what you mean

Re: Review Request 27244: ZOOKEEPER-2069

2014-12-02 Thread fpj
to do with this NIOLock? I'm not sure what you mean with race cocnditions in the comment. - fpj On Nov. 24, 2014, 7:01 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

Re: Review Request 28119: Documentation: The bookkeeper protocol

2014-11-19 Thread fpj
? - fpj On Nov. 17, 2014, 2:58 p.m., Ivan Kelly wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28119/ --- (Updated Nov. 17

Re: Review Request 27529: BOOKKEEPER-795 Race condition causes writes to hang if ledger is fences

2014-11-14 Thread fpj
On Nov. 12, 2014, 4:33 p.m., fpj wrote: I quite like the changes here, they make sense to me. I just have one clarification question and a small suggestion below. I'm happy with the changes. Any other comment here or should I get this in? On Nov. 12, 2014, 4:33 p.m., fpj wrote

Re: Review Request 27244: ZOOKEEPER-2069

2014-11-01 Thread fpj
this could happen, is this a potential race or just a sanity check? - fpj On Oct. 31, 2014, 10:57 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27244

RE: ZooKeeper 3.5.0-alpha planning

2014-07-21 Thread FPJ
+1 for having an RC this week. Since this is an alpha release, I think 72 biz hours is enough for the vote. -Flavio -Original Message- From: Patrick Hunt [mailto:ph...@apache.org] Sent: 21 July 2014 18:55 To: DevZooKeeper Subject: Re: ZooKeeper 3.5.0-alpha planning I fixed a

svn upgrade issue

2014-07-11 Thread FPJ
The precommit build is getting this error: [exec] svn: E155036: Please see the 'svn upgrade' command [exec] svn: E155036: The working copy at '/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk' [exec] is too old (format 10) to work with client version

RE: c client test output seems to be broken for trunk

2014-07-08 Thread FPJ
Yeah, I was pretty much sure we had a jira for this latest failure, but I couldn't find it... -Flavio -Original Message- From: Patrick Hunt [mailto:ph...@apache.org] Sent: 08 July 2014 17:00 To: DevZooKeeper Subject: Re: c client test output seems to be broken for trunk The

Re: Review Request 23081: ZOOKEEPER-827. enable r/o mode in C client library

2014-06-29 Thread fpj
if not necessary. - fpj On June 27, 2014, 6:45 p.m., Raul Gutierrez Segales wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23081

Re: Review Request 23081: ZOOKEEPER-827. enable r/o mode in C client library

2014-06-27 Thread fpj
spaces. src/c/tests/zkServer.sh https://reviews.apache.org/r/23081/#comment82449 why are you calling startPeer the mode with ro enabled? startReadOnly seems more clear, no? - fpj On June 26, 2014, 7:09 p.m., Raul Gutierrez Segales wrote

RE: intern project idea: decouple zab from zookeeper

2014-06-02 Thread FPJ
I have a few reasons for suggesting a separate project: - I don't see a reason for tying the releases of an independent implementation of Zab to ZooKeeper - The set of developers (and committers) interested in an independent implementation of Zab might be different compared to ZooKeeper; it

ApacheCon Europe

2014-05-28 Thread FPJ
If anyone here is interested in talking about ZK at ApacheCon, please let me know: http://events.linuxfoundation.org//events/apachecon-europe/program/cfp http://events.linuxfoundation.org/events/apachecon-europe/program/cfp It would be great to have a good presence there! -Flavio

RE: Reviews over the next few weeks

2014-04-23 Thread FPJ
Aiming for tomorrow sounds like a stretch, but I'll carve out some time to look at patches. There are 8 patch avaialables. -Flavio -Original Message- From: Ivan Kelly [mailto:iv...@apache.org] Sent: 23 April 2014 11:41 To: bookkeeper-dev@zookeeper.apache.org Subject: Reviews over

  1   2   >