Problem
ChannelFactory.resolveCandidates() hands the address list returned by Netty's
AddressResolverGroup straight to reattachHostnames() and rotate() without collapsing
duplicates. If a name maps to the same address more than once, that address is attempted more than
once within a single logical connect().
Each redundant attempt costs a full advanced.connection.connect-timeout when the address is not
answering, and because rotate() sorts candidates by toString() the duplicates end up adjacent,
so the stalls are consecutive. ChannelFactory#tryNextCandidate already documents a worst case of
N × connect-timeout for a name with N addresses; duplicates inflate that N with attempts that
cannot tell the driver anything it does not already know from the first one.
Where duplicates come from in practice: a zone file with a repeated A record, or a custom
AddressResolverGroup that stitches results together from more than one source.
Found while reviewing #890 (DRIVER-201, multi-address DNS resolution), which introduced this code
path. Deliberately left out of that PR to keep it scoped — duplicate A records are unusual, no
reviewer raised it, and #890 was close to being signed off. This issue is that follow-up.
Fix sketch
Collapse after reattachHostnames() and before rotate():
List<SocketAddress> unique = new ArrayList<>(new LinkedHashSet<>(reattachHostnames(address, addresses)));
result.complete(rotate(address, unique));
Two things make this safe:
LinkedHashSet collapses what we want it to. InetSocketAddress.equals compares the address
bytes and the port and ignores the attached host name, so two candidates for the same IP and port
are equal even if the resolver labelled them differently. Doing it after reattachHostnames()
also means the surviving entry carries the queried name, as every other candidate does.- Retrying the same address is not this loop's job. The candidate loop exists to try
different addresses; recovering from a transient failure at the same address is what the
reconnection schedule is for. So there is no behaviour worth preserving here.
Order is preserved, so nothing about rotation determinism changes.
Test
ChannelFactoryMultiAddressTest already covers the candidate loop with a stub resolver
(TestAddressResolverGroup) that answers with a fixed list, so a case that answers with the same
address twice and asserts a single connection attempt fits alongside the existing tests.
Problem
ChannelFactory.resolveCandidates()hands the address list returned by Netty'sAddressResolverGroupstraight toreattachHostnames()androtate()without collapsingduplicates. If a name maps to the same address more than once, that address is attempted more than
once within a single logical
connect().Each redundant attempt costs a full
advanced.connection.connect-timeoutwhen the address is notanswering, and because
rotate()sorts candidates bytoString()the duplicates end up adjacent,so the stalls are consecutive.
ChannelFactory#tryNextCandidatealready documents a worst case ofN × connect-timeoutfor a name with N addresses; duplicates inflate that N with attempts thatcannot tell the driver anything it does not already know from the first one.
Where duplicates come from in practice: a zone file with a repeated A record, or a custom
AddressResolverGroupthat stitches results together from more than one source.Found while reviewing #890 (DRIVER-201, multi-address DNS resolution), which introduced this code
path. Deliberately left out of that PR to keep it scoped — duplicate A records are unusual, no
reviewer raised it, and #890 was close to being signed off. This issue is that follow-up.
Fix sketch
Collapse after
reattachHostnames()and beforerotate():Two things make this safe:
LinkedHashSetcollapses what we want it to.InetSocketAddress.equalscompares the addressbytes and the port and ignores the attached host name, so two candidates for the same IP and port
are equal even if the resolver labelled them differently. Doing it after
reattachHostnames()also means the surviving entry carries the queried name, as every other candidate does.
different addresses; recovering from a transient failure at the same address is what the
reconnection schedule is for. So there is no behaviour worth preserving here.
Order is preserved, so nothing about rotation determinism changes.
Test
ChannelFactoryMultiAddressTestalready covers the candidate loop with a stub resolver(
TestAddressResolverGroup) that answers with a fixed list, so a case that answers with the sameaddress twice and asserts a single connection attempt fits alongside the existing tests.