Skip to content

fix(redis): keep taking writes when the card is too full to snapshot - #3452

Open
vpetersson-bot wants to merge 1 commit into
masterfrom
agent/anthias-claude/bd9f02d38a35
Open

vpetersson-bot wants to merge 1 commit into
masterfrom
agent/anthias-claude/bd9f02d38a35

Conversation

@vpetersson-bot

Copy link
Copy Markdown
Contributor

The bug

Redis defaults stop-writes-on-bgsave-error to yes: after one failed background save it rejects every write command with MISCONF until a save succeeds again. Our docker/Dockerfile.redis.j2 never overrode it.

On a player whose card has filled up, that converts a local disk problem into a whole-device outage — and one strictly worse than the condition that caused it. Redis is the celery broker and result backend, the Channels layer, and the viewer's anthias.viewer command bus, so when it stops taking writes, essentially everything stops.

Three Sentry groups are that single flag seen from three sides, all on current releases:

Sentry What it is Scale
ANTHIAS-6J celery worker never starts — first broker PING returns MISCONF, kombu reraises as ConnectionError, celery logs Unrecoverable error and exits fatal, new on 2026.10.0
ANTHIAS-4X viewer cannot publish the display resolution or reach the command bus, so the screen stops following the playlist 450 events from a single pi3 in one day
ANTHIAS-6M the underlying full card, reported directly through celerybeat's on-disk schedule new, separate device

ANTHIAS-6J and ANTHIAS-4X are on two different pi3s, so this is not one sick board.

Why the healthcheck never caught it

The redis-cli ping healthcheck in both compose templates keeps passing throughout: PING is a read, and only writes are refused. The service stays healthy, every depends_on: service_healthy gate opens, and the stack comes up into a state where nothing can write. That is why the guard added here is a test over the shipped CMD rather than a runtime probe.

The fix

Add --stop-writes-on-bgsave-error no to the redis-server CMD.

Nothing this image set out to persist is given up. The telemetry device_id — the thing the existing comment in that file exists to protect — rides on the AOF, which has its own error handling and is untouched by this flag. The RDB save points are the belt-and-braces copy, and losing a snapshot while the card is full costs nothing that a restart would not cost anyway.

It also hides no evidence: Redis still logs every failed bgsave, and a full card keeps announcing itself through the writes that genuinely cannot proceed (which is exactly what ANTHIAS-6M is). The device simply stops taking the whole player down with it.

Tests

tests/test_redis_persistence_flags.py, following the tests/test_core_dump_limits.py pattern of asserting over shipped config:

  • a failed snapshot must not disable writes;
  • the companion assertion that durability still rides on the AOF and the mounted --dir, since those are what make the trade above safe. Drop either and the trade stops holding.

Verified the new test fails against the un-fixed CMD and passes with it. Full non-integration suite: 2748 passed. Two failures reproduce unchanged on pristine master and are untouched by this branch — test_reboot_gate_still_skips_the_image_purge (the subject of open PR 3450) and a -n auto ordering flake in TestWaitForWaylandSocket, which passes in isolation.

Scope

Deliberately not addressed here: why these cards are filling. That is a separate problem with its own history (the 24.2 GB of core dumps behind the core-dump limits is one known cause), and it does not change the fact that a full card should degrade rather than black out the screen.

🤖 Generated with Claude Code

Redis defaults `stop-writes-on-bgsave-error` to `yes`: after a single
failed background save it rejects every write command with MISCONF
until a save succeeds again. On a player whose card has filled up,
that turns a local disk problem into a whole-device outage, and one
strictly worse than the condition that caused it.

Two Sentry groups are the same flag seen from either end. The celery
worker never gets started: its first broker PING comes back MISCONF,
kombu reraises it as a ConnectionError, and celery logs
"Unrecoverable error" and exits (ANTHIAS-6J, fatal). The viewer loses
the command bus and cannot publish the display resolution, so the
screen stops following the playlist -- one pi3 filed 450 of those in a
day (ANTHIAS-4X). Both are on current releases; a third group reports
the underlying full card directly, through celerybeat's on-disk
schedule (ANTHIAS-6M).

The compose healthcheck cannot catch any of this, which is why the
guard is a test over the shipped CMD instead: PING is a read, so it
keeps answering while every write fails and the service stays
`healthy` throughout.

Nothing this image set out to persist is given up. The telemetry
device_id rides on the AOF, which has its own error handling and is
untouched by this flag; the RDB save points are the belt-and-braces
copy, and losing a snapshot while the card is full costs nothing a
restart would not cost anyway. Redis still logs every failed bgsave,
and a full card keeps announcing itself through the writes that
genuinely cannot proceed, so no evidence is hidden -- the device just
stops taking the whole player down with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
@vpetersson-bot
vpetersson-bot requested a review from a team as a code owner November 7, 2026 09:08
Copilot AI lite review requested due to automatic review settings November 7, 2026 09:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.

@sonarqubecloud

sonarqubecloud Bot commented Nov 7, 2026

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants