From 542749b0d3282c8cfebb24faeb8928b710b1fc5d Mon Sep 17 00:00:00 2001 From: Julien Letessier Date: Thu, 15 Jan 2026 09:14:41 +0100 Subject: [PATCH] Reject cross-shard MSETNX Prevent MSETNX from being split across shards to preserve atomicity. Adds msetnx_atomicity test and runs it in the harness. --- src/Handler.cpp | 6 ++++++ test/msetnx_atomicity.py | 43 ++++++++++++++++++++++++++++++++++++++++ test/run.sh | 4 +++- 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 test/msetnx_atomicity.py diff --git a/src/Handler.cpp b/src/Handler.cpp index db10ddf..70de0cb 100644 --- a/src/Handler.cpp +++ b/src/Handler.cpp @@ -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()) { diff --git a/test/msetnx_atomicity.py b/test/msetnx_atomicity.py new file mode 100644 index 0000000..3e098af --- /dev/null +++ b/test/msetnx_atomicity.py @@ -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) diff --git a/test/run.sh b/test/run.sh index 6fa80dc..bdbd260 100755 --- a/test/run.sh +++ b/test/run.sh @@ -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