Rotate slaves

[TimWeils]

While using this connector in my code I noticed that the operations towards slaves are not balanced between all slave instances returned by Sentinel. Connector just tries to connect to slaves in the order returned by Sentinel and uses the first one that succeeds. I checked how other Redis Clients solve this problem and for example client for Python rotates the slaves in round-robin manner (as you can see here). I implemented similar rotation for this connector so that all slave instances are utilized equally.

[stepulak]

This is really helpful, thank you.

Could you please @pintsized or @piotrp look at this pull request?

[pintsized]

LGTM. Could you please add a test case to cover the change? You can find examples in sentinel.t.

[piotrp]

You shouldn't keep rotation state in a global variable, this should be kept in connector instance. And, preferably, be a opt-in behavior (or at least opt-out?).

A few technical remarks:

• you call #slaves a lot of times, this should be kept in a variable
• a new array should be created by tab_new

A generalized rotation function could look like this:

local function rotate_slaves_2(slaves, rotate_by)
    local slave_count = #slaves
    local rotated_slaves = tbl_new(#slaves, 0)

    for i, slave in ipairs(slaves) do
        local new_index = (i - rotate_by - 1) % slave_count + 1
        rotated_slaves[new_index] = slave
    end

    return rotated_slaves
end

[TimWeils]

Sorry for a delayed answer, I was busy with other things.

@piotrp Thanks for the suggestions. I moved the variable inside the connector instance and set slaves rotation as an opt-in.

@pintsized I added two tests that test the implementation. Due to the fact that redis instances are not "fresh" for each test I had to put the tests above the already existing ones. Also since sentinel doesn't guarantee in which order the slave instances are returned while listing them I had to add some if statements inside the tests. For example this one.

[TimWeils]

@piotrp @pintsized Just want to remind you that I made some changes to this PR and I would be really grateful if you take a look at it. 🙂

[piotrp]

A quick test rotating 4 "slaves" (originally ["a","b","c","d"]), with 8 iterations:

["b","c","d","a"]
["c","d","a","b"]
["d","a","b","c"]
["a","b","c","d"]
["b","c","d","a"]
["c","d","a","b"]
["d","a","b","c"]
["a","b","c","d"]

Looks fine to me.