Reject cross-shard MSETNX

Prevent MSETNX from being split across shards to preserve atomicity.

Adds msetnx_atomicity test and runs it in the harness.
This commit is contained in:
Julien Letessier 2026-01-15 09:14:41 +01:00
parent 781b4a68e9
commit 542749b0d3
3 changed files with 52 additions and 1 deletions

View File

@ -562,6 +562,12 @@ bool Handler::preHandleRequest(Request* req, const String& key)
return false;
}
switch (req->type()) {
if (req->type() == Command::Msetnx && mProxy->isSplitMultiKey()) {
ResponsePtr res = ResponseAlloc::create();
res->setErr("msetnx not supported across shards");
handleResponse(nullptr, req, res);
return true;
}
case Command::Ping:
case Command::Echo:
if (key.empty()) {

43
test/msetnx_atomicity.py Normal file
View File

@ -0,0 +1,43 @@
#!/usr/bin/env python
#
# Verify MSETNX does not partially apply changes when it returns 0
#
import argparse
import sys
import redis
def run_test(host, port):
c = redis.StrictRedis(host=host, port=port)
c.delete("msetnx_k1", "msetnx_k2")
c.set("msetnx_k1", "existing")
try:
r = c.execute_command("MSETNX", "msetnx_k1", "v1", "msetnx_k2", "v2")
except Exception as exc:
# If proxy rejects cross-shard MSETNX, this is acceptable.
return True
if r not in (0, b"0", False):
print("FAIL: expected MSETNX result 0, got:", r)
return False
if c.get("msetnx_k2") is not None:
print("FAIL: MSETNX partially applied when it returned 0")
return False
return True
if __name__ == "__main__":
parser = argparse.ArgumentParser(conflict_handler='resolve', description="MSETNX atomicity test")
parser.add_argument("-h", "--host", default="127.0.0.1")
parser.add_argument("-p", "--port", type=int, default=7617)
args = parser.parse_args()
if run_test(args.host, args.port):
print("PASS: MSETNX atomicity")
sys.exit(0)
print("FAIL: MSETNX atomicity")
sys.exit(1)

View File

@ -66,6 +66,7 @@ NULL_RESPONSE_EXIT=0
PUBSUB_LONG_EXIT=0
TRANSACTION_FORBID_EXIT=0
MGET_WRONG_TYPE_EXIT=0
MSETNX_ATOMICITY_EXIT=0
uv run python3 test/basic.py || BASIC_EXIT=$?
uv run python3 test/pubsub_minimal.py -p 7617 || PUBSUB_REDIS_EXIT=$?
@ -73,11 +74,12 @@ uv run python3 test/pubsub_minimal.py -p 6379 || PUBSUB_MINIMAL_EXIT=$?
uv run python3 test/pubsub.py || PUBSUB_EXIT=$?
uv run python3 test/pubsub_subscription_order.py -p 7617 || PUBSUB_ORDER_EXIT=$?
uv run python3 test/pubsub_parser_reset.py -p 7617 || PUBSUB_RESET_EXIT=$?
uv run python3 test/msetnx_atomicity.py -p 7617 || MSETNX_ATOMICITY_EXIT=$?
uv run python3 test/mget_wrong_type.py -p 7617 || MGET_WRONG_TYPE_EXIT=$?
uv run python3 test/transaction_forbid.py -p 7617 || TRANSACTION_FORBID_EXIT=$?
uv run python3 test/pubsub_long_name.py -p 7617 || PUBSUB_LONG_EXIT=$?
uv run python3 test/null_response_handling.py -p 7617 || NULL_RESPONSE_EXIT=$?
uv run python3 test/pubsub_message_response.py -p 7617 || PUBSUB_MESSAGE_EXIT=$?
TEST_EXIT=$((BASIC_EXIT + PUBSUB_REDIS_EXIT + PUBSUB_MINIMAL_EXIT + PUBSUB_EXIT + PUBSUB_MESSAGE_EXIT + PUBSUB_ORDER_EXIT + PUBSUB_RESET_EXIT + NULL_RESPONSE_EXIT + PUBSUB_LONG_EXIT + TRANSACTION_FORBID_EXIT + MGET_WRONG_TYPE_EXIT))
TEST_EXIT=$((BASIC_EXIT + PUBSUB_REDIS_EXIT + PUBSUB_MINIMAL_EXIT + PUBSUB_EXIT + PUBSUB_MESSAGE_EXIT + PUBSUB_ORDER_EXIT + PUBSUB_RESET_EXIT + NULL_RESPONSE_EXIT + PUBSUB_LONG_EXIT + TRANSACTION_FORBID_EXIT + MGET_WRONG_TYPE_EXIT + MSETNX_ATOMICITY_EXIT))
exit $TEST_EXIT