[
https://issues.apache.org/jira/browse/ZOOKEEPER-78?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12697343#action_12697343
]
Patrick Hunt commented on ZOOKEEPER-78:
---------------------------------------
comments on the c code:
the api methods are named with prefix zoo_mutex_*
seems to me it would be better to name them zoo_recipe_lock_* as prefix
this specifies that it's a recipe, in particular that it's implementing the
"lock" recipe
We would expect (and should document in src/recipe/readme) that c api methods
should be named in this fashion, ie zoo_recipe_<recipename>_*
otw the c global namespace is going to cause problems down the line
also easier for users to identify the recipe to which a particular method call
pertains
it's good to commit the configure scripts (see hadoop c code for which files)
this was lesson learned for cbinding & zkfuse, makes it easier for users to
build
ensure binary flag set in tar package for executable scripts
would be nice to have a readme to orient the new user in src/c
include how to build (easier if configure script is here)
out of the box the patch fails to run-check
Running Zookeeper_locktest::testlocksh: ./tests/zkServer.sh: Permission denied
sh: ./tests/zkServer.sh: Permission denied
: assertion : assertion
be sure to set the exec flag when committing zkServer to svn.
in tests might want to use localhost rather than 127.0.0.1 (if ipv6 only host?
or does ipv6 handle that?)
sleep(30) in test is unfortunate - it artificially inflates the test time.
having short test time is a win
any chance you can actively poll rather than just sleeping? something
reusable by other recipes would be grt
I don't see any logging in zoo_lock.c, I think we should log at least the error
conditions, also things like
loop retry at debug level. also operation is pretty complex, some logging in
there would help.
also some similar comments as java - for example getting client_id in
operation, this may be invalid
if the session is not established?
the api says:
* \return the return code.
for many of the methods. what does this mean?
> 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: Mahadev konar
> Fix For: 3.2.0
>
> Attachments: patch_with_including_Benjamin's_fix.patch,
> using_zookeeper_facade.patch, ZOOKEEPER-78.patch, ZOOKEEPER-78.patch,
> ZOOKEEPER-78.patch, ZOOKEEPER-78.patch, ZOOKEEPER-78.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.
