[PR #444] [MERGED] Expose TTLs on lookups #1445

New issue

Closed

opened 2026-03-16 02:06:45 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 02:06:45 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/444
Author: @hawkw
Created: 5/1/2018
Status: ✅ Merged
Merged: 5/3/2018
Merged by: @bluejekyll

Base: master ← Head: eliza/resolver-ttls

📝 Commits (10+)

7a39685 resolver: expose TTL on Lookup
905c243 resolver: make lookup TTLs accessible from LookupIp
e4e8886 resolver: use the lower TTL when appending
75cbbcd resolver: change lookup TTLs to deadlines rather than durations
4b61bad resolver: Change method names to better reflect that they're deadlines
3d37bc3 resolver: rename LruValue.ttl_until for consistency
7ad65fc resolver: fix test for Lookup deadlines
fda17eb Review feedback
526b8d7 resolver: use MAX_TTL rather than None when TTL is unknown
1dfc697 resolver: Review feedback

📊 Changes

6 files changed (+54 additions, -21 deletions)

View changed files

📝 integration-tests/tests/lookup_tests.rs (+1 -1)
📝 resolver/src/dns_lru.rs (+11 -11)
📝 resolver/src/hosts.rs (+3 -3)
📝 resolver/src/lookup.rs (+32 -5)
📝 resolver/src/lookup_ip.rs (+6 -0)
📝 resolver/src/resolver_future.rs (+1 -1)

📄 Description

This PR changes the trust-dns-resolver API to expose the TTLs of DNS
lookups, which is needed downstream in Conduit (runconduit/conduit#711).

Reviewers should note the following:

In some cases, Lookups were constructed in places where the TTL is not
known or there is no TTL (e.g. for localhost in lookup_state.rs).
Therefore, I made the TTL field on Lookup optional, and added a new
Lookup::new_with_ttl constructor, to avoid breaking existing code.
When appending two Lookups, if both have known TTLs, the returned
Lookup has the lower of the two TTLs. Thus, the appended Lookup
should time out when the shorter-lived of either of its constituents
does. This seemed like the correct behaviour to me, so that append
doesn't become a way for short-lived Lookups to escape their TTLs,
but I wasn't sure about this --- please let me know if this intuition
is incorrect!
I've added a simple unit test for the added TTL function in lookup.rs.

Signed-off-by: Eliza Weisman eliza@buoyant.io

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/hickory-dns/hickory-dns/pull/444 **Author:** [@hawkw](https://github.com/hawkw) **Created:** 5/1/2018 **Status:** ✅ Merged **Merged:** 5/3/2018 **Merged by:** [@bluejekyll](https://github.com/bluejekyll) **Base:** `master` ← **Head:** `eliza/resolver-ttls` --- ### 📝 Commits (10+) - [`7a39685`](https://github.com/hickory-dns/hickory-dns/commit/7a39685a146d53037bb2aef40b0b81fe15561446) resolver: expose TTL on Lookup - [`905c243`](https://github.com/hickory-dns/hickory-dns/commit/905c24388598dbdf492b02c1d003591853f4b747) resolver: make lookup TTLs accessible from LookupIp - [`e4e8886`](https://github.com/hickory-dns/hickory-dns/commit/e4e8886d5c416c632748e062c7115c420d744fc6) resolver: use the lower TTL when `append`ing - [`75cbbcd`](https://github.com/hickory-dns/hickory-dns/commit/75cbbcde7bf63c93a04ad72cc0cf813bb8e64e32) resolver: change lookup TTLs to deadlines rather than durations - [`4b61bad`](https://github.com/hickory-dns/hickory-dns/commit/4b61bad4a81278f68bac327f449307563832419a) resolver: Change method names to better reflect that they're deadlines - [`3d37bc3`](https://github.com/hickory-dns/hickory-dns/commit/3d37bc35c8b58278928b0d9f652788beff057f52) resolver: rename LruValue.ttl_until for consistency - [`7ad65fc`](https://github.com/hickory-dns/hickory-dns/commit/7ad65fc13a820275c53b2809778da4445e40b670) resolver: fix test for Lookup deadlines - [`fda17eb`](https://github.com/hickory-dns/hickory-dns/commit/fda17ebff03f6ed78aeb1902efaeae26fb0decdb) Review feedback - [`526b8d7`](https://github.com/hickory-dns/hickory-dns/commit/526b8d7f4de3bd92ab0ddcbf7d7a2e26895efce8) resolver: use `MAX_TTL` rather than `None` when TTL is unknown - [`1dfc697`](https://github.com/hickory-dns/hickory-dns/commit/1dfc6972d8c0e9652d3f2953f974e09e65e4d3a9) resolver: Review feedback ### 📊 Changes **6 files changed** (+54 additions, -21 deletions) <details> <summary>View changed files</summary> 📝 `integration-tests/tests/lookup_tests.rs` (+1 -1) 📝 `resolver/src/dns_lru.rs` (+11 -11) 📝 `resolver/src/hosts.rs` (+3 -3) 📝 `resolver/src/lookup.rs` (+32 -5) 📝 `resolver/src/lookup_ip.rs` (+6 -0) 📝 `resolver/src/resolver_future.rs` (+1 -1) </details> ### 📄 Description This PR changes the `trust-dns-resolver` API to expose the TTLs of DNS lookups, which is needed downstream in Conduit (runconduit/conduit#711). Reviewers should note the following: + In some cases, `Lookup`s were constructed in places where the TTL is not known or there is no TTL (e.g. for `localhost` in `lookup_state.rs`). Therefore, I made the TTL field on `Lookup` optional, and added a new `Lookup::new_with_ttl` constructor, to avoid breaking existing code. + When `append`ing two `Lookups`, if both have known TTLs, the returned `Lookup` has the lower of the two TTLs. Thus, the appended `Lookup` should time out when the shorter-lived of either of its constituents does. This _seemed_ like the correct behaviour to me, so that `append` doesn't become a way for short-lived `Lookup`s to escape their TTLs, but I wasn't sure about this --- please let me know if this intuition is incorrect! + I've added a simple unit test for the added `TTL` function in `lookup.rs`. Signed-off-by: Eliza Weisman <eliza@buoyant.io> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>