feat(change-stream): SQLite-backed garbage collector

[aldy505]

Introduces SqliteGarbageCollector and SqliteGarbageCollectorStream which implements ChangeStream.
This is required for filesystem & S3-compatible API backends.
Note that it's not wired to the backends yet.

Supersedes #582
Refs FS-482

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Collector is never constructed

Medium Severity

SqliteGarbageCollectorStream lives in a private module and is never re-exported or built by ChangeStreamFactory. No backend can select this collector, so the new feature is unreachable.

Additional Locations (1)

• objectstore-service/src/change_stream/garbage_collector_sqlite.rs#L17-L33

 

^{Reviewed by Cursor Bugbot for commit 7dbb16c. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 98.26389% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.42%. Comparing base (81dbdc8) to head (f95fc6a).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
...vice/src/change_stream/garbage_collector_sqlite.rs	98.26%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #653      +/-   ##
==========================================
+ Coverage   91.33%   91.42%   +0.09%     
==========================================
  Files         117      118       +1     
  Lines       24171    24497     +326     
==========================================
+ Hits        22077    22397     +320     
- Misses       2094     2100       +6     

Components	Coverage Δ
Rust Backend	94.88% <98.26%> (+0.05%)	⬆️
Rust Client	81.70% <ø> (ø)
Python Client	93.81% <ø> (ø)

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[linear-code]

FS-482

[matt-codecov]

a couple questions:

• do we have performance goals for self-hosted deployments? i have been burned by poor single-insert sqlite performance before. if we need high throughput, we may want to introduce batched queries, but it introduces some risk of inventory corruption if we crash without flushing batches
• do we have scaling requirements for self-hosted deployments? i've seen a few different "distributed sqlite" projects (https://rqlite.io/ / https://docs.fly.io/litefs/how-it-works/) but generally consider sqlite to be node-local

[aldy505]

• do we have performance goals for self-hosted deployments? i have been burned by poor single-insert sqlite performance before. if we need high throughput, we may want to introduce batched queries, but it introduces some risk of inventory corruption if we crash without flushing batches

If they're using k8s (the unofficial helm chart), they want to use the postgres implementation of this garbage collector. In which I will implement after SQLite is correctly implemented. This is how Taskbroker is doing as well, they provide both options, with SQLite as default.

• do we have scaling requirements for self-hosted deployments? i've seen a few different "distributed sqlite" projects (https://rqlite.io/ / https://docs.fly.io/litefs/how-it-works/) but generally consider sqlite to be node-local

I've contributed to rqlite before, but no thanks, let's not consider that approach.

[lcian]

The biggest point to resolve will be #653 (comment) but in the meantime we can get the rest of the things in order before we find a solution for that.

[matt-codecov]

#677 changed the ChangeStream trait to help write GC correctly. unfortunately you will need to rebase and tweak your PR to implement:

• begin_write(): create the GC record before committing in the storage backend
• begin_update(): lengthen the expiration in GC before committing in the storage backend
• commit_delete(): delete the GC record after deleting the stored object

if begin_write() or begin_update() fail, it's not safe to allow the backend commit to happen. otherwise we can permanently leak objects or cause GC to delete them prematurely. so those functions have to .await rather than spawn and they have to surface failure to the caller

if commit_delete() fails it's kind of harmless. GC will just have a ghost record floating around until its expiry. if the ghost record has manual expiration then it'll float around forever, but that's... probably a small edge case.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit f95fc6a. Configure here.}

[cursor]

Write upsert shortens expiry too early

High Severity

begin_write upserts expires_at unconditionally, so an overwrite can move an existing object's GC deadline earlier before storage succeeds. ChangeStream treats begin_* as an upper bound and does not promise a matching commit_*. A failed or in-flight write can therefore make the collector delete a still-valid object.

 

^{Reviewed by Cursor Bugbot for commit f95fc6a. Configure here.}