[PR #2596] [MERGED] Remove redundant cache insert in resolve_cnames() #3166

New issue

Closed

opened 2026-03-16 11:28:59 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 11:28:59 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/2596
Author: @divergentdave
Created: 11/21/2024
Status: ✅ Merged
Merged: 11/21/2024
Merged by: @marcus0x62

Base: main ← Head: david/remove-redundant-cache-insert

📝 Commits (1)

a524006 Remove redundant cache insert in resolve_cnames()

📊 Changes

1 file changed (+1 additions, -7 deletions)

View changed files

📝 crates/recursor/src/recursor_dns_handle.rs (+1 -7)

📄 Description

This removes a call to DnsLru::insert_records() in RecursorDnsHandle::resolve_cnames(). This is a follow-up to #2576. We can remove this call because it is always redundant with either a preceding cache fetch or cache insert. RecursorDnsHandle::resolve_cnames() is called from three places in RecursorDnsHandle::resolve(). The first callsite is after a successful cache hit, and results in re-inserting the same Lookup. The other two callsites are after a successful call to RecursorDnsHandle::lookup(), returning a positive response. The last thing that RecursorDnsHandle::lookup() does is call cache_response(), which calls DnsLru::insert_records(), and in doing so flattens a response into a Lookup. Then, the call to DnsLru::insert_records() in RecursorDnsHandle::resolve_cnames() would store the same lookup again.

I tested the performance impact of this change as before, using dnsperf and making the same request three million times. The improvement from eliminating these cache inserts was drastic.

before: 20206qps, 20099qps, 20866qps, 20929qps, 20919qps
after: 134957qps, 131468qps, 129588qps, 132800qps, 128908qps

_{🔄 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/2596 **Author:** [@divergentdave](https://github.com/divergentdave) **Created:** 11/21/2024 **Status:** ✅ Merged **Merged:** 11/21/2024 **Merged by:** [@marcus0x62](https://github.com/marcus0x62) **Base:** `main` ← **Head:** `david/remove-redundant-cache-insert` --- ### 📝 Commits (1) - [`a524006`](https://github.com/hickory-dns/hickory-dns/commit/a5240061efd72d4a343dc900387dbc8cb7fdf257) Remove redundant cache insert in resolve_cnames() ### 📊 Changes **1 file changed** (+1 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `crates/recursor/src/recursor_dns_handle.rs` (+1 -7) </details> ### 📄 Description This removes a call to `DnsLru::insert_records()` in `RecursorDnsHandle::resolve_cnames()`. This is a follow-up to #2576. We can remove this call because it is always redundant with either a preceding cache fetch or cache insert. `RecursorDnsHandle::resolve_cnames()` is called from three places in `RecursorDnsHandle::resolve()`. The first callsite is after a successful cache hit, and results in re-inserting the same `Lookup`. The other two callsites are after a successful call to `RecursorDnsHandle::lookup()`, returning a positive response. The last thing that `RecursorDnsHandle::lookup()` does is call `cache_response()`, which calls `DnsLru::insert_records()`, and in doing so flattens a response into a `Lookup`. Then, the call to `DnsLru::insert_records()` in `RecursorDnsHandle::resolve_cnames()` would store the same lookup again. I tested the performance impact of this change as before, using `dnsperf` and making the same request three million times. The improvement from eliminating these cache inserts was drastic. > before: 20206qps, 20099qps, 20866qps, 20929qps, 20919qps > after: 134957qps, 131468qps, 129588qps, 132800qps, 128908qps --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>