Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.x: Reconnect to original contact points #344

Conversation

Bouncheck
Copy link
Collaborator

@Bouncheck Bouncheck commented Sep 23, 2024

Copied from commit message:

Add option to consider initial contact points during reconnection

When control connection tries to reconnect usually it considers only nodes
provided by load balancing policy. Usually those do not include what was
initially passed to the driver but the recently seen alive nodes. In some
setups the IPs can keep changing so it may be useful to have an option to
try initial contact points as one of the options during reconnection.
Mainly if the contact point is a hostname.

This change adds the option to the QueryOptions to control that behaviour
and adds necessary logic to ControlConnection class. It is disabled
by default, meaning that default behaviour remains unchanged.

Additionally adds org.burningwave tools dependency.
This dependency has features that allow for easier host resolution mocking.

Adds MappedHostResolverProvider class for testing as a single entry point
for controlling hostname resolution.

Adds an option to CcmBridge Builder to specify cluster name. Driver checks the
cluster name when reconnecting so it will refuse to reconnect to a different
CcmBridge auto-generated name.

@Bouncheck Bouncheck self-assigned this Sep 23, 2024
@Bouncheck Bouncheck force-pushed the scylla-3.x-reconnect-to-original-contact-points branch 3 times, most recently from 91c198f to 0723236 Compare September 23, 2024 21:07
@Bouncheck Bouncheck marked this pull request as ready for review September 23, 2024 21:09
@Bouncheck
Copy link
Collaborator Author

I believe this should resolve #224
If the contact point is passed the following way

cluster =
          Cluster.builder()
              .addContactPointsWithPorts(
                  InetSocketAddress.createUnresolved("some.hostname", 9042))
...

then with this change the driver should keep trying to reach some.hostname when no other nodes respond.

@Bouncheck Bouncheck changed the title Scylla 3.x reconnect to original contact points 3.x: Reconnect to original contact points Sep 23, 2024
When control connection tries to reconnect usually it considers only nodes
provided by load balancing policy. Usually those do not include what was
initially passed to the driver but the recently seen alive nodes. In some
setups the IPs can keep changing so it may be useful to have an option to
try initial contact points as one of the options during reconnection.
Mainly if the contact point is a hostname.

This change adds the option to the `QueryOptions` to control that behaviour
and adds necessary logic to `ControlConnection` class. It is disabled
by default, meaning that default behaviour remains unchanged.

Additionally adds org.burningwave tools dependency.
This dependency has features that allow for easier host resolution mocking.

Adds MappedHostResolverProvider class for testing as a single entry point
for controlling hostname resolution.

Adds an option to CcmBridge Builder to specify cluster name. Driver checks the
cluster name when reconnecting so it will refuse to reconnect to a different
CcmBridge auto-generated name.
@Bouncheck Bouncheck force-pushed the scylla-3.x-reconnect-to-original-contact-points branch from 26ed1b4 to 8970735 Compare September 27, 2024 18:43
@Bouncheck
Copy link
Collaborator Author

Pushed v2

Copy link
Collaborator

@dkropachev dkropachev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Bouncheck , is it ready to be shipped ? Let's fix it and merge

@dkropachev dkropachev merged commit 1cfc2df into scylladb:scylla-3.x Nov 22, 2024
10 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants