fix(service): Move TTI renewal to the background by jan-auer · Pull Request #614 · getsentry/objectstore

jan-auer · 2026-09-07T14:08:04Z

Backend GET and HEAD operations currently renew TTI deadlines inline. These rewrites can race with concurrent tiered-storage commits and overwrite newer state.

This change makes backend reads side-effect-free. The service schedules bounded, best-effort renewals after successful reads. It adds extend-only conditional expiry updates for each backend and renews tiered objects blob-first before updating the matching redirect.

Ref FS-405

linear-code · 2026-09-07T14:08:16Z

FS-405

codecov · 2026-09-07T14:09:50Z

Codecov Report

❌ Patch coverage is 95.81006% with 60 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.89%. Comparing base (d5d3a65) to head (c5f179f).

Files with missing lines	Patch %	Lines
objectstore-service/src/backend/gcs.rs	89.82%	17 Missing ⚠️
objectstore-service/src/background.rs	91.42%	12 Missing ⚠️
objectstore-service/src/backend/local_fs.rs	96.31%	8 Missing ⚠️
objectstore-service/src/backend/bigtable.rs	97.32%	5 Missing ⚠️
objectstore-service/src/backend/s3_compatible.rs	94.93%	4 Missing ⚠️
objectstore-service/src/backend/testing.rs	50.00%	4 Missing ⚠️
objectstore-service/src/concurrency.rs	84.61%	4 Missing ⚠️
objectstore-types/src/metadata.rs	62.50%	3 Missing ⚠️
objectstore-service/src/backend/tiered.rs	99.26%	1 Missing ⚠️
objectstore-service/src/service.rs	99.58%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #614      +/-   ##
==========================================
+ Coverage   89.22%   89.89%   +0.66%     
==========================================
  Files         109      110       +1     
  Lines       19089    20308    +1219     
==========================================
+ Hits        17033    18256    +1223     
+ Misses       2056     2052       -4

Components	Coverage Δ
Rust Backend	`93.50% <95.81%> (+0.57%)`	⬆️
Rust Client	`81.97% <ø> (ø)`
Python Client	`93.56% <ø> (ø)`

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

