WIP: Feature/add window size

bolshakov / stoplight

:traffic_light: Traffic control for code.

http://bolshakov.github.io/stoplight/

MIT License

384 stars 40 forks source link

WIP: Feature/add window size #133

Closed thebigw4lrus closed 1 year ago

thebigw4lrus commented 4 years ago

Fixes #131

And there are several unhandled issues left which must be resolved before merging:

[x] Carefully cover new code with tests.
[ ] Do not break backward compatibility (The new algorithm most likely should use a separate namespace for keys)
[x] Update docs.

thebigw4lrus commented 4 years ago

@bolshakov Can you take a look to this?.. one thing I havent managed to resolve are the warnings. It seems it comes from the Hash ruby implementation used by fakeredis. I don't want to change fakeredis 😅 , so I my approach was to shut down the warnings. I didn't succeed at it. Any ideas?.

thebigw4lrus commented 4 years ago

@bolshakov I really need this 🙏 ... can you take a look? thanks!.

bolshakov commented 4 years ago

Hey, @thebigw4lrus thank you for your patience and effort. You've done a great job! 👍

I apologize for my absence.

Do not worry about warnings👌. The main concern now is backward compatibility. Imagine you're switching from the current version to a new one that uses Sorted Sets instead of lists. Thus your failures_key(light) is not empty. You'll get an error when one any attempt to perform Sorted Set commands on it:

127.0.0.1:6379> lpush foo bar
(integer) 2
127.0.0.1:6379> zadd foo 42 baz
(error) WRONGTYPE Operation against a key holding the wrong kind of value

It seems the only option is to alter between old and new behavior depending on a failure_key type. What do you think?

thebigw4lrus commented 4 years ago

Hi @bolshakov !. Yeah, we could go that way!.. just wondering if one have a declared light, and upgrade the gem, it will only go for the former behaviour right?(with no window size), hence, that person would never be able to benefit from this new feature right?.

How about to introduce a breaking version? (so all users would lost their recorded failures if they upgrade to the next version, but then we would have all users in the same library state). Maybe this could make sense in order to avoid backward compatible adjustment code. At the end, the gem upgrade could be done when the circuit remains closed.

What do you think?

bolshakov commented 4 years ago

@thebigw4lrus I experimented a bit and probably the simpler and more convenient to migrate keys from Lists to ZSets on first use. I'm working in that direction now

thebigw4lrus commented 4 years ago

Amazing!!! many thanks @bolshakov !. Let me know if I can help with such move.

bolshakov commented 1 year ago

@thebigw4lrus, thank you for your work ❤️. It took some time, but I merged your changes into the master branch. I started a different pr for that https://github.com/bolshakov/stoplight/pull/172

I'm going to include these changes in version 4.0