Maxwell-Guo commented on code in PR #2458:
URL: https://github.com/apache/cassandra/pull/2458#discussion_r1247382945
##########
src/java/org/apache/cassandra/locator/AlibabaCloudSnitch.java:
##########
@@ -17,130 +17,47 @@
*/
package org.apache.cassandra.locator;
-import java.io.DataInputStream;
-import java.io.FilterInputStream;
import java.io.IOException;
-import java.net.HttpURLConnection;
-import java.net.MalformedURLException;
-import java.net.SocketTimeoutException;
-import java.net.URL;
-import java.nio.charset.StandardCharsets;
-import java.util.Map;
-import org.apache.cassandra.db.SystemKeyspace;
-import org.apache.cassandra.exceptions.ConfigurationException;
-import org.apache.cassandra.gms.ApplicationState;
-import org.apache.cassandra.gms.EndpointState;
-import org.apache.cassandra.gms.Gossiper;
-import org.apache.cassandra.io.util.FileUtils;
-import org.apache.cassandra.utils.FBUtilities;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import
org.apache.cassandra.locator.AbstractCloudMetadataServiceConnector.DefaultCloudMetadataServiceConnector;
+
+import static
org.apache.cassandra.locator.AbstractCloudMetadataServiceConnector.METADATA_URL_PROPERTY;
/**
- * A snitch that assumes an ECS region is a DC and an ECS availability_zone
- * is a rack. This information is available in the config for the node. the
- * format of the zone-id is like :cn-hangzhou-a where cn means china, hangzhou
- * means the hangzhou region, a means the az id. We use cn-hangzhou as the dc,
- * and f as the zone-id.
+ * A snitch that assumes an ECS region is a DC and an ECS availability_zone
+ * is a rack. This information is available in the config for the node. the
+ * format of the zone-id is like :cn-hangzhou-a where cn means china, hangzhou
+ * means the hangzhou region, a means the az id. We use cn-hangzhou as the dc,
+ * and f as the zone-id.
*/
-public class AlibabaCloudSnitch extends AbstractNetworkTopologySnitch
+public class AlibabaCloudSnitch extends AbstractCloudMetadataServiceSnitch
{
- protected static final Logger logger =
LoggerFactory.getLogger(AlibabaCloudSnitch.class);
- protected static final String ZONE_NAME_QUERY_URL =
"http://100.100.100.200/latest/meta-data/zone-id";;
- private static final String DEFAULT_DC = "UNKNOWN-DC";
- private static final String DEFAULT_RACK = "UNKNOWN-RACK";
- private Map<InetAddressAndPort, Map<String, String>> savedEndpoints;
- protected String ecsZone;
- protected String ecsRegion;
-
- private static final int HTTP_CONNECT_TIMEOUT = 30000;
-
-
- public AlibabaCloudSnitch() throws MalformedURLException, IOException
- {
- String response = alibabaApiCall(ZONE_NAME_QUERY_URL);
- String[] splits = response.split("/");
- String az = splits[splits.length - 1];
-
- // Split "us-central1-a" or "asia-east1-a" into "us-central1"/"a" and
"asia-east1"/"a".
- splits = az.split("-");
- ecsZone = splits[splits.length - 1];
-
- int lastRegionIndex = az.lastIndexOf("-");
- ecsRegion = az.substring(0, lastRegionIndex);
+ private static final String DEFAULT_METADATA_SERVICE_URL =
"http://100.100.100.200";;
+ private static final String ZONE_NAME_QUERY_URL =
"/latest/meta-data/zone-id";
- String datacenterSuffix = (new SnitchProperties()).get("dc_suffix",
"");
- ecsRegion = ecsRegion.concat(datacenterSuffix);
- logger.info("AlibabaSnitch using region: {}, zone: {}.", ecsRegion,
ecsZone);
-
- }
-
- String alibabaApiCall(String url) throws ConfigurationException,
IOException, SocketTimeoutException
+ public AlibabaCloudSnitch() throws IOException
{
- // Populate the region and zone by introspection, fail if 404 on
metadata
- HttpURLConnection conn = (HttpURLConnection) new
URL(url).openConnection();
- DataInputStream d = null;
- try
- {
- conn.setConnectTimeout(HTTP_CONNECT_TIMEOUT);
- conn.setRequestMethod("GET");
-
- int code = conn.getResponseCode();
- if (code != HttpURLConnection.HTTP_OK)
- throw new ConfigurationException("AlibabaSnitch was unable to
execute the API call. Not an ecs node? and the returun code is " + code);
-
- // Read the information. I wish I could say (String)
conn.getContent() here...
- int cl = conn.getContentLength();
- byte[] b = new byte[cl];
- d = new DataInputStream((FilterInputStream) conn.getContent());
- d.readFully(b);
- return new String(b, StandardCharsets.UTF_8);
- }
- catch (SocketTimeoutException e)
- {
- throw new SocketTimeoutException("Timeout occurred reading a
response from the Alibaba ECS metadata");
- }
- finally
- {
- FileUtils.close(d);
- conn.disconnect();
- }
+ this(new SnitchProperties());
}
-
- @Override
- public String getRack(InetAddressAndPort endpoint)
+
+ public AlibabaCloudSnitch(SnitchProperties properties) throws IOException
{
- if (endpoint.equals(FBUtilities.getBroadcastAddressAndPort()))
- return ecsZone;
- EndpointState state =
Gossiper.instance.getEndpointStateForEndpoint(endpoint);
- if (state == null || state.getApplicationState(ApplicationState.RACK)
== null)
- {
- if (savedEndpoints == null)
- savedEndpoints = SystemKeyspace.loadDcRackInfo();
- if (savedEndpoints.containsKey(endpoint))
- return savedEndpoints.get(endpoint).get("rack");
- return DEFAULT_RACK;
- }
- return state.getApplicationState(ApplicationState.RACK).value;
-
+ this(properties, new
DefaultCloudMetadataServiceConnector(properties.get(METADATA_URL_PROPERTY,
Review Comment:
what about merge METADATA_URL_PROPERTY and ZONE_NAME_QUERY_URL together as
before not just split them into two Strings and make them configurable.
or make ZONE_NAME_QUERY_URL configurable through Snitchpropetry ? (In this
way I may suggest changing the name to ZONE_NAME_QUERY_URL_PATH)
##########
src/java/org/apache/cassandra/locator/AlibabaCloudSnitch.java:
##########
@@ -17,130 +17,47 @@
*/
package org.apache.cassandra.locator;
-import java.io.DataInputStream;
-import java.io.FilterInputStream;
import java.io.IOException;
-import java.net.HttpURLConnection;
-import java.net.MalformedURLException;
-import java.net.SocketTimeoutException;
-import java.net.URL;
-import java.nio.charset.StandardCharsets;
-import java.util.Map;
-import org.apache.cassandra.db.SystemKeyspace;
-import org.apache.cassandra.exceptions.ConfigurationException;
-import org.apache.cassandra.gms.ApplicationState;
-import org.apache.cassandra.gms.EndpointState;
-import org.apache.cassandra.gms.Gossiper;
-import org.apache.cassandra.io.util.FileUtils;
-import org.apache.cassandra.utils.FBUtilities;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import
org.apache.cassandra.locator.AbstractCloudMetadataServiceConnector.DefaultCloudMetadataServiceConnector;
+
+import static
org.apache.cassandra.locator.AbstractCloudMetadataServiceConnector.METADATA_URL_PROPERTY;
/**
- * A snitch that assumes an ECS region is a DC and an ECS availability_zone
- * is a rack. This information is available in the config for the node. the
- * format of the zone-id is like :cn-hangzhou-a where cn means china, hangzhou
- * means the hangzhou region, a means the az id. We use cn-hangzhou as the dc,
- * and f as the zone-id.
+ * A snitch that assumes an ECS region is a DC and an ECS availability_zone
+ * is a rack. This information is available in the config for the node. the
+ * format of the zone-id is like :cn-hangzhou-a where cn means china, hangzhou
+ * means the hangzhou region, a means the az id. We use cn-hangzhou as the dc,
+ * and f as the zone-id.
Review Comment:
sorry I make a misstake at the first time push this pr, this should be "a as
the zone-id" not f.
##########
src/java/org/apache/cassandra/locator/AbstractCloudMetadataServiceConnector.java:
##########
@@ -25,22 +25,39 @@
import java.net.URL;
import java.util.Map;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import static java.nio.charset.StandardCharsets.UTF_8;
abstract class AbstractCloudMetadataServiceConnector
{
+ static final String METADATA_URL_PROPERTY = "metadata_url";
+
+ public static class DefaultCloudMetadataServiceConnector extends
AbstractCloudMetadataServiceConnector
+ {
+ protected DefaultCloudMetadataServiceConnector(String
metadataServiceUrl)
Review Comment:
as said before , what about make this metaserviceUrl with the full url path
,not only the url address.
Or make both zone path configurable which may be more flexible.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
