This is an automated email from the ASF dual-hosted git repository. hulee pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/helix.git
commit 9381a80f82f55ec7dde7bfd68c7abe14d6145ce2 Author: Neal Sun <[email protected]> AuthorDate: Tue Apr 7 18:03:50 2020 -0700 Fix MetadataStoreDirectory routing data cache refresh bug (#933) This PR ensures that routing data cache is cleared before updating in ZkMetadataStoreDirectory. Also, this PR fixes an edge case during TrieRoutingData creation when no zk realms has any sharding key. --- .../metadatastore/ZkMetadataStoreDirectory.java | 5 ++++ .../TestZkMetadataStoreDirectory.java | 31 ++++++++++++++++++++++ .../helix/msdcommon/datamodel/TrieRoutingData.java | 19 +++++++++++++ .../msdcommon/datamodel/TestTrieRoutingData.java | 8 ++++++ 4 files changed, 63 insertions(+) diff --git a/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/ZkMetadataStoreDirectory.java b/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/ZkMetadataStoreDirectory.java index 42b2b17..a869618 100644 --- a/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/ZkMetadataStoreDirectory.java +++ b/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/ZkMetadataStoreDirectory.java @@ -321,6 +321,11 @@ public class ZkMetadataStoreDirectory implements MetadataStoreDirectory, Routing return; } + // Remove the raw data first in case of failure on creation + _realmToShardingKeysMap.remove(namespace); + // Remove routing data first in case of failure on creation + _routingDataMap.remove(namespace); + Map<String, List<String>> rawRoutingData; try { rawRoutingData = _routingDataReaderMap.get(namespace).getRoutingData(); diff --git a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java index 47d6fba..71e96ff 100644 --- a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java +++ b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java @@ -319,6 +319,37 @@ public class TestZkMetadataStoreDirectory extends AbstractTestClass { }, TestHelper.WAIT_DURATION)); } + @Test(dependsOnMethods = "testChildChangeCallback") + public void testDataDeletionCallback() throws Exception { + // For all namespaces, delete all routing data + _zkList.forEach(zk -> { + ZkClient zkClient = ZK_SERVER_MAP.get(zk).getZkClient(); + if (zkClient.exists(MetadataStoreRoutingConstants.ROUTING_DATA_PATH)) { + for (String zkRealm : zkClient + .getChildren(MetadataStoreRoutingConstants.ROUTING_DATA_PATH)) { + zkClient.delete(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + zkRealm); + } + } + } + ); + + // Confirm that TrieRoutingData has been removed due to missing routing data + Assert.assertTrue(TestHelper.verify(() -> { + for (String namespace : _routingZkAddrMap.keySet()) { + try { + _metadataStoreDirectory.getMetadataStoreRealm(namespace, "anyKey"); + return false; + } catch (IllegalStateException e) { + if (!e.getMessage().equals("Failed to get metadata store realm: Namespace " + namespace + + " contains either empty or invalid routing data!")) { + return false; + } + } + } + return true; + }, TestHelper.WAIT_DURATION)); + } + private void clearRoutingData() throws Exception { Assert.assertTrue(TestHelper.verify(() -> { for (String zk : _zkList) { diff --git a/metadata-store-directory-common/src/main/java/org/apache/helix/msdcommon/datamodel/TrieRoutingData.java b/metadata-store-directory-common/src/main/java/org/apache/helix/msdcommon/datamodel/TrieRoutingData.java index 94bb331..b174357 100644 --- a/metadata-store-directory-common/src/main/java/org/apache/helix/msdcommon/datamodel/TrieRoutingData.java +++ b/metadata-store-directory-common/src/main/java/org/apache/helix/msdcommon/datamodel/TrieRoutingData.java @@ -48,6 +48,10 @@ public class TrieRoutingData implements MetadataStoreRoutingData { throw new InvalidRoutingDataException("routingData cannot be null or empty"); } + if (!containsShardingKey(routingData)) { + throw new InvalidRoutingDataException("routingData needs at least 1 sharding key"); + } + if (isRootShardingKey(routingData)) { Map.Entry<String, List<String>> entry = routingData.entrySet().iterator().next(); _rootNode = new TrieNode(Collections.emptyMap(), "/", true, entry.getKey()); @@ -167,6 +171,21 @@ public class TrieRoutingData implements MetadataStoreRoutingData { } /* + * Checks if there is any sharding key in the routing data + * @param routingData - a mapping from "sharding keys" to "realm addresses" to be parsed into a + * trie + * @return whether there is any sharding key + */ + private boolean containsShardingKey(Map<String, List<String>> routingData) { + for (Map.Entry<String, List<String>> entry : routingData.entrySet()) { + if (entry.getValue().size() > 0) { + return true; + } + } + return false; + } + + /* * Checks for the edge case when the only sharding key in provided routing data is the delimiter. * When this is the case, the trie is valid and contains only one node, which * is the root node, and the root node is a leaf node with a realm address associated with it. diff --git a/metadata-store-directory-common/src/test/java/org/apache/helix/msdcommon/datamodel/TestTrieRoutingData.java b/metadata-store-directory-common/src/test/java/org/apache/helix/msdcommon/datamodel/TestTrieRoutingData.java index 2dc5dfe..44b7e6e 100644 --- a/metadata-store-directory-common/src/test/java/org/apache/helix/msdcommon/datamodel/TestTrieRoutingData.java +++ b/metadata-store-directory-common/src/test/java/org/apache/helix/msdcommon/datamodel/TestTrieRoutingData.java @@ -48,6 +48,14 @@ public class TestTrieRoutingData { } catch (InvalidRoutingDataException e) { Assert.assertTrue(e.getMessage().contains("routingData cannot be null or empty")); } + Map<String, List<String>> routingData = new HashMap<>(); + routingData.put("realmAddress", Collections.emptyList()); + try { + new TrieRoutingData(routingData); + Assert.fail("Expecting InvalidRoutingDataException"); + } catch (InvalidRoutingDataException e) { + Assert.assertTrue(e.getMessage().contains("routingData needs at least 1 sharding key")); + } } /**
