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

fix(expiration): don't add nottl items #155

Open
wants to merge 1 commit into
base: v3
Choose a base branch
from

Conversation

KyleSanderson
Copy link

There's a ton of performance work that can happen here. The expiration queue dominates my profiles on real-world workloads, but that's because it's not doing a swap and is instead always rewriting the head pointer.

I can try to take this on, if you'll take it upstream.

@swithek
Copy link
Contributor

swithek commented Nov 14, 2024

Thank your for the PR.

Omitting NoTTL items from the expiration queue is a good idea and this PR provides a decent starting point, but in the current state, it might produce some side effects. For example:

  • What happens if an item is first set with a non-zero TTL and then there's an update of the same item that changes its TTL to NoTTL?
  • There are other methods that depend on the expiration queue, for instance, the Len() method. Will it be able to handle NoTTL items not being in the queue?

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