[GH-ISSUE #2402] Conflict between `ResolverOpts` server selection options #977

New issue

Closed

opened 2026-03-16 01:09:15 +03:00 by kerem · 4 comments

kerem commented

2026-03-16 01:09:15 +03:00

Owner

Originally created by @andrewbaxter on GitHub (Sep 1, 2024).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2402

Sorry, I'm not sure this is really a feature, but it's not really a bug either (maybe?)

ResolverOpts has two parameters server_ordering_strategy and shuffle_dns_servers. They don't mention each other, but it seems like they both operate on the same data, and shuffle_dns_servers overwrites the server_ordering_strategy. This could cause unexpected behavior (like if you set shuffle_dns_servers and forget about it, then later tweak server_ordering_strategy to avoid some issue).

Originally created by @andrewbaxter on GitHub (Sep 1, 2024). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2402 Sorry, I'm not sure this is really a feature, but it's not really a bug either (maybe?) `ResolverOpts` has two parameters `server_ordering_strategy` and `shuffle_dns_servers`. They don't mention each other, but it seems like they both operate on the same data, and `shuffle_dns_servers` overwrites the `server_ordering_strategy`. This could cause unexpected behavior (like if you set `shuffle_dns_servers` and forget about it, then later tweak `server_ordering_strategy` to avoid some issue).

kerem

2026-03-16 01:09:15 +03:00

closed this issue
added the
cleanup
label

kerem commented

2026-03-16 01:09:31 +03:00

Author

Owner

@djc commented on GitHub (Sep 2, 2024):

server_ordering_strategy was added in #1766 (previously discussed in #1702) while shuffle_name_servers originates from #1920. It does indeed look like there is substantial overlap, which we completely missed during review. To summarize:

ServerOrderingStrategy::QueryStatistics re-orders the servers by a metric designed by a decayed round trip time once per request
ServerOrderingStrategy::UseProvidedOrder uses the specified order of the servers
shuffle_dns_servers will shuffle the list of servers for each run through the entire list of servers

So the interactions would be:

(QueryStatistics, true) doesn't really make sense, because now the statistics end up not being used due to shuffling
(QueryStatistics, false) is the default and seems like a good default case
(UseProvidedOrder, true) doesn't really make sense because shuffling makes it so that the provided order is not used
(UseProvidedOrder, false) seems a like a decent alternative

So we should probably drop shuffle_dns_servers as a separate option and maybe tweak the ServerOrderingStrategy. Maybe a good way out would be to provide an extra option shuffle: bool for QueryStatistics which some shuffling behavior but would weight the shuffle via the statistics?

@bluejekyll any opinions?

@djc commented on GitHub (Sep 2, 2024): `server_ordering_strategy` was added in #1766 (previously discussed in #1702) while `shuffle_name_servers` originates from #1920. It does indeed look like there is substantial overlap, which we completely missed during review. To summarize: * `ServerOrderingStrategy::QueryStatistics` re-orders the servers by a metric designed by a decayed round trip time once per request * `ServerOrderingStrategy::UseProvidedOrder` uses the specified order of the servers * `shuffle_dns_servers` will shuffle the list of servers for each run through the entire list of servers So the interactions would be: - `(QueryStatistics, true)` doesn't really make sense, because now the statistics end up not being used due to shuffling - `(QueryStatistics, false)` is the default and seems like a good default case - `(UseProvidedOrder, true)` doesn't really make sense because shuffling makes it so that the provided order is not used - `(UseProvidedOrder, false)` seems a like a decent alternative So we should probably drop `shuffle_dns_servers` as a separate option and maybe tweak the `ServerOrderingStrategy`. Maybe a good way out would be to provide an extra option `shuffle: bool` for `QueryStatistics` which some shuffling behavior but would weight the shuffle via the statistics? @bluejekyll any opinions?

kerem commented

2026-03-16 01:09:36 +03:00

Author

Owner

@samin-cf commented on GitHub (Dec 2, 2024):

Bump 🙂 @djc @bluejekyll

@samin-cf commented on GitHub (Dec 2, 2024): Bump 🙂 @djc @bluejekyll

kerem commented

2026-03-16 01:09:41 +03:00

Author

Owner

@djc commented on GitHub (Dec 2, 2024):

@marcus0x62 opinions?

@djc commented on GitHub (Dec 2, 2024): @marcus0x62 opinions?

kerem commented

2026-03-16 01:09:46 +03:00

Author

Owner

@bluejekyll commented on GitHub (Mar 2, 2025):

Yes, we should clean this up. Definitely don't want confusing behavior, so we should make it clearer how these work with each other.

@bluejekyll commented on GitHub (Mar 2, 2025): Yes, we should clean this up. Definitely don't want confusing behavior, so we should make it clearer how these work with each other.