Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

feat: clean up partition limiter state on schedule #72

Merged
merged 2 commits into from
Dec 13, 2023
Merged

Conversation

ellie
Copy link
Contributor

@ellie ellie commented Dec 13, 2023

May not be the cause of our issues, however is good to have regardless

Will want to test in dev to ensure performance is not impacted. Note that if it is impacted, we may want to do some sort of atomic swap. I don't think it will be an issue for where we are now though

@ellie ellie requested a review from xvello December 13, 2023 12:32
Copy link
Contributor

@xvello xvello left a comment

Choose a reason for hiding this comment

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

with nit

interval.tick().await;

self.limiter.retain_recent();
self.limiter.shrink_to_fit();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: because we know that we'll see new distinct_ids soon (as sessions churn from visitor to visitor), I don't think the cost of shrink is worth it now, I'd just run retain every 5 minutes, which should in theory keep the active key count bounded. Alternatively, we could shrink before we retain, to only re-allocate the map if it didn't grow back after the last retain.
But it's safe to merge as is and tune later.

@ellie ellie merged commit 454b96d into main Dec 13, 2023
4 checks passed
@ellie ellie deleted the ellie/retain branch December 13, 2023 13:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants