Reject EXEC containing write commands against RO replica.

Thanks to @soloestoy for discovering this issue in #5667.
This is an alternative fix in order to avoid both cycling the clients
and also disconnecting clients just having valid read-only transactions
pending.
This commit is contained in:
antirez 2018-12-11 11:39:18 +01:00
parent 086363babf
commit 274531396c
2 changed files with 20 additions and 0 deletions

View File

@ -35,6 +35,7 @@
void initClientMultiState(client *c) {
c->mstate.commands = NULL;
c->mstate.count = 0;
c->mstate.cmd_flags = 0;
}
/* Release all the resources associated with MULTI/EXEC state */
@ -67,6 +68,7 @@ void queueMultiCommand(client *c) {
for (j = 0; j < c->argc; j++)
incrRefCount(mc->argv[j]);
c->mstate.count++;
c->mstate.cmd_flags |= c->cmd->flags;
}
void discardTransaction(client *c) {
@ -137,6 +139,21 @@ void execCommand(client *c) {
goto handle_monitor;
}
/* If there are write commands inside the transaction, and this is a read
* only slave, we want to send an error. This happens when the transaction
* was initiated when the instance was a master or a writable replica and
* then the configuration changed (for example instance was turned into
* a replica). */
if (server.masterhost && server.repl_slave_ro &&
!(c->flags & CLIENT_MASTER) && c->mstate.cmd_flags & CMD_WRITE)
{
addReplyError(c,
"Transaction contains write commands but instance "
"is now a read-only slave. EXEC aborted.");
discardTransaction(c);
goto handle_monitor;
}
/* Exec all the queued commands */
unwatchAllKeys(c); /* Unwatch ASAP otherwise we'll waste CPU cycles */
orig_argv = c->argv;

View File

@ -654,6 +654,9 @@ typedef struct multiCmd {
typedef struct multiState {
multiCmd *commands; /* Array of MULTI commands */
int count; /* Total number of MULTI commands */
int cmd_flags; /* The accumulated command flags OR-ed together.
So if at least a command has a given flag, it
will be set in this field. */
int minreplicas; /* MINREPLICAS for synchronous replication */
time_t minreplicas_timeout; /* MINREPLICAS timeout as unixtime. */
} multiState;