You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by hu...@apache.org on 2020/08/14 18:14:03 UTC
[helix] 08/12: Make RoutingDataManager a pure Singleton with
double-checked locking
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 <hu...@linkedin.com>
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,