Don't force sliding window entries to be monotonic

[michaelkipper]

The code in #227 is not passing tests, because it forces an invariant on the SlidingWindow that entries be monotonic. This doesn't work, unfortunately, with host-based circuits, since individual processes perform a two-step operation of:

1. Getting the current timestamp, and
2. Pushing the timestamp onto the sliding window

These operations, performed without a lock, can be interleaved and violates the invariant.

I tried to make that operation atomic, but it breaks with tests that want to use Timecop.travel to modify the system time. Therefore, the only option I had was to relax the invariant.

This does change the runtime of the reject! function to Ω(n) from O(n). Given that n tends to be less than 100, even in host-based circuits with a large scale_factor, this shouldn't be a big deal.

cc: @Shopify/servcomm

[thegedge]

Some food for thought: when I was going through the code of SlidingWindow, my thought was that it doesn't feel like what I'd expect out of a sliding window data structure.

In particular, the fact that the caller has to manage timestamps doesn't make sense to me, because I would expect the sliding window to manage that. For example, I would expect to be able to write code like this:

# Width being time in seconds, and max_size being the size of the underlying ring buffer / storage data structure
window = SlidingWindow.new(width, max_size: 100)

# Can push arbitrary data, SlidingWindow manages timestamps
window.push("value")

# Lazily do reject, instead of caller having to decide how to reject
window.size

If it completely managed timestamps, I think it would be far easier to avoid these monotonicity issues.

[michaelkipper]

@thegedge: I agree that this is a very opinionated SlidingWindow. In particular, it's a window of timestamps, not data attached to timestamps. The key is that the max_size defines the number of timestamps that can be in the window, which makes it simple to figure out when to open the circuit.

# Can push arbitrary data, SlidingWindow manages timestamps
window.push("value")

I suppose this might work with Timecop.travel if you have the underlying C implementation use the Ruby version of time instead of primitives like time(2).

In any event, I still think that this PR is a simplification on #227, so a net win.

[thegedge]

LGTM

In any event, I still think that this PR is a simplification on #227, so a net win.

🙌

[michaelkipper]

Merging this to mkipper/global-circuit-breaker so it's easier to review and reason about.