assaf / vanity

Experiment Driven Development for Ruby
http://vanity.labnotes.org
MIT License
1.55k stars 269 forks source link

Fully destroy experiments using the Redis adapter #356

Closed urbanautomaton closed 5 years ago

urbanautomaton commented 5 years ago

Currently the redis adapter's #destroy_experiment method only removes experiment metadata; all participant data is retained even if the experiment is reset. This can lead to large numbers of old keys hanging around in redis instances, requiring manual cleanup.

This change uses redis's SCAN command[1] to delete all keys matching the experiment prefix, which should completely reset the experiment.

The SCAN command works in batches, each time returning a cursor value that can be used in subsequent calls; when the cursor "0" is returned, the scan is complete.

I've tried to test this here by making the shared adapter test more complete; I haven't got this running locally at the moment, however, so it's likely to be failing on travis and I'll patch it up with subsequent commits.

Is this something you'd be interested in merging, once the tests pass? The SCAN command was implemented in Redis 2.8.0, released in 2013; is this okay?

[1] https://redis.io/commands/scan

phillbaker commented 5 years ago

@urbanautomaton makes sense to me! Thanks for taking this on - if we can get the tests passing, happy to merge.

For redis compatibility, I think master already has a redis >= 3.x restriction, which will be in the next release, so should be fine!

urbanautomaton commented 5 years ago

Thanks for the response - the build's now green (with no further effort on the other adapters, either!) so I think this is ready. 😄

phillbaker commented 5 years ago

Awesome, will take a look tonight!

On Thu, Dec 6, 2018 at 7:50 AM Simon Coffey notifications@github.com wrote:

Thanks for the response - the build's now green (with no further effort on the other adapters, either!) so I think this is ready. 😄

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/assaf/vanity/pull/356#issuecomment-444860372, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFxKb27BrYYZOLz4hfZcrsBkSbf-lBxks5u2RJ9gaJpZM4ZDLzC .