Clarify documentation of method addContactPoints

Description

Resolution:
Per the conversation in the comments we've decided not to apply the suggested fix. I'm planning this ticket for a future version to remember to clarify the javadocs.


Initial report:
Some users complain about the fact that the method addContactPoints(String... addresses) throw an IllegalArgumentException when at least one host could not be resolved. I believe that it has been implemented this way to prevent potential typos which makes sense.

However let's say that my configuration is correct but between 2 startup, one host in the provided array of hosts has been removed from the DNS entries, in that particular case it will fail while we could expect it to work properly because we know that we have at least one provided host that can be resolved.

So I believe that it could be interesting to provide a new method that will only fail if all the provided hosts could not be resolved otherwise a warning message could be logged.

Something like this:

For more details about the problem, please check the related question on stackoverflow here

Environment

None

Pull Requests

None

Activity

Show:
Olivier Michallat
November 21, 2016, 10:22 PM

Sorry but I'm not convinced.

I don't think it's the best way to answer the original question on stackoverflow: they mention that their environment is very dynamic and VM addresses change very often. If so, making the code more lenient will only help you as long as at least one address is still valid. Once all the addresses have changed, the code fails again and you're back to square one.
If the environment is dynamic, the application should embrace this, and retrieve the addresses in a dynamic way (in that sense I agree with Jeff Jirsa's answer).

On the other hand, as mentioned by someone else, I agree that the Javadoc is not clear, I'll fix it to explicitly mention address resolution.

Pablo Matías Gomez
November 21, 2016, 11:54 PM
Edited

I don't agree. If the driver supports the state in which all cassandras can be down except for one, why can't we do the same for the host resolution? When saying that the environment is very dynamic doesn't mean that every host is going to change. For example you could have hosts A, B, and C. But at certain time, host b could not be resolved, but later maybe yes. So the app can run with hosts A and C, and later with A, B and C, without changing any config. It is the same logic as if the cassandra app is down, but the VM up.

Nicolas Filotto
November 22, 2016, 9:02 AM

@omichallat So If I understand your answer properly, for you, we have only 2 acceptable use cases: static environment with static configuration or very dynamic environment with dynamic configuration generation, something in the middle as described by Pablo (FYI Pablo is the original poster of the question on SO) should be managed as a very dynamic environment, did I get it right?

Olivier Michallat
November 22, 2016, 6:40 PM
Edited

If the driver supports the state in which all cassandras can be down except for one, why can't we do the same for the host resolution?

That's a bit different, a down node is a possible state of the cluster; a bad IP or DNS name is essentially invalid configuration.

When adding features to the driver, we have to strike a good balance between solving common problems and not spreading the API too thin (1). In this particular instance, I feel like the suggested approach:

  • only partially solves the issue (won't work when all addresses have changed)

  • has arguably better alternatives (dynamic configuration, think discovery services for example)

  • can be trivially implemented on the application side if you decide to go that route anyway

(1) there are already 5 variants of addContactPoints. The more we add, the more complicated it gets to pick the right one.
Also, should we provide addKnownContactPointsWithPortsOnly variants in case the user passes in unresolved InetSocketAddress instances?
What if you provide the contact points one by one with addContactPoint? Why not allow bad addresses in that case as well?

Nicolas Filotto
November 23, 2016, 7:49 AM

@omichallat as you wish, feel free to close it as won't fix then, as I don't have enough rights to do it.

Fixed

Assignee

Unassigned

Reporter

Nicolas Filotto

Labels

None

PM Priority

None

Affects versions

Fix versions

Doc Impact

None

Size

None

External issue ID

None

External issue ID

None

Components

Priority

Major
Configure