Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/303#discussion_r251685066
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
---
@@ -48,19 +50,21 @@
public class
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/303#discussion_r251672915
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
---
@@ -48,19 +50,21 @@
public class
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/257
Any thoughts on this @Randgalt . I've had a look at it, and the changes
seem fine, but equally, they're not adding a great deal (other than stopping
some error prone complaints).
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/278
I will merge this shortly, thanks @ramaraochavali and @dragonsinth
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/294
Looks good to me.
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/282
Thanks for the PR @alexbrasetvik I will merge this shortly.
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/285
Looks ok to me.
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/292
Looks good to me.
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/283
Looks good to me. I pushed a fixed to the TestTtlNodes unit test is it was
consistently failing for me.
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/281
Thanks for the fix @njhill
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/281
Yep, I've been meaning to merge, just having issues running some unit tests
(which are unrelated to this PR). I will try and merge it today.
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/243
This is just done for efficiency. Each client involved in the leader
election only cares when the child before it disconnects, because that implies
that it should now become leader. If any
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/281#discussion_r233280744
--- Diff:
curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentNode.java
---
@@ -445,7 +445,8 @@ private void createNode
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/281#discussion_r233227009
--- Diff:
curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentNode.java
---
@@ -445,7 +445,8 @@ private void createNode
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/280
It looks OK to me, but my knowledge of this section of Curator is very
limited. @Randgalt do you have any thoughts on this? If not, I'll merge.
---
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/281#discussion_r231379350
--- Diff:
curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentNode.java
---
@@ -444,8 +447,19 @@ private void
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/281#discussion_r231322157
--- Diff:
curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentNode.java
---
@@ -445,7 +445,8 @@ private void createNode
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/281
@njhill Thanks for the PR. Would it be possible to provide a unit test to
reproduce the problem and verify the fix?
cheers
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/275
Thanks for the PR. Looks good to me. I will merge to master shortly.
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/263
Looks good to me, will merge shortly.
---
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/257#discussion_r183254951
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/schema/Schema.java
---
@@ -105,7 +105,8 @@ public static SchemaBuilder
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/262
Ok, will merge this now.
Thanks @arrodrigues !
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/262
To Curator, don't they all look the same? Ultimately, Curator will just get
a Disconnected event?
Given that we can't tell the difference between them, I think that we just
have
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/262
My issue is the difference in timing in these events:
-If the connection between client and server is lost a disconnected event
is received essentially immediately.
-If a heart beat
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/262
I'm conflicted about the LOST injection stuff. I guess all we can do is err
on the side of caution and leave it as is. That will cover what is presumably
the most common case, where
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/262
That's a good point @arrodrigues
I don't think we can tell the difference between Disconnected event from ZK
due to loss of the underlying socket and a Disconected event due
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/262
The changes look good to me, thanks @arrodrigues
@Randgalt , I think the approach you're suggesting for setting up the
default session timeout stuff looks OK. I thought
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/262
Thanks for the updates, I think this looks good.
I still think we need to look at the default behaviour of the
ConnectionStateListener though. Currently, a LOST event will only
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/262#discussion_r179022253
--- Diff:
curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatch.java
---
@@ -222,6 +223,35 @@ public void
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/262#discussion_r179022050
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/state/ConnectionStateManager.java
---
@@ -253,6 +253,7 @@ private void
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/262#discussion_r179001383
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/state/ConnectionStateManager.java
---
@@ -253,6 +253,7 @@ private void
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/262#discussion_r179001451
--- Diff:
curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatch.java
---
@@ -222,6 +223,35 @@ public void
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/252
Looks good to me.
---
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/252#discussion_r169528858
--- Diff:
curator-framework/src/test/java/org/apache/curator/framework/ensemble/TestEnsembleProvider.java
---
@@ -0,0 +1,162
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/249
I think this looks OK.
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/248
Looks ok to me.
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/243
Thanks!
---
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/243#discussion_r154539096
--- Diff:
curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/testing/DummyLeaderLatch.java
---
@@ -0,0 +1,189
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/243#discussion_r154539053
--- Diff:
curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatchIsolatedZookeeper.java
---
@@ -0,0 +1,224
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/243#discussion_r154539158
--- Diff:
curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatchIsolatedZookeeper.java
---
@@ -0,0 +1,224
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/243#discussion_r154539068
--- Diff:
curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatchIsolatedZookeeper.java
---
@@ -0,0 +1,224
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/243#discussion_r154539070
--- Diff:
curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatchIsolatedZookeeper.java
---
@@ -0,0 +1,224
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/243#discussion_r154539071
--- Diff:
curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatchIsolatedZookeeper.java
---
@@ -0,0 +1,224
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/240
Looks ok to me.
---
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/238
Looks good to me.
---
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/236#discussion_r138482206
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
---
@@ -1212,7 +1212,21 @@ public String call
GitHub user cammckenzie opened a pull request:
https://github.com/apache/curator/pull/236
CURATOR-431 - Fixed stat population during create
-The stat object was not being populated if the create failed due to the
node already existing.
You can merge this pull request into a Git
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/223#discussion_r123912201
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/ExistsBuilderImpl.java
---
@@ -185,7 +185,7 @@ public Stat forPath(String
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/208
I think that what we've got is probably the best compromise we're going to
get.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/223
Sounds good to me, thanks @szekizoli
---
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 cammckenzie commented on the issue:
https://github.com/apache/curator/pull/223
I agree that this is now a 'feature' that the parents don't have the ACL
set. Perhaps we need to introduce another option to set the ACL on the parents?
---
If your project is set up
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/221
I had a bit more of a look at the failing test and see the following being
logged when the client.reconfig() call occurs. I'm not sure if these are issues
or not, but they certainly seem
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/221
TestFailedDeleteManager seems to be ok now, but I'm still seeing fairly
regular failure on TestReconfiguration.testNewMembers(). It's getting a
ConnectionLossException at Line 313
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/221
The reconfiguration case seems to be better now. I'm still seeing failures
on the
TestFailedDeleteManager.testLostSession
It seems that at line 77, it is sometimes getting
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/221
I'm still seeing the following fail. The TestFailedDeleteManager case is
intermittent, the reconfig one seems to be failing consistently.
Failed tests
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/218#discussion_r114693246
--- Diff:
curator-x-async/src/main/java/org/apache/curator/x/async/details/AsyncTransactionOpImpl.java
---
@@ -87,10 +87,7 @@ public
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/218#discussion_r114674883
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/api/transaction/TransactionCreateBuilder.java
---
@@ -18,17 +18,17
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/218#discussion_r114673449
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/api/transaction/TransactionCreateBuilder.java
---
@@ -18,17 +18,17
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/218#discussion_r114670214
--- Diff:
curator-x-async/src/main/java/org/apache/curator/x/async/details/AsyncTransactionOpImpl.java
---
@@ -77,6 +78,13 @@ public
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/218#discussion_r114670616
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/api/transaction/TransactionCreateBuilder.java
---
@@ -18,17 +18,17
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/171#discussion_r111854064
--- Diff:
curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentTtlNode.java
---
@@ -0,0 +1,58 @@
+package
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/171#discussion_r111854284
--- Diff:
curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentTtlNode.java
---
@@ -44,25 +45,49 @@ public void
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/171#discussion_r111849959
--- Diff:
curator-framework/src/test/java/org/apache/curator/framework/imps/TestTtlNodes.java
---
@@ -0,0 +1,88 @@
+/**
+ * Licensed
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/213
Ah, I see now. It's the Preconditions.checkState call that causes the
issue. I thought that we could just hold a reference to the builder and then
call forPath() on it when required
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/213
I think that the fix is fine, but why is the createMethod a lazily
instantiated reference? Couldn't it just be a normal reference that gets
created in the constructor? We seem to have
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/208
I can't think of a better option other than deprecating the 1 arg
constructor, and forcing clients to explicitly define their behaviour going
forward.
---
If your project is set up
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/208#discussion_r108049761
--- Diff:
curator-x-discovery/src/test/java/org/apache/curator/x/discovery/details/NewServiceInstance.java
---
@@ -0,0 +1,148
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/208#discussion_r108049783
--- Diff:
curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/JsonInstanceSerializer.java
---
@@ -16,30 +16,57
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/204#discussion_r105804310
--- Diff: pom.xml ---
@@ -62,22 +62,22 @@
3.4.8
-
2.7
-2.3.7
-2.10.3
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/204#discussion_r105800775
--- Diff: pom.xml ---
@@ -62,22 +62,22 @@
3.4.8
-
2.7
-2.3.7
-2.10.3
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/103
Curator 2.x only supports Java 1.6 so can't support the auto close features
in Java 1.7
Curator 3.x supports Java 1.7. The Locker class is present in this version.
cheers
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/197#discussion_r103351183
--- Diff:
curator-client/src/main/java/org/apache/curator/ConnectionState.java ---
@@ -283,12 +300,19 @@ private boolean checkState(Event.KeeperState
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/197#discussion_r103338590
--- Diff:
curator-client/src/main/java/org/apache/curator/ConnectionState.java ---
@@ -283,12 +300,19 @@ private boolean checkState(Event.KeeperState
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/197
Any thoughts on this @Randgalt , I'd like to merge before doing a build.
---
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 cammckenzie commented on the issue:
https://github.com/apache/curator/pull/197
I've run through some more tests and it all looks good.
My only concern is that we're introducing the Mockito just for this test
case. Could you move the test case into the curator
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/197#discussion_r98551865
--- Diff:
curator-client/src/main/java/org/apache/curator/ConnectionState.java ---
@@ -160,13 +162,33 @@ public void process(WatchedEvent event
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/197#discussion_r98367381
--- Diff:
curator-client/src/main/java/org/apache/curator/ConnectionState.java ---
@@ -160,13 +162,33 @@ public void process(WatchedEvent event
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/189
I don't have any real experience with the Java 8 asynch framework, but what
you've implemented looks good to me. I'll have a look at the doco next week
when I get some time (on leave
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/186
Is this fix specifically for the PersistentNode class? It seems like it
will potentially break a lot of stuff.
---
If your project is set up for it, you can reply to this email and have your
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/175
Looks good to me. I assume there's no easy way of providing unit tests for
the new functionality?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/173#discussion_r89223241
--- Diff:
curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderSelector.java
---
@@ -341,11 +342,41 @@ public Participant
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/173#discussion_r89212147
--- Diff:
curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderSelector.java
---
@@ -341,11 +342,41 @@ public Participant
GitHub user cammckenzie opened a pull request:
https://github.com/apache/curator/pull/173
CURATOR-358 - Fixed race condition with getLeader()
-If leadership changes between the getParticipantNodes() call and the
getLeader() internal call the NoNodeException is now handled
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/171#discussion_r86286940
--- Diff:
curator-framework/src/test/java/org/apache/curator/framework/imps/TestTtlNodes.java
---
@@ -0,0 +1,88 @@
+/**
+ * Licensed
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/171#discussion_r86286992
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
---
@@ -83,6 +85,13 @@ public CreateBuilderMain
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/170
Looks good to me, thanks for the PR.
---
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 cammckenzie commented on the issue:
https://github.com/apache/curator/pull/165
Thanks @lvfangmin, unfortunately I'm somewhat geographically challenged
living on the other side of the world. Sounds like fun though.
---
If your project is set up for it, you can reply
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/168
Thanks for the PR!
---
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
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/167
Looks ok to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/166
Looks ok to me
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/165
I've had a bit more of a look and I think that the changes are OK. As you
mentioned, the changes are only to internal classes. There is a loss of
granularity (The TimeTrace was using nanos
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/165
Thanks for the PR,
The content looks OK, but I think that the interface changes need to be
made backwards compatible. Can we change the OperationTrace to extend the
existing TimerTrace
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/162
Looks ok to me
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/161
Looks good to me, thanks for the patch!
---
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 cammckenzie commented on the issue:
https://github.com/apache/curator/pull/161
I'm not sure on the pattern for executors, probably just the whim of
whoever implemented the recipe I imagine.
I think that you should be able to do this asynchronously without
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/161
I don't think that this completely fixes the problem. The test fails
intermittently because it's possible that the server gets stopped while the
SharedValue is processing the initial connected
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/157
Ok, I've pushed some additional changes to fix your comments. Are you happy
for me to merge?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/157#discussion_r66906186
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/state/ConnectionStateManager.java
---
@@ -252,7 +252,9 @@ private void
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/157#discussion_r66905910
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/state/ConnectionStateManager.java
---
@@ -252,7 +252,9 @@ private void
GitHub user cammckenzie opened a pull request:
https://github.com/apache/curator/pull/157
Long session timeout issue
Fix for issue where LOST events were created at 4/3 of the session timeout
rather than at the end of session timeout.
You can merge this pull request into a Git
1 - 100 of 231 matches
Mail list logo
