HELIX-1269: improve semantics for BaseDataAccessor.remove()
Project: http://git-wip-us.apache.org/repos/asf/helix/repo Commit: http://git-wip-us.apache.org/repos/asf/helix/commit/a6863937 Tree: http://git-wip-us.apache.org/repos/asf/helix/tree/a6863937 Diff: http://git-wip-us.apache.org/repos/asf/helix/diff/a6863937 Branch: refs/heads/master Commit: a6863937c7d404ffdf703d8f2f7a0735b41ea197 Parents: ce167f5 Author: Harry Zhang <[email protected]> Authored: Tue Aug 28 12:18:27 2018 -0700 Committer: Junkai Xue <[email protected]> Committed: Mon Oct 29 13:47:54 2018 -0700 ---------------------------------------------------------------------- .../helix/manager/zk/ZkBaseDataAccessor.java | 21 ++++++++++++++------ .../manager/zk/TestZkBaseDataAccessor.java | 3 ++- 2 files changed, 17 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/helix/blob/a6863937/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java ---------------------------------------------------------------------- diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java b/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java index 7d0032e..6811766 100644 --- a/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java +++ b/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java @@ -530,18 +530,27 @@ public class ZkBaseDataAccessor<T> implements BaseDataAccessor<T> { } /** - * sync remove + * Sync remove. it tries to remove the ZNode and all its descendants if any, node does not exist + * is regarded as success */ @Override public boolean remove(String path, int options) { try { - // optimize on common path - return _zkClient.delete(path); + // operation will not throw exception when path successfully deleted or does not exist + // despite real error, operation will throw exception when path not empty, and in this + // case, we try to delete recursively + _zkClient.delete(path); } catch (ZkException e) { - LOG.warn(String.format("Caught exception when deleting %s with options %s.", path, options), - e); - return _zkClient.deleteRecursive(path); + LOG.debug("Failed to delete {} with opts {}, err: {}. Try recursive delete", path, options, + e.getMessage()); + try { + _zkClient.deleteRecursively(path); + } catch (HelixException he) { + LOG.error("Failed to delete {} recursively with opts {}.", path, options, he); + return false; + } } + return true; } /** http://git-wip-us.apache.org/repos/asf/helix/blob/a6863937/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBaseDataAccessor.java ---------------------------------------------------------------------- diff --git a/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBaseDataAccessor.java b/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBaseDataAccessor.java index bee1f9a..671ce80 100644 --- a/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBaseDataAccessor.java +++ b/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBaseDataAccessor.java @@ -243,8 +243,9 @@ public class TestZkBaseDataAccessor extends ZkUnitTestBase { ZNRecord record = new ZNRecord("msg_0"); ZkBaseDataAccessor<ZNRecord> accessor = new ZkBaseDataAccessor<ZNRecord>(_gZkClient); + // Base data accessor shall not fail when remove a non-exist path boolean success = accessor.remove(path, 0); - Assert.assertFalse(success); + Assert.assertTrue(success); success = accessor.create(path, record, AccessOption.PERSISTENT); Assert.assertTrue(success);
