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

Trimming sessions in batches #211

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

Trimming sessions in batches #211

wants to merge 1 commit into from

Conversation

bartzon
Copy link

@bartzon bartzon commented Jan 15, 2024

Due to some unforeseen events, we hit the MySQL timeout when trimming our sessions.
We've implemented a batched approach in our own codebase, but I figured this might be beneficial to the gem itself.

Not sure this change warrants any tests (and if so, what the best way is to implement these), so open to suggestions on that.

@bartzon
Copy link
Author

bartzon commented Jan 15, 2024

FYI, our codebase is checked by Sorbet, and I did see this error

Method 'delete_all' does not exist on 'NilClass component of 'T.nilable(ActiveRecord::Batches:: BatchEnumerator)' (fix available)

I was able to resolve it by using in_batches&.delete_all

Since this gem does not have Sorbet, I opted to not include the &. Maybe this should be added as well?

@@ -16,6 +16,7 @@ namespace 'db:sessions' do
cutoff_period = (ENV['SESSION_DAYS_TRIM_THRESHOLD'] || 30).to_i.days.ago
ActiveRecord::SessionStore::Session.
where("updated_at < ?", cutoff_period).
Copy link

Choose a reason for hiding this comment

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

Nice! However, even this solution gave me a timeout, but at least it deleted many batches instead of nothing

what i'm gonna try is add some throttling between each batch (like suggested in rails docs )
image

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