Repository: cassandra Updated Branches: refs/heads/trunk 8a5e1cbe2 -> fd44a69fc
Use standard Amazon naming for datacenter and rack in Ec2Snitch. patch by Gregory Ramsperger and jasobrown; reviewed by Joey Lynch for CASSANDRA-7839 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/fd44a69f Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/fd44a69f Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/fd44a69f Branch: refs/heads/trunk Commit: fd44a69fce135c6ddf7ebcba25d032a36a1d9064 Parents: 8a5e1cb Author: Gregory Ramsperger <ramsper...@users.noreply.github.com> Authored: Sat Apr 16 00:42:03 2016 -0500 Committer: Jason Brown <jasedbr...@gmail.com> Committed: Mon Apr 16 17:59:06 2018 -0700 ---------------------------------------------------------------------- CHANGES.txt | 1 + NEWS.txt | 4 + conf/cassandra-rackdc.properties | 12 ++ .../locator/DynamicEndpointSnitch.java | 5 + .../org/apache/cassandra/locator/Ec2Snitch.java | 109 +++++++++++++++-- .../cassandra/locator/IEndpointSnitch.java | 9 ++ .../cassandra/locator/SnitchProperties.java | 2 +- .../cassandra/service/StorageService.java | 26 ++++ test/conf/cassandra-rackdc.properties | 12 ++ .../apache/cassandra/locator/EC2SnitchTest.java | 118 ++++++++++++++++++- 10 files changed, 279 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/fd44a69f/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 0362d01..71d947a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0 + * Use standard Amazon naming for datacenter and rack in Ec2Snitch (CASSANDRA-7839) * Fix junit failure for SSTableReaderTest (CASSANDRA-14387) * Abstract write path for pluggable storage (CASSANDRA-14118) * nodetool describecluster should be more informative (CASSANDRA-13853) http://git-wip-us.apache.org/repos/asf/cassandra/blob/fd44a69f/NEWS.txt ---------------------------------------------------------------------- diff --git a/NEWS.txt b/NEWS.txt index df4c32a..3be52dc 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -141,6 +141,10 @@ Upgrading storage port or native port at nodes you must first upgrade the entire cluster and clients to 4.0 so they can handle the port not being consistent across the cluster. + - Names of AWS regions/availability zones have been cleaned up to more correctly + match the Amazon names. There is now a new option in conf/cassandra-rackdc.properties + that lets users enable the correct names for new clusters, or use the legacy + names for existing clusters. See conf/cassandra-rackdc.properties for details. Materialized Views ------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/fd44a69f/conf/cassandra-rackdc.properties ---------------------------------------------------------------------- diff --git a/conf/cassandra-rackdc.properties b/conf/cassandra-rackdc.properties index 2ea6043..cc472b4 100644 --- a/conf/cassandra-rackdc.properties +++ b/conf/cassandra-rackdc.properties @@ -25,3 +25,15 @@ rack=rack1 # Uncomment the following line to make this snitch prefer the internal ip when possible, as the Ec2MultiRegionSnitch does. # prefer_local=true + +# Datacenter and rack naming convention used by the Ec2Snitch and Ec2MultiRegionSnitch. +# Options are: +# legacy : datacenter name is the part of the availability zone name preceding the last "-" +# when the zone ends in -1 and includes the number if not -1. Rack is the portion of +# the availability zone name following the last "-". +# Examples: us-west-1a => dc: us-west, rack: 1a; us-west-2b => dc: us-west-2, rack: 2b; +# YOU MUST USE THIS VALUE IF YOU ARE UPGRADING A PRE-4.0 CLUSTER +# standard : Default value. datacenter name is the standard AWS region name, including the number. +# rack name is the region plus the availability zone letter. +# Examples: us-west-1a => dc: us-west-1, rack: us-west-1a; us-west-2b => dc: us-west-2, rack: us-west-2b; +# ec2_naming_scheme=standard http://git-wip-us.apache.org/repos/asf/cassandra/blob/fd44a69f/src/java/org/apache/cassandra/locator/DynamicEndpointSnitch.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/locator/DynamicEndpointSnitch.java b/src/java/org/apache/cassandra/locator/DynamicEndpointSnitch.java index 9ea7e05..010c892 100644 --- a/src/java/org/apache/cassandra/locator/DynamicEndpointSnitch.java +++ b/src/java/org/apache/cassandra/locator/DynamicEndpointSnitch.java @@ -448,4 +448,9 @@ public class DynamicEndpointSnitch extends AbstractEndpointSnitch implements ILa } return maxScore; } + + public boolean validate(Set<String> datacenters, Set<String> racks) + { + return subsnitch.validate(datacenters, racks); + } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/fd44a69f/src/java/org/apache/cassandra/locator/Ec2Snitch.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/locator/Ec2Snitch.java b/src/java/org/apache/cassandra/locator/Ec2Snitch.java index c7324c8..b6aafd3 100644 --- a/src/java/org/apache/cassandra/locator/Ec2Snitch.java +++ b/src/java/org/apache/cassandra/locator/Ec2Snitch.java @@ -24,7 +24,9 @@ import java.net.HttpURLConnection; import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Map; +import java.util.Set; +import com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.cassandra.db.SystemKeyspace; @@ -42,30 +44,63 @@ import org.apache.cassandra.utils.FBUtilities; public class Ec2Snitch extends AbstractNetworkTopologySnitch { protected static final Logger logger = LoggerFactory.getLogger(Ec2Snitch.class); - protected static final String ZONE_NAME_QUERY_URL = "http://169.254.169.254/latest/meta-data/placement/availability-zone";; + + private static final String SNITCH_PROP_NAMING_SCHEME = "ec2_naming_scheme"; + static final String EC2_NAMING_LEGACY = "legacy"; + private static final String EC2_NAMING_STANDARD = "standard"; + + private static final String ZONE_NAME_QUERY_URL = "http://169.254.169.254/latest/meta-data/placement/availability-zone";; private static final String DEFAULT_DC = "UNKNOWN-DC"; private static final String DEFAULT_RACK = "UNKNOWN-RACK"; + + final String ec2region; + private final String ec2zone; + private final boolean usingLegacyNaming; + private Map<InetAddressAndPort, Map<String, String>> savedEndpoints; - protected String ec2zone; - protected String ec2region; public Ec2Snitch() throws IOException, ConfigurationException { + this(new SnitchProperties()); + } + + public Ec2Snitch(SnitchProperties props) throws IOException, ConfigurationException + { String az = awsApiCall(ZONE_NAME_QUERY_URL); - // Split "us-east-1a" or "asia-1a" into "us-east"/"1a" and "asia"/"1a". - String[] splits = az.split("-"); - ec2zone = splits[splits.length - 1]; - // hack for CASSANDRA-4026 - ec2region = az.substring(0, az.length() - 1); - if (ec2region.endsWith("1")) - ec2region = az.substring(0, az.length() - 3); + // if using the full naming scheme, region name is created by removing letters from the + // end of the availability zone and zone is the full zone name + usingLegacyNaming = isUsingLegacyNaming(props); + String region; + if (usingLegacyNaming) + { + // Split "us-east-1a" or "asia-1a" into "us-east"/"1a" and "asia"/"1a". + String[] splits = az.split("-"); + ec2zone = splits[splits.length - 1]; + + // hack for CASSANDRA-4026 + region = az.substring(0, az.length() - 1); + if (region.endsWith("1")) + region = az.substring(0, az.length() - 3); + } + else + { + // grab the region name, which is embedded in the availability zone name. + // thus an AZ of "us-east-1a" yields the region name "us-east-1" + region = az.replaceFirst("[a-z]+$",""); + ec2zone = az; + } - String datacenterSuffix = (new SnitchProperties()).get("dc_suffix", ""); - ec2region = ec2region.concat(datacenterSuffix); + String datacenterSuffix = props.get("dc_suffix", ""); + ec2region = region.concat(datacenterSuffix); logger.info("EC2Snitch using region: {}, zone: {}.", ec2region, ec2zone); } + private static boolean isUsingLegacyNaming(SnitchProperties props) + { + return props.get(SNITCH_PROP_NAMING_SCHEME, EC2_NAMING_STANDARD).equalsIgnoreCase(EC2_NAMING_LEGACY); + } + String awsApiCall(String url) throws IOException, ConfigurationException { // Populate the region and zone by introspection, fail if 404 on metadata @@ -122,4 +157,54 @@ public class Ec2Snitch extends AbstractNetworkTopologySnitch } return state.getApplicationState(ApplicationState.DC).value; } + + @Override + public boolean validate(Set<String> datacenters, Set<String> racks) + { + return validate(datacenters, racks, usingLegacyNaming); + } + + @VisibleForTesting + static boolean validate(Set<String> datacenters, Set<String> racks, boolean usingLegacyNaming) + { + boolean valid = true; + + for (String dc : datacenters) + { + // predicated on the late-2017 AWS naming 'convention' that all region names end with a digit. + // Unfortunately, life isn't that simple. Since we allow custom datacenter suffixes (CASSANDRA-5155), + // an operator could conceiveably make the suffix "a", and thus create a region name that looks just like + // one of the region's availability zones. (for example, "us-east-1a"). + // Further, we can't make any assumptions of what that suffix might be by looking at this node's + // datacenterSuffix as conceivably their could be many different suffixes in play for a given region. + // + // Thus, the best we can do is make sure the region name follows + // the basic region naming pattern: "us-east-1<custom-suffix>" + boolean dcUsesLegacyFormat = !dc.matches("[a-z]+-[a-z].+-[\\d].*"); + if (dcUsesLegacyFormat != usingLegacyNaming) + valid = false; + } + + for (String rack : racks) + { + // predicated on late-2017 AWS naming 'convention' that AZs do not have a digit as the first char - + // we had that in our legacy AZ (rack) names. Thus we test to see if the rack is in the legacy format. + // + // NOTE: the allowed custom suffix only applies to datacenter (region) names, not availability zones. + boolean rackUsesLegacyFormat = rack.matches("[\\d][a-z]"); + if (rackUsesLegacyFormat != usingLegacyNaming) + valid = false; + } + + if (!valid) + { + logger.error("This ec2-enabled snitch appears to be using the {} naming scheme for regions, " + + "but existing nodes in cluster are using the opposite: region(s) = {}, availability zone(s) = {}. " + + "Please check the {} property in the {} configuration file for more details.", + usingLegacyNaming ? "legacy" : "standard", datacenters, racks, + SNITCH_PROP_NAMING_SCHEME, SnitchProperties.RACKDC_PROPERTY_FILENAME); + } + + return valid; + } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/fd44a69f/src/java/org/apache/cassandra/locator/IEndpointSnitch.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/locator/IEndpointSnitch.java b/src/java/org/apache/cassandra/locator/IEndpointSnitch.java index 00a1543..63d333b 100644 --- a/src/java/org/apache/cassandra/locator/IEndpointSnitch.java +++ b/src/java/org/apache/cassandra/locator/IEndpointSnitch.java @@ -19,6 +19,7 @@ package org.apache.cassandra.locator; import java.util.Collection; import java.util.List; +import java.util.Set; /** * This interface helps determine location of node in the datacenter relative to another node. @@ -63,4 +64,12 @@ public interface IEndpointSnitch * to be faster than 2 sequential queries, one against l1 followed by one against l2. */ public boolean isWorthMergingForRangeQuery(List<InetAddressAndPort> merged, List<InetAddressAndPort> l1, List<InetAddressAndPort> l2); + + /** + * Determine if the datacenter or rack values in the current node's snitch conflict with those passed in parameters. + */ + default boolean validate(Set<String> datacenters, Set<String> racks) + { + return true; + } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/fd44a69f/src/java/org/apache/cassandra/locator/SnitchProperties.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/locator/SnitchProperties.java b/src/java/org/apache/cassandra/locator/SnitchProperties.java index 158feef..afb6804 100644 --- a/src/java/org/apache/cassandra/locator/SnitchProperties.java +++ b/src/java/org/apache/cassandra/locator/SnitchProperties.java @@ -30,7 +30,7 @@ public class SnitchProperties private static final Logger logger = LoggerFactory.getLogger(SnitchProperties.class); public static final String RACKDC_PROPERTY_FILENAME = "cassandra-rackdc.properties"; - private Properties properties; + private final Properties properties; public SnitchProperties() { http://git-wip-us.apache.org/repos/asf/cassandra/blob/fd44a69f/src/java/org/apache/cassandra/service/StorageService.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index e5911f3..400ab75 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -482,6 +482,8 @@ public class StorageService extends NotificationBroadcasterSupport implements IE if (epStates.get(replaceAddress) == null) throw new RuntimeException(String.format("Cannot replace_address %s because it doesn't exist in gossip", replaceAddress)); + validateEndpointSnitch(epStates.values().iterator()); + try { VersionedValue tokensVersionedValue = epStates.get(replaceAddress).getApplicationState(ApplicationState.TOKENS); @@ -527,6 +529,8 @@ public class StorageService extends NotificationBroadcasterSupport implements IE FBUtilities.getBroadcastAddressAndPort())); } + validateEndpointSnitch(epStates.values().iterator()); + if (shouldBootstrap() && useStrictConsistency && !allowSimultaneousMoves()) { for (Map.Entry<InetAddressAndPort, EndpointState> entry : epStates.entrySet()) @@ -550,6 +554,28 @@ public class StorageService extends NotificationBroadcasterSupport implements IE } } + private static void validateEndpointSnitch(Iterator<EndpointState> endpointStates) + { + Set<String> datacenters = new HashSet<>(); + Set<String> racks = new HashSet<>(); + while (endpointStates.hasNext()) + { + EndpointState state = endpointStates.next(); + VersionedValue val = state.getApplicationState(ApplicationState.DC); + if (val != null) + datacenters.add(val.value); + val = state.getApplicationState(ApplicationState.RACK); + if (val != null) + racks.add(val.value); + } + + IEndpointSnitch snitch = DatabaseDescriptor.getEndpointSnitch(); + if (!snitch.validate(datacenters, racks)) + { + throw new IllegalStateException(); + } + } + private boolean allowSimultaneousMoves() { return allowSimultaneousMoves && DatabaseDescriptor.getNumTokens() == 1; http://git-wip-us.apache.org/repos/asf/cassandra/blob/fd44a69f/test/conf/cassandra-rackdc.properties ---------------------------------------------------------------------- diff --git a/test/conf/cassandra-rackdc.properties b/test/conf/cassandra-rackdc.properties index be2e7d2..742def3 100644 --- a/test/conf/cassandra-rackdc.properties +++ b/test/conf/cassandra-rackdc.properties @@ -22,3 +22,15 @@ rack=RAC1 # Add a suffix to a datacenter name. Used by the Ec2Snitch and Ec2MultiRegionSnitch # to append a string to the EC2 region name. #dc_suffix= + +# Datacenter and rack naming convention used by the Ec2Snitch and Ec2MultiRegionSnitch. +# Options are: +# legacy : datacenter name is the part of the availability zone name preceding the last "-" +# when the zone ends in -1 and includes the number if not -1. Rack is the portion of +# the availability zone name following the last "-". +# Examples: us-west-1a => dc: us-west, rack: 1a; us-west-2b => dc: us-west-2, rack: 2b; +# standard : datacenter name is the standard AWS region name, including the number. rack name is the +# region plus the availability zone letter. +# Examples: us-west-1a => dc: us-west-1, rack: us-west-1a; us-west-2b => dc: us-west-2, rack: us-west-2b; +# default: standard +ec2_naming_scheme=standard http://git-wip-us.apache.org/repos/asf/cassandra/blob/fd44a69f/test/unit/org/apache/cassandra/locator/EC2SnitchTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/locator/EC2SnitchTest.java b/test/unit/org/apache/cassandra/locator/EC2SnitchTest.java index 182ff1a..062bf69 100644 --- a/test/unit/org/apache/cassandra/locator/EC2SnitchTest.java +++ b/test/unit/org/apache/cassandra/locator/EC2SnitchTest.java @@ -20,11 +20,14 @@ package org.apache.cassandra.locator; import java.io.IOException; -import java.net.UnknownHostException; +import java.util.Collections; import java.util.EnumMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import org.junit.AfterClass; +import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; @@ -37,12 +40,21 @@ import org.apache.cassandra.gms.Gossiper; import org.apache.cassandra.gms.VersionedValue; import org.apache.cassandra.service.StorageService; +import static org.apache.cassandra.locator.Ec2Snitch.EC2_NAMING_LEGACY; import static org.junit.Assert.assertEquals; public class EC2SnitchTest { private static String az; + private final SnitchProperties legacySnitchProps = new SnitchProperties() + { + public String get(String propertyName, String defaultValue) + { + return propertyName.equals("ec2_naming_scheme") ? EC2_NAMING_LEGACY : super.get(propertyName, defaultValue); + } + }; + @BeforeClass public static void setup() throws Exception { @@ -60,6 +72,11 @@ public class EC2SnitchTest super(); } + public TestEC2Snitch(SnitchProperties props) throws IOException, ConfigurationException + { + super(props); + } + @Override String awsApiCall(String url) throws IOException, ConfigurationException { @@ -68,10 +85,10 @@ public class EC2SnitchTest } @Test - public void testRac() throws IOException, ConfigurationException + public void testLegacyRac() throws IOException, ConfigurationException { az = "us-east-1d"; - Ec2Snitch snitch = new TestEC2Snitch(); + Ec2Snitch snitch = new TestEC2Snitch(legacySnitchProps); InetAddressAndPort local = InetAddressAndPort.getByName("127.0.0.1"); InetAddressAndPort nonlocal = InetAddressAndPort.getByName("127.0.0.7"); @@ -87,17 +104,106 @@ public class EC2SnitchTest assertEquals("us-east", snitch.getDatacenter(local)); assertEquals("1d", snitch.getRack(local)); } - + @Test - public void testNewRegions() throws IOException, ConfigurationException + public void testLegacyNewRegions() throws IOException, ConfigurationException { az = "us-east-2d"; - Ec2Snitch snitch = new TestEC2Snitch(); + Ec2Snitch snitch = new TestEC2Snitch(legacySnitchProps); InetAddressAndPort local = InetAddressAndPort.getByName("127.0.0.1"); assertEquals("us-east-2", snitch.getDatacenter(local)); assertEquals("2d", snitch.getRack(local)); } + @Test + public void testFullNamingScheme() throws IOException, ConfigurationException + { + InetAddressAndPort local = InetAddressAndPort.getByName("127.0.0.1"); + az = "us-east-2d"; + Ec2Snitch snitch = new TestEC2Snitch(); + + assertEquals("us-east-2", snitch.getDatacenter(local)); + assertEquals("us-east-2d", snitch.getRack(local)); + + az = "us-west-1a"; + snitch = new TestEC2Snitch(); + + assertEquals("us-west-1", snitch.getDatacenter(local)); + assertEquals("us-west-1a", snitch.getRack(local)); + } + + @Test + public void validateDatacenter_RequiresLegacy_CorrectAmazonName() + { + Set<String> datacenters = new HashSet<>(); + datacenters.add("us-east-1"); + Assert.assertFalse(Ec2Snitch.validate(datacenters, Collections.emptySet(), true)); + } + + @Test + public void validateDatacenter_RequiresLegacy_LegacyName() + { + Set<String> datacenters = new HashSet<>(); + datacenters.add("us-east"); + Assert.assertTrue(Ec2Snitch.validate(datacenters, Collections.emptySet(), true)); + } + + @Test + public void validate_RequiresLegacy_HappyPath() + { + Set<String> datacenters = new HashSet<>(); + datacenters.add("us-east"); + Set<String> racks = new HashSet<>(); + racks.add("1a"); + Assert.assertTrue(Ec2Snitch.validate(datacenters, racks, true)); + } + + @Test + public void validate_RequiresLegacy_HappyPathWithDCSuffix() + { + Set<String> datacenters = new HashSet<>(); + datacenters.add("us-east_CUSTOM_SUFFIX"); + Set<String> racks = new HashSet<>(); + racks.add("1a"); + Assert.assertTrue(Ec2Snitch.validate(datacenters, racks, true)); + } + + @Test + public void validateRack_RequiresAmazonName_CorrectAmazonName() + { + Set<String> racks = new HashSet<>(); + racks.add("us-east-1a"); + Assert.assertTrue(Ec2Snitch.validate(Collections.emptySet(), racks, false)); + } + + @Test + public void validateRack_RequiresAmazonName_LegacyName() + { + Set<String> racks = new HashSet<>(); + racks.add("1a"); + Assert.assertFalse(Ec2Snitch.validate(Collections.emptySet(), racks, false)); + } + + @Test + public void validate_RequiresAmazonName_HappyPath() + { + Set<String> datacenters = new HashSet<>(); + datacenters.add("us-east-1"); + Set<String> racks = new HashSet<>(); + racks.add("us-east-1a"); + Assert.assertTrue(Ec2Snitch.validate(datacenters, racks, false)); + } + + @Test + public void validate_RequiresAmazonName_HappyPathWithDCSuffix() + { + Set<String> datacenters = new HashSet<>(); + datacenters.add("us-east-1_CUSTOM_SUFFIX"); + Set<String> racks = new HashSet<>(); + racks.add("us-east-1a"); + Assert.assertTrue(Ec2Snitch.validate(datacenters, racks, false)); + } + @AfterClass public static void tearDown() { --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
