Design issue - DemandMaster doesn't do what people want it to do

[TimLovellSmith]

An observation, from dealing with user questions:

What people want DemandMaster to do for them: "make sure they always actually send the command to the master, and they never see the error about writing to slave"

What it actually does: "throw an error if SE.redis guesses they are about to talk to the replica" - sometimes incorrectly, because SE.redis cached the server state.

Maybe there's an implementation which could, avoid the 'incorrectly because SE.redis cached the server state" bug?

Or maybe DemandMaster flag should just be removed, since it basically only makes things fail more often?

[mgravell]

When it comes to routing, it is a reality that the library will always be working only to the best of its knowledge, since it can't reasonably verify before every command (and even if it did: it would be a race condition).

Happy to discuss the topic, though! Can I ask: what is the scenario or scenarios that are your primary concerns? Also: I've quickly checked a few other repos to check I'm not "revealing" anything, but would it change your thoughts if I said there's active discussion around better "maintenance" notifications from servers, so that client libraries have better knowledge of server changes such as failovers?

Re the nature of DemandPrimary: the intent here is for read (not write) operations, where the caller prefers failure vs a replica read. That's quite a niche scenario, so I wonder whether in your case, PreferPrimary would be a better choice.

[TimLovellSmith]

I'm sure I see a nonrepresentative sample, but the questions that tend to come my way tend to be either

"my write command failed because SE.redis thinks I'm issuing it to a slave, and I have DemandMaster set!"
or
"my write command failed because SE.Redis sent it to a slave, even though I have DemandMaster set! How can this happen?"

[mgravell]

I'm guessing that in both of these cases, this was during or shortly after a failover; is that correct?

If so, then there are various ways we can improve things:

• library/application initiated topology change notifications; this is actually baked into SE.Redis but only works if SE.Redis performs the failover (or at least: the exact same convention is followed)
• we perform periodic topology polls; perfectly possible and quite easy to implement, but: how often? How long is it acceptable to be wrong?
• we react to certain error messages; we certainly already do this for -MOVED etc; I can check whether we do this for -READONLY
• server initiated topology change notifications; we already react to the Azure notifications, and as I hinted: we're discussing what can be built into regular Redis (although that would be server-version dependent)

Thoughts?

[TimLovellSmith]

That was my guess too. Or that there's an unexpected interval of time with no masters, is a runner up guess.

Reacting to certain error messages or notifications sounds like the smart thing to do, to avoid scaled-up polling costs as much as possible.

I've seen other language's libraries doing that, but in an effort to reduce costs, they seem to often use long polling times and then the overall experience is not really satisfactory for ensuring a quick reaction to a failover.

[TimLovellSmith]

I'm also starting to internally question my assumption that the current behavior is terrible.

It is basically doing something useful if you use it for read commands to avoid reading from replicas - that kind of makes sense! So what are the things I don't understand then:

• Why do we allow using it with write commands, which would fail anyway? (Except for clusters that allow writes to replicas!)
• Does it meet people's needs to avoid reading from replicas in best effort fashion? Or is the reason they're avoiding reading from replicas really because they want consistency, in which case they really don't want to ever be able to read from a replica even if SE.redis cached that its a master?
• Do write commands ever demand master automagically, so that people would get the error but without understanding why?
• Is the error message really good enough? It seems like instead of "You tried to send commands to a replica which is not allowed" we could be explaining this a bit differently.

[mgravell]

Without active server participation, I fear that polling is always going to be frustratingly slow, hence we don't really try to do that. I'll check on the -READONLY reaction, and you should see "maintenance events" ala pure Redis appear in a PR here soon.

[mgravell]

Your last bullet (communication/messaging) is certainly something we can improve. The rest are all good questions.