A brief database hiccup throws away every packet in flight, and nothing retries them #48

New issue

Open

opened 2026-11-03 18:52:27 +00:00 by skippy-bot · 0 comments

skippy-bot commented

2026-11-03 18:52:27 +00:00

Problem

When a batch insert fails for a database-level reason (connection reset during a failover, pool timeout, statement timeout, missing partition), the chunk is counted as invalid and the consumer returns an empty ack to GenStage. The producer immediately frees demand and hands over the next packets, so everything received during the error window is permanently lost while APRS-IS keeps sending. At roughly 100 packets per second, a 30 second failover is thousands of packets missing from a station's track and from history, with no dashboard counter that shows the loss.

Evidence

At 2f150f66:

lib/aprsme/packet_consumer.ex:363 is the only write path, Repo.insert_all(Aprsme.Packet, insert_attrs, @insert_opts).
lib/aprsme/packet_consumer.ex:370 sends non row errors down the else branch; :389 logs Batch insert of N packets failed, not retrying row by row, and :391 returns {0, invalid_count + length(rows)}.
lib/aprsme/packet_consumer.ex:420 is def row_error?(_error), do: false, so DBConnection.ConnectionError, DBConnection.OwnershipError and pool timeouts all take the discard path.
lib/aprsme/packet_consumer.ex:168 returns {:noreply, [], new_state} after processing, which GenStage reads as consumed, so nothing is redelivered.
lib/aprsme/packet_producer.ex:75 shows the demand loop that pulls the next packets as soon as the consumer replies.

The comment at :388 shows the per-row fallback was deliberately rejected, and that decision is right for a bad row. The gap is that a transient database failure also falls through to it with no batch-level retry.

Expected

A batch that fails on a database level is retried with backoff, or parked while consumption pauses, so a failover delays ingestion instead of deleting it. Only rows that genuinely violate a constraint are dropped, and any drop emits a counter an operator can watch.

How to verify

In test/aprsme/packet_consumer_test.exs, stub Repo.insert_all to raise DBConnection.ConnectionError once, push one batch, and assert the insert was attempted twice and the rows are stored. Today the second attempt never happens and no row lands.

Acceptance criteria

A transient database error retries the same batch and the rows are present afterwards.
Retries are bounded with backoff and do not spin while the pool is exhausted.
Dropped packets emit telemetry and appear on the dashboard.
The per-row salvage path for a genuinely bad row is unchanged.

Effort: M
Lens: reliability

## Problem When a batch insert fails for a database-level reason (connection reset during a failover, pool timeout, statement timeout, missing partition), the chunk is counted as invalid and the consumer returns an empty ack to GenStage. The producer immediately frees demand and hands over the next packets, so everything received during the error window is permanently lost while APRS-IS keeps sending. At roughly 100 packets per second, a 30 second failover is thousands of packets missing from a station's track and from history, with no dashboard counter that shows the loss. ## Evidence At `2f150f66`: - `lib/aprsme/packet_consumer.ex:363` is the only write path, `Repo.insert_all(Aprsme.Packet, insert_attrs, @insert_opts)`. - `lib/aprsme/packet_consumer.ex:370` sends non row errors down the else branch; `:389` logs `Batch insert of N packets failed, not retrying row by row`, and `:391` returns `{0, invalid_count + length(rows)}`. - `lib/aprsme/packet_consumer.ex:420` is `def row_error?(_error), do: false`, so `DBConnection.ConnectionError`, `DBConnection.OwnershipError` and pool timeouts all take the discard path. - `lib/aprsme/packet_consumer.ex:168` returns `{:noreply, [], new_state}` after processing, which GenStage reads as consumed, so nothing is redelivered. - `lib/aprsme/packet_producer.ex:75` shows the demand loop that pulls the next packets as soon as the consumer replies. The comment at `:388` shows the per-row fallback was deliberately rejected, and that decision is right for a bad row. The gap is that a transient database failure also falls through to it with no batch-level retry. ## Expected A batch that fails on a database level is retried with backoff, or parked while consumption pauses, so a failover delays ingestion instead of deleting it. Only rows that genuinely violate a constraint are dropped, and any drop emits a counter an operator can watch. ## How to verify In `test/aprsme/packet_consumer_test.exs`, stub `Repo.insert_all` to raise `DBConnection.ConnectionError` once, push one batch, and assert the insert was attempted twice and the rows are stored. Today the second attempt never happens and no row lands. ## Acceptance criteria - [ ] A transient database error retries the same batch and the rows are present afterwards. - [ ] Retries are bounded with backoff and do not spin while the pool is exhausted. - [ ] Dropped packets emit telemetry and appear on the dashboard. - [ ] The per-row salvage path for a genuinely bad row is unchanged. **Effort:** M **Lens:** reliability

skippy-bot added the

High

bug

effort: M

labels

2026-11-03 18:52:27 +00:00