MULTI/EXEC can commit partial writes from a command that returns a runtime error

[PragmaTwice]

Search before asking

• I had searched in the issues and found no similar issues.

Version

Unstable 567fcf6c26fc38367c1bda81ca0541c6ab861086 on Linux, using the default storage encoding. The reproduction only changes rocksdb.write_options.write_batch_max_bytes at runtime.

Minimal reproduce step

The following was reproduced four times on the current unstable build. The value 180 makes the first internal field write fit in the transaction batch while the next one reaches the batch limit. It may need a small adjustment if the storage encoding or namespace configuration differs.

CONFIG SET rocksdb.write_options.write_batch_max_bytes 0
DEL txhash
HSET txhash f1 old1 f2 old2 f3 old3
CONFIG SET rocksdb.write_options.write_batch_max_bytes 180

# Standalone control: this fails atomically and leaves every field unchanged.
HSET txhash f1 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA f2 BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB f3 CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC
HMGET txhash f1 f2 f3

# The same HSET inside MULTI leaks one of its writes into the shared batch.
MULTI
HSET txhash f1 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA f2 BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB f3 CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC
SET after ok
HMGET txhash f1 f2 f3
EXEC

CONFIG SET rocksdb.write_options.write_batch_max_bytes 0
HMGET txhash f1 f2 f3
GET after

The standalone HSET returns:

ERR Operation aborted: Memory limit reached

and its following HMGET returns old1, old2, old3.

What did you expect to see?

Runtime errors should not abort the remaining queued commands or roll back earlier successful commands. An ordinary atomic command such as HSET must not leave only some of its field updates behind. EXEC should therefore return the HSET error, commit SET after ok, and both HMGET calls should return old1, old2, old3.

What did you see instead?

EXEC returns:

1) ERR Operation aborted: Memory limit reached
2) OK
3) old1, old2, CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC

After EXEC, GET after returns ok, but HMGET still returns old1, old2, and the new f3 value. The failed HSET processes fields in reverse order: its first Put succeeds, the next Put returns MemoryLimit, and the successful Put remains in the transaction-wide batch.

Anything Else?

This is a transaction-wide issue rather than an HSET-specific one:

• Storage::BeginTxn creates one shared WriteBatchWithIndex for the whole EXEC.
• Storage::GetWriteBatchBase returns an observer to that shared batch in transaction mode, and Storage::Write returns OK without flushing it.
• A RocksDB batch operation that exceeds max_bytes rolls back only that individual operation. Earlier successful operations from the same Redis command remain in the shared batch.
• CommandExec correctly continues after runtime command errors, but it then commits those leftover operations because there is no per-command rollback boundary.

Many multi-record write paths are affected, including MSET/MSETEX/MSETNX, SADD/SREM, ZADD and range removals, list push/pop/trim, XADD/XTRIM, hash mutations, bitmap mutations, and other types. Depending on where the failure occurs, this can leave metadata inconsistent with subkeys, break ZSet's two-column-family index, or commit deletes from a failed trim/pop operation. write_batch_max_bytes is a deterministic trigger, but RocksDB read/write errors or decode/validation failures after earlier batch mutations have the same risk.

This is distinct from #2992, which handled errors from the final DB commit, and from #2554, which tracks transaction concurrency. The transaction-wide shared batch introduced for #1281 provides EXEC-level atomic commit, but it currently has no command-level savepoints.

A general fix likely needs savepoints at ordinary atomic command execution boundaries while the shared transaction batch is active: establish a savepoint before the command, PopSavePoint() after full success, and RollbackToSavePoint() on error. The boundary should cover PutLogData and associated index updates while preserving writes from previously successful commands. If rollback itself fails, the whole transaction batch should be discarded rather than committed.

This must not become an unconditional rollback around every outer command. Redis deliberately retains successful writes performed by an EVAL/Function before a later script runtime error. Nested redis.call commands still need their own atomic boundaries, but an outer script/function error must not roll back its already successful nested writes. For example, MULTI; EVAL "redis.call('SET',KEYS[1],'v'); error('boom')" 1 scriptkey; EXEC should return a script error while leaving scriptkey=v.

Regression tests should force a mid-command MemoryLimit, verify that earlier and later successful commands still commit, and verify that no value, metadata, secondary-index, or replication-log record from the failed atomic command survives. They should also protect the EVAL/Function behavior above so a command-level fix does not accidentally add Redis-incompatible script rollback.

Are you willing to submit a PR?

• I'm willing to submit a PR!