…newal * origin/main: feat(gcs): Add resumable uploads (#609)

lcian · 2026-09-09T12:39:07Z

+    /// of an absent row.
+    ///
+    /// Returns `true` when the update was applied or its requested state was
+    /// already satisfied. Returns `false` for an absent, expired, manual-policy,


Suggested change

/// already satisfied. Returns `false` for an absent, expired, manual-policy,

/// already satisfied. Returns `false` for an absent, expired,

Given that this performs a generic TieredUpdate, the fact that we fail the op when the policy is manual is probably more suited to rather be documented on TieredUpdate::SetExpiry.

lcian · 2026-09-09T12:45:45Z

-            // leave its update intact and let a future read evaluate the TTI again.
-            if response.status() == StatusCode::PRECONDITION_FAILED {
+            // A concurrent metadata writer won the CAS race. Leave its update
+            // intact; automatic renewal can be retried by a later read.


Suggested change

// intact; automatic renewal can be retried by a later read.

// intact.

lcian · 2026-09-09T12:48:48Z

+        download_url
+            .query_pairs_mut()
+            .append_pair("alt", "media")
+            .append_pair("ifGenerationMatch", &gcs_metadata.generation);


Why only ifGenerationMatch but not ifMetagenerationMatch I wonder?

We read metadata first and now need a matching payload. If only metadata changes in the meanwhile, the read is still valid.

lcian · 2026-09-09T12:52:54Z

+        // Renewal is an implementation detail of the original client read and
+        // must not add another client-operation COGS unit.


Right now this is true, but when we change the API to require explicit bumps, we'll have to change this.

That is true. Since we will only call set_expiry conditionally, I think we can change this right away.

lcian · 2026-09-09T13:08:59Z

+///
+/// Millisecond expiry timestamps are not unique revisions. A replacement inline
+/// object with the same row kind and expiry can therefore still be overwritten
+/// by this rewrite. A dedicated persisted revision token is deferred.


Suggested change

///

/// Millisecond expiry timestamps are not unique revisions. A replacement inline

/// object with the same row kind and expiry can therefore still be overwritten

/// by this rewrite. A dedicated persisted revision token is deferred.

It seems that this is not relevant.

lcian · 2026-09-09T14:47:13Z

+    worker: Option<RenewalWorker>,
+}
+
+impl Clone for RenewalScheduler {


Why is this needed?

The scheduler is like a handle to the worker. Instead of externally Arc'ing it, we clone it directly for the background metrics emitter and for stream executor.

sentry · 2026-09-09T17:17:48Z

+    pub fn schedule(&self, id: ObjectId, expire_at: SystemTime) {
+        let pending = Arc::clone(&self.inner.pending);
+        if !pending.pin().insert(id.clone()) {
+            objectstore_metrics::count!(
+                "service.expiry_renewal",
+                outcome = "skipped",
+                reason = "duplicate"
+            );
+            return;
+        }


Bug: A race condition in RenewalScheduler::schedule() can cause TTI renewal requests to be silently dropped due to incorrect ordering of deduplication and queue capacity checks.
_{Severity: LOW}

Suggested Fix

In the schedule() method, perform the queue capacity check using try_reserve() before inserting the object ID into the pending deduplication set. This ensures an ID is only marked as pending if a slot in the queue has been successfully reserved for it, preventing the race condition where a request is dropped after being falsely identified as a duplicate.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: objectstore-service/src/background.rs#L215-L224 Potential issue: A race condition exists in the `schedule()` method of the `RenewalScheduler`. An object ID is added to the `pending` deduplication set before checking if the renewal queue has capacity. If two threads call `schedule()` for the same ID concurrently while the queue is full, the first thread adds the ID but then fails the capacity check. The second thread sees the ID in the `pending` set, incorrectly assumes it's a duplicate, and returns. As a result, the renewal request is silently dropped by both threads.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

cursor

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

^{❌ 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 c5f179f. Configure here.}

cursor · 2026-09-09T17:21:58Z

        Ok(None)
    }

+    async fn compare_and_update(


Expired tombstones still redirect deletes

Low Severity

delete_non_tombstone still returns an expired tombstone as live. The same change treats expired entries as absent on reads, put_non_tombstone, and compare_and_write, so a delete can still follow the redirect and remove the long-term blob after get already reports not found.

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

fix(service): Move TTI renewal to the background

1599319

jan-auer added 13 commits September 7, 2026 18:10

refactor(service): Generalize tiered updates

b0d4e5d

style(service): Clean up imports

1dd270b

fix: Back out normalize_expiry

2288b45

fix: Clean up expiry handling

d64c199

fix: Simplify in memory backend

c298de6

fix: Cleanup GCS

ec464c7

fix: Further improvements to GCS

3fdfe2b

Merge remote-tracking branch 'origin/main' into fix/background-tti-re…

14abaf1

…newal * origin/main: feat(gcs): Add resumable uploads (#609)

ref: Partial local_fs cleanup

befad36

fix: S3 implementation

ea9204f

ref: Improve BigTable implementation

2163439

fix: Bigtable small stuff

92a7e84

ref: Simplify tiered storage

607c913

lcian reviewed Sep 9, 2026

View reviewed changes

ref: Improve background scheduling

d6de850

lcian reviewed Sep 9, 2026

View reviewed changes

jan-auer added 2 commits September 9, 2026 17:16

fix: Re-read on generation mismatch in GCS

6e8c840

fix: Simplify tests

c5f179f

jan-auer marked this pull request as ready for review September 9, 2026 17:13

jan-auer requested a review from a team as a code owner September 9, 2026 17:13

sentry Bot reviewed Sep 9, 2026

View reviewed changes

cursor Bot reviewed Sep 9, 2026

View reviewed changes

	/// already satisfied. Returns `false` for an absent, expired, manual-policy,
	/// already satisfied. Returns `false` for an absent, expired,

	// intact; automatic renewal can be retried by a later read.
	// intact.

		// Renewal is an implementation detail of the original client read and
		// must not add another client-operation COGS unit.

Uh oh!

Uh oh!

Conversation

jan-auer commented Sep 7, 2026

Uh oh!

linear-code Bot commented Sep 7, 2026

Uh oh!

codecov Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jan-auer Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sentry Bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Sep 9, 2026

Choose a reason for hiding this comment

Expired tombstones still redirect deletes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Sep 7, 2026 •

edited

Loading

jan-auer Sep 9, 2026 •

edited

Loading