feat: Add progressive backoff for failing IMAP connection attempts (#5443)

This way we avoid an immediate retry if the network is not yet ready exhausting the ratelimiter's
quota of two connection attempts. Also notify the ratelimiter only after a successful connection so
that it only limits the server load, but not connection attempts.
This commit is contained in:
iequidoo
2024-04-14 05:33:21 -03:00
committed by iequidoo
parent ae10ed5c40
commit 22f240dd4d

View File

@@ -5,11 +5,12 @@
use std::{ use std::{
cmp::max, cmp::max,
cmp::min,
collections::{BTreeMap, BTreeSet, HashMap}, collections::{BTreeMap, BTreeSet, HashMap},
iter::Peekable, iter::Peekable,
mem::take, mem::take,
sync::atomic::Ordering, sync::atomic::Ordering,
time::Duration, time::{Duration, UNIX_EPOCH},
}; };
use anyhow::{bail, format_err, Context as _, Result}; use anyhow::{bail, format_err, Context as _, Result};
@@ -19,6 +20,7 @@ use deltachat_contact_tools::{normalize_name, ContactAddress};
use futures::{FutureExt as _, StreamExt, TryStreamExt}; use futures::{FutureExt as _, StreamExt, TryStreamExt};
use futures_lite::FutureExt; use futures_lite::FutureExt;
use num_traits::FromPrimitive; use num_traits::FromPrimitive;
use rand::Rng;
use ratelimit::Ratelimit; use ratelimit::Ratelimit;
use crate::chat::{self, ChatId, ChatIdBlocked}; use crate::chat::{self, ChatId, ChatIdBlocked};
@@ -41,7 +43,7 @@ use crate::scheduler::connectivity::ConnectivityStore;
use crate::socks::Socks5Config; use crate::socks::Socks5Config;
use crate::sql; use crate::sql;
use crate::stock_str; use crate::stock_str;
use crate::tools::{create_id, duration_to_str}; use crate::tools::{self, create_id, duration_to_str};
pub(crate) mod capabilities; pub(crate) mod capabilities;
mod client; mod client;
@@ -81,10 +83,12 @@ pub(crate) struct Imap {
pub(crate) connectivity: ConnectivityStore, pub(crate) connectivity: ConnectivityStore,
/// Rate limit for IMAP connection attempts. conn_last_try: tools::Time,
conn_backoff_ms: u64,
/// Rate limit for successful IMAP connections.
/// ///
/// This rate limit prevents busy loop /// This rate limit prevents busy loop in case the server refuses logins
/// in case the server refuses connections
/// or in case connection gets dropped over and over due to IMAP bug, /// or in case connection gets dropped over and over due to IMAP bug,
/// e.g. the server returning invalid response to SELECT command /// e.g. the server returning invalid response to SELECT command
/// immediately after logging in or returning an error in response to LOGIN command /// immediately after logging in or returning an error in response to LOGIN command
@@ -247,6 +251,8 @@ impl Imap {
strict_tls, strict_tls,
login_failed_once: false, login_failed_once: false,
connectivity: Default::default(), connectivity: Default::default(),
conn_last_try: UNIX_EPOCH,
conn_backoff_ms: 0,
// 1 connection per minute + a burst of 2. // 1 connection per minute + a burst of 2.
ratelimit: Ratelimit::new(Duration::new(120, 0), 2.0), ratelimit: Ratelimit::new(Duration::new(120, 0), 2.0),
}; };
@@ -292,7 +298,15 @@ impl Imap {
bail!("IMAP operation attempted while it is torn down"); bail!("IMAP operation attempted while it is torn down");
} }
let ratelimit_duration = self.ratelimit.until_can_send(); let now = tools::Time::now();
let until_can_send = max(
min(self.conn_last_try, now)
.checked_add(Duration::from_millis(self.conn_backoff_ms))
.unwrap_or(now),
now,
)
.duration_since(now)?;
let ratelimit_duration = max(until_can_send, self.ratelimit.until_can_send());
if !ratelimit_duration.is_zero() { if !ratelimit_duration.is_zero() {
warn!( warn!(
context, context,
@@ -315,7 +329,16 @@ impl Imap {
info!(context, "Connecting to IMAP server"); info!(context, "Connecting to IMAP server");
self.connectivity.set_connecting(context).await; self.connectivity.set_connecting(context).await;
self.ratelimit.send();
self.conn_last_try = tools::Time::now();
const BACKOFF_MIN_MS: u64 = 2000;
const BACKOFF_MAX_MS: u64 = 80_000;
self.conn_backoff_ms = min(self.conn_backoff_ms, BACKOFF_MAX_MS / 2);
self.conn_backoff_ms = self.conn_backoff_ms.saturating_add(
rand::thread_rng().gen_range((self.conn_backoff_ms / 2)..=self.conn_backoff_ms),
);
self.conn_backoff_ms = max(BACKOFF_MIN_MS, self.conn_backoff_ms);
let connection_res: Result<Client> = let connection_res: Result<Client> =
if self.lp.security == Socket::Starttls || self.lp.security == Socket::Plain { if self.lp.security == Socket::Starttls || self.lp.security == Socket::Plain {
let imap_server: &str = self.lp.server.as_ref(); let imap_server: &str = self.lp.server.as_ref();
@@ -363,6 +386,8 @@ impl Imap {
} }
}; };
let client = connection_res?; let client = connection_res?;
self.conn_backoff_ms = BACKOFF_MIN_MS;
self.ratelimit.send();
let imap_user: &str = self.lp.user.as_ref(); let imap_user: &str = self.lp.user.as_ref();
let imap_pw: &str = self.lp.password.as_ref(); let imap_pw: &str = self.lp.password.as_ref();