Release notes: main moved the v2.32.0 notes to the released section, so this PR's bullet goes under v2.33.0's Bug fixes + maintenance. The test's one-line t.Cleanup literals are spread over lines.
The stored total was overwritten with one node's in-memory count, so only the leader could
write it (#2066) and publishes on other nodes were never counted. Every node now adds the
publishes since its last flush to the stored total, reads the total back, and moves its flush
marker to it, so peer counts are never added twice. The total loaded at startup is the first
marker, so a restart does not count it again; a single node counts exactly as before.
MessagesCapped also resolved since=<id> markers, with its own fallback
for an unknown one (the whole history), duplicating what the server does
with the resolved marker (sinceForTopic). Only tests still used it. It
now rejects since=<id>, and the tests use the two marker functions.
An expired marker looks the same as one not written yet, so the wait
delayed every poll of a phone that was offline for longer than the cache
duration by 2.5s, and cost about ten database lookups each. Replaying
from the marker's delivery here covers the unwritten markers this was
for: a message goes out on a node before it is written. A marker that
neither the cache nor this node knows replays the whole history again,
right away, as on main.
Waiting for the write, and polling the database meanwhile, made a fenced
node's clients a database storm: 20,000 of them, 10 lookups each, in a
few seconds, until catch-up reads took longer than the load balancer
allows. The topic knows when the marker went out here, so that replay
starts right away; only a marker nobody here knows is waited for.
A time-based replay from the last minute reads the primary: what a
lagging replica lacks went out live before the client arrived, so the
tail does not resend it.
The marker lookup already fell back to the primary, but the rows were
still read from the lagging replica, which lacks the newest ones. Those
went out live before the client arrived, so the tail does not resend
them either, and they were lost.
Cluster nodes write their batches independently, and late when one is
cut off from the database, so a message can be written after one that
was published later. A client whose marker is the late-written one never
got the later message: its row id is lower, and the node it reconnects
to delivered it before the client arrived, so the tail does not resend
it. since=<id> now also returns the rows published in the marker's
second or later with a lower row id. The marker's own second is replayed
as a consequence, which clients drop by id.
A marker that is not written yet (the client reconnected right after
receiving it) used to resolve to row 0 and replay the topic's entire
history. It is now looked up again for up to the batch timeout plus a
second first.
The shared insert helper only returns the values of a message in a fixed
order. Each backend writes its own INSERT text as a constant: PostgreSQL
the unnest statement with the array casts, SQLite the multi-row INSERT
head and row placeholder. No column names or types travel with the values
and no renaming between backends.
The batch writer built a different multi-row INSERT text for every batch
size, and pgx prepares each distinct text server-side and keeps up to 512
per connection. With pool connections now long-lived, every backend on
ntfy.sh accumulated hundreds of multi-MB plans: primary memory crept from
50% to 65% in three hours after 2.29.0. Locally, one backend grew from
20 MB to 1.15 GB after 1000 batch sizes and kept growing once the cache
was full.
The INSERT now takes one array parameter per column and unnests them, so
the statement text is the same for any batch size (32 MB after the same
workload). SQLite keeps the multi-row INSERT, chunked at 1000 rows. The
docs now recommend pool settings, including a connection lifetime.
Marking a claimed message published was wrong in the way a reviewer would
notice first: published is read by four other queries (since= replays, polls,
scheduled-message replacement, expiry), so a claimed-but-undelivered message
looked delivered to all of them, and a node dying after the claim lost the
push entirely.
The claim is now its own column. claimed_at holds the row off from the other
nodes, the claim statement is a single CTE that locks what it picks and skips
what another node is already claiming, and MarkPublished stays where it was,
after dispatch. So published still means delivered, and delivery is still at
least once.
The claim expires after two minutes, which is what makes a dead claimer
harmless: nothing would ever deliver its rows again if the claim were
permanent. Two minutes is far more than dispatching a claimed batch takes
(side effects are goroutine handoffs), and the claim also takes at most 1000
rows, so a backlog drains over several ticks instead of one node holding all
of it.
Schema 15 -> 16 adds the column; the migration test asserts the migrated
database is structurally identical to a fresh one, so the ALTER and the CREATE
TABLE cannot drift.
Tests: concurrent callers get disjoint sets, an abandoned claim is invisible
until it expires and claimable afterwards, and a published message never comes
back. Both halves of the reclaim test fail if the claim filter is dropped.
Every node's sender loop runs the same MessagesDue query, so once there is
more than one node, a scheduled message is delivered once per node. On
PostgreSQL the query is now SELECT ... FOR UPDATE SKIP LOCKED inside a
transaction, and the claimed rows are marked published in that transaction:
each row goes to exactly one caller and the others skip it instead of
blocking. SQLite and the in-memory store keep the old path.
This does change PostgreSQL semantics. A row used to be marked published
after dispatch returned, so a node dying mid-delivery re-sent on the next
tick; now it is marked before, so a node dying between claim and delivery
loses that message. The alternative is holding the row lock across Firebase,
web push, email and upstream delivery, where one stuck delivery would block
the claim path for every node. A narrow at-most-once window buys a
short-lived lock, and only for delayed messages.
The test runs six concurrent callers against 40 due messages and asserts each
was claimed exactly once; without the claim it reports "claimed 6 times".
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.
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.
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.
binwiederhier
Philipp C. Heckel