JExample - Interesting junit extension
This looks like it could be very useful for unit tests - basically allows you to easily reuse fixtures: http://smallwiki.unibe.ch/jexample Patrick
[jira] Commented: (ZOOKEEPER-318) remove locking in zk_hashtable.c or add locking in collect_keys()
[ https://issues.apache.org/jira/browse/ZOOKEEPER-318?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12676782#action_12676782 ] Mahadev konar commented on ZOOKEEPER-318: - i looked thorugh the code and what you suggest looks right to me. Thoguh, did you see any performance impact from using the lcoks? if not, I would proabably leave the locks their, since it prevents against future changes (say we have two threads accessing the methods in zk_hashtable.c).. what do you think? remove locking in zk_hashtable.c or add locking in collect_keys() - Key: ZOOKEEPER-318 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-318 Project: Zookeeper Issue Type: Bug Components: c client Affects Versions: 3.0.0, 3.0.1, 3.1.0 Reporter: Chris Darroch Fix For: 3.2.0, 4.0.0 Attachments: ZOOKEEPER-318.patch From a review of zk_hashtable.c it appears to me that all functions which manipulate the hashtables are called from the IO thread, and therefore any need for locking is obviated. If I'm wrong about that, then I think at a minimum collect_keys() should acquire a lock in the same manner as collect_session_watchers(). Both iterate over hashtable contents (in the latter case using copy_table()). However, from what I can see, the only function (besides the init/destroy functions used when creating a zhandle_t) called from the completion thread is deliverWatchers(), which simply iterates over a delivery list created from the hashtables by collectWatchers(). The activateWatcher() function contains comments which describe it being called by the completion thread, but in fact it is called by the IO thread in zookeeper_process(). I believe all calls to collectWatchers(), activateWatcher(), and collect_keys() are made by the IO thread in zookeeper_interest(), zookeeper_process(), check_events(), send_set_watches(), and handle_error(). Note that queue_session_event() is aliased as PROCESS_SESSION_EVENT, but appears only in handle_error() and check_events(). Also note that handle_error() is called only in zookeeper_process() and handle_socket_error_msg(), which is used only by the IO thread, so far as I can see. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-78) added a high level protocol/feature - for easy Leader Election or exclusive Write Lock creation
[ https://issues.apache.org/jira/browse/ZOOKEEPER-78?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12676802#action_12676802 ] Mahadev konar commented on ZOOKEEPER-78: sorry for my late response tom.. i havent had a real close look at the interfaces and methods in this patch myself, so thanks for reviewing.. I was mainly looking at the handling of zookeeper events. 1) I think you are rught that we should probably have call back methods with lockacruired and lockreleeased methods. The current implementation is too restrictive. 2) I am with you on this one as well... I hadn't implemented the lock interface just because I had the same reservations as you.. I think for now we should just leave it as it is without implementing the lock interface and see what our users have to say.. 3) agreed.. added a high level protocol/feature - for easy Leader Election or exclusive Write Lock creation --- Key: ZOOKEEPER-78 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-78 Project: Zookeeper Issue Type: New Feature Components: java client Affects Versions: 3.0.0 Reporter: james strachan Assignee: james strachan Fix For: 3.2.0 Attachments: patch_with_including_Benjamin's_fix.patch, using_zookeeper_facade.patch, ZOOKEEPER-78.patch, ZOOKEEPER-78.patch Here's a patch which adds a little WriteLock helper class for performing leader elections or creating exclusive locks in some directory znode. Note its an early cut; am sure we can improve it over time. The aim is to avoid folks having to use the low level ZK stuff but provide a simpler high level abstraction. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-319) add locking around auth info in zhandle_t
[
https://issues.apache.org/jira/browse/ZOOKEEPER-319?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mahadev konar updated ZOOKEEPER-319:
Attachment: ZOOKEEPER-319.patch
just a minor modification. my compiler fails on compiliing
consta char* auth_data without initialization.
+1 to the patch.
one minor nit --
- we have some logging in auth_completion_func() in zookeeper.c
{noformat}
zoo_lock_auth(zh);
if(rc!=0){
LOG_ERROR((Authentication scheme %s failed. Connection closed.,
zh-auth.scheme));
zh-state=ZOO_AUTH_FAILED_STATE;
}else{
zh-auth.state=1; // active
LOG_INFO((Authentication scheme %s succeeded, zh-auth.scheme));
}
if (zh-auth.completion) {
auth_completion = zh-auth.completion;
auth_data = zh-auth.data;
zh-auth.completion=0;
}
zoo_unlock_auth(zh);
{noformat}
Should we get rid of logging within the lock?
add locking around auth info in zhandle_t
-
Key: ZOOKEEPER-319
URL: https://issues.apache.org/jira/browse/ZOOKEEPER-319
Project: Zookeeper
Issue Type: Bug
Components: c client
Affects Versions: 3.0.0, 3.0.1, 3.1.0
Reporter: Chris Darroch
Fix For: 3.1.1, 3.2.0
Attachments: ZOOKEEPER-319.patch, ZOOKEEPER-319.patch
Looking over the zookeeper.c code it appears to me that the zoo_add_auth()
function may be called at any time by the user in their main thread. This
function alters the elements of the auth_info structure in the zhandle_t
structure.
Meanwhile, the IO thread may read those elements at any time in such
functions as send_auth_info() and auth_completion_func(). It seems
important, then, to add a lock which prevents data being read by the IO
thread while only partially changed by the user's thread. The attached patch
add such a lock.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-320) call auth completion in free_completions()
[ https://issues.apache.org/jira/browse/ZOOKEEPER-320?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12676825#action_12676825 ] Mahadev konar commented on ZOOKEEPER-320: - +1 for the second patch. call auth completion in free_completions() -- Key: ZOOKEEPER-320 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-320 Project: Zookeeper Issue Type: Bug Components: c client Affects Versions: 3.0.0, 3.0.1, 3.1.0 Reporter: Chris Darroch Fix For: 3.1.1, 3.2.0 Attachments: ZOOKEEPER-320-319.patch, ZOOKEEPER-320.patch If a client calls zoo_add_auth() with an invalid scheme (e.g., foo) the ZooKeeper server will mark their session expired and close the connection. However, the C client has returned immediately after queuing the new auth data to be sent with a ZOK return code. If the client then waits for their auth completion function to be called, they can wait forever, as no session event is ever delivered to that completion function. All other completion functions are notified of session events by free_completions(), which is called by cleanup_bufs() in handle_error() in handle_socket_error_msg(). In actual fact, what can happen (about 50% of the time, for me) is that the next call by the IO thread to flush_send_queue() calls send() from within send_buffer(), and receives a SIGPIPE signal during this send() call. Because the ZooKeeper C API is a library, it properly does not catch that signal. If the user's code is not catching that signal either, they experience an abort caused by an untrapped signal. If they are ignoring the signal -- which is common in context I'm working in, the Apache httpd server -- then flush_send_queue()'s error return code is EPIPE, which is logged by handle_socket_error_msg(), and all non-auth completion functions are notified of a session event. However, if the caller is waiting for their auth completion function, they wait forever while the IO thread tries repeatedly to reconnect and is rejected by the server as having an expired session. So, first of all, it would be useful to document in the C API portion of the programmer's guide that trapping or ignoring SIGPIPE is important, as this signal may be generated by the C API. Next, the two attached patches call the auth completion function, if any, in free_completions(), which fixes this problem for me. The second attached patch includes auth lock/unlock function, as per ZOOKEEPER-319. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Assigned: (ZOOKEEPER-320) call auth completion in free_completions()
[ https://issues.apache.org/jira/browse/ZOOKEEPER-320?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Mahadev konar reassigned ZOOKEEPER-320: --- Assignee: Chris Darroch call auth completion in free_completions() -- Key: ZOOKEEPER-320 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-320 Project: Zookeeper Issue Type: Bug Components: c client Affects Versions: 3.0.0, 3.0.1, 3.1.0 Reporter: Chris Darroch Assignee: Chris Darroch Fix For: 3.1.1, 3.2.0 Attachments: ZOOKEEPER-320-319.patch, ZOOKEEPER-320.patch If a client calls zoo_add_auth() with an invalid scheme (e.g., foo) the ZooKeeper server will mark their session expired and close the connection. However, the C client has returned immediately after queuing the new auth data to be sent with a ZOK return code. If the client then waits for their auth completion function to be called, they can wait forever, as no session event is ever delivered to that completion function. All other completion functions are notified of session events by free_completions(), which is called by cleanup_bufs() in handle_error() in handle_socket_error_msg(). In actual fact, what can happen (about 50% of the time, for me) is that the next call by the IO thread to flush_send_queue() calls send() from within send_buffer(), and receives a SIGPIPE signal during this send() call. Because the ZooKeeper C API is a library, it properly does not catch that signal. If the user's code is not catching that signal either, they experience an abort caused by an untrapped signal. If they are ignoring the signal -- which is common in context I'm working in, the Apache httpd server -- then flush_send_queue()'s error return code is EPIPE, which is logged by handle_socket_error_msg(), and all non-auth completion functions are notified of a session event. However, if the caller is waiting for their auth completion function, they wait forever while the IO thread tries repeatedly to reconnect and is rejected by the server as having an expired session. So, first of all, it would be useful to document in the C API portion of the programmer's guide that trapping or ignoring SIGPIPE is important, as this signal may be generated by the C API. Next, the two attached patches call the auth completion function, if any, in free_completions(), which fixes this problem for me. The second attached patch includes auth lock/unlock function, as per ZOOKEEPER-319. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-324) do not materialize strings in the server
[ https://issues.apache.org/jira/browse/ZOOKEEPER-324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12676829#action_12676829 ] Patrick Hunt commented on ZOOKEEPER-324: there's another issue related to this. we have multiple instances of the same string instantiated in various parts of the code (issue regardless of string vs byte representation) if canonicalize (like String intern) we can save significant amounts of memory. For example registering watches on a node (each client registering would use the same string, vs currently using diff strings). We just need to ensure the String/byte[] is immutable (not an issue with string) do not materialize strings in the server Key: ZOOKEEPER-324 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-324 Project: Zookeeper Issue Type: Improvement Components: server Reporter: Benjamin Reed We convert paths and authentication information to strings rather than byte[] even though we could work just as well with byte[] for our needs since we don't really interpret the strings. we are just doing basic pattern matching. the only really string manipulations we do with paths is to look for '/', but we could easily to that with byte[] since we use utf8 encoding for the strings. by not materializing the strings we save time doing the serializations and also space since most (almost all) of our strings are ASCII and thus just one byte. we could probably get by without even changing the jute spec if we make the generated classes check for a flag to see whether strings should be treated as byte[] or String. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
