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 f001cde49f5ede583154855865ea0450496d48d0 Author: Hunter Lee <[email protected]> AuthorDate: Fri Jul 24 15:49:07 2020 -0700 Make RoutingDataManager a pure Singleton with double-checked locking RoutingDataManager was a stateful static class, but since it contains caches, it would be better to make it a Singleton. This commit makes it a singleton and updates the code accordingly. --- .../helix/manager/zk/GenericZkHelixApiBuilder.java | 11 +++--- .../org/apache/helix/manager/zk/ZKHelixAdmin.java | 5 ++- .../multizk/TestMultiZkHelixJavaApis.java | 2 +- .../apache/helix/rest/server/ServerContext.java | 2 +- .../zookeeper/impl/client/DedicatedZkClient.java | 4 +- .../zookeeper/impl/client/FederatedZkClient.java | 4 +- .../zookeeper/impl/client/SharedZkClient.java | 4 +- .../zookeeper/routing/RoutingDataManager.java | 45 +++++++++++++++------- .../zookeeper/util/TestRoutingDataManager.java | 10 ++--- 9 files changed, 53 insertions(+), 34 deletions(-) diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/GenericZkHelixApiBuilder.java b/helix-core/src/main/java/org/apache/helix/manager/zk/GenericZkHelixApiBuilder.java index d02f67e..1a4a69b 100644 --- a/helix-core/src/main/java/org/apache/helix/manager/zk/GenericZkHelixApiBuilder.java +++ b/helix-core/src/main/java/org/apache/helix/manager/zk/GenericZkHelixApiBuilder.java @@ -203,11 +203,12 @@ public abstract class GenericZkHelixApiBuilder<B extends GenericZkHelixApiBuilde boolean isRoutingDataSourceEndpointSet = connectionConfig.getRoutingDataSourceEndpoint() != null && !connectionConfig .getRoutingDataSourceEndpoint().isEmpty(); - MetadataStoreRoutingData routingData = isRoutingDataSourceEndpointSet ? RoutingDataManager - .getMetadataStoreRoutingData( - RoutingDataReaderType.lookUp(connectionConfig.getRoutingDataSourceType()), - connectionConfig.getRoutingDataSourceEndpoint()) - : RoutingDataManager.getMetadataStoreRoutingData(); + MetadataStoreRoutingData routingData = + isRoutingDataSourceEndpointSet ? RoutingDataManager.getInstance() + .getMetadataStoreRoutingData( + RoutingDataReaderType.lookUp(connectionConfig.getRoutingDataSourceType()), + connectionConfig.getRoutingDataSourceEndpoint()) + : RoutingDataManager.getInstance().getMetadataStoreRoutingData(); return routingData.getMetadataStoreRealm(connectionConfig.getZkRealmShardingKey()); } } diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java index 1759116..1b3eb8f 100644 --- a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java +++ b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java @@ -969,9 +969,10 @@ public class ZKHelixAdmin implements HelixAdmin { _zkClient.getRealmAwareZkConnectionConfig().getRoutingDataSourceEndpoint(); if (routingDataSourceEndpoint == null || routingDataSourceEndpoint.isEmpty()) { // If endpoint is not given explicitly, use HTTP and the endpoint set in System Properties - realmToShardingKeys = RoutingDataManager.getRawRoutingData(); + realmToShardingKeys = RoutingDataManager.getInstance().getRawRoutingData(); } else { - realmToShardingKeys = RoutingDataManager.getRawRoutingData(RoutingDataReaderType + realmToShardingKeys = RoutingDataManager.getInstance().getRawRoutingData( + RoutingDataReaderType .lookUp(_zkClient.getRealmAwareZkConnectionConfig().getRoutingDataSourceType()), routingDataSourceEndpoint); } diff --git a/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java b/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java index d72318a..f48a96f 100644 --- a/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java +++ b/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java @@ -1039,7 +1039,7 @@ public class TestMultiZkHelixJavaApis { .setRoutingDataSourceEndpoint(_msdsEndpoint).build(); // Reset cached routing data - RoutingDataManager.reset(); + RoutingDataManager.getInstance().reset(); // Shutdown MSDS to ensure that these accessors are able to pull routing data from ZK _msds.stopServer(); diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/ServerContext.java b/helix-rest/src/main/java/org/apache/helix/rest/server/ServerContext.java index 176180f..dc19da7 100644 --- a/helix-rest/src/main/java/org/apache/helix/rest/server/ServerContext.java +++ b/helix-rest/src/main/java/org/apache/helix/rest/server/ServerContext.java @@ -323,7 +323,7 @@ public class ServerContext implements IZkDataListener, IZkChildListener, IZkStat _zkAddr); try { // Reset RoutingDataManager's cache - RoutingDataManager.reset(); + RoutingDataManager.getInstance().reset(); // All Helix APIs will be closed implicitly because ZkClient is closed if (_zkClient != null && !_zkClient.isClosed()) { _zkClient.close(); diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/DedicatedZkClient.java b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/DedicatedZkClient.java index dbe64f3..4527fe8 100644 --- a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/DedicatedZkClient.java +++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/DedicatedZkClient.java @@ -85,9 +85,9 @@ public class DedicatedZkClient implements RealmAwareZkClient { String routingDataSourceEndpoint = connectionConfig.getRoutingDataSourceEndpoint(); if (routingDataSourceEndpoint == null || routingDataSourceEndpoint.isEmpty()) { // If endpoint is not given explicitly, use HTTP and the endpoint set in System Properties - _metadataStoreRoutingData = RoutingDataManager.getMetadataStoreRoutingData(); + _metadataStoreRoutingData = RoutingDataManager.getInstance().getMetadataStoreRoutingData(); } else { - _metadataStoreRoutingData = RoutingDataManager.getMetadataStoreRoutingData( + _metadataStoreRoutingData = RoutingDataManager.getInstance().getMetadataStoreRoutingData( RoutingDataReaderType.lookUp(connectionConfig.getRoutingDataSourceType()), routingDataSourceEndpoint); } diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/FederatedZkClient.java b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/FederatedZkClient.java index 01e7fc7..11d0291 100644 --- a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/FederatedZkClient.java +++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/FederatedZkClient.java @@ -98,9 +98,9 @@ public class FederatedZkClient implements RealmAwareZkClient { String routingDataSourceEndpoint = connectionConfig.getRoutingDataSourceEndpoint(); if (routingDataSourceEndpoint == null || routingDataSourceEndpoint.isEmpty()) { // If endpoint is not given explicitly, use HTTP and the endpoint set in System Properties - _metadataStoreRoutingData = RoutingDataManager.getMetadataStoreRoutingData(); + _metadataStoreRoutingData = RoutingDataManager.getInstance().getMetadataStoreRoutingData(); } else { - _metadataStoreRoutingData = RoutingDataManager.getMetadataStoreRoutingData( + _metadataStoreRoutingData = RoutingDataManager.getInstance().getMetadataStoreRoutingData( RoutingDataReaderType.lookUp(connectionConfig.getRoutingDataSourceType()), routingDataSourceEndpoint); } diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/SharedZkClient.java b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/SharedZkClient.java index 7c3a562..14a4794 100644 --- a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/SharedZkClient.java +++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/SharedZkClient.java @@ -78,9 +78,9 @@ public class SharedZkClient implements RealmAwareZkClient { String routingDataSourceEndpoint = connectionConfig.getRoutingDataSourceEndpoint(); if (routingDataSourceEndpoint == null || routingDataSourceEndpoint.isEmpty()) { // If endpoint is not given explicitly, use HTTP and the endpoint set in System Properties - _metadataStoreRoutingData = RoutingDataManager.getMetadataStoreRoutingData(); + _metadataStoreRoutingData = RoutingDataManager.getInstance().getMetadataStoreRoutingData(); } else { - _metadataStoreRoutingData = RoutingDataManager.getMetadataStoreRoutingData( + _metadataStoreRoutingData = RoutingDataManager.getInstance().getMetadataStoreRoutingData( RoutingDataReaderType.lookUp(connectionConfig.getRoutingDataSourceType()), routingDataSourceEndpoint); } diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/routing/RoutingDataManager.java b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/routing/RoutingDataManager.java index e489088..bfc12aa 100644 --- a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/routing/RoutingDataManager.java +++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/routing/RoutingDataManager.java @@ -33,28 +33,47 @@ import org.apache.helix.zookeeper.exception.MultiZkException; /** - * RoutingDataManager is a static Singleton that + * RoutingDataManager is a Singleton that * 1. resolves RoutingDataReader based on the system config given * 2. caches routing data * 3. provides public methods for reading routing data from various sources (configurable) */ public class RoutingDataManager { /** HTTP call to MSDS is used to fetch routing data by default */ - private static final String DEFAULT_MSDS_ENDPOINT = + private final String DEFAULT_MSDS_ENDPOINT = System.getProperty(MetadataStoreRoutingConstants.MSDS_SERVER_ENDPOINT_KEY); /** Double-checked locking requires that the following fields be final (volatile) */ // The following map stands for (RoutingDataReaderType_endpoint ID, Raw Routing Data) - private static final Map<String, Map<String, List<String>>> _rawRoutingDataMap = + private final Map<String, Map<String, List<String>>> _rawRoutingDataMap = new ConcurrentHashMap<>(); // The following map stands for (RoutingDataReaderType_endpoint ID, MetadataStoreRoutingData) - private static final Map<String, MetadataStoreRoutingData> _metadataStoreRoutingDataMap = + private final Map<String, MetadataStoreRoutingData> _metadataStoreRoutingDataMap = new ConcurrentHashMap<>(); + // Singleton instance + private static RoutingDataManager _instance; + /** * This class is a Singleton. */ private RoutingDataManager() { + // Private constructor for Singleton + } + + /** + * Lazy initialization with double-checked locking. + * @return + */ + public static RoutingDataManager getInstance() { + if (_instance == null) { + synchronized (RoutingDataManager.class) { + if (_instance == null) { + _instance = new RoutingDataManager(); + } + } + } + return _instance; } /** @@ -62,7 +81,7 @@ public class RoutingDataManager { * config. * @return */ - public static Map<String, List<String>> getRawRoutingData() { + public Map<String, List<String>> getRawRoutingData() { if (DEFAULT_MSDS_ENDPOINT == null || DEFAULT_MSDS_ENDPOINT.isEmpty()) { throw new IllegalStateException( "HttpRoutingDataReader was unable to find a valid MSDS endpoint String in System " @@ -79,8 +98,8 @@ public class RoutingDataManager { * @param routingDataReaderType * @param endpoint */ - public static Map<String, List<String>> getRawRoutingData( - RoutingDataReaderType routingDataReaderType, String endpoint) { + public Map<String, List<String>> getRawRoutingData(RoutingDataReaderType routingDataReaderType, + String endpoint) { String routingDataCacheKey = getRoutingDataCacheKey(routingDataReaderType, endpoint); Map<String, List<String>> rawRoutingData = _rawRoutingDataMap.get(routingDataCacheKey); if (rawRoutingData == null) { @@ -101,8 +120,7 @@ public class RoutingDataManager { * MSDS configured in the JVM config. * @return MetadataStoreRoutingData */ - public static MetadataStoreRoutingData getMetadataStoreRoutingData() - throws InvalidRoutingDataException { + public MetadataStoreRoutingData getMetadataStoreRoutingData() throws InvalidRoutingDataException { if (DEFAULT_MSDS_ENDPOINT == null || DEFAULT_MSDS_ENDPOINT.isEmpty()) { throw new IllegalStateException( "HttpRoutingDataReader was unable to find a valid MSDS endpoint String in System " @@ -119,7 +137,7 @@ public class RoutingDataManager { * @throws IOException * @throws InvalidRoutingDataException */ - public static MetadataStoreRoutingData getMetadataStoreRoutingData( + public MetadataStoreRoutingData getMetadataStoreRoutingData( RoutingDataReaderType routingDataReaderType, String endpoint) throws InvalidRoutingDataException { String routingDataCacheKey = getRoutingDataCacheKey(routingDataReaderType, endpoint); @@ -141,7 +159,7 @@ public class RoutingDataManager { /** * Clears the statically-cached routing data and private fields. */ - public synchronized static void reset() { + public synchronized void reset() { _rawRoutingDataMap.clear(); _metadataStoreRoutingDataMap.clear(); } @@ -151,8 +169,7 @@ public class RoutingDataManager { * @param routingDataReaderType * @return */ - private static RoutingDataReader resolveRoutingDataReader( - RoutingDataReaderType routingDataReaderType) { + private RoutingDataReader resolveRoutingDataReader(RoutingDataReaderType routingDataReaderType) { // Instantiate an instance of routing data reader using the type try { return (RoutingDataReader) Class.forName(routingDataReaderType.getClassName()).newInstance(); @@ -169,7 +186,7 @@ public class RoutingDataManager { * @param endpoint * @return */ - private static String getRoutingDataCacheKey(RoutingDataReaderType routingDataReaderType, + private String getRoutingDataCacheKey(RoutingDataReaderType routingDataReaderType, String endpoint) { if (routingDataReaderType == null) { throw new MultiZkException("RoutingDataManager: RoutingDataReaderType cannot be null!"); diff --git a/zookeeper-api/src/test/java/org/apache/helix/zookeeper/util/TestRoutingDataManager.java b/zookeeper-api/src/test/java/org/apache/helix/zookeeper/util/TestRoutingDataManager.java index 2fcba96..57b59ee 100644 --- a/zookeeper-api/src/test/java/org/apache/helix/zookeeper/util/TestRoutingDataManager.java +++ b/zookeeper-api/src/test/java/org/apache/helix/zookeeper/util/TestRoutingDataManager.java @@ -68,14 +68,14 @@ public class TestRoutingDataManager extends ZkTestBase { @Test public void testGetRawRoutingData() { - Map<String, List<String>> rawRoutingData = RoutingDataManager.getRawRoutingData(); + Map<String, List<String>> rawRoutingData = RoutingDataManager.getInstance().getRawRoutingData(); TestConstants.FAKE_ROUTING_DATA.forEach((realm, keys) -> Assert .assertEquals(new HashSet(rawRoutingData.get(realm)), new HashSet(keys))); } @Test(dependsOnMethods = "testGetRawRoutingData") - public void testGetMetadataStoreRoutingData() throws IOException, InvalidRoutingDataException { - MetadataStoreRoutingData data = RoutingDataManager.getMetadataStoreRoutingData(); + public void testGetMetadataStoreRoutingData() throws InvalidRoutingDataException { + MetadataStoreRoutingData data = RoutingDataManager.getInstance().getMetadataStoreRoutingData(); Map<String, String> allMappings = data.getAllMappingUnderPath("/"); Map<String, Set<String>> groupedMappings = allMappings.entrySet().stream().collect(Collectors .groupingBy(Map.Entry::getValue, @@ -103,7 +103,7 @@ public class TestRoutingDataManager extends ZkTestBase { // HttpRoutingDataReader should still return old data because it's static // Make sure the results don't contain the new realm - Map<String, List<String>> rawRoutingData = RoutingDataManager.getRawRoutingData(); + Map<String, List<String>> rawRoutingData = RoutingDataManager.getInstance().getRawRoutingData(); Assert.assertFalse(rawRoutingData.containsKey(newRealm)); // Remove newRealm and check for equality @@ -112,7 +112,7 @@ public class TestRoutingDataManager extends ZkTestBase { TestConstants.FAKE_ROUTING_DATA.forEach((realm, keys) -> Assert .assertEquals(new HashSet(rawRoutingData.get(realm)), new HashSet(keys))); - MetadataStoreRoutingData data = RoutingDataManager.getMetadataStoreRoutingData(); + MetadataStoreRoutingData data = RoutingDataManager.getInstance().getMetadataStoreRoutingData(); Map<String, String> allMappings = data.getAllMappingUnderPath("/"); Map<String, Set<String>> groupedMappings = allMappings.entrySet().stream().collect(Collectors .groupingBy(Map.Entry::getValue,
