redis-namespace icon indicating copy to clipboard operation
redis-namespace copied to clipboard

Add ability to clear namespaces

Open fatkodima opened this issue 3 years ago • 5 comments

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.

fatkodima avatar Jul 03 '22 16:07 fatkodima

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

byroot avatar Jul 04 '22 18:07 byroot

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

Do you mean clearing namespaces is not a good idea?

fatkodima avatar Jul 04 '22 18:07 fatkodima

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.

byroot avatar Jul 04 '22 18:07 byroot

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 😃.

fatkodima avatar Jul 04 '22 18:07 fatkodima

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:

  • Added a changelog entry to describe this
  • Added a readme section
  • Added some inline code comments linking to this PR & describing in what scenario you'd use this
  • Adding a puts/log indicating this is a dangerous operation, can take a while to run, and should only be run if you are sure you know what you are doing.

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!

iloveitaly avatar Aug 13 '22 14:08 iloveitaly

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

fatkodima avatar Nov 12 '22 23:11 fatkodima

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

Tensho avatar Feb 22 '23 22:02 Tensho

@Tensho Oh, hi! 👋 😄

fatkodima avatar Feb 22 '23 22:02 fatkodima