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

Store block offsets in disk data cache #611

Merged
merged 4 commits into from
Nov 17, 2023

Conversation

passaro
Copy link
Contributor

@passaro passaro commented Nov 17, 2023

Description of change

Add the block offset to the header stored in each cache block. This allows for additional consistency checks when reading or writing to the cache, where the offset can be compared with the block index.

Does this change impact existing behavior?

It does change the on disk cache format, but it is still currently under the caching feature flag.


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

Signed-off-by: Alessandro Passaro <[email protected]>
@passaro passaro had a problem deploying to PR integration tests November 17, 2023 10:20 — with GitHub Actions Failure
@passaro passaro had a problem deploying to PR integration tests November 17, 2023 10:20 — with GitHub Actions Failure
@passaro passaro had a problem deploying to PR integration tests November 17, 2023 10:20 — with GitHub Actions Failure
@passaro passaro had a problem deploying to PR integration tests November 17, 2023 10:20 — with GitHub Actions Failure
@passaro passaro temporarily deployed to PR integration tests November 17, 2023 10:43 — with GitHub Actions Inactive
@passaro passaro temporarily deployed to PR integration tests November 17, 2023 10:43 — with GitHub Actions Inactive
@passaro passaro temporarily deployed to PR integration tests November 17, 2023 10:43 — with GitHub Actions Inactive
@passaro passaro temporarily deployed to PR integration tests November 17, 2023 10:43 — with GitHub Actions Inactive
@passaro passaro requested a review from dannycjones November 17, 2023 10:43
dannycjones
dannycjones previously approved these changes Nov 17, 2023
Copy link
Contributor

@dannycjones dannycjones left a comment

Choose a reason for hiding this comment

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

In the interest of stabilizing the format on disk, I'm inclined to merge as is.

I do like the idea we discussed of having some kind of guard type that wraps something and requires a certain set of metadata to be provided before allowing code access to the contents. Let's do this at a later point.

mountpoint-s3/src/data_cache/disk_data_cache.rs Outdated Show resolved Hide resolved
mountpoint-s3/src/data_cache/disk_data_cache.rs Outdated Show resolved Hide resolved
Signed-off-by: Alessandro Passaro <[email protected]>
Signed-off-by: Alessandro Passaro <[email protected]>
@passaro passaro added this pull request to the merge queue Nov 17, 2023
Merged via the queue into awslabs:main with commit b23eaca Nov 17, 2023
18 checks passed
@passaro passaro deleted the store_block_offset branch November 17, 2023 15:56
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