Settings reload: no DNS at build time, don't refuse over old failures #38

Merged

jcoffey-dev merged 1 commits from fix/reload-resilient-build-errors into main

2026-09-24 19:19:34 +00:00

jcoffey-dev commented

2026-09-24 18:59:02 +00:00

Owner

Problem

In the 3-node rehearsal, every settings reload failed across the whole cluster because one node couldn't resolve the Pyzor host.

PyzorConfig::parse (crates/common/src/config/mailstore/spamfilter.rs:479-496) runs lookup_host while the settings are being built, and a failed lookup counts as a build error.
reload_registry (crates/common/src/cache/reload.rs:120-129) only swaps in the new core when the whole build has no errors.
So the reload is refused on that node. ReloadSettings only broadcasts after a successful local reload, so no other node reloads either.
At boot, the same error is only logged and the server runs with whatever did build.

(a) No network I/O while the settings are built

Pyzor. The config now keeps host and port, and PyzorConfig::address() resolves when a message is checked:

An IP address is used as it is.
A hostname is resolved once and reused for 5 minutes.
The lookup counts against the Pyzor timeout.
A failed lookup is a spam.pyzor-error for that message, like an unreachable server already was.

Milter. MtaMilter had the same problem, and also used a blocking to_socket_addrs() inside async code (crates/common/src/config/smtp/session.rs). An IP address is kept as it is. A hostname is now resolved on each connection in MilterClient::connect, inside the existing connect timeout.

I searched the rest of the config build for network or DNS I/O. The only other cases already don't fail the build:

Directories that can't connect: LDAP/SQL/OIDC become Unavailable with a warning (DIR-21).
The AI model locality check (AI-2): it only warns.

(b) One failing object shouldn't freeze every reload

Choice: a reload is refused only for new errors, meaning errors in objects that built fine when the running settings were built. Errors in objects that were already failing then no longer block it.

The rules:

