yannh / redis-dump-go

Backup & Restore your Redis server - FAST
MIT License
280 stars 58 forks source link

Add km flag #12

Closed Thxzzzzz closed 4 years ago

Thxzzzzz commented 4 years ago

USE KEYS command instead of SCAN, for redis version below 2.8.0

yannh commented 4 years ago

Hi @Thxzzzzz , thanks for the Pull Request! I'd be fine with the feature - but I have some concerns on the implementation - see comments. :+1:

Thxzzzzz commented 4 years ago

Hi @Thxzzzzz , thanks for the Pull Request! I'd be fine with the feature - but I have some concerns on the implementation - see comments. 👍 You are right , there are better way to implementation. Anyway, Thanks your project, It really help me a lot

yannh commented 4 years ago

@Thxzzzzz mind checking out https://github.com/yannh/redis-dump-go/pull/14 ? I am looking for something more explicit than -km, though I m not sure it's great :) Note that some of the code is taken from an older version from redis-dump-go, since this is how it worked before it was changed to use Scan instead - which is more consistent and less memory intensive.

Thxzzzzz commented 4 years ago

@Thxzzzzz mind checking out #14 ? I am looking for something more explicit than -km, though I m not sure it's great :) Note that some of the code is taken from an older version from redis-dump-go, since this is how it worked before it was changed to use Scan instead - which is more consistent and less memory intensive.

noscan is better then -km. but i think there are too many params on function DumpDB and DumpServer , maybe replace it with a struct or functional options pattern?

yannh commented 4 years ago

Yep, that would be a nice improvement.. I ll merge this for now, might refactor later! Thanks!

yannh commented 4 years ago

Released v0.4.1, hope it works for you!