One write path for both backends; the magic column count is gone. A column added to the list
but not to insertMessageArgs (or vice versa) now fails every message write on every backend.
The message cache decides how many batches may wait for its writer (queueBufferedBatches in
message/cache.go, next to the shutdown flush timeout it has to fit in); util stays policy-free.
On cron bursts (top of the hour), the single batch writer fell behind: it wrote one INSERT round
trip per message, so against a busy remote Postgres it managed ~100 rows/s while ~300/s arrived.
New messages took up to ~40s to reach the database (polls missed them), and publishes blocked on
the unbuffered queue for up to ~25s.
- Postgres: write each batch with one multi-row INSERT (up to 1000 rows per statement) instead of
one statement per row. SQLite keeps the row-by-row path.
- Skip DeleteScheduledBySequenceID when the client did not set a sequence ID: the sequence ID is
then the message's own fresh ID, so no scheduled message can match. This saves a transaction on
the primary for almost every publish.
- Buffer up to 100 batches in the BatchingQueue, so a briefly slow writer does not stall publishes.
- dbtest: add a latency proxy (CreateTestPostgresWithLatency, WithLatency) to test round trips.
A review of this PR pointed out that the cap was computed from a separate
read of the available tokens: a read and a reservation are two operations, so
anything consuming the limiter in between makes the read stale and the burn
lands deeper than one burst. The cap now comes from each reservation's own
delay, which the limiter computes atomically with the consumption, and the
chunk that would run past one burst of replenishment is cancelled.
That holds the cap against any amount of ordinary concurrent consumption. It
does not make concurrent BURNS of one limiter safe: two burners can each
reserve a chunk that looked acceptable, and the cap is then exceeded by about
a chunk per racing burner (roughly three bursts with 32 of them, measured).
Both the old and the new code behave that way, so this is a documented
contract rather than a fix: callers burning one limiter from several
goroutines serialize, which RateLimiter.Burn does with its own mutex and the
cluster does by burning peer usage from a single flush loop.
Tests cover the two invariants that hold: the cap survives ordinary traffic
racing the burn, and serialized burners share one burst of debt instead of
each taking one.
A per-run key space stops concurrent runs from deleting each other's objects,
but it also means nothing ever revisits the keys of a run that was killed
before its cleanup, so those leak forever. The fixed prefix this replaced at
least swept itself on the next run.
The first test to construct an s3 store now sweeps the shared test root once
per binary, deleting anything older than two hours: a test takes seconds and a
CI job minutes, so nothing that old can belong to a live run. Failures to list
or delete are logged, not fatal, since this is hygiene and not the test.
The age filter is a pure function with its own tests, including that an object
whose modification time could not be read is left alone rather than treated as
infinitely old.
This is a backstop. A lifecycle rule on the bucket (expire testpkg-* after a
day) is the real fix, since it also covers a bucket nobody runs tests against
for a while and cannot be skipped by a crash.
The s3-backed tests shared one prefix in the shared CI bucket, and both setup
and cleanup delete everything under the prefix they are given, so two
concurrent CI runs deleted each other's objects mid-test. It surfaced as
NoSuchKey 404s in TestStore_Sync/s3 on a pull request that touches neither
attachments nor s3.
Keys now live under <prefix>/testpkg-attachment/<random per run>/<test name>,
the same scoping s3/client_test.go already uses.
Five option names and their validation, ahead of the implementation, so the
config surface is settled and the cluster change is only code. The
experimental- prefix is there because the names may still change.
The options parse into server.Config and the cross-field rules are enforced:
a listen address needs database-url, a secret and a node id; a secret without
a listen address is an error rather than clustering being silently off; a
wildcard bind needs an explicit advertise URL; a negative linger is rejected.
Setting experimental-cluster-listen then refuses to start the server, since
accepting the options and running single-node would look like a working
cluster. The cluster change deletes that one line.
A bounded non-blocking batching queue, a token-burn helper for consumption
that happened elsewhere, and MaxTime. Nothing uses them yet; they land on
their own so they can be reviewed as primitives rather than inside the
cluster diff.
LingerQueue differs from BatchingQueue in that producers never block: a full
queue drops and says so, and Close flushes the remainder before closing the
consumer channel, so per-peer queues can come and go with the nodes.
BurnTokens caps the debt it creates at one burst beyond what is available, so
usage reported from another node can drain a bucket but cannot lock a visitor
out for an unbounded time.
They each build their own schema and their own registries, so they were
waiting out lease and hold-off windows one after another for no reason: 27s
for the package, 26s of it in six tests. With t.Parallel they overlap, and
the tick loops poll every 50ms instead of 10ms, which cuts the query rate
they put on the database by five while staying 40x finer than the windows
they measure.
Package is 8.6s now, stable over five runs with -race, and each safeguard is
still independently covered: removing the observation-freshness check fails
the heartbeat-without-observing test, and removing the hold-off restart fails
the frozen-leader test.
Both gaps came out of a review: removing either safeguard left the suite
green, so neither was actually tested.
- Observation freshness: a node can keep its registration fresh while its
view of the registry goes stale, and it has to stop believing then, because
the hold-off it earned rests on an observation that no longer holds. The new
test keeps the incumbent registering but never refreshing while a returning
lower-id node earns its own hold-off; without the check both believe.
- The clock decision: the old test wrote a row an hour in the past and would
have passed just as well with a node-side cutoff, since the test process and
the database share a clock. It now asserts that the heartbeat, the cutoff and
the prune all take their instants from the database, plus the behaviour that
follows (a row an hour-fast node would write still counts as live, one from
an hour-slow node does not). Its comment also said "an hour ahead" while the
row subtracted an hour.
The lease and the hold-off were both one node TTL, and the hold-off is the
one that wants to be longer: the TTL also decides how fast a dead peer leaves
the fan-out set and how fast an isolated node notices it is isolated, so it
has to stay short, while leadership would rather be slow and calm.
The hold-off is two TTLs now and the lease stays at one, so belief still ends
exactly when peers stop counting this node as live. With the default 30s TTL
a successor waits 60s, a unit of work that starts under valid leadership has
60s before a second leader is possible, and a crash leaves up to ~90s with no
leader, which costs a couple of manager ticks of pruning and nothing else.
A test asserts the invariant the factors have to satisfy (lease < TTL +
hold-off), since that is what keeps a deposed leader's belief ending before
any successor's begins.
The doc comment also states what the windows do not promise. A leader-gated
job that blocks inside a database call can sit there past its own lease:
measured on three hosts with the database partitioned away from the leader,
an unbounded call kept a node acting as leader for 14.3s after its lease
expired, 8s of that alongside the new leader. Anything whose double execution
matters needs its own claim row or has to be idempotent.
Registry calls are bounded at ttl/2, so a 200ms test TTL left 100ms per
query. That is fine against a local Postgres and times out on a CI service
container, which is what broke the pipeline. One second instead.
Found by running two real processes against Postgres: freezing the leader
with SIGSTOP and thawing it gave 436ms of two leaders. The hold-off was
measured from when this node first saw itself as the lowest live id, and a
frozen process keeps that observation across the pause, so one successful
heartbeat on resume made the hold-off look long satisfied while the incumbent
had not yet re-read the registry.
A gap in observation no longer counts: Refresh restarts the hold-off whenever
it has not run for a TTL, and belief now also requires a recent Refresh, not
just a recent Register. The regression test freezes a leader by not ticking
it, then thaws it with one register plus one refresh and requires that it does
not believe yet.
First piece of multi-node ntfy: the membership table, and leadership derived
from it. Nothing uses it yet, so this is inert on a single node.
Each node upserts its own row (node_id, advertise_url, last_heartbeat) every
few seconds and reads the other fresh rows to find its peers. Reads are split
in two on purpose: Refresh queries the table and is called from the heartbeat
loop, while Peers only returns the snapshot of the last Refresh, so a message
path can ask who the peers are without ever waiting on the database, not even
during an outage.
Leadership is not a separate mechanism. The leader is the live node with the
lowest id, which every node can work out from the data it already reads, and
three conditions keep two nodes from both believing it:
- it is the lowest live id as of its last refresh;
- its own registration is fresh, dated from when the heartbeat was issued, so
a node cut off from the database stops believing within one TTL, and stops
before its peers write it off (its deadline is measured from before the
write they saw);
- it has been the lowest live id for a whole TTL, because a returning
lower-id node would otherwise believe at once while the incumbent still
believes until its next read.
Heartbeats are stamped by the database and the liveness cutoff is evaluated
in SQL: these timestamps are compared across nodes, so one clock has to
decide. Tests cover the failure modes rather than just the happy path: a
leader that stops heartbeating, a returning lower-id node preempting, a node
whose database is gone, and a row written by a badly skewed clock.
The wrapper only exposed the context-free database/sql methods, so callers
that need a deadline (or cancellation on shutdown) had to reach for
Primary() directly. Delegates to the primary, like Exec and Query.
The since marker was resolved by an inline subquery on the replica. A message
ID the replica had not seen yet (a client reconnecting right after receiving
a message, ahead of replication) resolved to 'id > 0' and replayed the topic's
whole retained history to that client.
Resolution is now its own lookup: on the replica, and only if the ID is
unknown there, on the primary. At most two point lookups before the replay,
and none on the primary unless a replica is configured. An ID unknown to the
primary as well (expired) keeps the full-replay fallback.
Now that the signal handler waits for Stop, two holes in Stop matter:
- Stop waits for every store to close, and those waits have no deadline of
their own: attachment.Store.Close waits for the sync loop, whose
AttachmentsWithSizes query runs against the database (and S3 deletes are in
there too). A wedged database parked Stop before it ever reached the message
cache, whose own 5s bound was then useless. Stop now gives up after 10s.
- A signal can arrive before Run has set up its listeners. Stop closed the
stores and consumed stopOnce, and Run then started listeners against closed
stores and blocked on the listener error, so the process neither served nor
exited and a second signal could not stop it. Run bails out when the server
is already stopped, outside the lock (a stuck shutdown holds it) and again
under it.
The final batch write has no deadline (no statement_timeout, and database/sql
is used without a context), so an unresponsive database would hold Stop until
systemd's stop timeout fires and SIGKILLs us. Cache.Close now gives up after
5 seconds, both on handing the pending messages to the batch writer and on
waiting for it, and logs that it did.
Without pool_max_idle_conns, database/sql keeps only 2 idle connections, so
any burst above that closed connections after use and re-opened them on the
next query: a DNS lookup, TCP + TLS handshake and SCRAM auth per query. Under
load this shows up as HTTP 500s whenever the resolver hiccups ("hostname
resolving error"). ntfy.sh runs pool_max_conns=30 without an idle setting, so
it churns connections the same way.
pool_max_idle_conns now defaults to pool_max_conns.
On Postgres all stores share one pool, and closeDatabases closed the user
manager first, which closed the pool under the message cache's final batch
write ("Cannot write message batch: sql: database is closed").
ensureUser checks v.User(), the handler re-reads it, and a concurrent
anonymous request on the same shared IP-keyed visitor can SetUser(nil) in
between -> nil dereference (~8x/day on ntfy.sh). Return 401 instead. The
regression test makes the race deterministic.
With cache-batch-size/-timeout set (ntfy.sh: 100 / 1500ms), accepted messages
wait in a BatchingQueue before they are written. Nothing drained that queue on
shutdown, and `ntfy serve` did not handle SIGTERM at all (only SIGHUP), so
every restart dropped up to one batch timeout's worth of accepted messages
from the cache: live subscribers got them, pollers and since= replays never
did.
- util.BatchingQueue.Close emits the pending elements and closes the output
channel (Enqueue after Close is a no-op)
- message.Cache.Close closes the queue and waits for the batch writer before
closing the database
- serve stops the server on SIGTERM/SIGINT and waits for Stop to finish (Run
returns as soon as the listeners close); Stop is idempotent now, since the
signal handler and serve both call it
binwiederhier