[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 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 #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 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_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-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-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_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_r186759876 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -111,9 +154,18 @@ public InetSocketAddress next(long spinDelay

[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 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 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

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

2016-09-24 Thread fpj
views.apache.org/r/51546/#comment218207> 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://rev

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

2016-10-02 Thread fpj
paces. src/java/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

[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\""

[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 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 #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 will be

[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_r85642202 --- 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_r85641975 --- Diff: src/java/main/org/apache/zookeeper/server/ServerCnxn.java --- @@ -51,7 +47,7 @@ final public static Object me = new Object

[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 as

[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_r85642051 --- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java --- @@ -0,0 +1,131 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[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 #97: ZOOKEEPER-2624: Add test script for pull request...

2016-10-31 Thread fpj
GitHub user fpj opened a pull request: https://github.com/apache/zookeeper/pull/97 ZOOKEEPER-2624: Add test script for pull requests You can merge this pull request into a Git repository by running: $ git pull https://github.com/fpj/zookeeper ZK-2624 Alternatively you can

[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 f

[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 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 (the

[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. If

[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 be

[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 addre

[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 #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 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 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 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 this

[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 will be

[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 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 mak

[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 to this

[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 GitH

[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 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_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_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_r89264869 --- Diff: src/java/main/org/apache/jute/compiler/JRecord.java --- @@ -403,168 +410,165 @@ public void genJavaCode(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_r89264584 --- Diff: src/java/main/org/apache/jute/compiler/JRecord.java --- @@ -403,168 +410,165 @@ public void genJavaCode(File outputDirectory) throws IOException

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

2016-11-23 Thread fpj
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/102 @hanm I've left a few comments, but in general looks pretty good. I noticed that there are also unaddressed comments from @lvfangmin and @eribeiro . Could you take care of those so that we can

[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

[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 on

[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 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

[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

[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 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

Review Request: ZOOKEEPER-465: Ledger size in bytes

2011-01-07 Thread fpj
/bookkeeper/test/org/apache/bookkeeper/test/BookieReadWriteTest.java 1055984 trunk/src/contrib/bookkeeper/test/org/apache/bookkeeper/test/LedgerRecoveryTest.java 1055984 Diff: https://reviews.apache.org/r/234/diff Testing --- Thanks, fpj

Re: Review Request: ZOOKEEPER-465: Ledger size in bytes

2011-01-07 Thread fpj
://reviews.apache.org/r/234/diff Testing --- Thanks, fpj

Re: Review Request: Move blocking read/write calls to SendWorker and RecvWorker Threads

2011-01-09 Thread fpj
java/test/org/apache/zookeeper/test/CnxManagerTest.java <https://reviews.apache.org/r/240/#comment179> If this is commented out, we should probably remove these lines from the patch. - fpj On 2011-01-07 02:21:38, Vishal Kher wrote: > > ---

Re: Review Request: Move blocking read/write calls to SendWorker and RecvWorker Threads

2011-01-18 Thread fpj
> On 2011-01-09 06:48:15, fpj wrote: > > trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, > > line 896 > > <https://reviews.apache.org/r/240/diff/1/?file=9407#file9407line896> > > > > Shouldn't we call sw.sh

Re: Review Request: Move blocking read/write calls to SendWorker and RecvWorker Threads

2011-01-20 Thread fpj
> On 2011-01-09 06:48:15, fpj wrote: > > trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, > > line 336 > > <https://reviews.apache.org/r/240/diff/1/?file=9407#file9407line336> > > > > If this error is fatal, the

Re: Review Request: ZOOKEEPER-465: Ledger size in bytes

2011-01-24 Thread fpj
r/src/java/org/apache/bookkeeper/client/PendingReadOp.java, > > line 134 > > <https://reviews.apache.org/r/234/diff/1/?file=9386#file9386line134> > > > > we should be using the METADATA_LENGTH here right? METADATA_LENGTH is 32 bytes, while here we need 24. In real

Re: Review Request: Move blocking read/write calls to SendWorker and RecvWorker Threads

2011-01-27 Thread fpj
27;ll mark it to ship once I do it, and I need a 3.3.3 patch for that. - fpj On 2011-01-17 03:55:41, Vishal Kher wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request: ZOOKEEPER-465: Ledger size in bytes

2011-02-01 Thread fpj
/LedgerRecoveryTest.java 1055997 Diff: https://reviews.apache.org/r/234/diff Testing --- Thanks, fpj

Review Request: ZOOKEEPER-335.

2011-06-09 Thread fpj
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/873/ --- Review request for zookeeper, fpj, Vishal Kher, and Benjamin Reed. Summary

Re: Review Request: ZOOKEEPER-335.

2011-06-09 Thread fpj
#comment1705> I was wondering why you removed this line. I suspect this was causing the bug you mention before, but I was wondering if you have some more insight about it. - fpj On 2011-06-09 14:39:27, fpj wrote: > >

Re: Review Request: ZOOKEEPER-1177: Optimzations for memory and locking in the WatchManager

2011-10-19 Thread fpj
id. Can't we move acquiring the lock to deletePathId()? Moving it would reduce the block size running under the lock. /src/java/test/org/apache/zookeeper/server/WatchManagerTest.java <https://reviews.apache.org/r/2399/#comment6008> Shouldn't contain an author t

Re: Review Request: ZOOKEEPER-1177: Optimzations for memory and locking in the WatchManager

2011-10-20 Thread fpj
let me > > know and I will try to change this to use long and implement a LongBitSet. My comment was triggered by a comment in the code about wrapping around, so I was wondering about making it a long to make it less likely that we wrap around. I agree with you observation about t

Review Request: ZOOKEEPER-1292: FLETest is flaky

2011-12-15 Thread fpj
for FLE. Diffs - /src/java/test/org/apache/zookeeper/test/FLETest.java 1214749 Diff: https://reviews.apache.org/r/3208/diff Testing --- Thanks, fpj

Re: Review Request: ZOOKEEPER-1292: FLETest is flaky

2011-12-15 Thread fpj
Reed. Summary --- Improved test for FLE. This addresses bug ZOOKEEPER-1292. https://issues.apache.org/jira/browse/ZOOKEEPER-1292 Diffs - /src/java/test/org/apache/zookeeper/test/FLETest.java 1214749 Diff: https://reviews.apache.org/r/3208/diff Testing --- Thanks, fpj

Re: Review Request: Add zk.updateServerList(newServerList)

2012-02-08 Thread fpj
0804> You may want to have the comments on top of the line, so that it doesn't overflow. - fpj On 2012-02-07 22:42:04, Alexander Shraer wrote: > > --- > This is an automatically

Re: Review Request: BOOKKEEPER-203: improve ledger manager interface to remove zookeeper dependency on metadata operations.

2012-04-03 Thread fpj
o strongly about it, since the comment before the class says that it is for bookies. bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java <https://reviews.apache.org/r/4603/#comment14405> The comment here should not refer t

Re: Review Request: Consolidate membership management

2012-04-19 Thread fpj
729/#comment15606> Clarification. This is the only new test, so I'm assuming that forward compatibility is being tested through the other tests that are modified to work with the membership file. - fpj On 2012-04-15 20:49:35, Alexander Shraer wrote: > > ---

Re: Review Request: Consolidate membership management

2012-04-19 Thread fpj
> On 2012-04-19 08:12:24, fpj wrote: > > One quick clarification. Is the reason why you want to have a separate file > > for membership to separate membership from the other configuration > > parameters? If so, by having this change in, we are saying that other > >

Re: Review Request: Add zk.updateServerList(newServerList)

2012-11-13 Thread fpj
le paragraphs and perhaps highlight the example by putting it in a separate box? /src/java/main/org/apache/zookeeper/client/StaticHostProvider.java <https://reviews.apache.org/r/3781/#comment28726> This javadoc text is overflowing a bit. /src/java/main/org/apache/zookeeper/client/

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

2016-03-27 Thread fpj
org/r/25160/#comment188435> 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: > > -

[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 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

[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 this

[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 wel

[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 #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 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

[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 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_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_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-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-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-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_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_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_r97446760 --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java --- @@ -87,15 +73,69 @@ public StaticHostProvider(Collection serverAddresses

[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 #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 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 as

[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

  1   2   >