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.

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

rsc: Delete multiple blob files per task by config #1652

Merged
merged 3 commits into from
Sep 25, 2024

Conversation

V-FEXrt
Copy link
Contributor

@V-FEXrt V-FEXrt commented Sep 24, 2024

Blob file deletion previously launched a task per file. This has significant overhead when deleting 100s of thousands of files.

This change allows the config to specify how many files should be deleted per task

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/bin/rsc/config.rs Outdated Show resolved Hide resolved
pub chunk_size: u32,
// Maximum number of files to delete from the disk per task
pub file_chunk_size: usize,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is usize always going to be less than u32?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no usize is the number of bits a pointer on the host has (so pretty much u64 everywhere). The API that the value goes into expects a usize so it was easier to let the config system do validation instead of pushing it all the way into the eviction task

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure thats fine.

@V-FEXrt V-FEXrt merged commit 52eb7bb into master Sep 25, 2024
11 checks passed
@V-FEXrt V-FEXrt deleted the rsc-multi-delete-task branch September 25, 2024 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants