Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rsc: Optimize blob eviction deletion #1615

Merged
merged 3 commits into from
Aug 1, 2024
Merged

rsc: Optimize blob eviction deletion #1615

merged 3 commits into from
Aug 1, 2024

Conversation

V-FEXrt
Copy link
Contributor

@V-FEXrt V-FEXrt commented Jul 31, 2024

  • Adds index to blob(updated_at) which speeds up the TTL check for old blobs
  • Completes the blob eviction lookup and delete in one query to not waste time repeating work.
  • Makes delete query much more static instead of passing in 16k variables

ag-eitilt
ag-eitilt approved these changes Jul 31, 2024
View reviewed changes
Copy link
Collaborator

@ag-eitilt ag-eitilt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely seems like a reasonable optimization. I imagine we used to do something else with the blobs between the fetch and the deletion?

colinschmidt
colinschmidt approved these changes Jul 31, 2024
View reviewed changes
Copy link
Contributor

@colinschmidt colinschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

rust/rsc/src/database.rs

Ok(affected)
DELETE from blob b
WHERE b.id IN (SELECT id FROM eligible_blob_ids)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it not possible to just do IN (eligible_blob_ids.id) or something like that? I'm surprised to see a select is needed again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope :( I tried that. As fair as I could tell this didn't actually effect the query plan since WITH is kind of just writing an alias for a subquery

@V-FEXrt
Copy link
Contributor Author

V-FEXrt commented Aug 1, 2024

I imagine we used to do something else with the blobs between the fetch and the deletion?

Not really. It was previously just split into two queries for simplicity reasons

@V-FEXrt V-FEXrt merged commit ff0aeb3 into master Aug 1, 2024
11 checks passed
@V-FEXrt V-FEXrt deleted the rsc-optimize-blob-delete branch August 1, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ag-eitilt ag-eitilt ag-eitilt approved these changes

@colinschmidt colinschmidt colinschmidt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@V-FEXrt @colinschmidt @ag-eitilt