[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[ https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17677984#comment-17677984 ] Stefan Miklosovic edited comment on CASSANDRA-14361 at 1/17/23 9:13 PM: Both runs (yours too) look good to me. I ll merge this (1) (1) https://github.com/instaclustr/cassandra/commit/69113b85f94bd3d4e23cd119fac8de782e2d914e was (Author: smiklosovic): Both runs (yours too) looks good tome. I ll merge this (1) (1) https://github.com/instaclustr/cassandra/commit/69113b85f94bd3d4e23cd119fac8de782e2d914e > Allow SimpleSeedProvider to resolve multiple IPs per DNS name > - > > Key: CASSANDRA-14361 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14361 > Project: Cassandra > Issue Type: Improvement > Components: Local/Config >Reporter: Ben Bromhead >Assignee: Stefan Miklosovic >Priority: Low > Fix For: 4.x > > Time Spent: 1h 20m > Remaining Estimate: 0h > > Currently SimpleSeedProvider can accept a comma separated string of IPs or > hostnames as the set of Cassandra seeds. hostnames are resolved via > InetAddress.getByName, which will only return the first IP associated with an > A, or CNAME record. > By changing to InetAddress.getAllByName, existing behavior is preserved, but > now Cassandra can discover multiple IP address per record, allowing seed > discovery by DNS to be a little easier. > Some examples of improved workflows with this change include: > * specify the DNS name of a headless service in Kubernetes which will > resolve to all IP addresses of pods within that service. > * seed discovery for multi-region clusters via AWS route53, AzureDNS etc > * Other common DNS service discovery mechanisms. > The only behavior this is likely to impact would be where users are relying > on the fact that getByName only returns a single IP address. > I can't imagine any scenario where that is a sane choice. Even when that > choice has been made, it only impacts the first startup of Cassandra and > would not be on any critical path. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[ https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17677769#comment-17677769 ] Stefan Miklosovic edited comment on CASSANDRA-14361 at 1/17/23 1:45 PM: I have made "false" to be default instead of "true" in this commit https://github.com/apache/cassandra/pull/2067/commits/bbcd2054f917f70f953e0bfa44402e6b60f809a5 Sure, I ll build the rest and let you know. was (Author: smiklosovic): I have made "false" to be default instead of "true" in this commit https://github.com/apache/cassandra/pull/2067/commits/bbcd2054f917f70f953e0bfa44402e6b60f809a5 > Allow SimpleSeedProvider to resolve multiple IPs per DNS name > - > > Key: CASSANDRA-14361 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14361 > Project: Cassandra > Issue Type: Improvement > Components: Local/Config >Reporter: Ben Bromhead >Assignee: Stefan Miklosovic >Priority: Low > Fix For: 4.x > > Time Spent: 1h 20m > Remaining Estimate: 0h > > Currently SimpleSeedProvider can accept a comma separated string of IPs or > hostnames as the set of Cassandra seeds. hostnames are resolved via > InetAddress.getByName, which will only return the first IP associated with an > A, or CNAME record. > By changing to InetAddress.getAllByName, existing behavior is preserved, but > now Cassandra can discover multiple IP address per record, allowing seed > discovery by DNS to be a little easier. > Some examples of improved workflows with this change include: > * specify the DNS name of a headless service in Kubernetes which will > resolve to all IP addresses of pods within that service. > * seed discovery for multi-region clusters via AWS route53, AzureDNS etc > * Other common DNS service discovery mechanisms. > The only behavior this is likely to impact would be where users are relying > on the fact that getByName only returns a single IP address. > I can't imagine any scenario where that is a sane choice. Even when that > choice has been made, it only impacts the first startup of Cassandra and > would not be on any critical path. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[ https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17675959#comment-17675959 ] Andres de la Peña edited comment on CASSANDRA-14361 at 1/12/23 11:42 AM: - Changes look good to me, +1. I have started [a CI run|https://app.circleci.com/pipelines/github/adelapena/cassandra/2545/workflows/eaa36a20-abab-4a3d-950f-36c692920534] including 500 repetitions of the new test. Thanks for notifying the mail list, I guess no one will oppose the new dependency. Let's wait a bit before merge to see if someone has any objection. was (Author: adelapena): Changes look good to me, +1. I have started [a CI run|https://app.circleci.com/pipelines/github/adelapena/cassandra/2545/workflows/eaa36a20-abab-4a3d-950f-36c692920534] including 500 repetitions of the new test. Thanks for notifying the mail list, I guess no one will oppose the new dependency. Let's wait a bit before merge to see in anyone has any objection. > Allow SimpleSeedProvider to resolve multiple IPs per DNS name > - > > Key: CASSANDRA-14361 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14361 > Project: Cassandra > Issue Type: Improvement > Components: Local/Config >Reporter: Ben Bromhead >Assignee: Stefan Miklosovic >Priority: Low > Fix For: 4.x > > Time Spent: 1h 20m > Remaining Estimate: 0h > > Currently SimpleSeedProvider can accept a comma separated string of IPs or > hostnames as the set of Cassandra seeds. hostnames are resolved via > InetAddress.getByName, which will only return the first IP associated with an > A, or CNAME record. > By changing to InetAddress.getAllByName, existing behavior is preserved, but > now Cassandra can discover multiple IP address per record, allowing seed > discovery by DNS to be a little easier. > Some examples of improved workflows with this change include: > * specify the DNS name of a headless service in Kubernetes which will > resolve to all IP addresses of pods within that service. > * seed discovery for multi-region clusters via AWS route53, AzureDNS etc > * Other common DNS service discovery mechanisms. > The only behavior this is likely to impact would be where users are relying > on the fact that getByName only returns a single IP address. > I can't imagine any scenario where that is a sane choice. Even when that > choice has been made, it only impacts the first startup of Cassandra and > would not be on any critical path. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[ https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17675860#comment-17675860 ] Stefan Miklosovic edited comment on CASSANDRA-14361 at 1/12/23 8:18 AM: Thanks [~adelapena], I have already incorporated your feedback. Please confirm and I will build it yet one more time before the merge. I have sent email about mockito-inline here (1). We might wait couple days more but it seems like David is fine with that, at least. (1) https://lists.apache.org/thread/tpswxpgdpvj4lfovk4gj9dxyqyjtwv6w was (Author: smiklosovic): Thanks [~adelapena], I have already incorporated your feedback. Please confirm and I will build it yet one more time before the merge. > Allow SimpleSeedProvider to resolve multiple IPs per DNS name > - > > Key: CASSANDRA-14361 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14361 > Project: Cassandra > Issue Type: Improvement > Components: Local/Config >Reporter: Ben Bromhead >Assignee: Stefan Miklosovic >Priority: Low > Fix For: 4.x > > Time Spent: 1h 10m > Remaining Estimate: 0h > > Currently SimpleSeedProvider can accept a comma separated string of IPs or > hostnames as the set of Cassandra seeds. hostnames are resolved via > InetAddress.getByName, which will only return the first IP associated with an > A, or CNAME record. > By changing to InetAddress.getAllByName, existing behavior is preserved, but > now Cassandra can discover multiple IP address per record, allowing seed > discovery by DNS to be a little easier. > Some examples of improved workflows with this change include: > * specify the DNS name of a headless service in Kubernetes which will > resolve to all IP addresses of pods within that service. > * seed discovery for multi-region clusters via AWS route53, AzureDNS etc > * Other common DNS service discovery mechanisms. > The only behavior this is likely to impact would be where users are relying > on the fact that getByName only returns a single IP address. > I can't imagine any scenario where that is a sane choice. Even when that > choice has been made, it only impacts the first startup of Cassandra and > would not be on any critical path. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[ https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17656109#comment-17656109 ] Stefan Miklosovic edited comment on CASSANDRA-14361 at 1/9/23 2:03 PM: --- I incorporated the feedback of [~adelapena] in my branch here (rebased + squashed what was there and added the changes on top in one commit). I have added that parameter in seed provider and removed it from top-level cassandra.yaml. [~adelapena] would you mind to review again? PR: https://github.com/apache/cassandra/pull/2067/commits the build is running here: https://ci-cassandra.apache.org/view/patches/job/Cassandra-devbranch/2167/ EDIT: btw I am not completely sure how to test this other than empirically. The problem I see, if I remember correctly, is that Mockito is not able to mock static methods and we are using such method in that logic. Ideally I would like to say what IPs so and so hostname returns and based on the configuration property in seed provider I would assert their length however I just see that mocking of static methods was not possible prior Mockito 3.4.0 and we are on 4.7.0 so it should be possible. I ll try to do a test as well but I think it is good to review already. was (Author: smiklosovic): I incorporated the feedback of [~adelapena] in my branch here (rebased + squashed what was there and added the changes on top in one commit). I have added that parameter in seed provider and removed it from top-level cassandra.yaml. [~adelapena] would you mind to review again? PR: https://github.com/apache/cassandra/pull/2067/commits the build is running here: https://ci-cassandra.apache.org/view/patches/job/Cassandra-devbranch/2167/ > Allow SimpleSeedProvider to resolve multiple IPs per DNS name > - > > Key: CASSANDRA-14361 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14361 > Project: Cassandra > Issue Type: Improvement > Components: Local/Config >Reporter: Ben Bromhead >Assignee: Stefan Miklosovic >Priority: Low > Fix For: 4.x > > Time Spent: 50m > Remaining Estimate: 0h > > Currently SimpleSeedProvider can accept a comma separated string of IPs or > hostnames as the set of Cassandra seeds. hostnames are resolved via > InetAddress.getByName, which will only return the first IP associated with an > A, or CNAME record. > By changing to InetAddress.getAllByName, existing behavior is preserved, but > now Cassandra can discover multiple IP address per record, allowing seed > discovery by DNS to be a little easier. > Some examples of improved workflows with this change include: > * specify the DNS name of a headless service in Kubernetes which will > resolve to all IP addresses of pods within that service. > * seed discovery for multi-region clusters via AWS route53, AzureDNS etc > * Other common DNS service discovery mechanisms. > The only behavior this is likely to impact would be where users are relying > on the fact that getByName only returns a single IP address. > I can't imagine any scenario where that is a sane choice. Even when that > choice has been made, it only impacts the first startup of Cassandra and > would not be on any critical path. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[
https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17649711#comment-17649711
]
Andres de la Peña edited comment on CASSANDRA-14361 at 12/20/22 12:09 PM:
--
[~smiklosovic] being an improvement I think it should go to trunk only.
[~jkoppenhofer] as mentioned above, there are still a few nits to resolve on
[the PR|https://github.com/apache/cassandra/pull/599], and the suggestion about
the {{seed_provider}} section. We will also need a rebase and a CI run, given
that this has been inactive for a while.
was (Author: adelapena):
[~smiklosovic] being an improvement I think it should go to trunk only.
[~jkoppenhofer] as mentioned above, there are still a few nits to resolve on
[the PR|https://github.com/apache/cassandra/pull/599], and the suggestion about
the {{seed_provider}} section.
> Allow SimpleSeedProvider to resolve multiple IPs per DNS name
> -
>
> Key: CASSANDRA-14361
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14361
> Project: Cassandra
> Issue Type: Improvement
> Components: Local/Config
>Reporter: Ben Bromhead
>Assignee: Stefan Miklosovic
>Priority: Low
> Fix For: 4.0.x
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Currently SimpleSeedProvider can accept a comma separated string of IPs or
> hostnames as the set of Cassandra seeds. hostnames are resolved via
> InetAddress.getByName, which will only return the first IP associated with an
> A, or CNAME record.
> By changing to InetAddress.getAllByName, existing behavior is preserved, but
> now Cassandra can discover multiple IP address per record, allowing seed
> discovery by DNS to be a little easier.
> Some examples of improved workflows with this change include:
> * specify the DNS name of a headless service in Kubernetes which will
> resolve to all IP addresses of pods within that service.
> * seed discovery for multi-region clusters via AWS route53, AzureDNS etc
> * Other common DNS service discovery mechanisms.
> The only behavior this is likely to impact would be where users are relying
> on the fact that getByName only returns a single IP address.
> I can't imagine any scenario where that is a sane choice. Even when that
> choice has been made, it only impacts the first startup of Cassandra and
> would not be on any critical path.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[
https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17649711#comment-17649711
]
Andres de la Peña edited comment on CASSANDRA-14361 at 12/20/22 12:08 PM:
--
[~smiklosovic] being an improvement I think it should go to trunk only.
[~jkoppenhofer] as mentioned above, there are still a few nits to resolve on
[the PR|https://github.com/apache/cassandra/pull/599], and the suggestion about
the {{seed_provider}} section.
was (Author: adelapena):
[~smiklosovic] being an improvement I think it should go to trunk only.
[~jkoppenhofer] as mentioned above, there are still a few nits to resolve on
[the PR|https://github.com/apache/cassandra/pull/599], and the suggestion about
the {{seed_provider}} section.
> Allow SimpleSeedProvider to resolve multiple IPs per DNS name
> -
>
> Key: CASSANDRA-14361
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14361
> Project: Cassandra
> Issue Type: Improvement
> Components: Local/Config
>Reporter: Ben Bromhead
>Assignee: Stefan Miklosovic
>Priority: Low
> Fix For: 4.0.x
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Currently SimpleSeedProvider can accept a comma separated string of IPs or
> hostnames as the set of Cassandra seeds. hostnames are resolved via
> InetAddress.getByName, which will only return the first IP associated with an
> A, or CNAME record.
> By changing to InetAddress.getAllByName, existing behavior is preserved, but
> now Cassandra can discover multiple IP address per record, allowing seed
> discovery by DNS to be a little easier.
> Some examples of improved workflows with this change include:
> * specify the DNS name of a headless service in Kubernetes which will
> resolve to all IP addresses of pods within that service.
> * seed discovery for multi-region clusters via AWS route53, AzureDNS etc
> * Other common DNS service discovery mechanisms.
> The only behavior this is likely to impact would be where users are relying
> on the fact that getByName only returns a single IP address.
> I can't imagine any scenario where that is a sane choice. Even when that
> choice has been made, it only impacts the first startup of Cassandra and
> would not be on any critical path.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[ https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17648721#comment-17648721 ] Stefan Miklosovic edited comment on CASSANDRA-14361 at 12/16/22 4:39 PM: - [~adelapena] What branches do we want this to be in? I think just trunk will be ok as this is basically a new feature? was (Author: smiklosovic): [~adelapena] What branches we this to be in? I think just trunk will be ok as this is basically a new feature? > Allow SimpleSeedProvider to resolve multiple IPs per DNS name > - > > Key: CASSANDRA-14361 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14361 > Project: Cassandra > Issue Type: Improvement > Components: Local/Config >Reporter: Ben Bromhead >Assignee: Stefan Miklosovic >Priority: Low > Fix For: 4.0.x > > Time Spent: 40m > Remaining Estimate: 0h > > Currently SimpleSeedProvider can accept a comma separated string of IPs or > hostnames as the set of Cassandra seeds. hostnames are resolved via > InetAddress.getByName, which will only return the first IP associated with an > A, or CNAME record. > By changing to InetAddress.getAllByName, existing behavior is preserved, but > now Cassandra can discover multiple IP address per record, allowing seed > discovery by DNS to be a little easier. > Some examples of improved workflows with this change include: > * specify the DNS name of a headless service in Kubernetes which will > resolve to all IP addresses of pods within that service. > * seed discovery for multi-region clusters via AWS route53, AzureDNS etc > * Other common DNS service discovery mechanisms. > The only behavior this is likely to impact would be where users are relying > on the fact that getByName only returns a single IP address. > I can't imagine any scenario where that is a sane choice. Even when that > choice has been made, it only impacts the first startup of Cassandra and > would not be on any critical path. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[
https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17212343#comment-17212343
]
Andres de la Peña edited comment on CASSANDRA-14361 at 10/12/20, 12:55 PM:
---
[~benbromhead] changes look mostly good, but I'm getting a
{{NullPointerPointerException}} during startup that doesn't even allow to start
the server. That happens because of my suggestion of using {{getPortOrDefault}}
in
[{{InetAddressAndPort.getAllByNameOverrideDefaults}}|https://github.com/apache/cassandra/blob/f9de59c5856fc3fee1aaf1f9c09aa63cf39b10ee/src/java/org/apache/cassandra/locator/InetAddressAndPort.java#L234].
This is because {{HostAndPort#getPortOrDefault}} expects an {{int}} argument
and it's receiving a {{null}} instead, so we should either keep using it and
pass {{InetAddressAndPort.defaultPort}} to it, or leave it as it was.
Additionally, I still think it would be nice to place the threshold property in
{{cassandra.yaml}}. Note that there is [a new section for safety thresholds at
{{cassandra.yaml}}|https://github.com/apache/cassandra/blob/trunk/conf/cassandra.yaml#L1217-L1273],
that would be the place to put it. That way we would give visibility and some
documentation to the new property, so users know that it exists and how to use
it.
was (Author: adelapena):
[~benbromhead] changes look mostly good, but I'm getting a
{{NullPointerPointerException}} during startup that doesn't even allow to start
the server. That happens because of my suggestion of using {{getPortOrDefault}}
in
[{{InetAddressAndPort.getAllByNameOverrideDefaults}}|https://github.com/apache/cassandra/blob/f9de59c5856fc3fee1aaf1f9c09aa63cf39b10ee/src/java/org/apache/cassandra/locator/InetAddressAndPort.java#L234].
This is because {{HostAndPort#getPortOrDefault}} expects an {{int}} argument
and it's receiving a {{null}} instead, so we should either keep using it and
pass {{InetAddressAndPort.defaultPort}} to it, or leave it as it was.
Additionally, I still think it would be nice to place the threshold property in
{{cassandra.yaml}}. Note that there is [a new section for safety thresholds at
{{cassandra.yaml}}|https://github.com/apache/cassandra/blob/trunk/conf/cassandra.yaml#L1217-L1273],
that would be the place to put it.
> Allow SimpleSeedProvider to resolve multiple IPs per DNS name
> -
>
> Key: CASSANDRA-14361
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14361
> Project: Cassandra
> Issue Type: Improvement
> Components: Local/Config
>Reporter: Ben Bromhead
>Assignee: Ben Bromhead
>Priority: Low
> Fix For: 4.0.x
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Currently SimpleSeedProvider can accept a comma separated string of IPs or
> hostnames as the set of Cassandra seeds. hostnames are resolved via
> InetAddress.getByName, which will only return the first IP associated with an
> A, or CNAME record.
> By changing to InetAddress.getAllByName, existing behavior is preserved, but
> now Cassandra can discover multiple IP address per record, allowing seed
> discovery by DNS to be a little easier.
> Some examples of improved workflows with this change include:
> * specify the DNS name of a headless service in Kubernetes which will
> resolve to all IP addresses of pods within that service.
> * seed discovery for multi-region clusters via AWS route53, AzureDNS etc
> * Other common DNS service discovery mechanisms.
> The only behavior this is likely to impact would be where users are relying
> on the fact that getByName only returns a single IP address.
> I can't imagine any scenario where that is a sane choice. Even when that
> choice has been made, it only impacts the first startup of Cassandra and
> would not be on any critical path.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[
https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17107259#comment-17107259
]
Andres de la Peña edited comment on CASSANDRA-14361 at 5/14/20, 1:59 PM:
-
[~benbromhead] it seems the patch is needing a rebase, it conflicts with
CASSANDRA-7544. Besides that, I think that the threshold property would
probably be better placed in cassandra.yaml.
Also, I'm not sure if we want a property to disable the {{getAllByName}} call
and fallback to the old behaviour, so it doesn't create problems with existing
setups, wdyt? Probably it's not needed since 4.0 is a major version, although
we should add a note in NEWS.txt about the new behaviour.
was (Author: adelapena):
[~benbromhead] it seems the patch is needing a rebase, it conflicts with
CASSANDRA-7544. Besides that, I think that the threshold property would
probably be better placed in cassandra.yaml.
Also, I'm not sure if we want a property to disable the {{getAllByName}} call
and fallback to the old behaviour, so it doesn't create problems with existing
setups, wdyt?
> Allow SimpleSeedProvider to resolve multiple IPs per DNS name
> -
>
> Key: CASSANDRA-14361
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14361
> Project: Cassandra
> Issue Type: Improvement
> Components: Local/Config
>Reporter: Ben Bromhead
>Assignee: Ben Bromhead
>Priority: Low
> Fix For: 4.0
>
>
> Currently SimpleSeedProvider can accept a comma separated string of IPs or
> hostnames as the set of Cassandra seeds. hostnames are resolved via
> InetAddress.getByName, which will only return the first IP associated with an
> A, or CNAME record.
> By changing to InetAddress.getAllByName, existing behavior is preserved, but
> now Cassandra can discover multiple IP address per record, allowing seed
> discovery by DNS to be a little easier.
> Some examples of improved workflows with this change include:
> * specify the DNS name of a headless service in Kubernetes which will
> resolve to all IP addresses of pods within that service.
> * seed discovery for multi-region clusters via AWS route53, AzureDNS etc
> * Other common DNS service discovery mechanisms.
> The only behavior this is likely to impact would be where users are relying
> on the fact that getByName only returns a single IP address.
> I can't imagine any scenario where that is a sane choice. Even when that
> choice has been made, it only impacts the first startup of Cassandra and
> would not be on any critical path.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[ https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16425809#comment-16425809 ] Ben Bromhead edited comment on CASSANDRA-14361 at 4/4/18 4:30 PM: -- I would argue that you need to be intentional about your DNS records, but I'm willing to concede that it moves config further away from the system which can hide the reason for behavior. Would it be worth adding some info/debug level logging around what IPs got resolved for each time getSeeds() gets called? At least that way the behavior of what gets looked up is observable and the answer to any potential WTFs is in the logs. was (Author: benbromhead): I would argue you need to be intentional about your DNS records, but I'm willing to concede that it moves config further away from the system which can hide the reason for behavior. Would it be worth adding some info/debug level logging around what IPs got resolved for each time getSeeds() gets called? At least that way the behavior of what gets looked up is observable and the answer to any potential WTFs is in the logs. > Allow SimpleSeedProvider to resolve multiple IPs per DNS name > - > > Key: CASSANDRA-14361 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14361 > Project: Cassandra > Issue Type: Improvement > Components: Configuration >Reporter: Ben Bromhead >Assignee: Ben Bromhead >Priority: Minor > Fix For: 4.0 > > > Currently SimpleSeedProvider can accept a comma separated string of IPs or > hostnames as the set of Cassandra seeds. hostnames are resolved via > InetAddress.getByName, which will only return the first IP associated with an > A, or CNAME record. > By changing to InetAddress.getAllByName, existing behavior is preserved, but > now Cassandra can discover multiple IP address per record, allowing seed > discovery by DNS to be a little easier. > Some examples of improved workflows with this change include: > * specify the DNS name of a headless service in Kubernetes which will > resolve to all IP addresses of pods within that service. > * seed discovery for multi-region clusters via AWS route53, AzureDNS etc > * Other common DNS service discovery mechanisms. > The only behavior this is likely to impact would be where users are relying > on the fact that getByName only returns a single IP address. > I can't imagine any scenario where that is a sane choice. Even when that > choice has been made, it only impacts the first startup of Cassandra and > would not be on any critical path. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[ https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16425723#comment-16425723 ] Jon Haddad edited comment on CASSANDRA-14361 at 4/4/18 3:31 PM: I agree with you, you could *explicitly* put 1000 seeds in there. Would you expect there to be 1000 potential seeds in your gossip list by putting 1 thing in the seed list? People don't even mostly know about this behavior. I'm not sure it's optimal, and I'm not arguing against changing it, I'm just pointing it out and saying it's worth a discussion. A point against it being, the new behavior might be a bit surprising (and maybe it doesn't actually change at all) was (Author: rustyrazorblade): I agree with you, you could *explicitly* put 1000 seeds in there. Would you expect there to be 1000 potential seeds in your gossip list by putting 1 thing in the seed list? People don't even mostly know about this behavior. I'm not sure it's optimal, and I'm not arguing against changing it, I'm just pointing it out and saying it's worth a discussion. A point against it being, the new behavior might be a bit surprising. > Allow SimpleSeedProvider to resolve multiple IPs per DNS name > - > > Key: CASSANDRA-14361 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14361 > Project: Cassandra > Issue Type: Improvement > Components: Configuration >Reporter: Ben Bromhead >Assignee: Ben Bromhead >Priority: Minor > Fix For: 4.0 > > > Currently SimpleSeedProvider can accept a comma separated string of IPs or > hostnames as the set of Cassandra seeds. hostnames are resolved via > InetAddress.getByName, which will only return the first IP associated with an > A, or CNAME record. > By changing to InetAddress.getAllByName, existing behavior is preserved, but > now Cassandra can discover multiple IP address per record, allowing seed > discovery by DNS to be a little easier. > Some examples of improved workflows with this change include: > * specify the DNS name of a headless service in Kubernetes which will > resolve to all IP addresses of pods within that service. > * seed discovery for multi-region clusters via AWS route53, AzureDNS etc > * Other common DNS service discovery mechanisms. > The only behavior this is likely to impact would be where users are relying > on the fact that getByName only returns a single IP address. > I can't imagine any scenario where that is a sane choice. Even when that > choice has been made, it only impacts the first startup of Cassandra and > would not be on any critical path. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[
https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16425647#comment-16425647
]
Ben Bromhead edited comment on CASSANDRA-14361 at 4/4/18 3:15 PM:
--
{quote}JVM property {{networkaddress.cache.ttl}} must be set otherwise
operators will have to do a rolling restart of the cluster each time the seed
list changes (unless default is not {{-1}} on their platforms).
{quote}
Caching behavior remains the same, given operators relying on hostnames in the
seed list would need to be aware of this anyway, whether it looks up one IP or
multiple. Also the JVM will only cache forever by default if a security manager
is installed. If a security manager is not installed each specific JVM
implementations can have a potentially different TTL, but a defined TTL
nonetheless. See
[https://docs.oracle.com/javase/8/docs/technotes/guides/net/properties.html].
{quote}This would also affect the third gossip round:
[https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/gms/Gossiper.java#L185]
{quote}
Ah, of course.
If you are already using hostnames in the seed list, the behavior would still
be the same? Other than your list of seeds potentially containing seed IPs you
might not expect, if you are returning multiple IPs per A record, but not
relying on the behavior. Resulting in potentially more gossip rounds to hit the
IP you used to expect?
I've marked this as a 4.0 patch, so I think a change in behavior would be fine.
I've created another ticket
https://issues.apache.org/jira/browse/CASSANDRA-14364 to document seed behavior
with relation to the third gossip round and DNS (including caching) behavior.
was (Author: benbromhead):
{quote}JVM property {{networkaddress.cache.ttl}} must be set otherwise
operators will have to do a rolling restart of the cluster each time the seed
list changes (unless default is not {{-1}} on their platforms).
{quote}
Caching behavior remains the same, given operators relying on hostnames in the
seed list would need to be aware of this anyway, whether it looks up one IP or
multiple. Also the JVM will only cache forever by default if a security manager
is installed. If a security manager is not installed each specific JVM
implementations can have a potentially different TTL, but a defined TTL
nonetheless. See
[https://docs.oracle.com/javase/8/docs/technotes/guides/net/properties.html].
{quote}This would also affect the third gossip round:
[https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/gms/Gossiper.java#L185]
{quote}
Ah, of course.
If you are already using hostnames in the seed list, the behavior would still
be the same? Other than your list of seeds potentially containing seed IPs you
might not expect, if you are returning multiple IPs per A record, but not
relying on the behavior. Resulting in potentially more gossip rounds to hit the
IP you used to expect?
I've marked this as a 4.0 patch, so I think a change in behavior would be fine.
I've created another ticket
https://issues.apache.org/jira/browse/CASSANDRA-14364 to document seed behavior
with relation to the third gossip round and DNS (including caching) behavior.
> Allow SimpleSeedProvider to resolve multiple IPs per DNS name
> -
>
> Key: CASSANDRA-14361
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14361
> Project: Cassandra
> Issue Type: Improvement
> Components: Configuration
>Reporter: Ben Bromhead
>Assignee: Ben Bromhead
>Priority: Minor
> Fix For: 4.0
>
>
> Currently SimpleSeedProvider can accept a comma separated string of IPs or
> hostnames as the set of Cassandra seeds. hostnames are resolved via
> InetAddress.getByName, which will only return the first IP associated with an
> A, or CNAME record.
> By changing to InetAddress.getAllByName, existing behavior is preserved, but
> now Cassandra can discover multiple IP address per record, allowing seed
> discovery by DNS to be a little easier.
> Some examples of improved workflows with this change include:
> * specify the DNS name of a headless service in Kubernetes which will
> resolve to all IP addresses of pods within that service.
> * seed discovery for multi-region clusters via AWS route53, AzureDNS etc
> * Other common DNS service discovery mechanisms.
> The only behavior this is likely to impact would be where users are relying
> on the fact that getByName only returns a single IP address.
> I can't imagine any scenario where that is a sane choice. Even when that
> choice has been made, it only impacts the first startup of Cassandra and
> would not be on any critical path.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (CASSANDRA-14361) Allow SimpleSeedProvider to resolve multiple IPs per DNS name
[
https://issues.apache.org/jira/browse/CASSANDRA-14361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16425647#comment-16425647
]
Ben Bromhead edited comment on CASSANDRA-14361 at 4/4/18 2:52 PM:
--
{quote}JVM property {{networkaddress.cache.ttl}} must be set otherwise
operators will have to do a rolling restart of the cluster each time the seed
list changes (unless default is not {{-1}} on their platforms).
{quote}
Caching behavior remains the same, given operators relying on hostnames in the
seed list would need to be aware of this anyway, whether it looks up one IP or
multiple. Also the JVM will only cache forever by default if a security manager
is installed. If a security manager is not installed each specific JVM
implementations can have a potentially different TTL, but a defined TTL
nonetheless. See
[https://docs.oracle.com/javase/8/docs/technotes/guides/net/properties.html].
{quote}This would also affect the third gossip round:
[https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/gms/Gossiper.java#L185]
{quote}
Ah, of course.
If you are already using hostnames in the seed list, the behavior would still
be the same? Other than your list of seeds potentially containing seed IPs you
might not expect, if you are returning multiple IPs per A record, but not
relying on the behavior. Resulting in potentially more gossip rounds to hit the
IP you used to expect?
I've marked this as a 4.0 patch, so I think a change in behavior would be fine.
I've created another ticket
https://issues.apache.org/jira/browse/CASSANDRA-14364 to document seed behavior
with relation to the third gossip round and DNS (including caching) behavior.
was (Author: benbromhead):
{quote}
JVM property networkaddress.cache.ttl must be set otherwise operators
will have to do a rolling restart of the cluster each time the seed list
changes (unless default is not -1 on their platforms).
{quote}
Caching behavior remains the same, given operators relying on hostnames in the
seed list would need to be aware of this anyway, whether it looks up one IP or
multiple. Also the JVM will only cache forever by default if a security manager
is installed. If a security manager is not installed each specific JVM
implementations can have a potentially different TTL, but a defined TTL
nonetheless. See
[https://docs.oracle.com/javase/8/docs/technotes/guides/net/properties.html].
{quote}
This would also affect the third gossip round:
[https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/gms/Gossiper.java#L185]
{quote}
Ah, of course.
If you are already using hostnames in the seed list, the behavior would still
be the same? Other than your list of seeds potentially containing seed IPs you
might not expect, if you are returning multiple IPs per A record, but not
relying on the behavior. Resulting in potentially more gossip rounds to hit the
IP you used to expect?
I've marked this as a 4.0 patch, so I think a change in behavior would be fine.
I've created another ticket
https://issues.apache.org/jira/browse/CASSANDRA-14364 to document seed behavior
with relation to the third gossip round and DNS (including caching) behavior.
> Allow SimpleSeedProvider to resolve multiple IPs per DNS name
> -
>
> Key: CASSANDRA-14361
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14361
> Project: Cassandra
> Issue Type: Improvement
> Components: Configuration
>Reporter: Ben Bromhead
>Assignee: Ben Bromhead
>Priority: Minor
> Fix For: 4.0
>
>
> Currently SimpleSeedProvider can accept a comma separated string of IPs or
> hostnames as the set of Cassandra seeds. hostnames are resolved via
> InetAddress.getByName, which will only return the first IP associated with an
> A, or CNAME record.
> By changing to InetAddress.getAllByName, existing behavior is preserved, but
> now Cassandra can discover multiple IP address per record, allowing seed
> discovery by DNS to be a little easier.
> Some examples of improved workflows with this change include:
> * specify the DNS name of a headless service in Kubernetes which will
> resolve to all IP addresses of pods within that service.
> * seed discovery for multi-region clusters via AWS route53, AzureDNS etc
> * Other common DNS service discovery mechanisms.
> The only behavior this is likely to impact would be where users are relying
> on the fact that getByName only returns a single IP address.
> I can't imagine any scenario where that is a sane choice. Even when that
> choice has been made, it only impacts the first startup of Cassandra and
> would not be on any critical path.
--
This message was sent by Atlassian
