hadoop-zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (JIRA)" <j...@apache.org>
Subject [jira] Commented: (ZOOKEEPER-729) Recursively delete a znode - zkCli.sh rmr /node
Date Fri, 09 Apr 2010 02:16:50 GMT

    [ https://issues.apache.org/jira/browse/ZOOKEEPER-729?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12855251#action_12855251
] 

Henry Robinson commented on ZOOKEEPER-729:
------------------------------------------

Patch looks pretty good! A few nits:

1. Your tab level seems to be set to two spaces, but the standard is to use four tabs per
space
2. I would change the LOG levels to DEBUG instead of INFO.
3. Consider calling listSubTree listSubTreeBFS? I agree about making the strategy explicit,
I feel this is the best way to do it. 
4. You should be explicit in the listSubTree comment that this is *not* an atomic snapshot
and does not necessarily represent the state of the sub-tree as it ever was - particularly
given that this is a public method. 
5. Similarly, explicitly note in the comments that you are deleting all nodes regardless of
their version. 
6. Did you decide against including a -r option for delete? I'm +1 on that idea rather than
rmr, but don't feel that strongly. 
7. I would remove the TODO comments - I'm not a huge fan of them (anymore :) ). There is no
separator constant, the BFS thing should be taken care of by renaming and we must make a decision
about dealing with the async requests - my take is just to do what you are doing and just
run through the entire tree. 

Finally - we need a test for this! I know there's not a lot of coverage of ZooKeeper, which
is not your fault - but I'm intending to be fairly militant about tests :) Check out e.g.
ACLRootTest for a structure to get a server up and running and a client to connect to it.
A couple of tests which verify the behaviour of both variants would be great. Ideally, we'd
be able to test the behaviour when a create interleaves with a recursive delete, but that
might still be hard to do without mocking up some of the code.

I'll be happy to commit this once these are sorted out, thanks for your patience.

> Recursively delete a znode  - zkCli.sh rmr /node
> ------------------------------------------------
>
>                 Key: ZOOKEEPER-729
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-729
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Kay Kay
>            Assignee: Kay Kay
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-729.patch, ZOOKEEPER-729.patch
>
>
> Recursively delete a given znode in zookeeper, from the command-line. 
> New operation "rmr" added to zkclient. 
> $ ./zkCli.sh rmr /node 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message