Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/310
@lvfangmin
I am trying to reproduce the issue you have seen here, and I have not been
able to do so. The test either fails for me with the same leader being elected
each time, or on
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/310
Apparently for some reason I don't understand if I don't run all of the
tests in QuorumPeerMainTest The old leader is elected again each time.
---
GitHub user revans2 opened a pull request:
https://github.com/apache/zookeeper/pull/453
ZOOKEEPER-2845: Send a SNAP if transactions cannot be verified.
I will be creating a patch/pull request for 3.4 and 3.5 too, but I wanted
to get a pull request up for others to look at ASAP
GitHub user revans2 opened a pull request:
https://github.com/apache/zookeeper/pull/454
ZOOKEEPER-2845: Send a SNAP if transactions cannot be verified. (3.5)
This is the version of #453 for the 3.5 branch
You can merge this pull request into a Git repository by running:
$ git
GitHub user revans2 opened a pull request:
https://github.com/apache/zookeeper/pull/455
ZOOKEEPER-2845: Send a SNAP if transactions cannot be verified.
This is the version of #453 for the 3.4 branch
You can merge this pull request into a Git repository by running:
$ git pull
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/453
Thank you to everyone who reviewed the patch, but with the help of Fangmin
Lv I found one case that the original patch didn't cover. I have reworked the
patch to cover that case, but to do
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r167885280
--- Diff: src/java/main/org/apache/zookeeper/server/ZKDatabase.java ---
@@ -233,14 +233,32 @@ public long getDataTreeLastProcessedZxid
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/453
@anmolnar I will add some kind of a test. I ran into a lot of issues with
`testTxnAheadSnapInRetainDB`. I could not get it to run correctly against
master as it would always end up electing the
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/453
@anmolnar I added in an updated version of the test in #310. The issue
turned out to be a race condition where the original leader would time out
clients and then would join the new quorum too
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r168793764
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java ---
@@ -888,4 +888,127 @@ public void testWithOnlyMinSessionTimeout
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r168794042
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java ---
@@ -888,4 +888,127 @@ public void testWithOnlyMinSessionTimeout
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r168795646
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java ---
@@ -435,7 +435,7 @@ private void waitForOne(ZooKeeper zk, States
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r168807853
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java ---
@@ -888,4 +888,127 @@ public void testWithOnlyMinSessionTimeout
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r168807914
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java ---
@@ -888,4 +888,127 @@ public void testWithOnlyMinSessionTimeout
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r168807976
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java ---
@@ -888,4 +888,127 @@ public void testWithOnlyMinSessionTimeout
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r168807943
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java ---
@@ -888,4 +888,127 @@ public void testWithOnlyMinSessionTimeout
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/453
@afine and @anmolnar I think I have addressed all of your review comments,
except for the one about the change to `waitForOne` and I am happy to adjust
however you want there.
---
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/453#discussion_r169662234
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java ---
@@ -888,4 +923,103 @@ public void testWithOnlyMinSessionTimeout
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/453
@afine
I have addressed you most recent comments. If you want me to squash
commits please let me know.
I have a pull request for the 3.5 branch #454 and for the 3.4 branch #455
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/454
I just rebased this and pulled in all of the changes made to the main test.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/455
I just rebased this and pulled in all of the changes made to the main test.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/453
@afine all of the changes in this branch are now in the pull requests to
the 3.5 and 3.5 branches,
---
Github user revans2 closed the pull request at:
https://github.com/apache/zookeeper/pull/454
---
Github user revans2 closed the pull request at:
https://github.com/apache/zookeeper/pull/455
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/453
Thanks @afine I closed them.
---
GitHub user revans2 opened a pull request:
https://github.com/apache/zookeeper/pull/282
ZOOKEEPER-1782: Let a SASL super user be super
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/revans2/zookeeper ZOOKEEPER-1782
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/282
@hanm Thanks for the review. I think I have addressed all of your review
comments. I originally put the patch up in 2013 and I have not touched it
since so I honestly don't remember a lot
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/282#discussion_r122698936
--- Diff:
src/java/main/org/apache/zookeeper/server/auth/SASLAuthenticationProvider.java
---
@@ -38,11 +38,6 @@ public String getScheme
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/282
@arshadmohammad I think I have addressed your review comments
---
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
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/282#discussion_r123330561
--- Diff:
src/java/main/org/apache/zookeeper/server/auth/SASLAuthenticationProvider.java
---
@@ -38,11 +38,6 @@ public String getScheme
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/282
@arshadmohammad if everything looks good I will squash my commits
---
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
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/282
@hanm I didn't add in `zookeeper.superUser`. I am happy to document it if
you tell me where to make that change. I don't know where the documentation
source is stored. I am happy to
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/282
@hanm I added in the docs.
---
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
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/310
@lvfangmin any update on getting a pull request for the actual fix?
---
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/148#discussion_r96435980
--- Diff: ivy.xml ---
@@ -41,13 +41,20
GitHub user revans2 opened a pull request:
https://github.com/apache/zookeeper/pull/157
ZOOKEEPER-2678: Discovery and Sync can take a very long time on large DB
This patch addresses recovery time when a leader is lost on a large DB.
It does this by not clearing the DB
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/157#discussion_r98449817
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/Learner.java ---
@@ -364,10 +367,12 @@ else if (qp.getType() == Leader.SNAP
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/157
@fpj Thanks for the review I will update the comments and start porting it
to other lines.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
GitHub user revans2 opened a pull request:
https://github.com/apache/zookeeper/pull/159
ZOOKEEPER-2678: Discovery and Sync can take a very long time on large DBs
This is the master version of #157
You can merge this pull request into a Git repository by running:
$ git pull
GitHub user revans2 opened a pull request:
https://github.com/apache/zookeeper/pull/158
ZOOKEEPER-2678: Discovery and Sync can take a very long time on large DBs
This is the 3.5 version of #157
You can merge this pull request into a Git repository by running:
$ git pull https
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/157
@fpj I addressed your review comments, and I also found another race in the
ZAB test that I addressed. Apparently the log line I removed was taking enough
time for the transactions to be fully
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/157
@eribeiro will do. It is a clean cherry pick between master and 3.5
---
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
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/158
Closing this because master applies cleanly to 3.5 as well
---
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 user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/159
This also applies cleanly to the 3.5 line #157 is for the 3.4 line
---
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
Github user revans2 closed the pull request at:
https://github.com/apache/zookeeper/pull/158
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/157#discussion_r98744573
--- Diff: src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java
---
@@ -839,12 +839,25 @@ public void converseWithFollower(InputArchive ia
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/157
@hanm I addressed your review comments.
---
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 user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/157#discussion_r99347551
--- Diff: src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java
---
@@ -839,12 +839,25 @@ public void converseWithFollower(InputArchive ia
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/157
@afine I updated the test to spy on the `LearnerZooKeeperServer` instance
and check if and when `takeSnapshot` was called. I think this fulfills your
desires, but with tests there can always be
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/157
Is there any more I need to do to get this merged in?
---
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 user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/159#discussion_r100552192
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/Learner.java ---
@@ -498,14 +504,19 @@ else if (qp.getType() == Leader.SNAP
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/159
@hanm thanks for the reviews I removed the incorrect comment.
---
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
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/157
@rakeshadr If it makes you feel any better we have been running with an
older version of this patch in production for a while. We have used it as part
of a rolling upgrade at least 10 times in
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101775786
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * Licensed to the Apache Software
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102229805
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r10749
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/180
Oh I also had the idea that it might be nice to provide feedback on the
snap command for both JMX and the Admin command. You could provide more then
just the last zxid, but you could also
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102245094
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102283801
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException
Github user revans2 closed the pull request at:
https://github.com/apache/zookeeper/pull/157
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is
GitHub user revans2 opened a pull request:
https://github.com/apache/zookeeper/pull/648
ZOOKEEPER-3156: Add in option to canonicalize host name
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/revans2/zookeeper ZOOKEEPER-3156-3.4
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/648#discussion_r221376870
--- Diff: src/java/main/org/apache/zookeeper/ClientCnxn.java ---
@@ -990,6 +992,27 @@ private void sendPing() {
private boolean
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/648#discussion_r221377101
--- Diff: src/java/main/org/apache/zookeeper/ClientCnxn.java ---
@@ -990,6 +992,27 @@ private void sendPing() {
private boolean
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/648
@anmolnar I think I have addressed all of your comments. Please have
another look.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/648
I just addressed the review comments if things look good I am happy to
squash commits. I'll also put up pull requests to the other lines, now that it
looks like we have the majority of the
GitHub user revans2 opened a pull request:
https://github.com/apache/zookeeper/pull/652
ZOOKEEPER-3156: Add in option to canonicalize host name
This is the master and 3.5 version of #648. It should apply cleanly to
both lines, but if you want a separate pull request for each I am
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/652#discussion_r221749153
--- Diff: src/java/main/org/apache/zookeeper/ClientCnxn.java ---
@@ -1102,7 +1103,32 @@ private void startConnect(InetSocketAddress addr)
throws
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/648
Sorry yes. I'll get on the test case...
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/648
So I am running into some issues with the tests as all of the methods to
InetSocketAddress are marked as final and as such I cannot mock them. This
means I would need a set of host names that are
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/648
Looks like it is mockito-all not powermock-api-mockito. I can try and
change it and see.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/648
I tried to add in powermock too, but it conflicts with mockito (power mock
uses a much older version of mockito-core). I'll try and use a different API
for powermock instead of mockito.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/652
Added in the test.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/648
I just added in a test and made the code match closer to how master/3.5
work for the canonical host name.
---
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/652#discussion_r221956355
--- Diff: src/java/main/org/apache/zookeeper/ClientCnxn.java ---
@@ -793,7 +794,87 @@ public RWServerFoundException(String msg) {
super
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/648
There were some comments in the other pull request about the test code. I
will address them in both places.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/652
@anmolnar and @lvfangmin I think I have addressed all of your review
comments. I named the new class SaslServerPrincipal but if you have a
different idea for a name I am happy to change it.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/652
Ping @eolivelli @lvfangmin @anmolnar
I think I have addressed all of the concerns on both branches. The test
failure here looks unrelated to the changes that I made.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/648
Thanks for all of the reviews I just rebased and squashed commits.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/652
Thanks for all of the reviews I just rebased and squashed commits.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/652
I just rebased to deal with the directories being moved around.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/652
Ping any hope in getting this merged in?
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/652
@anmolnar sorry it took me so long. I was out on vacation.
---
Github user revans2 commented on the issue:
https://github.com/apache/zookeeper/pull/648
Thanks
---
Github user revans2 closed the pull request at:
https://github.com/apache/zookeeper/pull/648
---
84 matches
Mail list logo
