You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "Maxwell-Guo (via GitHub)" <gi...@apache.org> on 2023/06/30 03:57:14 UTC
[GitHub] [cassandra] Maxwell-Guo commented on a diff in pull request #2458: CASSANDRA-18438
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org