You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/05/27 04:08:47 UTC
[GitHub] [cassandra] adelapena opened a new pull request #599: CASSANDRA-14361 Allow SimpleSeedProvider to resolve multiple IPs per DNS name
adelapena opened a new pull request #599:
URL: https://github.com/apache/cassandra/pull/599
----------------------------------------------------------------
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: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #599: CASSANDRA-14361 Allow SimpleSeedProvider to resolve multiple IPs per DNS name
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #599:
URL: https://github.com/apache/cassandra/pull/599#discussion_r503853602
##########
File path: src/java/org/apache/cassandra/locator/InetAddressAndPort.java
##########
@@ -24,12 +24,17 @@
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.nio.ByteBuffer;
+import java.util.Collection;
Review comment:
Nit: this import is not used
##########
File path: src/java/org/apache/cassandra/locator/InetAddressAndPort.java
##########
@@ -24,12 +24,17 @@
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.nio.ByteBuffer;
+import java.util.Collection;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
import java.util.Objects;
import java.util.regex.Pattern;
import com.google.common.base.Preconditions;
import com.google.common.net.HostAndPort;
+import org.apache.cassandra.config.DatabaseDescriptor;
Review comment:
Nit: this import is not used
##########
File path: src/java/org/apache/cassandra/locator/SimpleSeedProvider.java
##########
@@ -53,15 +55,30 @@ public SimpleSeedProvider(Map<String, String> args) {}
try
{
if(!host.trim().isEmpty()) {
- seeds.add(InetAddressAndPort.getByName(host.trim()));
+ if (DatabaseDescriptor.useMultiIPsPerDNSRecord()) {
+ List<InetAddressAndPort> resolvedSeeds = InetAddressAndPort.getAllByName(host.trim());
+ seeds.addAll(resolvedSeeds);
+ logger.debug("{} resolves to {}", host, resolvedSeeds);
+
+ } else {
+ InetAddressAndPort addressAndPort = InetAddressAndPort.getByName(host.trim());
+ seeds.add(addressAndPort);
+ logger.debug("Only resolving one IP per DNS record - {} resolves to {}", host, addressAndPort);
+ }
}
+
}
catch (UnknownHostException ex)
{
// not fatal... DD will bark if there end up being zero seeds.
logger.warn("Seed provider couldn't lookup host {}", host);
}
}
+
+
Review comment:
Nit: double blank line
```suggestion
```
##########
File path: src/java/org/apache/cassandra/locator/SimpleSeedProvider.java
##########
@@ -53,15 +55,30 @@ public SimpleSeedProvider(Map<String, String> args) {}
try
{
if(!host.trim().isEmpty()) {
- seeds.add(InetAddressAndPort.getByName(host.trim()));
+ if (DatabaseDescriptor.useMultiIPsPerDNSRecord()) {
Review comment:
Nit: According to [the project's code style](https://cassandra.apache.org/doc/latest/development/code_style.html):
```suggestion
if (DatabaseDescriptor.useMultiIPsPerDNSRecord())
{
```
##########
File path: src/java/org/apache/cassandra/locator/SimpleSeedProvider.java
##########
@@ -53,15 +55,30 @@ public SimpleSeedProvider(Map<String, String> args) {}
try
{
if(!host.trim().isEmpty()) {
- seeds.add(InetAddressAndPort.getByName(host.trim()));
+ if (DatabaseDescriptor.useMultiIPsPerDNSRecord()) {
+ List<InetAddressAndPort> resolvedSeeds = InetAddressAndPort.getAllByName(host.trim());
+ seeds.addAll(resolvedSeeds);
+ logger.debug("{} resolves to {}", host, resolvedSeeds);
+
+ } else {
Review comment:
```suggestion
}
else
{
```
----------------------------------------------------------------
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: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #599: CASSANDRA-14361 Allow SimpleSeedProvider to resolve multiple IPs per DNS name
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #599:
URL: https://github.com/apache/cassandra/pull/599#discussion_r503857543
##########
File path: src/java/org/apache/cassandra/locator/SimpleSeedProvider.java
##########
@@ -32,6 +33,7 @@
public class SimpleSeedProvider implements SeedProvider
{
private static final Logger logger = LoggerFactory.getLogger(SimpleSeedProvider.class);
+ private static final int seedCountWarnThreshold = Integer.valueOf(System.getProperty("cassandra.seed_count_warn_threshold", "20"));
Review comment:
We can use `parseInt`:
```suggestion
private static final int seedCountWarnThreshold = Integer.parseInt(System.getProperty("cassandra.seed_count_warn_threshold", "20"));
```
----------------------------------------------------------------
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: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #599: CASSANDRA-14361 Allow SimpleSeedProvider to resolve multiple IPs per DNS name
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #599:
URL: https://github.com/apache/cassandra/pull/599#discussion_r430365264
##########
File path: src/java/org/apache/cassandra/locator/InetAddressAndPort.java
##########
@@ -159,6 +163,29 @@ public static InetAddressAndPort getByName(String name) throws UnknownHostExcept
return getByNameOverrideDefaults(name, null);
}
+
+ public static List<InetAddressAndPort> getAllByName(String name) throws UnknownHostException
+ {
+ return getAllByNameOverrideDefaults(name, null);
+ }
+
+ /**
+ *
+ * @param name Hostname + optional ports string
+ * @param port Port to connect on, overridden by values in hostname string, defaults to DatabaseDescriptor default if not specified anywhere.
+ */
+ public static List<InetAddressAndPort> getAllByNameOverrideDefaults(String name, Integer port) throws UnknownHostException
+ {
+ HostAndPort hap = HostAndPort.fromString(name);
+ if (hap.hasPort())
+ {
+ port = hap.getPort();
+ }
+ Integer finalPort = port;
Review comment:
We could use `HostAndPort#getPortOrDefault`:
```suggestion
Integer finalPort = hap.getPortOrDefault(port);
```
##########
File path: src/java/org/apache/cassandra/locator/InetAddressAndPort.java
##########
@@ -159,6 +163,29 @@ public static InetAddressAndPort getByName(String name) throws UnknownHostExcept
return getByNameOverrideDefaults(name, null);
}
+
+ public static List<InetAddressAndPort> getAllByName(String name) throws UnknownHostException
+ {
+ return getAllByNameOverrideDefaults(name, null);
+ }
+
+ /**
+ *
+ * @param name Hostname + optional ports string
+ * @param port Port to connect on, overridden by values in hostname string, defaults to DatabaseDescriptor default if not specified anywhere.
+ */
+ public static List<InetAddressAndPort> getAllByNameOverrideDefaults(String name, Integer port) throws UnknownHostException
+ {
+ HostAndPort hap = HostAndPort.fromString(name);
+ if (hap.hasPort())
+ {
+ port = hap.getPort();
+ }
+ Integer finalPort = port;
+
+ return Stream.of(InetAddress.getAllByName(hap.getHost())).map((address) -> getByAddressOverrideDefaults(address, finalPort)).collect(Collectors.toList());
Review comment:
Nit: we could break this line
```suggestion
return Stream.of(InetAddress.getAllByName(hap.getHost()))
.map((address) -> getByAddressOverrideDefaults(address, finalPort))
.collect(Collectors.toList());
```
----------------------------------------------------------------
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: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org