Skip to content

Conversation

@palfrey
Copy link
Member

@palfrey palfrey commented Nov 4, 2025

Description

There's a case where if you've got a filesystem store that's small enough, you can have a case where your entry gets evicted during insertion, and right now this causes a deadlock. To some extent, this is mostly a bad config (i.e. you should have a bigger store), but I suspect the same error can occur with much larger stores, depending on the exact eviction priorities on items. More specifically, we shouldn't be able to cause deadlock no matter how bad the config is, so this needs fixing either way.

Type of change

Please delete options that aren't relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

bazel test //...

Checklist

  • Updated documentation if needed
  • Tests added/amended
  • bazel test //... passes locally
  • PR is contained in a single commit, using git amend see some docs

This change is Reviewable

@palfrey palfrey changed the title New filesystem test for eviction breaking Eviction breaking side case fix for filesystem store Nov 5, 2025
@palfrey palfrey force-pushed the nested-evicition-break branch 2 times, most recently from 432209e to e0dbc60 Compare November 6, 2025 16:48
// file. To support this edge case, we first move the file to a temp file and point
// target file location to the new temp file. `unref()` should only ever be called once.
#[inline]
async fn unref(&self) {
Copy link
Member Author

@palfrey palfrey Nov 6, 2025

Choose a reason for hiding this comment

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

A lot of this is whitespace changes as it had a double-nested {/} for some reason.

@palfrey palfrey force-pushed the nested-evicition-break branch from 9537bdd to 0f68eab Compare November 18, 2025 23:00
@palfrey palfrey marked this pull request as ready for review November 19, 2025 10:00
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.

1 participant