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

Add temporary way to configure amount of data prefetched per file handle #1021

Merged
merged 3 commits into from
Sep 18, 2024

Conversation

dannycjones
Copy link
Contributor

Description of change

In some use cases (such as those in multi-tenant settings), a very large number of file handles may be opened concurrently. While the default prefetching works fine for environments with fewer open file handles, we see that Mountpoint can use far too much memory otherwise.

This change introduces a temporary way to scale down the maximum amount that Mountpoint will prefetch per file handle. Today, the default is 2GiB and the new parameter can be used to reduce that.

The end goal is to make this parameter obsolete through the work in #987 to make prefetching scale down when available memory is low.

Relevant issues: #502, #566, #630, #987

Does this change impact existing behavior?

No existing behavior is impacted. This allows a temporary way to override the prefetch window size.

When it is removed, it will not trigger any issue (so long as the root cause is solved on memory usage). We use an environment variable such that this can be removed without breaking deployments.

Does this change need a changelog entry in any of the crates?

This change does not add a changelog entry as we do not recommend using this override.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

This can be removed at any time.

Signed-off-by: Daniel Carl Jones <[email protected]>
mountpoint-s3/src/prefetch.rs Outdated Show resolved Hide resolved
mountpoint-s3/src/prefetch.rs Show resolved Hide resolved
Copy link
Contributor

@passaro passaro left a comment

Choose a reason for hiding this comment

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

LGTM

@dannycjones dannycjones added this pull request to the merge queue Sep 18, 2024
Merged via the queue into awslabs:main with commit de6d145 Sep 18, 2024
23 checks passed
@dannycjones dannycjones deleted the temporary-prefetch-size-config branch September 18, 2024 16:40
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