[aldy505]

Introduces GcStorage trait, that will be implemented by various backend. Also implemented SQLite here.

Supersedes #582
Refs FS-482

[cursor]

Stale Bugbot comment from a previous run.

[codecov]

Codecov Report

❌ Patch coverage is 97.59615% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.57%. Comparing base (1c2db3b) to head (1fe4d59).

Files with missing lines	Patch %	Lines
...bjectstore-service/src/garbage_collector/sqlite.rs	97.59%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #653      +/-   ##
==========================================
+ Coverage   91.52%   91.57%   +0.05%     
==========================================
  Files         116      117       +1     
  Lines       22175    22383     +208     
==========================================
+ Hits        20295    20498     +203     
- Misses       1880     1885       +5     

Components	Coverage Δ
Rust Backend	94.72% <97.59%> (+0.03%)	⬆️
Rust Client	84.41% <ø> (ø)
Python Client	93.88% <ø> (ø)

☔ 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.}

[sentry-junior]

sqlx is a hard dependency of objectstore-service, and it's pulled in with features this code doesn't need. tls-rustls-ring-native-roots and chrono aren't needed for a local SQLite file, and nothing in garbage_collector_sqlite.rs uses chrono. Trimming the features to runtime-tokio + sqlite avoids compiling the TLS stack for this. It's also worth putting it behind a feature (like storage-cogs), so builds that only use GCS or Bigtable don't compile bundled SQLite.

Posted by mistake; feedback was meant to be shared privately.

[github-actions]

Changelog Preview

This PR lands in these workspaces:

• objectstore-server: CalVer

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).

---

New Features

• Sqlite implementation by aldy505 in #653

---

_{🤖 This preview updates automatically when you update the PR.}