resque / redis-namespace

This gem adds a Redis::Namespace class which can be used to namespace Redis keys.
http://redis.io
MIT License
695 stars 192 forks source link

Add ability to clear namespaces #202

Closed fatkodima closed 1 year ago

fatkodima commented 2 years ago

There is often a need to clear the namespace and since there is no an implementation of this method, people usually bake their own implementations of clearing.

People also are/were believing that flushall/flushdb clears only the namespace (see #56).

This PR uses scan, but it is available only in redis >= 2.8.0, so it drops for iterating over all keys for older versions.

byroot commented 2 years ago

I really don't think this is a good idea. Even with scan the client side performance is pretty much unpredictable.

fatkodima commented 2 years ago

I really don't think this is a good idea.

Do you mean clearing namespaces is not a good idea?

byroot commented 2 years ago

Yes. This could take a very long time to run.

Also to give you a bit of context, we're just making sure redis-namespace somewhat keep working, but I really don't recommend using it, it's a pain to maintain as Redis server adds new commands or change the arguments.

So I'm fine with merging quick fixes or updates to the command list, but I'm not (and I don't think anyone) is actively developing that gem, so I'm not really open to new features.

fatkodima commented 2 years ago

Yes, understandable. Sure, this can take a bit of time.

Personally, if I would have a few namespaces in my app, I would prefer using separate dbs and flush them when I want to clear. Or maybe make namespaces somewhat dynamic: like passing through env variables. And when you want to clear - just change that value.

But when people have quite a few namespaces, for example in some multi-tenant app, each tenant has its own namespace and not a lot of data, then I think it makes sense to use this gem instead of separate databases. Or when the user has not so much of the data, he can also use this helper to clear his namespace (for example https://github.com/department-of-veterans-affairs/vets-api/pull/421/files).

But, yeah, these are hypothetical examples. I haven't worked with a multi tenant app or the need for many namespaces, so can't say 😃.

iloveitaly commented 2 years ago

I can see the use case here. Makes sense, although I share @byroot's concerns about speed. It's a dangerous operation that looks too easy to run right now.

What if we:

I agree that this project is mostly in maintenance mode, but if great folks like @fatkodima want to contribute more features, I'm all for it!

fatkodima commented 1 year ago

@iloveitaly Thank you for the suggestions. Added suggested warnings/comments/etc. Please, take a look.

Tensho commented 1 year ago

@fatkodima Thank you for insistence with this. That's exactly what I need currently for our dynamic test environment cleanup.

fatkodima commented 1 year ago

@Tensho Oh, hi! 👋 😄