Reresolve hostnames as fallback when all hosts are unreachable #1708
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@martin-sucha Could you please take a look? |
There was a problem hiding this comment.
@wprzytula thank you for the pull request! This is indeed a useful fix. I've added a few comments inline.
wprzytula
force-pushed
the
reresolve-dns-upstream
branch
from
35aaf59
to
25a06a8
Compare
|
I've addressed comments and retested. |
If all nodes in the cluster change their IPs at one time, driver used to no longer be able to ever contact the cluster; the only solution was to restart the driver. A fallback is added to the control connection `reconnect()` logic so that when no known host is reachable, all hostnames provided in ClusterConfig (initial contact points) are reresolved and control connection is attempted to be opened to any of them. If this succeeds, a metadata fetch is issued normally and the whole cluster is discovered with its new IPs. For the cluster to correctly learn new IPs in case that nodes are accessible indirectly (e.g. through a proxy), that is, by translated address and not `rpc_address` or `broadcast_address`, the code introduced in apache#1682 was extended to remove and re-add a host also when its translated address changed (even when its internal address stays the same). As a bonus, a misnamed variable `hostport` is renamed to a suitable `hostaddr`.
wprzytula
force-pushed
the
reresolve-dns-upstream
branch
from
25a06a8
to
b9737dd
Compare
|
@martin-sucha Done. |
|
Merged. Thank you! Would you be willing to contribute a test that would ensure that the function does not break in the future? |
|
I'm afraid that automated testing of DNS changes is too much effort. The manual tests that I ran were complex: they involved:
|
This was referenced Jul 3, 2023
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If all nodes in the cluster change their IPs at one time, driver used to no longer be able to ever contact the cluster; the only solution was to restart the driver. A fallback is added to the control connection
reconnect()logic so that when no known host is reachable, all hostnames provided in ClusterConfig (initial contact points) are reresolved and control connection is attempted to be opened to any of them. If this succeeds, a metadata fetch is issued normally and the whole cluster is discovered with its new IPs.For the cluster to correctly learn new IPs in case that nodes are accessible indirectly (e.g. through a proxy), that is, by translated address and not
rpc_addressorbroadcast_address, the code introduced in #1682 is extended to remove and re-add a host also when its translated address changed (even when its internal address stays the same).As a bonus, a misnamed variable
hostportis renamed to a suitablehostaddr.