Elasticache: Abort incomplete pipelines and transactions upon reconnect

[alavers]

Elasticache severs the connection immediately after it returns a
READONLY error. This can sometimes leave queued up pipelined commands
in an inconsistent state when the connection is reestablished. For
example, if a pipeline has 6 commands and the second one generates a
READONLY error, Elasticache will only return results for the first two
before severing the connection. Upon reconnect, the pipeline still
thinks it has 6 commands to send but the commandQueue has only 4. This
fix will detect any pipeline command sets that only had a partial
response before connection loss, and abort them.

This Elasticache behavior also affects transactions. If reconnectOnError
returns 2, some transaction fragments may end up in the offlineQueue.
This fix will check the offlineQueue for any such transaction fragments
and abort them, so that we don't send mismatched multi/exec to redis
upon reconnection.

• Introduced piplineIndex property on pipelined commands to allow for later
  cleanup
• Added a routine to event_handler that aborts any pipelined commands inside
  commandQueue and offlineQueue that were interrupted in the middle of the
  pipeline
• Added a routine to event_handler that removes any transaction
  fragments from the offline queue
• Introduced inTransaction property on commands to simplify pipeline logic
• Added a flags param to mock_server to allow the Elasticache disconnect
  behavior to be simulated
• Added a reconnect_on_error test case for transactions
• Added some test cases testing for correct handling of this unique elasticache
  behavior
• Added unit tests to validate inTransaction and pipelineIndex setting

Fixes #965

---

You can simulate this Elasticache disconnect behavior on a local redis instance by modifying the source like so:

diff --git a/src/server.c b/src/server.c
index f6faa61a..31721551 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2702,7 +2702,9 @@ int processCommand(client *c) {
         !(c->flags & CLIENT_MASTER) &&
         c->cmd->flags & CMD_WRITE)
     {
+        flagTransaction(c);
         addReply(c, shared.roslaveerr);
+        c->flags |= CLIENT_CLOSE_AFTER_REPLY;
         return C_OK;
     }

(Thanks for the idea @luin)

[alavers]

I do feel like this approach is treating the downstream symptoms rather than the root cause of the problem. The tradeoff is that it isolates connection handling cleanup stuff inside event_handler so that transaction.ts and pipeline.ts can be more decoupled from reconnection behaviors.

I'm open to other ways of approaching this.

[luin]

Awesome! Merged 🍻

[ioredis-robot]

🎉 This PR is included in version 4.16.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