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

LRU callbacks may raise #47

Closed
wants to merge 1 commit into from
Closed

Conversation

crusaderky
Copy link
Collaborator

@crusaderky crusaderky commented Nov 19, 2021

If an LRU on_evict callback raises an exception, e.g. out of disk space, keep the key in memory instead of accidentally losing it.

Completely untested.
@ncclementi could you pick this up from here?

@ncclementi
Copy link
Member

Thanks @crusaderky , I'll pick it up from here.

This was referenced Nov 23, 2021
Comment on lines +64 to +65
while self.total_weight > self.n:
self.evict()
Copy link
Collaborator Author

@crusaderky crusaderky Nov 24, 2021

Choose a reason for hiding this comment

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

There are 3 use cases:

  1. weight <= self.n. In this case, the while loop is guaranteed to stop before key is evicted.
  2. weight > self.n and the sum of all other keys in self.d is enough to fill slow. so you will raise on line 72 and again on again on line 65 at some point in the middle of the while iteration. This is desirable.
  3. weight > self.n and the sum of all other keys fits in slow. The while won't stop until it tries and - we know in advance - fail to store key too into slow. This can be avoided with the following:
Suggested change
while self.total_weight > self.n:
self.evict()
while self.total_weight > self.n and len(self.d) > 1:
self.evict()

@crusaderky
Copy link
Collaborator Author

Superseded by #48

@crusaderky crusaderky closed this Nov 29, 2021
@crusaderky crusaderky deleted the slow_raises branch February 16, 2022 15:12
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