feat: Add batch TOUCH operation with optimized implementation#477
feat: Add batch TOUCH operation with optimized implementation#477lcian wants to merge 7 commits into
Conversation
Add an `exists` operation for batch requests that checks whether objects exist, returning 200/404 without payload. Key optimization: instead of N individual backend calls, Bigtable uses 1-3 RPCs (batch ReadRows for metadata, optional payload fetch for stale TTI subset, batch MutateRows for TTI bumps). GCS uses concurrent fetch_gcs_metadata calls. Tiered storage coordinates HV metadata batch → LT exists batch for tombstones. The batch endpoint partitions exists ops from others, routes exists through a dedicated `service.check_exists_batch()` path, and merges results with the streaming executor output.
lcian
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #477 +/- ##
==========================================
- Coverage 86.91% 85.99% -0.93%
==========================================
Files 77 79 +2
Lines 11398 11900 +502
==========================================
+ Hits 9907 10233 +326
- Misses 1491 1667 +176
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
lcian
changed the title
lcian
changed the title
| use objectstore_types::metadata::{Compression, Metadata}; | ||
| use percent_encoding::NON_ALPHANUMERIC; | ||
| use reqwest::header::{CONTENT_TYPE, HeaderMap, HeaderName, HeaderValue}; | ||
| use reqwest::multipart::Part; |
There was a problem hiding this comment.
This is all just plumbing.
| Operation::Get(_) | Operation::Touch(_) => { | ||
| objectstore_types::auth::Permission::ObjectRead | ||
| } | ||
| Operation::Insert(_) => objectstore_types::auth::Permission::ObjectWrite, |
There was a problem hiding this comment.
Touch operations are unconditionally exempt from rate limiting despite triggering expensive backend writes
In objectstore-server/src/endpoints/batch.rs (line 55), all Touch operations skip the rate limiter (!matches!(op, Operation::Touch(_))) with the comment 'lightweight existence checks', but each touch triggers real backend mutations: a GCS metadata rewrite or a Bigtable MutateRows call. An authenticated caller with ObjectRead permission can issue unlimited batch requests with up to 1,000 touch keys each, with no throttle, flooding the backend storage tier.
Verification
Bypass at objectstore-server/src/endpoints/batch.rs:55 — the per-operation rate-limit check is skipped for Operation::Touch. Touch requires only ObjectRead permission (streaming.rs:108–110). state.service.touch_batch (service.rs) acquires a single concurrency permit for the whole batch; the Bigtable backend executes a ReadRows + MutateRows that writes a GC/TTI policy on every matching row, and the GCS backend performs up to 20 concurrent metadata rewrites per batch. MAX_OPERATIONS=1000 caps per-request size but no inter-request throttle applies, while every other op kind (Get/Insert/Delete) still passes through rate_limiter.check. Any read-token holder can therefore flood the storage tier with unlimited backend mutations.
Identified by Warden security-review · 6WH-KV2
|
Unfortunately this can create internal inconsistency issues as we're using |
This adds the HEAD operation to the Rust client API, and makes it batchable. The intent is to use this in `sentry-cli` to do a batch check for existing Snapshots, bumping their TTI, and subsequently only upload the missing ones. This is possible given that we use the hash of the object as its key for snapshots. Note that we will also need to change the expiration policy for Snapshots to TTI instead of TTL (currently used). A singular HEAD request maps to a HEAD HTTP request. A batch HEAD request goes through the existing `objectstore-service/src/streaming.rs` pipeline that executes operations with concurrency bounded to 10% of the existing service concurrency limit. This is the "dumb" version of the much more optimized #477. In that PR, I optimized the implementation down to the backend level, most importantly using `ReadRows` (plural) and `WriteRows` for Bigtable. Unfortunately, that creates some possible inconsistency scenarios where, ultimately, LT blobs could remain orphaned. `WriteRows` is not conditional, so it could overwrite a new tombstone that comes in between the `ReadRows` to read all tombstones and the `WriteRows` to bump the TTI of existing ones. Close FS-335
1 participant
This adds a new TOUCH operation that, most importantly, can be batched.
Semantics: if an object exists, and has a TTI expiration policy, refresh its TTI.
The
Backendtrait gains a new methodtouch_batchthat can provide an optimized implementation for executing multiple TOUCH operations. The default implementation just callsget_metadatain a loop and converts the result.The GCS backend has an "optimized" implementation that executes the requests to GCS with bounded concurrency (20).
HighVolumeBackendalso gains a new methodget_tiered_metadata_batchwith a similar default implementation.The Bigtable backend has an optimized version where only two requests are used, a
ReadRowsto get the rows and aMutateRowsto bump the TTI on all of them in bulk.The name is inspired by the UNIX
touchcommand, which bumps the date a file was last modified, although it's also different because it can also create files, which our operation doesn't.More discussion on the naming is welcome.
The previous name was EXISTS, which IMO didn't really make it clear that this bumps TTI, so TOUCH might make that a bit clearer.
As a bonus, this also adds a HEAD API to the client, needed because with the way we have implemented batching, each batchable operation also needs its own non-batch implementation, and for TOUCH this is simply a HEAD request where we discard the metadata.