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

optimization Cleaner #39

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

prog-supdex
Copy link
Contributor

@prog-supdex prog-supdex commented Sep 18, 2023

Here was added a new HASH key objects:list-created-times

After creating subscriptions (and channels later), we will add a subscription's name and the current time to this hash.
We will use it in GraphQL::AnyCable::Cleaner to remove the old keys, who has created_time + config.subscription_expiration_seconds

But there is one problem.
If we have config.subscription_expiration_seconds, we will set EXPIRE here
It means that when Redis destroys this key, this key will remain storing in objects:list-created-times hash

I also considered a separate key, something like this

time:graphql-subscription:some_unique_hash

In other words, every subscription had a Redis key, where has a name time:#{subscription_key_name} and value as the current date
But I refused this idea because it creates a lot of records in Redis (one subscription = one time:subscription), but there was a plus - we could set expire for every key

This is not the final PR

@palkan palkan marked this pull request as draft September 18, 2023 20:37
@palkan
Copy link
Member

palkan commented Sep 18, 2023

Let's separate the docs update and the addition of objects:list-created-times into two PRs.

@prog-supdex prog-supdex changed the title Add docs and optimization Cleaner optimization Cleaner Sep 19, 2023
@prog-supdex prog-supdex force-pushed the opt/key-object-expiry-date-separately branch 2 times, most recently from 0d32aba to 38b0aaa Compare September 19, 2023 16:15
@prog-supdex
Copy link
Contributor Author

Now, It works like that

We have two additional keys, subscription-storage-time and channel-storage-time, which are used to fill up a subscription and channel keys when we do not have subscription_expiration_seconds because if we have the setting subscription_expiration_seconds, it means that subscriptions and channels will be deleted automatically by Redis because in that case, we set TTL for subscription and channel keys

The GraphQL::AnyCable::Cleaner cleans subscriptions and channels using the subscription-storage-time and channel-storage-time keys. It will remove subscription/channel keys with keys from storage if their created_time is earlier than Time.now - config.subscription_expiration_seconds

But, maybe we need to add an ability to set an argument to methods clean_subscriptions and clean_channels, which will be in charge of the time_point, before which the keys should be deleted

Also, if a channel/subscription is deleted, it will remove this key from storage-time

@prog-supdex prog-supdex marked this pull request as ready for review September 19, 2023 17:04
…ng created_time of subscriptions and channels
@prog-supdex prog-supdex force-pushed the opt/key-object-expiry-date-separately branch from 38b0aaa to 99f9ec4 Compare September 20, 2023 15:13
@prog-supdex prog-supdex force-pushed the opt/key-object-expiry-date-separately branch from 99f9ec4 to aab684c Compare September 20, 2023 15:15
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