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 TTL pruning logic #15

Merged
merged 9 commits into from
Jan 7, 2020
Merged

Fix TTL pruning logic #15

merged 9 commits into from
Jan 7, 2020

Conversation

jazg
Copy link
Member

@jazg jazg commented Dec 23, 2019

There are currently two issues with the pruning logic.

  1. Data is divided into slots (current time / length of interval) which represent discrete time intervals. These slots are dropped when their distance is >= 1 from the current time slot. This means if data is inserted at the start of a slot and at the end, they will both be pruned when the next slot is created.

  2. Inserting data with the same key does not update the time at which the data will be pruned. For example (assuming we have one hour prune intervals), if the user inserts data at 3pm and updates the data at 3:30pm, the pruning still occurs at 4pm since the key has not changed.

This PR does two things to fix these issues.

  1. Increases the distance from 1 to 2. So (assuming the prune interval is one hour) rather than pruning data that is at most 1 hour old, it is pruned after at least 1 hour and at most 2 hours.

  2. Removes the key from the previous slot when the data is updated.

@jazg jazg changed the title Fix pruning logic Fix TTL pruning logic Dec 23, 2019
@loongy loongy requested a review from tok-kkk January 6, 2020 03:56
@loongy loongy added the bug Something isn't working label Jan 6, 2020
@jazg jazg requested a review from loongy as a code owner January 6, 2020 22:40
loongy
loongy previously approved these changes Jan 7, 2020
Copy link
Contributor

@loongy loongy left a comment

Choose a reason for hiding this comment

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

LGTM. I have left one question and one comment, but nothing that would prevent this from being merged.

cache/ttl/ttl.go Outdated Show resolved Hide resolved
cache/ttl/ttl.go Outdated Show resolved Hide resolved
@jazg jazg merged commit 01e19eb into master Jan 7, 2020
@jazg jazg deleted the fix/ttl branch January 7, 2020 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants