Skip to content

RM-220: Purge the database before the archiver, and bump stale dependencies - #4

Open
biast12 wants to merge 2 commits into
masterfrom
RM-220
Open

RM-220: Purge the database before the archiver, and bump stale dependencies#4
biast12 wants to merge 2 commits into
masterfrom
RM-220

Conversation

@biast12

@biast12 biast12 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Description

The guild purge has been failing against the current schema; the fix itself is in TicketsBot-cloud/database#41.

  • Reversed the purge order. The archiver delete ran first and is irreversible, so every database failure destroyed that guild's S3 transcripts while leaving all the Postgres rows behind — repeated on every 6-hour retry. The database now goes first; both halves are idempotent, so a failure is safe to retry and leaves the transcripts intact.
  • purgeGuild returns error instead of bool. Six distinct failure causes collapsed into one boolean, and the caller re-logged what the callee had already logged, so a single failed purge produced two errors that read as two separate faults.
  • Restored the poll deadline that cf92174 removed. That commit dropped context.WithTimeout(..., 5*time.Minute) and the ctx.Err() guard, leaving the status loop with no attempt cap and no deadline. Also capped the backoff, which grew without bound.
  • isBotInServer no longer rebuilds its context from context.Background(), so it honours caller cancellation.
  • Added sentry.Flush before exit. In ONESHOT mode the process returned from main immediately after Run(), dropping exactly the buffered errors you would be trying to read.
  • Run's only fatal path used log.Printf, so a failure to fetch the guild list never reached Sentry. Now uses zap.
  • Named the retention window RetentionPeriod, left at 28 days to match current behaviour. cf92174's title says 30, so that may be worth settling separately — changing it shifts data retention.

THIS PR REQIRES TicketsBot-cloud/database#41 and TicketsBot-cloud/logarchiver#10 TO BE ACCEPTED FIRST
THE DATABASE AND THE ARCHIVERCLIENT (and maybe common too) NEEDS TO BE BUMPED IN HERE BEFORE THIS SHOULD GET ACCEPTED

Type of Change

  • Bug fix
  • New feature
  • Breaking change

Testing

Tables should be cleaned up now

Checklist

  • My code follows the style of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

Refactors daemon purge handling to return errors, purge database data before irreversible transcript deletion, and add bounded polling with timeout/backoff when waiting on logarchiver status. Improves failure reporting by surfacing detailed per-object errors and fixes context usage for guild checks. Also updates main loop cleanup so one-shot runs flush Sentry/loggers before exit, and bumps TicketsBot cloud dependencies (with local common/database replaces enabled).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs:bump_db PR references database repo changes needs:gomod_comment go.mod has uncommented local replace directives type:bug Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant