Only set broadcast_rpc_address on Ec2MultiRegionSnitch if it's not set Patch by Paulo Motta; reviewed by Tyler Hobbs for CASSANDRA-11356
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/91f7387e Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/91f7387e Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/91f7387e Branch: refs/heads/cassandra-3.8 Commit: 91f7387e1f785b18321777311a5c3416af0663c2 Parents: e86d531 Author: Paulo Motta <pauloricard...@gmail.com> Authored: Tue Aug 2 12:42:20 2016 -0300 Committer: Tyler Hobbs <tylerlho...@gmail.com> Committed: Fri Aug 12 18:25:19 2016 -0500 ---------------------------------------------------------------------- CHANGES.txt | 1 + NEWS.txt | 6 ++++ .../cassandra/config/DatabaseDescriptor.java | 6 ++-- .../cassandra/locator/Ec2MultiRegionSnitch.java | 6 +++- .../cassandra/service/StorageService.java | 4 +-- .../org/apache/cassandra/transport/Server.java | 2 +- .../org/apache/cassandra/utils/FBUtilities.java | 20 ++++++++++++ .../apache/cassandra/utils/FBUtilitiesTest.java | 32 ++++++++++++++++++++ 8 files changed, 71 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/91f7387e/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index ddc6720..394598a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.2.8 + * Only set broadcast_rpc_address on Ec2MultiRegionSnitch if it's not set (CASSANDRA-11357) * Update StorageProxy range metrics for timeouts, failures and unavailables (CASSANDRA-9507) * Add Sigar to classes included in clientutil.jar (CASSANDRA-11635) * Add decay to histograms and timers used for metrics (CASSANDRA-11752) http://git-wip-us.apache.org/repos/asf/cassandra/blob/91f7387e/NEWS.txt ---------------------------------------------------------------------- diff --git a/NEWS.txt b/NEWS.txt index a3ba0dd..f0712eb 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -21,6 +21,12 @@ New features - JSON timestamps are now in UTC and contain the timezone information, see CASSANDRA-11137 for more details. +Upgrading +--------- + - Ec2MultiRegionSnitch will no longer automatically set broadcast_rpc_address + to the public instance IP if this property is defined on cassandra.yaml. + + 2.2.6 ===== http://git-wip-us.apache.org/repos/asf/cassandra/blob/91f7387e/src/java/org/apache/cassandra/config/DatabaseDescriptor.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index 6e46725..75f80b9 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -174,7 +174,7 @@ public class DatabaseDescriptor } @VisibleForTesting - static void applyAddressConfig(Config config) throws ConfigurationException + public static void applyAddressConfig(Config config) throws ConfigurationException { listenAddress = null; rpcAddress = null; @@ -266,7 +266,6 @@ public class DatabaseDescriptor if (rpcAddress.isAnyLocalAddress()) throw new ConfigurationException("If rpc_address is set to a wildcard address (" + config.rpc_address + "), then " + "you must set broadcast_rpc_address to a value other than " + config.rpc_address, false); - broadcastRpcAddress = rpcAddress; } } @@ -1269,6 +1268,9 @@ public class DatabaseDescriptor broadcastRpcAddress = broadcastRPCAddr; } + /** + * May be null, please use {@link FBUtilities#getBroadcastRpcAddress()} instead. + */ public static InetAddress getBroadcastRpcAddress() { return broadcastRpcAddress; http://git-wip-us.apache.org/repos/asf/cassandra/blob/91f7387e/src/java/org/apache/cassandra/locator/Ec2MultiRegionSnitch.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/locator/Ec2MultiRegionSnitch.java b/src/java/org/apache/cassandra/locator/Ec2MultiRegionSnitch.java index ab1b5d0..b32ca84 100644 --- a/src/java/org/apache/cassandra/locator/Ec2MultiRegionSnitch.java +++ b/src/java/org/apache/cassandra/locator/Ec2MultiRegionSnitch.java @@ -51,7 +51,11 @@ public class Ec2MultiRegionSnitch extends Ec2Snitch localPrivateAddress = awsApiCall(PRIVATE_IP_QUERY_URL); // use the Public IP to broadcast Address to other nodes. DatabaseDescriptor.setBroadcastAddress(localPublicAddress); - DatabaseDescriptor.setBroadcastRpcAddress(localPublicAddress); + if (DatabaseDescriptor.getBroadcastRpcAddress() == null) + { + logger.info("broadcast_rpc_address unset, broadcasting public IP as rpc_address: {}", localPublicAddress); + DatabaseDescriptor.setBroadcastRpcAddress(localPublicAddress); + } } @Override http://git-wip-us.apache.org/repos/asf/cassandra/blob/91f7387e/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 fa04595..3148f5e 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -697,7 +697,7 @@ public class StorageService extends NotificationBroadcasterSupport implements IE getTokenMetadata().updateHostId(localHostId, FBUtilities.getBroadcastAddress()); appStates.put(ApplicationState.NET_VERSION, valueFactory.networkVersion()); appStates.put(ApplicationState.HOST_ID, valueFactory.hostId(localHostId)); - appStates.put(ApplicationState.RPC_ADDRESS, valueFactory.rpcaddress(DatabaseDescriptor.getBroadcastRpcAddress())); + appStates.put(ApplicationState.RPC_ADDRESS, valueFactory.rpcaddress(FBUtilities.getBroadcastRpcAddress())); appStates.put(ApplicationState.RELEASE_VERSION, valueFactory.releaseVersion()); logger.info("Starting up server gossip"); Gossiper.instance.register(this); @@ -1282,7 +1282,7 @@ public class StorageService extends NotificationBroadcasterSupport implements IE public String getRpcaddress(InetAddress endpoint) { if (endpoint.equals(FBUtilities.getBroadcastAddress())) - return DatabaseDescriptor.getBroadcastRpcAddress().getHostAddress(); + return FBUtilities.getBroadcastRpcAddress().getHostAddress(); else if (Gossiper.instance.getEndpointStateForEndpoint(endpoint).getApplicationState(ApplicationState.RPC_ADDRESS) == null) return endpoint.getHostAddress(); else http://git-wip-us.apache.org/repos/asf/cassandra/blob/91f7387e/src/java/org/apache/cassandra/transport/Server.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/transport/Server.java b/src/java/org/apache/cassandra/transport/Server.java index 5c0d9d2..d1047f9 100644 --- a/src/java/org/apache/cassandra/transport/Server.java +++ b/src/java/org/apache/cassandra/transport/Server.java @@ -486,7 +486,7 @@ public class Server implements CassandraDaemon.Server // which is not useful to any driver and in fact may cauase serious problems to some drivers, // see CASSANDRA-10052 if (!endpoint.equals(FBUtilities.getBroadcastAddress()) && - event.nodeAddress().equals(DatabaseDescriptor.getBroadcastRpcAddress())) + event.nodeAddress().equals(FBUtilities.getBroadcastRpcAddress())) return; send(event); http://git-wip-us.apache.org/repos/asf/cassandra/blob/91f7387e/src/java/org/apache/cassandra/utils/FBUtilities.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/utils/FBUtilities.java b/src/java/org/apache/cassandra/utils/FBUtilities.java index 9eda878..23a2c2e 100644 --- a/src/java/org/apache/cassandra/utils/FBUtilities.java +++ b/src/java/org/apache/cassandra/utils/FBUtilities.java @@ -34,6 +34,7 @@ import java.util.zip.Checksum; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.collect.AbstractIterator; import org.apache.commons.lang3.StringUtils; @@ -74,6 +75,7 @@ public class FBUtilities private static volatile InetAddress localInetAddress; private static volatile InetAddress broadcastInetAddress; + private static volatile InetAddress broadcastRpcAddress; public static int getAvailableProcessors() { @@ -147,6 +149,16 @@ public class FBUtilities return broadcastInetAddress; } + + public static InetAddress getBroadcastRpcAddress() + { + if (broadcastRpcAddress == null) + broadcastRpcAddress = DatabaseDescriptor.getBroadcastRpcAddress() == null + ? DatabaseDescriptor.getRpcAddress() + : DatabaseDescriptor.getBroadcastRpcAddress(); + return broadcastRpcAddress; + } + public static Collection<InetAddress> getAllLocalAddresses() { Set<InetAddress> localAddresses = new HashSet<InetAddress>(); @@ -824,4 +836,12 @@ public class FBUtilities throw new RuntimeException(e); } } + + @VisibleForTesting + protected static void reset() + { + localInetAddress = null; + broadcastInetAddress = null; + broadcastRpcAddress = null; + } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/91f7387e/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java b/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java index c82bcc9..5b86252 100644 --- a/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java +++ b/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java @@ -21,6 +21,7 @@ package org.apache.cassandra.utils; import static org.junit.Assert.fail; import java.io.IOException; +import java.net.InetAddress; import java.nio.ByteBuffer; import java.nio.charset.CharacterCodingException; import java.nio.charset.StandardCharsets; @@ -30,6 +31,9 @@ import org.junit.Test; import java.util.Map; import java.util.TreeMap; +import org.apache.cassandra.config.Config; +import org.apache.cassandra.config.DatabaseDescriptor; + import static org.junit.Assert.assertEquals; public class FBUtilitiesTest @@ -95,4 +99,32 @@ public class FBUtilitiesTest ByteBuffer bytes = ByteBuffer.wrap(new byte[]{(byte)0xff, (byte)0xfe}); ByteBufferUtil.string(bytes, StandardCharsets.UTF_8); } + + @Test + public void testGetBroadcastRpcAddress() throws Exception + { + //When both rpc_address and broadcast_rpc_address are null, it should return the local address (from DD.applyAddressConfig) + FBUtilities.reset(); + Config testConfig = DatabaseDescriptor.loadConfig(); + testConfig.rpc_address = null; + testConfig.broadcast_rpc_address = null; + DatabaseDescriptor.applyAddressConfig(testConfig); + assertEquals(FBUtilities.getLocalAddress(), FBUtilities.getBroadcastRpcAddress()); + + //When rpc_address is defined and broadcast_rpc_address is null, it should return the rpc_address + FBUtilities.reset(); + testConfig.rpc_address = "127.0.0.2"; + testConfig.broadcast_rpc_address = null; + DatabaseDescriptor.applyAddressConfig(testConfig); + assertEquals(InetAddress.getByName("127.0.0.2"), FBUtilities.getBroadcastRpcAddress()); + + //When both rpc_address and broadcast_rpc_address are defined, it should return broadcast_rpc_address + FBUtilities.reset(); + testConfig.rpc_address = "127.0.0.2"; + testConfig.broadcast_rpc_address = "127.0.0.3"; + DatabaseDescriptor.applyAddressConfig(testConfig); + assertEquals(InetAddress.getByName("127.0.0.3"), FBUtilities.getBroadcastRpcAddress()); + + FBUtilities.reset(); + } }