Purge file from the negative cache on create #1250
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
force-pushed
the
invalidate-negative-cache
branch
from
740d632 to
db09a8b
Compare
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
|
I think it probably does want a changelog entry but under the 'other changes' list |
Signed-off-by: Christian Hagemeier <chagem@amazon.com>
c-hagem
force-pushed
the
invalidate-negative-cache
branch
from
db09a8b to
31ec9b9
Compare
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
c-hagem
requested a deployment
to
PR integration tests
GitHub Actions
Waiting
passaro
reviewed
Jan 31, 2025
| @@ -2,6 +2,9 @@ | |||
|
|
|||
| * Add support for appending to objects originally uploaded with a CRC64-NVME checksum. ([#1235](https://github.com/awslabs/mountpoint-s3/pull/1235)) | |||
|
|
|||
| ### Other Changes | |||
| * Fix an issue where file names were not expelled from negative cache. | |||
There was a problem hiding this comment.
We should include a link to this PR, like in the other entries:
Suggested change
| * Fix an issue where file names were not expelled from negative cache. | |
| * Fix an issue where file names were not expelled from negative cache. ([#1250](https://github.com/awslabs/mountpoint-s3/pull/1250)) |
There was a problem hiding this comment.
Also:
- why "file names"? Doesn't it apply to directories as well?
- we should qualify when the issue occurs.
| @@ -367,6 +367,8 @@ impl Superblock { | |||
| let inode = self | |||
| .inner | |||
| .create_inode_locked(&parent_inode, &mut parent_state, name, kind, state, true)?; | |||
| // Expel inode from negative cache, if it is present | |||
| self.inner.negative_cache.remove(parent_inode.ino(), name); | |||
There was a problem hiding this comment.
Should we consider updating the cache in create_inode_locked? It would require changes to the update_from_remote code path, but it could be a better trade off.
|
nit: I'd use "purge" rather than "expel" in the PR/changelog/comments |
c-hagem
changed the title
|
Regarding tests, I was not able to reproduce this issue easily as a Rust test -- I tried to re-create the Python script I used in Rust, however that test worked as expected. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds code to remove entries from the negative cache once the new file is created.
Does not need a changelog entry, as it's only a small bugfix.
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).