ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-729

Recursively delete a znode - zkCli.sh rmr /node

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.4.0
    • Component/s: java client
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Recursively delete a given znode in zookeeper, from the command-line.

      New operation "rmr" added to zkclient.

      $ ./zkCli.sh rmr /node

      1. ASF.LICENSE.NOT.GRANTED--ZOOKEEPER-729.patch
        10 kB
        Karthik K
      2. ASF.LICENSE.NOT.GRANTED--ZOOKEEPER-729.patch
        10 kB
        Karthik K
      3. ZOOKEEPER-729.patch
        9 kB
        Karthik K
      4. ZOOKEEPER-729.patch
        5 kB
        Karthik K
      5. ZOOKEEPER-729.patch
        3 kB
        Karthik K

        Issue Links

          Activity

          Hide
          Harsh J added a comment -

          With ZOOKEEPER-1326, this command was renamed into 'deleteall' and the 'rmr' was deprecated - in 3.5+.

          Show
          Harsh J added a comment - With ZOOKEEPER-1326 , this command was renamed into 'deleteall' and the 'rmr' was deprecated - in 3.5+.
          Hide
          Karthik K added a comment -

          Thanks Henry for committing this / Patrick for helping review this.

          Show
          Karthik K added a comment - Thanks Henry for committing this / Patrick for helping review this.
          Hide
          Hudson added a comment -

          Integrated in ZooKeeper-trunk #794 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/794/)
          ZOOKEEPER-729. Java client API to recursively delete a subtree. (Kay Kay via henry)

          Show
          Hudson added a comment - Integrated in ZooKeeper-trunk #794 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/794/ ) ZOOKEEPER-729 . Java client API to recursively delete a subtree. (Kay Kay via henry)
          Hide
          Henry Robinson added a comment -

          I just committed this (had to move the test file into java/, but otherwise committed as submitted) - thanks Kay!

          Show
          Henry Robinson added a comment - I just committed this (had to move the test file into java/, but otherwise committed as submitted) - thanks Kay!
          Hide
          Patrick Hunt added a comment -

          That's fine Henry, you might just run the findbugs and RAT directly to verify (it's in build.xml, looks ok to the naked eye though).

          Great job Kay, Thanks!

          Show
          Patrick Hunt added a comment - That's fine Henry, you might just run the findbugs and RAT directly to verify (it's in build.xml, looks ok to the naked eye though). Great job Kay, Thanks!
          Hide
          Henry Robinson added a comment -

          Hudson can't post messages to JIRA currently, but as Kay says the build looked good. So I'm +1 on this patch, and will commit it later today unless anyone shouts at this breach of protocol

          Show
          Henry Robinson added a comment - Hudson can't post messages to JIRA currently, but as Kay says the build looked good. So I'm +1 on this patch, and will commit it later today unless anyone shouts at this breach of protocol
          Hide
          Karthik K added a comment -

          so - good to go here ?

          Show
          Karthik K added a comment - so - good to go here ?
          Hide
          Karthik K added a comment -

          fyi - build # 55 is green with the latest patch.

          Show
          Karthik K added a comment - fyi - build # 55 is green with the latest patch.
          Hide
          Karthik K added a comment -

          release audit warnings mentions about - ZookeeperTest .

          http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/51/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt .

          New patch added - that hopefully addresses the issue.

          Show
          Karthik K added a comment - release audit warnings mentions about - ZookeeperTest . http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/51/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt . New patch added - that hopefully addresses the issue.
          Hide
          Karthik K added a comment -

          Added javadoc to ZookeeperTest + license headers

          Show
          Karthik K added a comment - Added javadoc to ZookeeperTest + license headers
          Hide
          Patrick Hunt added a comment -

          [exec] -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

          Also, there's all kinds of fallout because of the security breach from friday. afaict infra ops is still making changes.

          I'll ping Giri again (may be a known issue).

          but it seems that hudson is unable to update JIRA. so Something is still borked regardless.

          Show
          Patrick Hunt added a comment - [exec] -1 javadoc. The javadoc tool appears to have generated 1 warning messages. Also, there's all kinds of fallout because of the security breach from friday. afaict infra ops is still making changes. I'll ping Giri again (may be a known issue). but it seems that hudson is unable to update JIRA. so Something is still borked regardless.
          Hide
          Henry Robinson added a comment -

          I think it's due to a documentation warning as there is a link target in one of the headers that doesn't exist.

          Show
          Henry Robinson added a comment - I think it's due to a documentation warning as there is a link target in one of the headers that doesn't exist.
          Hide
          Karthik K added a comment -

          The build seems to have failed, although the tests seem to have passed.

          http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/51/testReport/org.apache.zookeeper/ZooKeeperTest/ .

          Any idea why the build failed ?

          Show
          Karthik K added a comment - The build seems to have failed, although the tests seem to have passed. http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/51/testReport/org.apache.zookeeper/ZooKeeperTest/ . Any idea why the build failed ?
          Hide
          Patrick Hunt added a comment -

          It's not you Kay, Hudson has been borked, Giri only just recently got it back.
          sometimes it seems to forget pending, it's running now:
          http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/51/

          Show
          Patrick Hunt added a comment - It's not you Kay, Hudson has been borked, Giri only just recently got it back. sometimes it seems to forget pending, it's running now: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/51/
          Hide
          Karthik K added a comment -

          Patrick -Thanks for helping flipping the status. I did try to do that a couple of times ago, but hudson does not seem to pick up after the first patch !

          Show
          Karthik K added a comment - Patrick -Thanks for helping flipping the status. I did try to do that a couple of times ago, but hudson does not seem to pick up after the first patch !
          Hide
          Henry Robinson added a comment -

          +1, patch looks good to me - waiting for HudsonBot to give us its verdict.

          Show
          Henry Robinson added a comment - +1, patch looks good to me - waiting for HudsonBot to give us its verdict.
          Hide
          Patrick Hunt added a comment -

          FYI Kay, you want to "submit patch" link once you have a good patch, in order for hudson qabot to get triggered and trigger the addl review workflow (committer)

          Show
          Patrick Hunt added a comment - FYI Kay, you want to "submit patch" link once you have a good patch, in order for hudson qabot to get triggered and trigger the addl review workflow (committer)
          Hide
          Karthik K added a comment -

          1 /2 . Fixed
          3. Renamed the method with BFS suffix. Strategy of tree crawling can come in a subsequent patch, I guess and would be out of scope for this one.
          4.5. comments added.
          6. Left as it is .
          7. TODO-s should be out.

          ZooKeeperTest added with method - testDeleteRecursive() .

          callback for async of delete recursive needs to be more meaningful than what it is now.

          Show
          Karthik K added a comment - 1 /2 . Fixed 3. Renamed the method with BFS suffix. Strategy of tree crawling can come in a subsequent patch, I guess and would be out of scope for this one. 4.5. comments added. 6. Left as it is . 7. TODO-s should be out. ZooKeeperTest added with method - testDeleteRecursive() . callback for async of delete recursive needs to be more meaningful than what it is now.
          Hide
          Karthik K added a comment -

          Thanks Henry / Patrick for the review. Tests is something that I was looking to add, but could not find a template to start with. Will look into that a bit more over the w/e.

          Henry: as far as the command args goes, please see the couple of my comments above. I don't have a preference with rmr / -r , but I believe some light-weight library would help us a lot here and in the future to be more flexible. Will take care of the rest in the next patch.

          Show
          Karthik K added a comment - Thanks Henry / Patrick for the review. Tests is something that I was looking to add, but could not find a template to start with. Will look into that a bit more over the w/e. Henry: as far as the command args goes, please see the couple of my comments above. I don't have a preference with rmr / -r , but I believe some light-weight library would help us a lot here and in the future to be more flexible. Will take care of the rest in the next patch.
          Hide
          Patrick Hunt added a comment -

          Sorry Kay, we're trying to catchup with some things after the release... No excuses though. Thanks Henry for picking this up,
          esp the focus on testing. We're always looking for ppl interested in eventually becoming committers, this requires some
          significant contributions, esp with a focus on working with the community (doing great here) and focus on quality (testing).

          Take a look at this example of testing ZooKeeperMain code, you can probably base your own (maybe general
          ZooKeeperMainTest) off this:

          src/java/test/org/apache/zookeeper/test/ZooKeeperQuotaTest.java

          Show
          Patrick Hunt added a comment - Sorry Kay, we're trying to catchup with some things after the release... No excuses though. Thanks Henry for picking this up, esp the focus on testing. We're always looking for ppl interested in eventually becoming committers, this requires some significant contributions, esp with a focus on working with the community (doing great here) and focus on quality (testing). Take a look at this example of testing ZooKeeperMain code, you can probably base your own (maybe general ZooKeeperMainTest) off this: src/java/test/org/apache/zookeeper/test/ZooKeeperQuotaTest.java
          Hide
          Henry Robinson added a comment -

          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.

          Show
          Henry Robinson added a comment - 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.
          Hide
          Karthik K added a comment -

          Can you help look into this before this gets out of sync with the trunk. Thanks.

          Show
          Karthik K added a comment - Can you help look into this before this gets out of sync with the trunk. Thanks.
          Hide
          Karthik K added a comment -
          Is there some way to package it up such that the user doesn't have to type lots of classpath jars?

          A quick look at the examples, says that it is very light-weight , with not much dependencies. But because you have started publishing mvn artifacts with 3.3.0 , clients of zk should not be having a problem about the number, say.

          , just an issue of carrying around extra baggage.

          true. i brought this up, because for extending the command-line functionality and grouping options, it would make it simpler, without worrying about the position of the args and give a getopt-like feel .

          Show
          Karthik K added a comment - Is there some way to package it up such that the user doesn't have to type lots of classpath jars? A quick look at the examples, says that it is very light-weight , with not much dependencies. But because you have started publishing mvn artifacts with 3.3.0 , clients of zk should not be having a problem about the number, say. , just an issue of carrying around extra baggage. true. i brought this up, because for extending the command-line functionality and grouping options, it would make it simpler, without worrying about the position of the args and give a getopt-like feel .
          Hide
          Patrick Hunt added a comment -

          Can you make it optional similar to jline? I suspect not... Is there some way to package it up such that the user
          doesn't have to type lots of classpath jars?

          https://args4j.dev.java.net/

          the code is MIT license so that's ok, just an issue of carrying around extra baggage...

          Show
          Patrick Hunt added a comment - Can you make it optional similar to jline? I suspect not... Is there some way to package it up such that the user doesn't have to type lots of classpath jars? https://args4j.dev.java.net/ the code is MIT license so that's ok, just an issue of carrying around extra baggage...
          Hide
          Karthik K added a comment -

          With respect to the command line argument, out of curiosity , can we have something based on args4j or something similar to that for better flexibility of commands.

          Tried to fit 'delete -r' in the picture, but already $2 is reserved for the number of watchers. So, checking for $3 to be '-r' and making a decision , seems a little bit round-about since we will be ignoring $2 , for the number of watchers in the recursive delete scenario ( it is -1, ). Hence left that part as before.

          Show
          Karthik K added a comment - With respect to the command line argument, out of curiosity , can we have something based on args4j or something similar to that for better flexibility of commands. Tried to fit 'delete -r' in the picture, but already $2 is reserved for the number of watchers. So, checking for $3 to be '-r' and making a decision , seems a little bit round-about since we will be ignoring $2 , for the number of watchers in the recursive delete scenario ( it is -1, ). Hence left that part as before.
          Hide
          Karthik K added a comment -

          LOG is in, replacing System.out/.err for Zookeeper.

          Comments added to delete functionality indicating the expected behavior in case one of the sub-node deletions fails with an exception.

          new public method deleteRecursive() added , ( async method ).

          Show
          Karthik K added a comment - LOG is in, replacing System.out/.err for Zookeeper. Comments added to delete functionality indicating the expected behavior in case one of the sub-node deletions fails with an exception. new public method deleteRecursive() added , ( async method ).
          Hide
          Patrick Hunt added a comment -

          Given Henry's comments sync might be better, albeit slower. You could stop right away if you failed to delete a znode.

          Show
          Patrick Hunt added a comment - Given Henry's comments sync might be better, albeit slower. You could stop right away if you failed to delete a znode.
          Hide
          Patrick Hunt added a comment -

          Would it be ok if we have a separate jira to track it.

          sure. link it to this one.

          that was an inspiration from hdfs

          that's fine, I was just thinking to keep down the proliferation of cmds... (personally I would have modeled all the commands on unix fs commands...)

          not sure what effect async will have

          In ZK all operations are ordered. So if a single client session calls async delete(/foo/a/b) then async delete(/foo/a), then async delete(/foo) it will be deleted in that same order, guaranteed. You will get callbacks with success/fail of each operation, so you
          would need to wait for those to be sure. sync is fine, just saying async is faster which might be important if the tree is large...

          You should get rid of the System.out.println lines

          I think we want to use system out for user interaction, LOG is for debugging (at least that's what we're doing in that class today.)
          Typically in unix you don't message to the console for expected

          phunt@valhalla:~$ touch foo
          phunt@valhalla:~$ rm foo
          phunt@valhalla:~$

          just for error conditions. I'd put those printlns for delete into the LOG (success/debug information) and println only if the
          user's operation failed to do what was expected.

          Show
          Patrick Hunt added a comment - Would it be ok if we have a separate jira to track it. sure. link it to this one. that was an inspiration from hdfs that's fine, I was just thinking to keep down the proliferation of cmds... (personally I would have modeled all the commands on unix fs commands...) not sure what effect async will have In ZK all operations are ordered. So if a single client session calls async delete(/foo/a/b) then async delete(/foo/a), then async delete(/foo) it will be deleted in that same order, guaranteed. You will get callbacks with success/fail of each operation, so you would need to wait for those to be sure. sync is fine, just saying async is faster which might be important if the tree is large... You should get rid of the System.out.println lines I think we want to use system out for user interaction, LOG is for debugging (at least that's what we're doing in that class today.) Typically in unix you don't message to the console for expected phunt@valhalla:~$ touch foo phunt@valhalla:~$ rm foo phunt@valhalla:~$ just for error conditions. I'd put those printlns for delete into the LOG (success/debug information) and println only if the user's operation failed to do what was expected.
          Hide
          Karthik K added a comment -

          It would be great if you could include this change in the c client as well. Would you mind updating
          the patch for that as well?

          Would it be ok if we have a separate jira to track it. I can update the same, but the motivation for me was , when we reset a katta cluster , I wanted to have a single command to deal with resets. For consistency purposes, I can have a look at the C client as well.

          You added a new "rmr" command, rather could you just add a "-r" option to the delete command (see create for
          similar command line options on a command)

          makes sense. that was an inspiration from hdfs, to avoid keystrokes, as you might have figured out already

          I see that you are using sync operations, did you consider using async? It would be much faster and would be a
          great example for users

          I wanted to delete the nodes in order, the leaves and then the root, and was not sure what effect async will have , in that, w.r.t race conditions. Also I was testing from the cmd line and wanted to be sure that when the command returns, every child node has indeed been deleted.

          Can you add some more hints ?

          1. You should get rid of the System.out.println lines - use LOG if you want to save something.

          Oops. Saw, System.err elsewhere in ZookeeperMain and confused that to be the standard with Zookeeper .

          3. If delete fails due to a NoNode exception or similar, this operation aborts I think. I'm not sure whether that's a good idea or not, but it should be documented in the comments.

          I guess, aborting is ok, so we know what is happening. May be - there could be another parameter to indicate if we are continuing / aborting.

          Something I wanted to flag up - this algorithm is not wait-free because if another process is continually adding child nodes in the right place, this will never finish.

          This is true. As I had mentioned in the assumptions - this is mostly to be used as a 1-step reset for apps, and the client apps are assumed to have been brought down completely.
          May be- for now - indicating the information in the comments would do ?

          Show
          Karthik K added a comment - It would be great if you could include this change in the c client as well. Would you mind updating the patch for that as well? Would it be ok if we have a separate jira to track it. I can update the same, but the motivation for me was , when we reset a katta cluster , I wanted to have a single command to deal with resets. For consistency purposes, I can have a look at the C client as well. You added a new "rmr" command, rather could you just add a "-r" option to the delete command (see create for similar command line options on a command) makes sense. that was an inspiration from hdfs, to avoid keystrokes, as you might have figured out already I see that you are using sync operations, did you consider using async? It would be much faster and would be a great example for users I wanted to delete the nodes in order, the leaves and then the root, and was not sure what effect async will have , in that, w.r.t race conditions. Also I was testing from the cmd line and wanted to be sure that when the command returns, every child node has indeed been deleted. Can you add some more hints ? 1. You should get rid of the System.out.println lines - use LOG if you want to save something. Oops. Saw, System.err elsewhere in ZookeeperMain and confused that to be the standard with Zookeeper . 3. If delete fails due to a NoNode exception or similar, this operation aborts I think. I'm not sure whether that's a good idea or not, but it should be documented in the comments. I guess, aborting is ok, so we know what is happening. May be - there could be another parameter to indicate if we are continuing / aborting. Something I wanted to flag up - this algorithm is not wait-free because if another process is continually adding child nodes in the right place, this will never finish. This is true. As I had mentioned in the assumptions - this is mostly to be used as a 1-step reset for apps, and the client apps are assumed to have been brought down completely. May be- for now - indicating the information in the comments would do ?
          Hide
          Henry Robinson added a comment -

          Thanks for the patch, Kay!

          I have a couple of review comments:

          1. You should get rid of the System.out.println lines - use LOG if you want to save something.
          2. It might be cleaner to use a LinkedList of nodes, always prepending to the front, then you can walk through it in order, but I don't mind which you do.
          3. If delete fails due to a NoNode exception or similar, this operation aborts I think. I'm not sure whether that's a good idea or not, but it should be documented in the comments.

          Something I wanted to flag up - this algorithm is not wait-free because if another process is continually adding child nodes in the right place, this will never finish. It is, however, obstruction-free. ISTR that ZooKeeper's wait-freedom was a significant feature in one of Flavio's presentations that I read. Are we ok relaxing this restriction for the client libraries for an unlikely edge case?

          (My vote is yes, we probably are, but this is not just an academic distinction - you can build systems predicated on wait-freedom that will fail if they are actually just obstruction-free).

          Show
          Henry Robinson added a comment - Thanks for the patch, Kay! I have a couple of review comments: 1. You should get rid of the System.out.println lines - use LOG if you want to save something. 2. It might be cleaner to use a LinkedList of nodes, always prepending to the front, then you can walk through it in order, but I don't mind which you do. 3. If delete fails due to a NoNode exception or similar, this operation aborts I think. I'm not sure whether that's a good idea or not, but it should be documented in the comments. Something I wanted to flag up - this algorithm is not wait-free because if another process is continually adding child nodes in the right place, this will never finish. It is, however, obstruction-free. ISTR that ZooKeeper's wait-freedom was a significant feature in one of Flavio's presentations that I read. Are we ok relaxing this restriction for the client libraries for an unlikely edge case? (My vote is yes, we probably are, but this is not just an academic distinction - you can build systems predicated on wait-freedom that will fail if they are actually just obstruction-free).
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12439903/ZOOKEEPER-729.patch
          against trunk revision 925362.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no tests are needed for this patch.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/39/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/39/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/39/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12439903/ZOOKEEPER-729.patch against trunk revision 925362. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no tests are needed for this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/39/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/39/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/39/console This message is automatically generated.
          Hide
          Patrick Hunt added a comment -

          Thanks Kay, since this is a new feature and not a fix I'm marking for just 3.4.0

          It would be great if you could include this change in the c client as well. Would you mind updating
          the patch for that as well?

          You added a new "rmr" command, rather could you just add a "-r" option to the delete command (see create for
          similar command line options on a command)

          I see that you are using sync operations, did you consider using async? It would be much faster and would be a
          great example for users.

          Show
          Patrick Hunt added a comment - Thanks Kay, since this is a new feature and not a fix I'm marking for just 3.4.0 It would be great if you could include this change in the c client as well. Would you mind updating the patch for that as well? You added a new "rmr" command, rather could you just add a "-r" option to the delete command (see create for similar command line options on a command) I see that you are using sync operations, did you consider using async? It would be much faster and would be a great example for users.
          Hide
          Karthik K added a comment -

          Retrieve the entire tree on the client side and then start deleting from the leaves to the root.

          assumptions:
          the service performing the updates to the root znode is brought down completely and the recursive delete completes then .

          No atomicity guaranteed.

          An efficient version would involve modifying the .jute and sending the path to the server and deleting on the same. But then, for now - this works.

          Show
          Karthik K added a comment - Retrieve the entire tree on the client side and then start deleting from the leaves to the root. assumptions: the service performing the updates to the root znode is brought down completely and the recursive delete completes then . No atomicity guaranteed. An efficient version would involve modifying the .jute and sending the path to the server and deleting on the same. But then, for now - this works.

            People

            • Assignee:
              Karthik K
              Reporter:
              Karthik K
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development