You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@curator.apache.org by brienwheeler-sqrrl <gi...@git.apache.org> on 2014/10/24 18:34:59 UTC
[GitHub] curator pull request: CURATOR-140: Allow InterProcessReadWriteLock...
GitHub user brienwheeler-sqrrl opened a pull request:
https://github.com/apache/curator/pull/48
CURATOR-140: Allow InterProcessReadWriteLock consumer to set lock node d...
Simple change to plumb through the ability to set node data from the consumer of InterProcessReadWriteLock.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sqrrldata/curator CURATOR-140
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/curator/pull/48.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #48
----
commit 77a1afbcc074d9ef87ba3bfc7d3cb641ad2f727e
Author: Brien Wheeler <br...@briens-macbook-pro.local>
Date: 2014-10-24T16:33:16Z
CURATOR-140: Allow InterProcessReadWriteLock consumer to set lock node data.
----
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-140: Allow InterProcessReadWriteLock...
Posted by cammckenzie <gi...@git.apache.org>.
Github user cammckenzie commented on the pull request:
https://github.com/apache/curator/pull/48#issuecomment-61018683
Thanks for the patch, it looks good. Would you be able to provide a unit test that covers the new functionality?
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-140: Allow InterProcessReadWriteLock...
Posted by ohshazbot <gi...@git.apache.org>.
Github user ohshazbot commented on a diff in the pull request:
https://github.com/apache/curator/pull/48#discussion_r19763228
--- Diff: curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestInterProcessReadWriteLock.java ---
@@ -261,6 +261,39 @@ public Void call() throws Exception
Assert.assertTrue(maxConcurrentCount.get() > 1);
}
+ @Test
+ public void testSetNodeData() throws Exception
+ {
+ CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+
+ try
+ {
+ client.start();
+
+ final byte[] nodeData = new byte[] { 1, 2, 3, 4 };
+
+ InterProcessReadWriteLock lock = new InterProcessReadWriteLock(client, "/lock", nodeData);
+
+ // mutate passed-in node data, lock has made copy
+ nodeData[0] = 5;
--- End diff --
Looks like it's to ensure that the passed in data is being copied, so any alterations to the byte array after the fact don't cause some sort of partial view of data.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-140: Allow InterProcessReadWriteLock...
Posted by brienwheeler-sqrrl <gi...@git.apache.org>.
Github user brienwheeler-sqrrl commented on the pull request:
https://github.com/apache/curator/pull/48#issuecomment-61111726
Sure thing, I'll look into that over the next few days.
Cheers,
Brien
On Wed, Oct 29, 2014 at 6:48 PM, cammckenzie <no...@github.com>
wrote:
> Thanks for the patch, it looks good. Would you be able to provide a unit
> test that covers the new functionality?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/curator/pull/48#issuecomment-61018683>.
>
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-140: Allow InterProcessReadWriteLock...
Posted by brienwheeler-sqrrl <gi...@git.apache.org>.
Github user brienwheeler-sqrrl commented on the pull request:
https://github.com/apache/curator/pull/48#issuecomment-61538081
Hi, I just pushed a test for setting the node data for CURATOR-140.
Cheers,
Brien
On Wed, Oct 29, 2014 at 6:48 PM, cammckenzie <no...@github.com>
wrote:
> Thanks for the patch, it looks good. Would you be able to provide a unit
> test that covers the new functionality?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/curator/pull/48#issuecomment-61018683>.
>
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-140: Allow InterProcessReadWriteLock...
Posted by cammckenzie <gi...@git.apache.org>.
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/48#discussion_r19763101
--- Diff: curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestInterProcessReadWriteLock.java ---
@@ -261,6 +261,39 @@ public Void call() throws Exception
Assert.assertTrue(maxConcurrentCount.get() > 1);
}
+ @Test
+ public void testSetNodeData() throws Exception
+ {
+ CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+
+ try
+ {
+ client.start();
+
+ final byte[] nodeData = new byte[] { 1, 2, 3, 4 };
+
+ InterProcessReadWriteLock lock = new InterProcessReadWriteLock(client, "/lock", nodeData);
+
+ // mutate passed-in node data, lock has made copy
+ nodeData[0] = 5;
--- End diff --
What's this for?
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-140: Allow InterProcessReadWriteLock...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/curator/pull/48
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-140: Allow InterProcessReadWriteLock...
Posted by cammckenzie <gi...@git.apache.org>.
Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/48#discussion_r19764502
--- Diff: curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestInterProcessReadWriteLock.java ---
@@ -261,6 +261,39 @@ public Void call() throws Exception
Assert.assertTrue(maxConcurrentCount.get() > 1);
}
+ @Test
+ public void testSetNodeData() throws Exception
+ {
+ CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+
+ try
+ {
+ client.start();
+
+ final byte[] nodeData = new byte[] { 1, 2, 3, 4 };
+
+ InterProcessReadWriteLock lock = new InterProcessReadWriteLock(client, "/lock", nodeData);
+
+ // mutate passed-in node data, lock has made copy
+ nodeData[0] = 5;
--- End diff --
Ok, just wasn't obvious because there's no explicit assert checking that the data in the InterProcessReadWriteLock is different to the mutated nodeData array. I notice that there's no way of getting this data out of the InterProcessReadWriteLock to do an explicit assert though, so I think that this is fine.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-140: Allow InterProcessReadWriteLock...
Posted by brienwheeler-sqrrl <gi...@git.apache.org>.
Github user brienwheeler-sqrrl commented on a diff in the pull request:
https://github.com/apache/curator/pull/48#discussion_r19763277
--- Diff: curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestInterProcessReadWriteLock.java ---
@@ -261,6 +261,39 @@ public Void call() throws Exception
Assert.assertTrue(maxConcurrentCount.get() > 1);
}
+ @Test
+ public void testSetNodeData() throws Exception
+ {
+ CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+
+ try
+ {
+ client.start();
+
+ final byte[] nodeData = new byte[] { 1, 2, 3, 4 };
+
+ InterProcessReadWriteLock lock = new InterProcessReadWriteLock(client, "/lock", nodeData);
+
+ // mutate passed-in node data, lock has made copy
+ nodeData[0] = 5;
--- End diff --
Yep, that's why I put in the post-creation mutation of the passed-in node data array.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---