[jira] Created: (ZOOKEEPER-936) zkpython is leaking ACL_vector
zkpython is leaking ACL_vector -- Key: ZOOKEEPER-936 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-936 Project: Zookeeper Issue Type: Bug Components: contrib-bindings Reporter: Gustavo Niemeyer It looks like there are no calls to deallocate_ACL_vector() within zookeeper.c in the zkpython binding, which means that (at least) the result of zoo_get_acl() must be leaking. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-733) use netty to handle client connections
[ https://issues.apache.org/jira/browse/ZOOKEEPER-733?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12878710#action_12878710 ] Gustavo Niemeyer commented on ZOOKEEPER-733: Hi Patrick, Our most pressing need is actually on securing the client=>server communication indeed. The basic need is simply to require a specific certificate to connect and communicate securely with the server. Any clients which don't hold the certificate should be denied access. We're certainly available for debating on any open issues you'd like to debate on around this. > use netty to handle client connections > -- > > Key: ZOOKEEPER-733 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-733 > Project: Zookeeper > Issue Type: Improvement >Reporter: Benjamin Reed > Attachments: accessive.jar, flowctl.zip, moved.zip, > QuorumTestFailed_sessionmoved_TRACE_LOG.txt.gz, ZOOKEEPER-733.patch, > ZOOKEEPER-733.patch, ZOOKEEPER-733.patch > > > we currently have our own asynchronous NIO socket engine to be able to handle > lots of clients with a single thread. over time the engine has become more > complicated. we would also like the engine to use multiple threads on > machines with lots of cores. plus, we would like to be able to support things > like SSL. if we switch to netty, we can simplify our code and get the > previously mentioned benefits. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Created: (ZOOKEEPER-732) Improper translation of error into Python exception
Improper translation of error into Python exception --- Key: ZOOKEEPER-732 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-732 Project: Zookeeper Issue Type: Bug Components: contrib-bindings Affects Versions: 3.2.2 Reporter: Gustavo Niemeyer Priority: Minor Apparently errors returned by the C library are not being correctly converted into a Python exception in some cases: >>> zookeeper.get_children(0, "/", None) Traceback (most recent call last): File "", line 1, in SystemError: error return without exception set -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation behavior in zkpython may be removed
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12791901#action_12791901
]
Gustavo Niemeyer commented on ZOOKEEPER-600:
I believe it's ready for integration.
> TODO pondering about allocation behavior in zkpython may be removed
> ---
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
>Affects Versions: 3.2.1
>Reporter: Gustavo Niemeyer
>Assignee: Gustavo Niemeyer
>Priority: Trivial
> Fix For: 3.3.0
>
> Attachments: deallocate-vector.patch
>
>
> I suppose the TODO below is referring to the "path" variable which is passed
> in as an output variable to PyArg_ParseTuple right below. The TODO may be
> removed, since the code is right. Code using PyArg_ParseTuple will borrow
> the reference from the calling code, since there's a stack behind the call to
> the enclosing function (pyzoo_get_children in this case) which won't go away
> until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===
> --- src/contrib/zkpython/src/c/zookeeper.c(revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c(working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
>int zkhid;
>char *path;
>PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-627) zkpython arbitrarily restricts the size of a 'get' to 512 bytes
[ https://issues.apache.org/jira/browse/ZOOKEEPER-627?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12791662#action_12791662 ] Gustavo Niemeyer commented on ZOOKEEPER-627: I vote for fixing it at some point. It'd be nice to fix the style as well (parenthesis, etc), in addition to the spacing. > zkpython arbitrarily restricts the size of a 'get' to 512 bytes > --- > > Key: ZOOKEEPER-627 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-627 > Project: Zookeeper > Issue Type: Bug > Components: contrib-bindings >Reporter: Henry Robinson >Assignee: Henry Robinson > Attachments: ZOOKEEPER-627.patch, ZOOKEEPER-627.patch, > ZOOKEEPER-627.patch > > > Reported on the list: > " > I'm working on using ZooKeeper for an internal application at Digg. I've > been using the zkpython package and I just noticed that the data I was > receiving from a zookeeper.get() call was being truncated. After some quick > digging I found that zookeeper.c limits the data returned to 512 characters > (see > http://svn.apache.org/viewvc/hadoop/zookeeper/tags/release-3.2.2/src/contrib/zkpython/src/c/zookeeper.c?view=markup > line 855). > Is there a reason for this? The only information regarding node size that > I've read is that it should not exceed 1MB so this limit seems a bit > arbitrary and restrictive. > Thanks for the great work! > Rich" -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-627) zkpython arbitrarily restricts the size of a 'get' to 512 bytes
[ https://issues.apache.org/jira/browse/ZOOKEEPER-627?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12791353#action_12791353 ] Gustavo Niemeyer commented on ZOOKEEPER-627: Thanks for the fix Henry. I think there's a minor detail to sort out before merging it: it looks like the buffer is being freed before it's actually used to build the return value. > zkpython arbitrarily restricts the size of a 'get' to 512 bytes > --- > > Key: ZOOKEEPER-627 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-627 > Project: Zookeeper > Issue Type: Bug > Components: contrib-bindings >Reporter: Henry Robinson >Assignee: Henry Robinson > Attachments: ZOOKEEPER-627.patch > > > Reported on the list: > " > I'm working on using ZooKeeper for an internal application at Digg. I've > been using the zkpython package and I just noticed that the data I was > receiving from a zookeeper.get() call was being truncated. After some quick > digging I found that zookeeper.c limits the data returned to 512 characters > (see > http://svn.apache.org/viewvc/hadoop/zookeeper/tags/release-3.2.2/src/contrib/zkpython/src/c/zookeeper.c?view=markup > line 855). > Is there a reason for this? The only information regarding node size that > I've read is that it should not exceed 1MB so this limit seems a bit > arbitrary and restrictive. > Thanks for the great work! > Rich" -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-600) TODO pondering about allocation behavior in zkpython may be removed
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gustavo Niemeyer updated ZOOKEEPER-600:
---
Attachment: deallocate-vector.patch
The attached patch should fix this problem. It also reuses existing code, and
fixes a few other issues around the former problem, with return values not
being properly checked for errors.
I'm afraid there are several instances of variables which are not checked for
error conditions in the module, unfortunately. :-( I'm not going to try fixing
these in this JIRA, though.
> TODO pondering about allocation behavior in zkpython may be removed
> ---
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
>Affects Versions: 3.2.1
>Reporter: Gustavo Niemeyer
>Assignee: Gustavo Niemeyer
>Priority: Trivial
> Fix For: 3.3.0
>
> Attachments: deallocate-vector.patch
>
>
> I suppose the TODO below is referring to the "path" variable which is passed
> in as an output variable to PyArg_ParseTuple right below. The TODO may be
> removed, since the code is right. Code using PyArg_ParseTuple will borrow
> the reference from the calling code, since there's a stack behind the call to
> the enclosing function (pyzoo_get_children in this case) which won't go away
> until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===
> --- src/contrib/zkpython/src/c/zookeeper.c(revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c(working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
>int zkhid;
>char *path;
>PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation behavior in zkpython may be removed
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12783933#action_12783933
]
Gustavo Niemeyer commented on ZOOKEEPER-600:
Patrick, thanks for pointing me to the conventions, and sorry for missing it
earlier.
I'll give it a shot and submit a patch soon.
> TODO pondering about allocation behavior in zkpython may be removed
> ---
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
>Affects Versions: 3.2.1
>Reporter: Gustavo Niemeyer
>Assignee: Gustavo Niemeyer
>Priority: Trivial
> Fix For: 3.3.0
>
>
> I suppose the TODO below is referring to the "path" variable which is passed
> in as an output variable to PyArg_ParseTuple right below. The TODO may be
> removed, since the code is right. Code using PyArg_ParseTuple will borrow
> the reference from the calling code, since there's a stack behind the call to
> the enclosing function (pyzoo_get_children in this case) which won't go away
> until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===
> --- src/contrib/zkpython/src/c/zookeeper.c(revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c(working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
>int zkhid;
>char *path;
>PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation behavior in zkpython may be removed
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12783931#action_12783931
]
Gustavo Niemeyer commented on ZOOKEEPER-600:
Ah, I see.
Yes, PyString_FromString will definitely copy the string over, so the
strings.data array is leaking if it has data allocated dynamically.
In addition to this, PyString_FromString and PyList_New are both allocating
memory, and thus they may fail to return a proper object. This isn't being
checked currently.
> TODO pondering about allocation behavior in zkpython may be removed
> ---
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
>Affects Versions: 3.2.1
>Reporter: Gustavo Niemeyer
>Assignee: Gustavo Niemeyer
>Priority: Trivial
> Fix For: 3.3.0
>
>
> I suppose the TODO below is referring to the "path" variable which is passed
> in as an output variable to PyArg_ParseTuple right below. The TODO may be
> removed, since the code is right. Code using PyArg_ParseTuple will borrow
> the reference from the calling code, since there's a stack behind the call to
> the enclosing function (pyzoo_get_children in this case) which won't go away
> until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===
> --- src/contrib/zkpython/src/c/zookeeper.c(revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c(working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
>int zkhid;
>char *path;
>PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Created: (ZOOKEEPER-600) TODO pondering about allocation behavior in zkpython may be removed
TODO pondering about allocation behavior in zkpython may be removed
---
Key: ZOOKEEPER-600
URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
Project: Zookeeper
Issue Type: Task
Reporter: Gustavo Niemeyer
Priority: Trivial
I suppose the TODO below is referring to the "path" variable which is passed in
as an output variable to PyArg_ParseTuple right below. The TODO may be
removed, since the code is right. Code using PyArg_ParseTuple will borrow the
reference from the calling code, since there's a stack behind the call to the
enclosing function (pyzoo_get_children in this case) which won't go away until
the function returns.
Index: src/contrib/zkpython/src/c/zookeeper.c
===
--- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
+++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
@@ -774,8 +774,6 @@
static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
{
- // TO DO: Does Python copy the string or the reference? If it's the former
- // we should free the String_vector
int zkhid;
char *path;
PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-599) Changes to FLE and QuorumCnxManager to support Observers
[ https://issues.apache.org/jira/browse/ZOOKEEPER-599?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12783885#action_12783885 ] Gustavo Niemeyer commented on ZOOKEEPER-599: I'm sorry.. I clicked on "submit a patch", but didn't notice that the context for the patch was the JIRA I was looking at, rather than the project itself. Please disregard the above comment. > Changes to FLE and QuorumCnxManager to support Observers > > > Key: ZOOKEEPER-599 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-599 > Project: Zookeeper > Issue Type: Improvement >Reporter: Flavio Paiva Junqueira >Assignee: Flavio Paiva Junqueira > Fix For: 3.3.0 > > Attachments: ZOOKEEPER-599.patch, ZOOKEEPER-599.patch, > ZOOKEEPER-599.patch > > > Observers currently can only use electionAlg 0, which is not the default, > supported leader implementation. This issue will fix it. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-599) Changes to FLE and QuorumCnxManager to support Observers
[
https://issues.apache.org/jira/browse/ZOOKEEPER-599?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gustavo Niemeyer updated ZOOKEEPER-599:
---
Status: Patch Available (was: Open)
I suppose the TODO below is referring to the "path" variable which is passed in
as an output variable to PyArg_ParseTuple right below. The TODO may be
removed, since the code is right. Code using PyArg_ParseTuple will borrow the
reference from the calling code, since there's a stack behind the call to the
enclosing function (pyzoo_get_children in this case) which won't go away until
the function returns.
Index: src/contrib/zkpython/src/c/zookeeper.c
===
--- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
+++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
@@ -774,8 +774,6 @@
static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
{
- // TO DO: Does Python copy the string or the reference? If it's the former
- // we should free the String_vector
int zkhid;
char *path;
PyObject *watcherfn = Py_None;
> Changes to FLE and QuorumCnxManager to support Observers
>
>
> Key: ZOOKEEPER-599
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-599
> Project: Zookeeper
> Issue Type: Improvement
>Reporter: Flavio Paiva Junqueira
>Assignee: Flavio Paiva Junqueira
> Fix For: 3.3.0
>
> Attachments: ZOOKEEPER-599.patch, ZOOKEEPER-599.patch,
> ZOOKEEPER-599.patch
>
>
> Observers currently can only use electionAlg 0, which is not the default,
> supported leader implementation. This issue will fix it.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-368) Observers
[
https://issues.apache.org/jira/browse/ZOOKEEPER-368?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12731366#action_12731366
]
Gustavo Niemeyer commented on ZOOKEEPER-368:
I've checked out the patch and have some minor questions/comments to make.
Please note that I'm a "mortal" as per the terminology above, so I apologize
for the lack of deeper understanding. :-)
[1]
+for (FollowerHandler f : observingFollowers) {
+if (!self.viewContains(f.sid))
+f.queuePacket(qp);
I'm missing the reasoning why the !self.viewContains() test is needed here.
For the untrained eye, it sounds redundant with the fact that there are
separate lists for observers and followers. Might be nice to add the reasoning
as a comment in the code, for the next pair of eyes passing over it.
[2]
+ * @param zxid
+ * @param proposal
+ */
+public void inform(Proposal proposal) {
There's no zxid parameter.
[3]
I'm slightly concerned about point (5) from Patrick too. If anyone can
subscribe as an Observer, it means that the ACLs for reading become very
ineffective for protecting data in the ensemble (e.g. passwords, etc). I
understand that it's tricky to address this in the right way right now since
there's no concept of authentication between servers, but it'd be awesome if
such an approach could be considered in the near future for the right way of
solving this, rather than simply whitelisting by IP. The whitelist solution
might be a good short term approach, but it's also a very weak approach for
securing information.
[4]
-forwardingFollowers.remove(follower);
-}
+forwardingFollowers.remove(follower);
+}
(...)
-Socket s = ss.accept();
+Socket s = ss.accept();
(...)
-self.setCurrentVote(result.vote);
+self.setCurrentVote(result.vote);
There are a few changes in the diff which are just adding trailing whitespace
at the end of some lines.
[5]
The VIEWCHANGE message type isn't being sent anywhere in the patch. Is it
preparing for a forthcoming change?
[6]
These changes look very nice overall! (again, for an untrained eye) Thanks for
working on this Henry.
> Observers
> -
>
> Key: ZOOKEEPER-368
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-368
> Project: Zookeeper
> Issue Type: New Feature
> Components: quorum
>Reporter: Flavio Paiva Junqueira
>Assignee: Henry Robinson
> Attachments: ZOOKEEPER-368.patch, ZOOKEEPER-368.patch,
> ZOOKEEPER-368.patch, ZOOKEEPER-368.patch
>
>
> Currently, all servers of an ensemble participate actively in reaching
> agreement on the order of ZooKeeper transactions. That is, all followers
> receive proposals, acknowledge them, and receive commit messages from the
> leader. A leader issues commit messages once it receives acknowledgments from
> a quorum of followers. For cross-colo operation, it would be useful to have a
> third role: observer. Using Paxos terminology, observers are similar to
> learners. An observer does not participate actively in the agreement step of
> the atomic broadcast protocol. Instead, it only commits proposals that have
> been accepted by some quorum of followers.
> One simple solution to implement observers is to have the leader forwarding
> commit messages not only to followers but also to observers, and have
> observers applying transactions according to the order followers agreed upon.
> In the current implementation of the protocol, however, commit messages do
> not carry their corresponding transaction payload because all servers
> different from the leader are followers and followers receive such a payload
> first through a proposal message. Just forwarding commit messages as they
> currently are to an observer consequently is not sufficient. We have a couple
> of options:
> 1- Include the transaction payload along in commit messages to observers;
> 2- Send proposals to observers as well.
> Number 2 is simpler to implement because it doesn't require changing the
> protocol implementation, but it increases traffic slightly. The performance
> impact due to such an increase might be insignificant, though.
> For scalability purposes, we may consider having followers also forwarding
> commit messages to observers. With this option, observers can connect to
> followers, and receive messages from followers. This choice is important to
> avoid increasing the load on the leader with the number of observers.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
