An on-device drilldown (Pixel, real Gmail over LTE) proved the root cause of the 31–40 s uncached-message stall: Gmail server-side throttles LibreMail's connect-per-operation IMAP. Every op was a fresh CONNECT + TLS + LOGIN, and full-history backfill's body+attachment prefetch generated ~601 connections in ~22 min, which trips (and sustains) Gmail's per-account rate/bandwidth clamp — body download collapses to ~4 KB/s and the clamp persists once tripped. The live gauge peaked at only 5 (Gmail allows ~15), so it is connection volume/rate, not count. Cross-provider control: Outlook IMAP on the same device opened in 2–3 s.
Connection reuse collapses ~601 sockets → ~1 warm socket per account, removing the throttle's trigger. The reuse cache was already built by the #125 spike but left OFF. This PR turns it on and hardens it.
Part of #357 — this is Part 2 of its proposed approach ("Warm/interactive IMAP connection — revisit #125"). Scoped to connection reuse only; #357's prefetch half and the on-device A/B remain, so #357 stays open. Origin/design: #125 (closed spike, docs/perf/issue-125-*).
How reuse is enabled (with a safety switch)
New BuildConfig.IMAP_CONNECTION_REUSE (default true) drives the production ImapClient no-arg @Inject constructor. Safety switch: to disable if a server misbehaves with a kept-alive socket, flip it to "false" in app/build.gradle.kts — a build-config change, no Kotlin edit, no logic change. The internal ImapClient(reuseConnections, reuseIdleTimeoutMillis) constructor stays the test/harness seam.
Universal — applies to all providers including Outlook. No per-provider caps / rate-limits / 429-backoff here; that is a separate effort (#356/#360–#364) and explicitly out of scope.
Hardening ImapConnectionCache for production
Transparent stale recovery. Broadened drop-detection to Angus's own iap.ConnectionException (and a MessagingExceptioncaused by one) — the real signal folder.open() throws on a server-dropped idle socket, which the previous IOException-only check missed, so the reconnect never actually fired. A dropped reused socket is now rebuilt once and the op retried, so callers see no spurious error. A genuine application error (e.g. message-not-found) is not retried.
Idle eviction.evictIdle() closes a connection unused past the reuse idle timeout (default 5 min), swept every 2 min by IdleService; a connection currently in use is skipped (non-blocking tryLock).
Teardown.IdleService tears down reused connections on the low-battery push-teardown path (#88/#89/#90), mirroring the IDLE-connection teardown.
Concurrency. One connection per account behind a per-account Mutex. Coexists with IMAP IDLE (its own separate connection), so reuse adds ≤1 socket/account — far under provider limits.
PII-free AppLog on the lifecycle (open / reuse-hit / reconnect-stale / evict / teardown), keyed by an opaque per-cache ordinal (never host/user/secret), plus the #358ImapPerf breadcrumb (shows connect≈0ms on a reuse hit).
Tests (fast gate; no emulator)
ImapConnectionCacheTest — reuse, retry-once stale recovery, narrow drop detection, deterministic idle eviction (injected clock), teardown.
ImapFolderOpenLatencyTest (GreenMail + counting proxy) — N ops share one connection/LOGIN; a force-dropped socket is transparently reconnected; an app error does not reconnect; idle eviction issues a LOGOUT and the next op reconnects.
Correctness suites (ImapClientTest / ImapClientBackfillTest / MailBackfillerTest) pinned to reuse-off so their connect-per-op assertions are unchanged.
Head-of-line blocking (by design). One socket per account means backfill and interactive ops on the same account now serialize on the per-account mutex (they previously used separate throwaway sockets). This is the known single-connection trade-off; a bounded pool / dedicated interactive lane is the deferred follow-up (#355). The on-device A/B will measure it.
Mutation at-least-once (documented, deferred). The retry re-runs the whole op, so a mutation dropped mid-flight is at-least-once. Flag sets are idempotent; the dominant real case — a socket the server dropped while idle, detected on the next op's first command before any mutation is issued — is safe. A copy-then-expunge move interrupted between its two halves is the rare exception left to the mutation-idempotency review noted in the #125 spike doc.
Not auto-merged: this is a behaviour-changing core-IMAP-path change; the coordinator will run an on-device reuse ON/OFF A/B on Gmail to prove the win first.
## Why
An on-device drilldown (Pixel, real Gmail over LTE) **proved** the root cause of the 31–40 s uncached-message stall: Gmail **server-side throttles** LibreMail's **connect-per-operation** IMAP. Every op was a fresh `CONNECT + TLS + LOGIN`, and full-history backfill's body+attachment prefetch generated **~601 connections in ~22 min**, which trips (and sustains) Gmail's per-account rate/bandwidth clamp — body download collapses to ~4 KB/s and the clamp *persists once tripped*. The `live` gauge peaked at only **5** (Gmail allows ~15), so it is connection **volume/rate**, not count. Cross-provider control: **Outlook IMAP on the same device opened in 2–3 s**.
Connection reuse collapses ~601 sockets → **~1 warm socket per account**, removing the throttle's trigger. The reuse cache was already built by the #125 spike but left **OFF**. This PR turns it on and hardens it.
**Part of #357** — this is Part 2 of its proposed approach ("Warm/interactive IMAP connection — revisit #125"). Scoped to **connection reuse only**; #357's prefetch half and the on-device A/B remain, so #357 stays open. Origin/design: #125 (closed spike, `docs/perf/issue-125-*`).
## How reuse is enabled (with a safety switch)
- New `BuildConfig.IMAP_CONNECTION_REUSE` (**default `true`**) drives the production `ImapClient` no-arg `@Inject` constructor. **Safety switch:** to disable if a server misbehaves with a kept-alive socket, flip it to `"false"` in `app/build.gradle.kts` — a build-config change, no Kotlin edit, no logic change. The internal `ImapClient(reuseConnections, reuseIdleTimeoutMillis)` constructor stays the test/harness seam.
- **Universal** — applies to all providers **including Outlook**. No per-provider caps / rate-limits / 429-backoff here; that is a separate effort (#356/#360–#364) and explicitly out of scope.
## Hardening `ImapConnectionCache` for production
- **Transparent stale recovery.** Broadened drop-detection to Angus's own `iap.ConnectionException` (and a `MessagingException` *caused by* one) — the real signal `folder.open()` throws on a server-dropped idle socket, which the previous IOException-only check **missed**, so the reconnect never actually fired. A dropped reused socket is now rebuilt **once** and the op retried, so callers see no spurious error. A genuine application error (e.g. message-not-found) is **not** retried.
- **Idle eviction.** `evictIdle()` closes a connection unused past the reuse idle timeout (default 5 min), swept every 2 min by `IdleService`; a connection currently in use is skipped (non-blocking `tryLock`).
- **Teardown.** `IdleService` tears down reused connections on the low-battery push-teardown path (#88/#89/#90), mirroring the IDLE-connection teardown.
- **Concurrency.** One connection per account behind a per-account `Mutex`. Coexists with IMAP IDLE (its own separate connection), so reuse adds ≤1 socket/account — far under provider limits.
- **PII-free `AppLog`** on the lifecycle (open / reuse-hit / reconnect-stale / evict / teardown), keyed by an **opaque per-cache ordinal** (never host/user/secret), plus the #358 `ImapPerf` breadcrumb (shows `connect≈0ms` on a reuse hit).
## Tests (fast gate; no emulator)
- **`ImapConnectionCacheTest`** — reuse, retry-once stale recovery, narrow drop detection, deterministic idle eviction (injected clock), teardown.
- **`ImapFolderOpenLatencyTest`** (GreenMail + counting proxy) — N ops share one connection/LOGIN; a force-dropped socket is transparently reconnected; an app error does **not** reconnect; idle eviction issues a `LOGOUT` and the next op reconnects.
- Correctness suites (`ImapClientTest` / `ImapClientBackfillTest` / `MailBackfillerTest`) pinned to reuse-off so their connect-per-op assertions are unchanged.
**Fast gate green:** `assembleDebug` · `testDebugUnitTest` · `compileDebugAndroidTestKotlin` · `lintDebug` · `ktlintCheck` · `detekt`.
## Risk / edge cases
- **Head-of-line blocking (by design).** One socket per account means backfill and interactive ops on the same account now serialize on the per-account mutex (they previously used separate throwaway sockets). This is the known single-connection trade-off; a bounded pool / dedicated interactive lane is the deferred follow-up (#355). The on-device A/B will measure it.
- **Mutation at-least-once (documented, deferred).** The retry re-runs the whole op, so a *mutation* dropped mid-flight is at-least-once. Flag sets are idempotent; the dominant real case — a socket the server dropped while idle, detected on the next op's first command **before** any mutation is issued — is safe. A copy-then-expunge move interrupted between its two halves is the rare exception left to the mutation-idempotency review noted in the #125 spike doc.
- Not auto-merged: this is a behaviour-changing core-IMAP-path change; the coordinator will run an on-device reuse ON/OFF A/B on Gmail to prove the win first.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
JMR-dev
(Migrated from github.com)
reviewed 2026-07-06 00:56:52 +00:00
Claude - Yes — pushed 81a3b7e. isConnectionDrop now leads with guard clauses (definite-drop types, then a not-MessagingException early return) instead of the when expression, main path un-nested. One subtlety worth flagging: FolderClosedException/StoreClosedException are themselves MessagingException subtypes, so their guard has to stay ordered before the MessagingException one (else they'd get gated on a .cause they don't carry) — left a comment on it. Behavior-identical; all 8 existing ImapConnectionCacheTest cases still pass, including the ones that pin that ordering.
Claude - Yes — pushed 81a3b7e. `isConnectionDrop` now leads with guard clauses (definite-drop types, then a not-MessagingException early return) instead of the `when` expression, main path un-nested. One subtlety worth flagging: FolderClosedException/StoreClosedException are themselves MessagingException subtypes, so their guard has to stay ordered before the MessagingException one (else they'd get gated on a `.cause` they don't carry) — left a comment on it. Behavior-identical; all 8 existing ImapConnectionCacheTest cases still pass, including the ones that pin that ordering.
JMR-dev
(Migrated from github.com)
reviewed 2026-07-06 02:19:35 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Why
An on-device drilldown (Pixel, real Gmail over LTE) proved the root cause of the 31–40 s uncached-message stall: Gmail server-side throttles LibreMail's connect-per-operation IMAP. Every op was a fresh
CONNECT + TLS + LOGIN, and full-history backfill's body+attachment prefetch generated ~601 connections in ~22 min, which trips (and sustains) Gmail's per-account rate/bandwidth clamp — body download collapses to ~4 KB/s and the clamp persists once tripped. Thelivegauge peaked at only 5 (Gmail allows ~15), so it is connection volume/rate, not count. Cross-provider control: Outlook IMAP on the same device opened in 2–3 s.Connection reuse collapses ~601 sockets → ~1 warm socket per account, removing the throttle's trigger. The reuse cache was already built by the #125 spike but left OFF. This PR turns it on and hardens it.
Part of #357 — this is Part 2 of its proposed approach ("Warm/interactive IMAP connection — revisit #125"). Scoped to connection reuse only; #357's prefetch half and the on-device A/B remain, so #357 stays open. Origin/design: #125 (closed spike,
docs/perf/issue-125-*).How reuse is enabled (with a safety switch)
BuildConfig.IMAP_CONNECTION_REUSE(defaulttrue) drives the productionImapClientno-arg@Injectconstructor. Safety switch: to disable if a server misbehaves with a kept-alive socket, flip it to"false"inapp/build.gradle.kts— a build-config change, no Kotlin edit, no logic change. The internalImapClient(reuseConnections, reuseIdleTimeoutMillis)constructor stays the test/harness seam.Hardening
ImapConnectionCachefor productioniap.ConnectionException(and aMessagingExceptioncaused by one) — the real signalfolder.open()throws on a server-dropped idle socket, which the previous IOException-only check missed, so the reconnect never actually fired. A dropped reused socket is now rebuilt once and the op retried, so callers see no spurious error. A genuine application error (e.g. message-not-found) is not retried.evictIdle()closes a connection unused past the reuse idle timeout (default 5 min), swept every 2 min byIdleService; a connection currently in use is skipped (non-blockingtryLock).IdleServicetears down reused connections on the low-battery push-teardown path (#88/#89/#90), mirroring the IDLE-connection teardown.Mutex. Coexists with IMAP IDLE (its own separate connection), so reuse adds ≤1 socket/account — far under provider limits.AppLogon the lifecycle (open / reuse-hit / reconnect-stale / evict / teardown), keyed by an opaque per-cache ordinal (never host/user/secret), plus the #358ImapPerfbreadcrumb (showsconnect≈0mson a reuse hit).Tests (fast gate; no emulator)
ImapConnectionCacheTest— reuse, retry-once stale recovery, narrow drop detection, deterministic idle eviction (injected clock), teardown.ImapFolderOpenLatencyTest(GreenMail + counting proxy) — N ops share one connection/LOGIN; a force-dropped socket is transparently reconnected; an app error does not reconnect; idle eviction issues aLOGOUTand the next op reconnects.ImapClientTest/ImapClientBackfillTest/MailBackfillerTest) pinned to reuse-off so their connect-per-op assertions are unchanged.Fast gate green:
assembleDebug·testDebugUnitTest·compileDebugAndroidTestKotlin·lintDebug·ktlintCheck·detekt.Risk / edge cases
🤖 Generated with Claude Code
@@ -97,15 +186,42 @@ internal class ImapConnectionCache(private val connect: (ImapConnectionParams) -private fun key(params: ImapConnectionParams): String =Could this be an early return pattern - order the false condition first?
@@ -97,15 +186,42 @@ internal class ImapConnectionCache(private val connect: (ImapConnectionParams) -private fun key(params: ImapConnectionParams): String =Claude - Yes — pushed
81a3b7e.isConnectionDropnow leads with guard clauses (definite-drop types, then a not-MessagingException early return) instead of thewhenexpression, main path un-nested. One subtlety worth flagging: FolderClosedException/StoreClosedException are themselves MessagingException subtypes, so their guard has to stay ordered before the MessagingException one (else they'd get gated on a.causethey don't carry) — left a comment on it. Behavior-identical; all 8 existing ImapConnectionCacheTest cases still pass, including the ones that pin that ordering.@@ -97,15 +186,42 @@ internal class ImapConnectionCache(private val connect: (ImapConnectionParams) -private fun key(params: ImapConnectionParams): String =Fixed it to my preferred style.