standalone REDIRECT: Fix scripting and further MULTI/EXEC scenarios

[gmbnomis]

In cluster mode, the necessity of a "MOVED" response is mainly determined based on the keys of a command whereas in standalone redirect mode, the necessity of a "REDIRECT" response is determined based on the command's flags.

It turns out that looking at keys is necessary. Moreover, the currently used command flags do not encompass all situations in which a REDIRECT is necessary:

• WATCH command (redirect in READWRITE mode is necessary)
• transactions with no keys must not be redirected
• scripting scenarios: scripts (with keys) must be redirected in some situations (issue [BUG] "CLIENT CAPA redirect" has no effect in scripts #868).

The new logic is a heavily "distilled" version of getNodeByQuery used in cluster mode. As we don't need to look at slots and their consistency, the information necessary is the flags of the command and whether it accesses at least one key.

Fixes #868

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.00%. Comparing base (221ff44) to head (d7e26c5).
Report is 37 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #1781      +/-   ##
============================================
- Coverage     71.18%   71.00%   -0.18%     
============================================
  Files           123      123              
  Lines         65619    65698      +79     
============================================
- Hits          46712    46652      -60     
- Misses        18907    19046     +139     

Files with missing lines	Coverage Δ
src/server.c	87.63% <100.00%> (+0.09%)	⬆️

... and 40 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[soloestoy]

WATCH command (redirect in READWRITE mode is necessary)

This makes sense.

transactions with no keys must not be redirected

I don't understand, this already aligns with the current behavior.

scripting scenarios: scripts (with keys) must be redirected in some situations

Can you elaborate it? EVAL and EVAL_RO would or not be redirected in different scenarios?

[gmbnomis]

transactions with no keys must not be redirected

I don't understand, this already aligns with the current behavior.

Mostly yes, I suppose, except when a script with no keys is used, see test case "replica allows MULTI that accesses no key". So, I think we need to look at the keys the commands within a transaction use, not only at the flags of those commands.

scripting scenarios: scripts (with keys) must be redirected in some situations

Can you elaborate it? EVAL and EVAL_RO would or not be redirected in different scenarios?

The proposal is to behave like in cluster mode, which basically means that EVAL and EVAL_RO do not behave differently when it comes to redirects (I think this is because EVAL_RO is rather new. If one does not have EVAL_RO, it makes sense for EVAL to assume that keys are used for reading in READONLY mode.) Thus:

In READWRITE mode on replica:

• Script without key is not redirected
• Script with keys is redirected (both EVAL and EVAL_RO)

In READONLY mode on replica:

• Script without keys is not redirected

• Script with keys is not redirected (both EVAL and EVAL_RO)

  There is a difference in the reported error, though, if the script actually tries to write. For EVAL_RO this is ERR Write commands are not allowed from read-only scripts. And for EVAL this is READONLY You can't write against a read only replica. See also the corresponding discussion for cluster mode at [BUG] Cluster: Inconsistent MOVED/READONLY errors for read-only client #869 (comment).