From 561ebd07f4666c4814c38a4d04b610079c9c31f4 Mon Sep 17 00:00:00 2001
From: Frank Denis <github@pureftpd.org>
Date: Tue, 5 May 2020 17:27:28 +0200
Subject: [PATCH] client_ttl_jitter -> client_ttl_holdon

---
 example-encrypted-dns.toml |  8 ++++----
 src/cache.rs               | 16 +++++++++++++++-
 src/config.rs              |  2 +-
 src/globals.rs             |  2 +-
 src/main.rs                |  2 +-
 src/resolver.rs            |  8 +++++---
 6 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/example-encrypted-dns.toml b/example-encrypted-dns.toml
index 4ef1c3d..d992640 100644
--- a/example-encrypted-dns.toml
+++ b/example-encrypted-dns.toml
@@ -83,11 +83,11 @@ cache_ttl_max = 86400
 cache_ttl_error = 600
 
 
-## DNS cache: to avoid bursts of traffic when an RRSET expires,
-## introduce jitter to the decreasing TTL sent to clients when the
-## actual TTL goes below that value.
+## DNS cache: to avoid bursts of traffic for popular queries when an
+## RRSET expires, hold a TTL received from an upstream server for
+## `client_ttl_holdon` seconds before decreasing it in client responses.
 
-client_ttl_jitter = 60
+client_ttl_holdon = 60
 
 
 ## Run as a background process
diff --git a/src/cache.rs b/src/cache.rs
index 9f10b46..d9da4b7 100644
--- a/src/cache.rs
+++ b/src/cache.rs
@@ -9,6 +9,7 @@ use std::sync::Arc;
 pub struct CachedResponse {
     response: Vec<u8>,
     expiry: Instant,
+    original_ttl: u32,
 }
 
 impl CachedResponse {
@@ -16,24 +17,37 @@ impl CachedResponse {
         let ttl = dns::min_ttl(&response, cache.ttl_min, cache.ttl_max, cache.ttl_error)
             .unwrap_or(cache.ttl_error);
         let expiry = Instant::recent() + Duration::from_secs(u64::from(ttl));
-        CachedResponse { response, expiry }
+        CachedResponse {
+            response,
+            expiry,
+            original_ttl: ttl,
+        }
     }
 
+    #[inline]
     pub fn set_tid(&mut self, tid: u16) {
         dns::set_tid(&mut self.response, tid)
     }
 
+    #[inline]
     pub fn into_response(self) -> Vec<u8> {
         self.response
     }
 
+    #[inline]
     pub fn has_expired(&self) -> bool {
         Instant::recent() > self.expiry
     }
 
+    #[inline]
     pub fn ttl(&self) -> u32 {
         (self.expiry - Instant::recent()).as_secs() as _
     }
+
+    #[inline]
+    pub fn original_ttl(&self) -> u32 {
+        self.original_ttl
+    }
 }
 
 #[derive(Clone, Derivative)]
diff --git a/src/config.rs b/src/config.rs
index b88acb8..192949a 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -82,7 +82,7 @@ pub struct Config {
     pub pid_file: Option<PathBuf>,
     pub log_file: Option<PathBuf>,
     pub my_ip: Option<String>,
-    pub client_ttl_jitter: Option<u32>,
+    pub client_ttl_holdon: Option<u32>,
     #[cfg(feature = "metrics")]
     pub metrics: Option<MetricsConfig>,
     pub anonymized_dns: Option<AnonymizedDNSConfig>,
diff --git a/src/globals.rs b/src/globals.rs
index e8a3fb0..c855fdc 100644
--- a/src/globals.rs
+++ b/src/globals.rs
@@ -49,7 +49,7 @@ pub struct Globals {
     pub anonymized_dns_allow_non_reserved_ports: bool,
     pub anonymized_dns_blacklisted_ips: Vec<IpAddr>,
     pub access_control_tokens: Option<Vec<String>>,
-    pub client_ttl_jitter: u32,
+    pub client_ttl_holdon: u32,
     pub my_ip: Option<Vec<u8>>,
     #[cfg(feature = "metrics")]
     #[derivative(Debug = "ignore")]
diff --git a/src/main.rs b/src/main.rs
index 5322d2c..fbcfb61 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -705,7 +705,7 @@ fn main() -> Result<(), Error> {
         anonymized_dns_blacklisted_ips,
         access_control_tokens,
         my_ip: config.my_ip.map(|ip| ip.as_bytes().to_ascii_lowercase()),
-        client_ttl_jitter: config.client_ttl_jitter.unwrap_or(60),
+        client_ttl_holdon: config.client_ttl_holdon.unwrap_or(60),
         #[cfg(feature = "metrics")]
         varz: Varz::default(),
     });
diff --git a/src/resolver.rs b/src/resolver.rs
index b851cd3..83374d9 100644
--- a/src/resolver.rs
+++ b/src/resolver.rs
@@ -7,6 +7,7 @@ use crate::ClientCtx;
 use byteorder::{BigEndian, ByteOrder};
 use rand::prelude::*;
 use siphasher::sip128::Hasher128;
+use std::cmp;
 use std::hash::Hasher;
 use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6};
 use tokio::net::{TcpStream, UdpSocket};
@@ -226,11 +227,12 @@ pub async fn get_cached_response_or_resolve(
                 #[cfg(feature = "metrics")]
                 globals.varz.client_queries_cached.inc();
                 cached_response.set_tid(original_tid);
+                let original_ttl = cached_response.original_ttl();
                 let mut ttl = cached_response.ttl();
-                if ttl < globals.client_ttl_jitter {
-                    let jitter = rand::thread_rng().gen::<u32>() % globals.client_ttl_jitter;
-                    ttl = globals.client_ttl_jitter.saturating_add(jitter);
+                if ttl.saturating_add(globals.client_ttl_holdon) > original_ttl {
+                    ttl = original_ttl;
                 }
+                ttl = cmp::max(1, ttl);
                 let mut response = cached_response.into_response();
                 dns::set_ttl(&mut response, ttl)?;
                 dns::recase_qname(&mut response, &packet_qname)?;