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 ETag into DiskDataCache hashed block path #594

Merged
merged 3 commits into from
Nov 10, 2023

Conversation

dannycjones
Copy link
Contributor

@dannycjones dannycjones commented Nov 3, 2023

Description of change

We discussed in #593 to hash the S3 key, ETag, and possibly block ID together when defining a path for the disk-based data cache. This is the PR that implements that change.

I'm encoding only the ETag as I still see value in keeping the block ID visible in the path. This allows us to do things like invalidate all of the blocks for a given S3 object version (version defined by its ETag).

Previous discussions:

Relevant issues:

Does this change impact existing behavior?

No change to released behavior. This changes file paths for the blocks on disk for the data cache, which is behind a build-time 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: Daniel Carl Jones <djonesoa@amazon.com>
Signed-off-by: Daniel Carl Jones <djonesoa@amazon.com>
Signed-off-by: Daniel Carl Jones <djonesoa@amazon.com>
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 Nov 10, 2023
Merged via the queue into awslabs:main with commit 689ebe3 Nov 10, 2023
18 checks passed
@dannycjones dannycjones deleted the data-cache-move-etag-into-hash branch November 10, 2023 17:14
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.

None yet

3 participants