You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/07/21 06:50:46 UTC
[GitHub] [helix] narendly opened a new pull request #1163: Implement RoutingDataManager to replace HttpRoutingDataReader
narendly opened a new pull request #1163:
URL: https://github.com/apache/helix/pull/1163
### Issues
- [x] My PR addresses the following Helix issues and references them in the PR description:
Resolves #1162
### Description
- [x] Here are some details about my PR, including screenshots of any UI changes:
We want to make the routing data source configurable. As such, using HttpRoutingDataReader as the static Singleton that fetches and caches the routing data is no longer appropriate. This change adds an implementation of RoutingDataManager and the new RoutingDataReader interface, with HttpRoutingDataReader as one of its implementations. This is the first step towards making routing data source configurable - other readers will be added in the future commits.
### Tests
- [x] The following tests are written for this issue:
TestRoutingDataManager.java
- [x] The following is the result of the "mvn test" command on the appropriate module:
zookeeper-api:
```
[INFO] Tests run: 37, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 54.08 s - in TestSuite
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 37, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO] -----------------
[INFO] BUILD SUCCESS
```
helix-core:
### Commits
- [x] My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation (Optional)
- [ ] In case of new functionality, my PR adds documentation in the following wiki page:
(Link the GitHub wiki you added)
### Code Quality
- [x] My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] dasahcc commented on a change in pull request #1163: Implement RoutingDataManager to replace HttpRoutingDataReader
Posted by GitBox <gi...@apache.org>.
dasahcc commented on a change in pull request #1163:
URL: https://github.com/apache/helix/pull/1163#discussion_r459643553
##########
File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/routing/RoutingDataManager.java
##########
@@ -0,0 +1,179 @@
+package org.apache.helix.zookeeper.routing;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.helix.msdcommon.constant.MetadataStoreRoutingConstants;
+import org.apache.helix.msdcommon.datamodel.MetadataStoreRoutingData;
+import org.apache.helix.msdcommon.datamodel.TrieRoutingData;
+import org.apache.helix.msdcommon.exception.InvalidRoutingDataException;
+import org.apache.helix.zookeeper.constant.RoutingDataReaderType;
+import org.apache.helix.zookeeper.exception.MultiZkException;
+
+
+/**
+ * RoutingDataManager is a static 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 =
+ 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 =
+ new ConcurrentHashMap<>();
+ // The following map stands for (RoutingDataReaderType_endpoint ID, MetadataStoreRoutingData)
+ private static final Map<String, MetadataStoreRoutingData> _metadataStoreRoutingDataMap =
+ new ConcurrentHashMap<>();
+
+ /**
+ * This class is a Singleton.
+ */
+ private RoutingDataManager() {
+ }
+
+ /**
+ * Fetches routing data from the data source via HTTP by querying the MSDS configured in the JVM
+ * config.
+ * @return
+ */
+ public static 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 "
+ + "Properties!");
+ }
+ return getRawRoutingData(RoutingDataReaderType.HTTP, DEFAULT_MSDS_ENDPOINT);
Review comment:
This should be configurable in your next PR, right?
##########
File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/routing/RoutingDataManager.java
##########
@@ -0,0 +1,179 @@
+package org.apache.helix.zookeeper.routing;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.helix.msdcommon.constant.MetadataStoreRoutingConstants;
+import org.apache.helix.msdcommon.datamodel.MetadataStoreRoutingData;
+import org.apache.helix.msdcommon.datamodel.TrieRoutingData;
+import org.apache.helix.msdcommon.exception.InvalidRoutingDataException;
+import org.apache.helix.zookeeper.constant.RoutingDataReaderType;
+import org.apache.helix.zookeeper.exception.MultiZkException;
+
+
+/**
+ * RoutingDataManager is a static 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 =
+ 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 =
+ new ConcurrentHashMap<>();
+ // The following map stands for (RoutingDataReaderType_endpoint ID, MetadataStoreRoutingData)
+ private static final Map<String, MetadataStoreRoutingData> _metadataStoreRoutingDataMap =
+ new ConcurrentHashMap<>();
+
+ /**
+ * This class is a Singleton.
+ */
+ private RoutingDataManager() {
+ }
+
+ /**
+ * Fetches routing data from the data source via HTTP by querying the MSDS configured in the JVM
+ * config.
+ * @return
+ */
+ public static 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 "
+ + "Properties!");
+ }
+ return getRawRoutingData(RoutingDataReaderType.HTTP, DEFAULT_MSDS_ENDPOINT);
+ }
+
+ /**
+ * Fetches routing data from the data source via HTTP.
+ * @return a mapping from "metadata store realm addresses" to lists of
+ * "metadata store sharding keys", where the sharding keys in a value list all route to
+ * the realm address in the key disallows a meaningful mapping to be returned.
+ * @param routingDataReaderType
+ * @param endpoint
+ */
+ public static 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) {
+ synchronized (RoutingDataManager.class) {
+ rawRoutingData = _rawRoutingDataMap.get(routingDataCacheKey);
+ if (rawRoutingData == null) {
+ RoutingDataReader reader = resolveRoutingDataReader(routingDataReaderType);
+ rawRoutingData = reader.getRawRoutingData(endpoint);
+ _rawRoutingDataMap.put(routingDataCacheKey, rawRoutingData);
+ }
+ }
+ }
+ return rawRoutingData;
+ }
+
+ /**
+ * Returns the routing data read from MSDS in a MetadataStoreRoutingData format by querying the
+ * MSDS configured in the JVM config.
+ * @return MetadataStoreRoutingData
+ */
+ public static 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 "
+ + "Properties!");
+ }
+ return getMetadataStoreRoutingData(RoutingDataReaderType.HTTP, DEFAULT_MSDS_ENDPOINT);
+ }
+
+ /**
+ * Returns the routing data read from MSDS as a MetadataStoreRoutingData object.
+ * @param routingDataReaderType
+ * @param endpoint
+ * @return
+ * @throws IOException
+ * @throws InvalidRoutingDataException
+ */
+ public static MetadataStoreRoutingData getMetadataStoreRoutingData(
+ RoutingDataReaderType routingDataReaderType, String endpoint)
+ throws InvalidRoutingDataException {
+ String routingDataCacheKey = getRoutingDataCacheKey(routingDataReaderType, endpoint);
+ MetadataStoreRoutingData metadataStoreRoutingData =
+ _metadataStoreRoutingDataMap.get(routingDataCacheKey);
+ if (metadataStoreRoutingData == null) {
+ synchronized (RoutingDataManager.class) {
+ metadataStoreRoutingData = _metadataStoreRoutingDataMap.get(routingDataCacheKey);
+ if (metadataStoreRoutingData == null) {
+ metadataStoreRoutingData =
+ new TrieRoutingData(getRawRoutingData(routingDataReaderType, endpoint));
+ _metadataStoreRoutingDataMap.put(routingDataCacheKey, metadataStoreRoutingData);
+ }
+ }
+ }
+ return metadataStoreRoutingData;
+ }
+
+ /**
+ * Clears the statically-cached routing data and private fields.
+ */
+ public synchronized static void reset() {
+ _rawRoutingDataMap.clear();
+ _metadataStoreRoutingDataMap.clear();
+ }
+
+ /**
+ * Returns an appropriate instance of RoutingDataReader given the type.
+ * @param routingDataReaderType
+ * @return
+ */
+ private static RoutingDataReader resolveRoutingDataReader(
+ RoutingDataReaderType routingDataReaderType) {
+ // Instantiate an instance of routing data reader using the type
+ try {
+ return (RoutingDataReader) Class.forName(routingDataReaderType.getClassName()).newInstance();
+ } catch (ClassNotFoundException | IllegalAccessException | InstantiationException e) {
+ throw new MultiZkException(
Review comment:
Do we want to add some logs here for debugging purpose? Because rely on exception stack trace could be too long when we handle it.
##########
File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/routing/HttpRoutingDataReader.java
##########
@@ -0,0 +1,132 @@
+package org.apache.helix.zookeeper.routing;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.helix.msdcommon.constant.MetadataStoreRoutingConstants;
+import org.apache.helix.msdcommon.datamodel.MetadataStoreRoutingData;
+import org.apache.helix.msdcommon.datamodel.TrieRoutingData;
+import org.apache.helix.msdcommon.exception.InvalidRoutingDataException;
+import org.apache.helix.zookeeper.exception.MultiZkException;
+import org.apache.http.HttpEntity;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.DefaultBackoffStrategy;
+import org.apache.http.impl.client.DefaultHttpRequestRetryHandler;
+import org.apache.http.impl.client.HttpClients;
+import org.apache.http.util.EntityUtils;
+
+
+/**
+ * HTTP-based RoutingDataReader that makes an HTTP call to Metadata Store Directory Service (REST)
+ * to fetch routing data.
+ */
+public class HttpRoutingDataReader implements RoutingDataReader {
+ private static final int DEFAULT_HTTP_TIMEOUT_IN_MS = 5000;
+
+ /**
+ * Returns an object form of metadata store routing data.
+ * @param endpoint
+ * @return
+ */
+ @Override
+ public MetadataStoreRoutingData getMetadataStoreRoutingData(String endpoint) {
+ try {
+ return new TrieRoutingData(getRawRoutingData(endpoint));
+ } catch (InvalidRoutingDataException e) {
+ throw new MultiZkException(e);
+ }
+ }
+
+ /**
+ * Returns a map form of metadata store routing data.
+ * The map fields stand for metadata store realm address (key), and a corresponding list of ZK
+ * path sharding keys (key).
+ * @param endpoint
+ * @return
+ */
+ @Override
+ public Map<String, List<String>> getRawRoutingData(String endpoint) {
+ try {
+ String routingDataJson = getAllRoutingData(endpoint);
+ return parseRoutingData(routingDataJson);
+ } catch (IOException e) {
+ throw new MultiZkException(e);
+ }
+ }
+
+ /**
+ * Makes an HTTP call to fetch all routing data.
+ * @return
+ * @throws IOException
+ */
+ private String getAllRoutingData(String msdsEndpoint) throws IOException {
+ // Note that MSDS_ENDPOINT should provide high-availability - it risks becoming a single point
+ // of failure if it's backed by a single IP address/host
+ // Retry count is 3 by default.
+ HttpGet requestAllData = new HttpGet(
+ msdsEndpoint + MetadataStoreRoutingConstants.MSDS_GET_ALL_ROUTING_DATA_ENDPOINT);
+
+ // Define timeout configs
+ RequestConfig config = RequestConfig.custom().setConnectTimeout(DEFAULT_HTTP_TIMEOUT_IN_MS)
+ .setConnectionRequestTimeout(DEFAULT_HTTP_TIMEOUT_IN_MS)
+ .setSocketTimeout(DEFAULT_HTTP_TIMEOUT_IN_MS).build();
+
+ try (CloseableHttpClient httpClient = HttpClients.custom().setDefaultRequestConfig(config)
+ .setConnectionBackoffStrategy(new DefaultBackoffStrategy())
+ .setRetryHandler(new DefaultHttpRequestRetryHandler()).build()) {
+ // Return the JSON because try-resources clause closes the CloseableHttpResponse
+ HttpEntity entity = httpClient.execute(requestAllData).getEntity();
+ if (entity == null) {
+ throw new IOException("Response's entity is null!");
+ }
+ return EntityUtils.toString(entity, "UTF-8");
+ }
Review comment:
Shall we have finally for returning something if there some exception in the try-with-resource statement?
##########
File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/routing/RoutingDataManager.java
##########
@@ -0,0 +1,179 @@
+package org.apache.helix.zookeeper.routing;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.helix.msdcommon.constant.MetadataStoreRoutingConstants;
+import org.apache.helix.msdcommon.datamodel.MetadataStoreRoutingData;
+import org.apache.helix.msdcommon.datamodel.TrieRoutingData;
+import org.apache.helix.msdcommon.exception.InvalidRoutingDataException;
+import org.apache.helix.zookeeper.constant.RoutingDataReaderType;
+import org.apache.helix.zookeeper.exception.MultiZkException;
+
+
+/**
+ * RoutingDataManager is a static 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 =
+ 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 =
+ new ConcurrentHashMap<>();
+ // The following map stands for (RoutingDataReaderType_endpoint ID, MetadataStoreRoutingData)
+ private static final Map<String, MetadataStoreRoutingData> _metadataStoreRoutingDataMap =
+ new ConcurrentHashMap<>();
+
+ /**
+ * This class is a Singleton.
+ */
+ private RoutingDataManager() {
+ }
Review comment:
If we would like to make it as singleton, we dont have to make all methods to be static but just add a static method to get the singleton instance. Somehow, this implementation mixing static utils with singleton implementation.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1163: Implement RoutingDataManager to replace HttpRoutingDataReader
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1163:
URL: https://github.com/apache/helix/pull/1163#discussion_r458489015
##########
File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/routing/RoutingDataManager.java
##########
@@ -0,0 +1,180 @@
+package org.apache.helix.zookeeper.routing;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.helix.msdcommon.constant.MetadataStoreRoutingConstants;
+import org.apache.helix.msdcommon.datamodel.MetadataStoreRoutingData;
+import org.apache.helix.msdcommon.datamodel.TrieRoutingData;
+import org.apache.helix.msdcommon.exception.InvalidRoutingDataException;
+import org.apache.helix.zookeeper.constant.RoutingDataReaderType;
+import org.apache.helix.zookeeper.exception.MultiZkException;
+
+
+/**
+ * RoutingDataManager is a static 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 =
+ 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 =
+ new ConcurrentHashMap<>();
+ // The following map stands for (RoutingDataReaderType_endpoint ID, MetadataStoreRoutingData)
+ private static final Map<String, MetadataStoreRoutingData> _metadataStoreRoutingDataMap =
+ new ConcurrentHashMap<>();
+
+ /**
+ * This class is a Singleton.
+ */
+ private RoutingDataManager() {
+ }
+
+ /**
+ * Fetches routing data from the data source via HTTP by querying the MSDS configured in the JVM
+ * config.
+ * @return
+ */
+ public static 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 "
+ + "Properties!");
+ }
+ return getRawRoutingData(RoutingDataReaderType.HTTP, DEFAULT_MSDS_ENDPOINT);
+ }
+
+ /**
+ * Fetches routing data from the data source via HTTP.
+ * @return a mapping from "metadata store realm addresses" to lists of
+ * "metadata store sharding keys", where the sharding keys in a value list all route to
+ * the realm address in the key disallows a meaningful mapping to be returned.
+ * @param routingDataReaderType
+ * @param endpoint
+ */
+ public static 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) {
+ synchronized (RoutingDataManager.class) {
+ rawRoutingData = _rawRoutingDataMap.get(routingDataCacheKey);
+ if (rawRoutingData == null) {
+ RoutingDataReader reader = resolveRoutingDataReader(routingDataReaderType);
+ rawRoutingData = reader.getRawRoutingData(endpoint);
+ _rawRoutingDataMap.put(routingDataCacheKey, rawRoutingData);
+ }
+ }
+ }
+ return rawRoutingData;
+ }
+
+ /**
+ * Returns the routing data read from MSDS in a MetadataStoreRoutingData format by querying the
+ * MSDS configured in the JVM config.
+ * @return MetadataStoreRoutingData
+ */
+ public static 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 "
+ + "Properties!");
+ }
+ return getMetadataStoreRoutingData(RoutingDataReaderType.HTTP, DEFAULT_MSDS_ENDPOINT);
+ }
+
+ /**
+ * Returns the routing data read from MSDS as a MetadataStoreRoutingData object.
+ * @param routingDataReaderType
+ * @param endpoint
+ * @return
+ * @throws IOException
+ * @throws InvalidRoutingDataException
+ */
+ public static MetadataStoreRoutingData getMetadataStoreRoutingData(
+ RoutingDataReaderType routingDataReaderType, String endpoint)
+ throws InvalidRoutingDataException {
+ String routingDataCacheKey = getRoutingDataCacheKey(routingDataReaderType, endpoint);
+ MetadataStoreRoutingData metadataStoreRoutingData =
+ _metadataStoreRoutingDataMap.get(routingDataCacheKey);
+ if (metadataStoreRoutingData == null) {
+ synchronized (RoutingDataManager.class) {
+ metadataStoreRoutingData = _metadataStoreRoutingDataMap.get(routingDataCacheKey);
+ if (metadataStoreRoutingData == null) {
+ metadataStoreRoutingData =
+ new TrieRoutingData(getRawRoutingData(routingDataReaderType, endpoint));
+ _metadataStoreRoutingDataMap.put(routingDataCacheKey, metadataStoreRoutingData);
+ }
+ }
+ }
+ return metadataStoreRoutingData;
+ }
+
+ /**
+ * Clears the statically-cached routing data and private fields.
+ */
+ public synchronized static void reset() {
+ _rawRoutingDataMap.clear();
+ _metadataStoreRoutingDataMap.clear();
+ }
+
+ /**
+ * Returns an appropriate instance of RoutingDataReader given the type.
+ * @param routingDataReaderType
+ * @return
+ */
+ private static RoutingDataReader resolveRoutingDataReader(
+ RoutingDataReaderType routingDataReaderType) {
+ // RoutingDataReaderType.HTTP by default if not found
+ routingDataReaderType =
+ routingDataReaderType == null ? RoutingDataReaderType.HTTP : routingDataReaderType;
+
+ // Instantiate an instance of routing data reader using the type
+ try {
+ return (RoutingDataReader) Class.forName(routingDataReaderType.getClassName()).newInstance();
+ } catch (ClassNotFoundException | IllegalAccessException | InstantiationException e) {
+ throw new MultiZkException(
+ "RoutingDataManager: failed to instantiate RoutingDataReader! ReaderType className: "
+ + routingDataReaderType.getClassName(), e);
+ }
+ }
+
+ /**
+ * Constructs a key for the cache lookup.
+ * @param routingDataReaderType
+ * @param endpoint
+ * @return
+ */
+ private static String getRoutingDataCacheKey(RoutingDataReaderType routingDataReaderType,
+ String endpoint) {
+ return routingDataReaderType.name() + "_" + endpoint;
Review comment:
Good comment. `resolveRoutingDataReader`'s check should move to `getRoutingDataCacheKey`, and we shouldn't expect the type to be null. Updated.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1163: Implement RoutingDataManager to replace HttpRoutingDataReader
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1163:
URL: https://github.com/apache/helix/pull/1163#discussion_r458488787
##########
File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/constant/RoutingSystemPropertyKeys.java
##########
@@ -0,0 +1,31 @@
+package org.apache.helix.zookeeper.constant;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * This class contains various routing-related system property keys for multi-zk clients.
+ */
+public class RoutingSystemPropertyKeys {
+
+ /**
+ * This static constant is used to refer to which implementation of RoutingDataReader to use.
+ */
+ public static final String ROUTING_DATA_READER_TYPE = "routing.data.reader.type";
Review comment:
Yes. This will be used in later PRs.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly closed pull request #1163: Implement RoutingDataManager to replace HttpRoutingDataReader
Posted by GitBox <gi...@apache.org>.
narendly closed pull request #1163:
URL: https://github.com/apache/helix/pull/1163
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] NealSun96 commented on a change in pull request #1163: Implement RoutingDataManager to replace HttpRoutingDataReader
Posted by GitBox <gi...@apache.org>.
NealSun96 commented on a change in pull request #1163:
URL: https://github.com/apache/helix/pull/1163#discussion_r458438417
##########
File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/routing/RoutingDataManager.java
##########
@@ -0,0 +1,180 @@
+package org.apache.helix.zookeeper.routing;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.helix.msdcommon.constant.MetadataStoreRoutingConstants;
+import org.apache.helix.msdcommon.datamodel.MetadataStoreRoutingData;
+import org.apache.helix.msdcommon.datamodel.TrieRoutingData;
+import org.apache.helix.msdcommon.exception.InvalidRoutingDataException;
+import org.apache.helix.zookeeper.constant.RoutingDataReaderType;
+import org.apache.helix.zookeeper.exception.MultiZkException;
+
+
+/**
+ * RoutingDataManager is a static 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 =
+ 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 =
+ new ConcurrentHashMap<>();
+ // The following map stands for (RoutingDataReaderType_endpoint ID, MetadataStoreRoutingData)
+ private static final Map<String, MetadataStoreRoutingData> _metadataStoreRoutingDataMap =
+ new ConcurrentHashMap<>();
+
+ /**
+ * This class is a Singleton.
+ */
+ private RoutingDataManager() {
+ }
+
+ /**
+ * Fetches routing data from the data source via HTTP by querying the MSDS configured in the JVM
+ * config.
+ * @return
+ */
+ public static 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 "
+ + "Properties!");
+ }
+ return getRawRoutingData(RoutingDataReaderType.HTTP, DEFAULT_MSDS_ENDPOINT);
+ }
+
+ /**
+ * Fetches routing data from the data source via HTTP.
+ * @return a mapping from "metadata store realm addresses" to lists of
+ * "metadata store sharding keys", where the sharding keys in a value list all route to
+ * the realm address in the key disallows a meaningful mapping to be returned.
+ * @param routingDataReaderType
+ * @param endpoint
+ */
+ public static 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) {
+ synchronized (RoutingDataManager.class) {
+ rawRoutingData = _rawRoutingDataMap.get(routingDataCacheKey);
+ if (rawRoutingData == null) {
+ RoutingDataReader reader = resolveRoutingDataReader(routingDataReaderType);
+ rawRoutingData = reader.getRawRoutingData(endpoint);
+ _rawRoutingDataMap.put(routingDataCacheKey, rawRoutingData);
+ }
+ }
+ }
+ return rawRoutingData;
+ }
+
+ /**
+ * Returns the routing data read from MSDS in a MetadataStoreRoutingData format by querying the
+ * MSDS configured in the JVM config.
+ * @return MetadataStoreRoutingData
+ */
+ public static 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 "
+ + "Properties!");
+ }
+ return getMetadataStoreRoutingData(RoutingDataReaderType.HTTP, DEFAULT_MSDS_ENDPOINT);
+ }
+
+ /**
+ * Returns the routing data read from MSDS as a MetadataStoreRoutingData object.
+ * @param routingDataReaderType
+ * @param endpoint
+ * @return
+ * @throws IOException
+ * @throws InvalidRoutingDataException
+ */
+ public static MetadataStoreRoutingData getMetadataStoreRoutingData(
+ RoutingDataReaderType routingDataReaderType, String endpoint)
+ throws InvalidRoutingDataException {
+ String routingDataCacheKey = getRoutingDataCacheKey(routingDataReaderType, endpoint);
+ MetadataStoreRoutingData metadataStoreRoutingData =
+ _metadataStoreRoutingDataMap.get(routingDataCacheKey);
+ if (metadataStoreRoutingData == null) {
+ synchronized (RoutingDataManager.class) {
+ metadataStoreRoutingData = _metadataStoreRoutingDataMap.get(routingDataCacheKey);
+ if (metadataStoreRoutingData == null) {
+ metadataStoreRoutingData =
+ new TrieRoutingData(getRawRoutingData(routingDataReaderType, endpoint));
+ _metadataStoreRoutingDataMap.put(routingDataCacheKey, metadataStoreRoutingData);
+ }
+ }
+ }
+ return metadataStoreRoutingData;
+ }
+
+ /**
+ * Clears the statically-cached routing data and private fields.
+ */
+ public synchronized static void reset() {
+ _rawRoutingDataMap.clear();
+ _metadataStoreRoutingDataMap.clear();
+ }
+
+ /**
+ * Returns an appropriate instance of RoutingDataReader given the type.
+ * @param routingDataReaderType
+ * @return
+ */
+ private static RoutingDataReader resolveRoutingDataReader(
+ RoutingDataReaderType routingDataReaderType) {
+ // RoutingDataReaderType.HTTP by default if not found
+ routingDataReaderType =
+ routingDataReaderType == null ? RoutingDataReaderType.HTTP : routingDataReaderType;
+
+ // Instantiate an instance of routing data reader using the type
+ try {
+ return (RoutingDataReader) Class.forName(routingDataReaderType.getClassName()).newInstance();
+ } catch (ClassNotFoundException | IllegalAccessException | InstantiationException e) {
+ throw new MultiZkException(
+ "RoutingDataManager: failed to instantiate RoutingDataReader! ReaderType className: "
+ + routingDataReaderType.getClassName(), e);
+ }
+ }
+
+ /**
+ * Constructs a key for the cache lookup.
+ * @param routingDataReaderType
+ * @param endpoint
+ * @return
+ */
+ private static String getRoutingDataCacheKey(RoutingDataReaderType routingDataReaderType,
+ String endpoint) {
+ return routingDataReaderType.name() + "_" + endpoint;
Review comment:
If in `resolveRoutingDataReader()` we assume `routingDataReaderType` could be null, it's possible to have a NullPointerExcpetion here.
##########
File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/constant/RoutingSystemPropertyKeys.java
##########
@@ -0,0 +1,31 @@
+package org.apache.helix.zookeeper.constant;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * This class contains various routing-related system property keys for multi-zk clients.
+ */
+public class RoutingSystemPropertyKeys {
+
+ /**
+ * This static constant is used to refer to which implementation of RoutingDataReader to use.
+ */
+ public static final String ROUTING_DATA_READER_TYPE = "routing.data.reader.type";
Review comment:
This doesn't have a usage right now. I assume it will be used once the reader type is configurable by the users?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org