440273c48da99c920d20efef32b68b73fdd159d4
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 clawbot
|
440273c48d |
Evict archive writers on deletion and sweep idle archives (closes #89)
check / check (push) Successful in 3m53s
The per-webhook archiveWriter registry in the database delivery target was never evicted, so a deleted webhook's writer -- and any archive file handle open within its debounce window -- lingered for the process lifetime. Separately, expiry pruning ran only when an archive was (re)opened, and reopens only happen on writes, so an archive belonging to a webhook that stopped receiving events kept its expired rows forever. Eviction: a new one-method delivery.WebhookEvictor interface (kept separate from Notifier: archiving lifecycle is not notification) is implemented by the Engine and injected into the handlers. Deleting a webhook, or deleting its last database target, drops the writer from the registry and closes its handle under the writer's own mutex, so eviction can never race an in-flight write. An evicted writer refuses further writes rather than reopening a file nothing holds. The archive file is deliberately left on disk: it is long-term storage an operator may want to keep or move away, and destroying it as a side effect of deleting a webhook would be unrecoverable. Idle sweep: a new ArchiveSweeper, modelled on the event RetentionReaper (fx lifecycle hooks, cancellable context, WaitGroup, ticker loop), prunes archives whose database target declares a positive expiry. It reuses the existing RETENTION_SWEEP_INTERVAL rather than adding a config key. It never creates an archive -- a missing file is skipped, and the reopen uses SQLite mode=rw so the file cannot be conjured even if it disappears mid-sweep -- routes the prune through the per-webhook writer so its mutex orders the sweep against concurrent writes, and leaves the archive closed so the move-the-file-away workflow keeps working. A failure for one webhook is logged and the sweep continues. Archives with no expiry or the expiry "never" are untouched. The sweep loop's context is rooted at context.Background(), not at the fx OnStart hook context. The hook context carries fx's 15 second start timeout, so a loop derived from it is cancelled three quarters of an hour before the first tick under the default one-hour interval, giving a sweeper that never sweeps. OnStop still cancels the loop and waits on the WaitGroup, so shutdown is unchanged. The sweep also never leaves a registry entry behind. Reaching the writer through the ordinary create-and-cache accessor would let a sweep that raced a webhook deletion re-insert a writer for a webhook that no longer exists, which nothing would ever evict again -- the very leak this change closes. An entry the sweep has to create is marked sweep-owned and released when the prune finishes, unless a delivery claimed it meanwhile, in which case it belongs to the registry and an eviction can still reach it. A writer evicted underneath a sweep is an ordinary interleaving and is logged at debug, not error. Each of those guards is pinned by a test that fails when the guard is removed. The adopt-during-sweep window -- a delivery claiming the sweep's own registry entry while that sweep is still running -- is driven directly against the registry, because a delivery placed between two sweeps never reaches the release path at all. The requirement that an idle archive ends the sweep closed is asserted both on a writer proven to hold an open handle beforehand and, end to end, on a delivery-owned entry the sweep keeps, rather than on an entry the sweep has already released and which therefore reports "not open" either way. The "never" expiry short circuit is checked against an archive file that has never been migrated, so any open of it would be observable as a created table. |
||
|
clawbot
|
4dd4dfa5eb |
chore: consolidate DBURL into DATA_DIR, codebase audit for 1.0.0
check / check (push) Successful in 56s
DBURL → DATA_DIR consolidation:
- Remove DBURL env var entirely; main DB now lives at {DATA_DIR}/webhooker.db
- database.go constructs DB path from config.DataDir, ensures dir exists
- Update DATA_DIR prod default from /data/events to /data
- Update all tests to use DataDir instead of DBURL
- Update Dockerfile: /data (not /data/events) for all SQLite databases
- Update README configuration table, Docker examples, architecture docs
Dead code removal:
- Remove unused IndexResponse struct (handlers/index.go)
- Remove unused TemplateData struct (handlers/handlers.go)
Stale comment cleanup:
- Remove TODO in server.go (DB cleanup handled by fx lifecycle)
- Fix nolint:golint → nolint:revive on ServerParams for consistency
- Clean up verbose middleware/routing comments in routes.go
- Fix TODO fan-out description (worker pool, not goroutine-per-target)
.gitignore fixes:
- Add data/ directory to gitignore
- Remove stale config.yaml entry (env-only config since rework)
|
||
|
clawbot
|
32bd40b313 |
refactor: self-contained delivery tasks — engine delivers without DB reads in happy path
check / check (push) Successful in 58s
The webhook handler now builds DeliveryTask structs carrying all target config and event data inline (for bodies ≤16KB) and sends them through the delivery channel. In the happy path, the engine delivers without reading from any database — it only writes to record delivery results. For large bodies (≥16KB), Body is nil and the engine fetches it from the per-webhook database on demand. Retry timers also carry the full DeliveryTask, so retries avoid unnecessary DB reads. The database is used for crash recovery only: on startup the engine scans for interrupted pending/retrying deliveries and re-queues them. Implements owner feedback from issue #15: > the message in the <=16KB case should have everything it needs to do > its delivery. it shouldn't touch the db until it has a success or > failure to record. |
||
|
clawbot
|
43c22a9e9a |
feat: implement per-webhook event databases
check / check (push) Successful in 1m50s
Split data storage into main application DB (config only) and
per-webhook event databases (one SQLite file per webhook).
Architecture changes:
- New WebhookDBManager component manages per-webhook DB lifecycle
(create, open, cache, delete) with lazy connection pooling via sync.Map
- Main DB (DBURL) stores only config: Users, Webhooks, Entrypoints,
Targets, APIKeys
- Per-webhook DBs (DATA_DIR) store Events, Deliveries, DeliveryResults
in files named events-{webhook_uuid}.db
- New DATA_DIR env var (default: ./data dev, /data/events prod)
Behavioral changes:
- Webhook creation creates per-webhook DB file
- Webhook deletion hard-deletes per-webhook DB file (config soft-deleted)
- Event ingestion writes to per-webhook DB, not main DB
- Delivery engine polls all per-webhook DBs for pending deliveries
- Database target type marks delivery as immediately successful (events
are already in the dedicated per-webhook DB)
- Event log UI reads from per-webhook DBs with targets from main DB
- Existing webhooks without DB files get them created lazily
Removed:
- ArchivedEvent model (was a half-measure, replaced by per-webhook DBs)
- Event/Delivery/DeliveryResult removed from main DB migrations
Added:
- Comprehensive tests for WebhookDBManager (create, delete, lazy
creation, delivery workflow, multiple webhooks, close all)
- Dockerfile creates /data/events directory
README updates:
- Per-webhook event databases documented as implemented (was Phase 2)
- DATA_DIR added to configuration table
- Docker instructions updated with data volume mount
- Data model diagram updated
- TODO updated (database separation moved to completed)
Closes #15
|