Recording. Boot, and every applied reload, record which objects failed (Data::build_errors).
Known failures. An object on that list is already missing from the running settings, exactly as at boot. Applying the reload loses nothing there. Its errors are logged and returned as ReloadResult::known_errors, but they don't block.
New failures. An error in an object that works now still refuses the reload. Otherwise a bad edit (say, an expression that doesn't compile) would silently take a working object out of service. The admin gets the error instead.
After a fix. Once an object builds again it drops off the list, so a later failure there blocks again.
Scope. This covers the full rebuild (directories, telemetry, core, listeners). The certificate, lookup, blocked-IP and application reloads only build their own objects, so they're unchanged.

Options I considered and rejected:

Always apply, like boot. Easy, but a typo would silently remove a working queue strategy or listener.
Apply with only the failed object kept at its previous value. Core::parse builds one core from everything; there's no per-object merge, and adding one would touch every parser.

Clearer error. ReloadSettings now fails with Settings were not reloaded. <Object> with id <id>: <message> (plus (N more in the server log.) when there are several), and the objectId is set as before. A reload refused after a directory change now logs its errors instead of only "Settings didn't reload".

Tests

system::reload::reload_tests (new, RocksDb):

Enable Pyzor with host pyzor.invalid, then ReloadSettings: succeeds, and the running config has the host. On main this step fails with Invalid address: failed to lookup address information: Name or service not known. That's the rehearsal error, reproduced here.
With an IP host, address() needs no lookup.
Two console tracers (only one is allowed) plus a Pyzor ratio change, then ReloadSettings: refused with Settings were not reloaded. Tracer with id …: Only one console tracer is allowed, and the ratio is unchanged.
Record that error as the running build's (as boot would), then ReloadSettings: applied, with the error listed in known_errors.
Remove the tracers, then add them again: refused again.

Other runs (RocksDb):

Test	Result
`smtp::inbound::*` (25 tests, antispam and milter included)	pass
`system::system_tests`	pass
`telemetry::telemetry_tests`	pass

smtp::inbound::milter::milter_session now uses localhost for its milter, so it exercises the connect-time lookup.

## Problem In the 3-node rehearsal, every settings reload failed across the whole cluster because one node couldn't resolve the Pyzor host. - `PyzorConfig::parse` (`crates/common/src/config/mailstore/spamfilter.rs:479-496`) runs `lookup_host` while the settings are being built, and a failed lookup counts as a build error. - `reload_registry` (`crates/common/src/cache/reload.rs:120-129`) only swaps in the new core when the whole build has no errors. - So the reload is refused on that node. `ReloadSettings` only broadcasts after a successful local reload, so no other node reloads either. - At boot, the same error is only logged and the server runs with whatever did build. ## (a) No network I/O while the settings are built **Pyzor.** The config now keeps `host` and `port`, and `PyzorConfig::address()` resolves when a message is checked: - An IP address is used as it is. - A hostname is resolved once and reused for 5 minutes. - The lookup counts against the Pyzor timeout. - A failed lookup is a `spam.pyzor-error` for that message, like an unreachable server already was. **Milter.** `MtaMilter` had the same problem, and also used a blocking `to_socket_addrs()` inside async code (`crates/common/src/config/smtp/session.rs`). An IP address is kept as it is. A hostname is now resolved on each connection in `MilterClient::connect`, inside the existing connect timeout. I searched the rest of the config build for network or DNS I/O. The only other cases already don't fail the build: - Directories that can't connect: LDAP/SQL/OIDC become `Unavailable` with a warning (DIR-21). - The AI model locality check (AI-2): it only warns. ## (b) One failing object shouldn't freeze every reload **Choice:** a reload is refused only for **new** errors, meaning errors in objects that built fine when the running settings were built. Errors in objects that were already failing then no longer block it. The rules: - **Recording.** Boot, and every applied reload, record which objects failed (`Data::build_errors`). - **Known failures.** An object on that list is already missing from the running settings, exactly as at boot. Applying the reload loses nothing there. Its errors are logged and returned as `ReloadResult::known_errors`, but they don't block. - **New failures.** An error in an object that works now still refuses the reload. Otherwise a bad edit (say, an expression that doesn't compile) would silently take a working object out of service. The admin gets the error instead. - **After a fix.** Once an object builds again it drops off the list, so a later failure there blocks again. - **Scope.** This covers the full rebuild (directories, telemetry, core, listeners). The certificate, lookup, blocked-IP and application reloads only build their own objects, so they're unchanged. Options I considered and rejected: - **Always apply, like boot.** Easy, but a typo would silently remove a working queue strategy or listener. - **Apply with only the failed object kept at its previous value.** `Core::parse` builds one core from everything; there's no per-object merge, and adding one would touch every parser. **Clearer error.** `ReloadSettings` now fails with `Settings were not reloaded. <Object> with id <id>: <message>` (plus `(N more in the server log.)` when there are several), and the `objectId` is set as before. A reload refused after a directory change now logs its errors instead of only "Settings didn't reload". ## Tests `system::reload::reload_tests` (new, RocksDb): 1. Enable Pyzor with host `pyzor.invalid`, then `ReloadSettings`: succeeds, and the running config has the host. On `main` this step fails with `Invalid address: failed to lookup address information: Name or service not known`. That's the rehearsal error, reproduced here. 2. With an IP host, `address()` needs no lookup. 3. Two console tracers (only one is allowed) plus a Pyzor ratio change, then `ReloadSettings`: refused with `Settings were not reloaded. Tracer with id …: Only one console tracer is allowed`, and the ratio is unchanged. 4. Record that error as the running build's (as boot would), then `ReloadSettings`: applied, with the error listed in `known_errors`. 5. Remove the tracers, then add them again: refused again. Other runs (RocksDb): | Test | Result | |---|---| | `smtp::inbound::*` (25 tests, antispam and milter included) | pass | | `system::system_tests` | pass | | `telemetry::telemetry_tests` | pass | `smtp::inbound::milter::milter_session` now uses `localhost` for its milter, so it exercises the connect-time lookup.

jcoffey-dev added 1 commit 2026-09-24 19:16:01 +00:00

Settings reload: no DNS at build time, don't refuse over old failures

ci / build (pull_request) Successful in 3m22s

Details

ci / fork-checks (pull_request) Successful in 44s

Details

999ae12cc7

A 3-node rehearsal found every settings reload refused, cluster-wide,
because one node couldn't resolve the Pyzor server:

- PyzorConfig::parse resolved the host while building the settings and
  made a failed lookup a build error. It now keeps the host and port and
  resolves when a message is checked (an IP address is used as is, a
  name is reused for five minutes, the lookup counts against the Pyzor
  timeout). A failure there is a Pyzor error for that message.
- A milter's hostname was resolved the same way, with a blocking
  to_socket_addrs in async code. An IP address is kept; a name is now
  resolved on each connection.

Other build-time I/O is already non-fatal: directories that can't
connect become unavailable with a warning (DIR-21), and the AI model
locality check only warns.

reload_registry swapped the core only when the whole build was free of
errors, while boot runs with whatever built. One failing object thus
refused every later reload, and the running settings went stale. Now a
reload is refused only for errors in objects that built when the
running settings were built (at boot or by the last applied reload):
applying it would lose those. Objects that already failed then are
missing from the running settings anyway, as at boot, so their errors
are logged and returned as known_errors but don't hold the reload back.
Refusing on new errors keeps a bad edit from taking a working object
out of service; the admin gets the error instead.

ReloadSettings now says "Settings were not reloaded." and names the
object and its error ("Tracer with id ...: Only one console tracer is
allowed"), with a count of any further errors. A refused reload after a
directory change logs its errors too.

system::reload::reload_tests (new): with Pyzor enabled on an
unresolvable host, ReloadSettings succeeds (on main it fails with
"Invalid address: failed to lookup address information"); an IP host
needs no lookup; a new build error refuses the reload, names the object
and leaves the running settings unchanged; the same error, once known
from the running settings' build, no longer blocks; once fixed, a new
error there blocks again. smtp::inbound::milter's session test now
names its milter "localhost", so the connect-time lookup is exercised.

jcoffey-dev force-pushed fix/reload-resilient-build-errors from f55087dd9b to 999ae12cc7

2026-09-24 19:16:01 +00:00

Compare

jcoffey-dev merged commit 19eb25a426 into main

2026-09-24 19:19:34 +00:00

jcoffey-dev deleted branch fix/reload-resilient-build-errors

2026-09-24 19:19:34 +00:00

jcoffey-dev referenced this pull request

2026-09-24 19:30:15 +00:00

Registry writes apply to the running settings without ReloadSettings #39