narendly commented on a change in pull request #731: Add TrieRoutingData constructor URL: https://github.com/apache/helix/pull/731#discussion_r377260364
########## File path: helix-rest/src/main/java/org/apache/helix/rest/metadatastore/ZkMetadataStoreDirectory.java ########## @@ -156,34 +156,42 @@ public boolean deleteShardingKey(String namespace, String realm, String sharding /** * Callback for updating the cached routing data. - * Note: this method should not synchronize on the class or the map. We do not want namespaces blocking each other. + * Note: this method should not synchronize on the class or the map. We do not want namespaces + * blocking each other. * Threadsafe map is used for _realmToShardingKeysMap. - * The global consistency of the in-memory routing data is not a requirement (eventual consistency is enough). + * The global consistency of the in-memory routing data is not a requirement (eventual consistency + * is enough). * @param namespace */ @Override public void refreshRoutingData(String namespace) { - // Safe to ignore the callback if routingDataMap is null. + // Safe to ignore the callback if any of the mapping is null. // If routingDataMap is null, then it will be populated by the constructor anyway // If routingDataMap is not null, then it's safe for the callback function to update it + if (_routingZkAddressMap == null || _routingDataMap == null || _realmToShardingKeysMap == null) { + LOG.error("Construction is not completed! "); + return; + } // Check if namespace exists; otherwise, return as a NOP and log it if (!_routingZkAddressMap.containsKey(namespace)) { - LOG.error("Failed to refresh internally-cached routing data! Namespace not found: " + namespace); + LOG.error("Failed to refresh internally-cached routing data! Namespace not found: {}", + namespace); + return; } try { - _realmToShardingKeysMap.put(namespace, _routingDataReaderMap.get(namespace).getRoutingData()); + Map<String, List<String>> rawRoutingData = + _routingDataReaderMap.get(namespace).getRoutingData(); + _realmToShardingKeysMap.put(namespace, rawRoutingData); + + MetadataStoreRoutingData routingData = new TrieRoutingData(rawRoutingData); + _routingDataMap.put(namespace, routingData); } catch (InvalidRoutingDataException e) { - LOG.error("Failed to get routing data for namespace: " + namespace + "!"); + LOG.error("Failed to refresh cached routing data for namespace {}, exception: {}", namespace, + e.getMessage()); Review comment: Nit: usually passing the error itself is enough: `LOG.error("Failed to refresh cached routing data for namespace {}", namespace, e);` ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org