summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authordormando <dormando@rydia.net>2023-03-13 16:57:23 -0700
committerdormando <dormando@rydia.net>2023-03-26 16:48:37 -0700
commitdb0e5b33b0d1043bb51346e7d7cf9bd317a64921 (patch)
tree194b4865ec600ae0ddabaa3bc5b85a30cc84c2ae
parent6c80728209acdb46629db8db3868d59d627ec33e (diff)
downloadmemcached-db0e5b33b0d1043bb51346e7d7cf9bd317a64921.tar.gz
proxy: fix reversal of pipelined backend queries
If a client sends multiple requests in the same packet, the proxy would reverse the requests before sending them to the backend. They would return to client in the correct order because top level responses are sent in the order they were created. In practice I guess this is rarely noticed. If a client sends a series of commands where the first one generates a syntax error, all prior commands would still succeed. It would also trip people up if they test pipelining commands as read-your-write would fail as the write gets ordered after the read. Did run into this before, but I thought it was just the ascii multiget code reversing keys, which would be harmless as the whole command has to complete regardless of key order.
-rw-r--r--proto_proxy.c7
-rw-r--r--t/proxyunits.t16
2 files changed, 10 insertions, 13 deletions
diff --git a/proto_proxy.c b/proto_proxy.c
index b01064d..3e17441 100644
--- a/proto_proxy.c
+++ b/proto_proxy.c
@@ -224,6 +224,8 @@ void proxy_thread_init(void *ctx, LIBEVENT_THREAD *thr) {
}
// ctx_stack is a stack of io_pending_proxy_t's.
+// head of q->s_ctx is the "newest" request so we must push into the head
+// of the next queue, as requests are dequeued from the head
void proxy_submit_cb(io_queue_t *q) {
proxy_event_thread_t *e = ((proxy_ctx_t *)q->ctx)->proxy_io_thread;
io_pending_proxy_t *p = q->stack_ctx;
@@ -270,11 +272,10 @@ void proxy_submit_cb(io_queue_t *q) {
be = p->backend;
if (be->use_io_thread) {
- // insert into tail so head is oldest request.
- STAILQ_INSERT_TAIL(&head, p, io_next);
+ STAILQ_INSERT_HEAD(&head, p, io_next);
} else {
// emulate some of handler_dequeue()
- STAILQ_INSERT_TAIL(&be->io_head, p, io_next);
+ STAILQ_INSERT_HEAD(&be->io_head, p, io_next);
if (be->io_next == NULL) {
be->io_next = p;
}
diff --git a/t/proxyunits.t b/t/proxyunits.t
index 284cb09..9d8b302 100644
--- a/t/proxyunits.t
+++ b/t/proxyunits.t
@@ -343,19 +343,15 @@ check_version($ps);
my $be = $mbe[0];
my $cmd = "get /b/a /b/b /b/c\r\n";
print $ps $cmd;
- # NOTE: the proxy ends up reversing the keys to the backend, but returns keys in the
- # proper order. This is undesireable but not problematic: because of how
- # ascii multiget syntax works the server cannot start responding until all
- # answers are resolved anyway.
- is(scalar <$be>, "get /b/c\r\n", "multiget breakdown c");
- is(scalar <$be>, "get /b/b\r\n", "multiget breakdown b");
is(scalar <$be>, "get /b/a\r\n", "multiget breakdown a");
+ is(scalar <$be>, "get /b/b\r\n", "multiget breakdown b");
+ is(scalar <$be>, "get /b/c\r\n", "multiget breakdown c");
- print $be "VALUE /b/c 0 1\r\nc\r\n",
+ print $be "VALUE /b/a 0 1\r\na\r\n",
"END\r\n",
"VALUE /b/b 0 1\r\nb\r\n",
"END\r\n",
- "VALUE /b/a 0 1\r\na\r\n",
+ "VALUE /b/c 0 1\r\nc\r\n",
"END\r\n";
for my $key ('a', 'b', 'c') {
@@ -366,9 +362,9 @@ check_version($ps);
# Test multiget workaround with misses (known bug)
print $ps $cmd;
- is(scalar <$be>, "get /b/c\r\n", "multiget breakdown c");
- is(scalar <$be>, "get /b/b\r\n", "multiget breakdown b");
is(scalar <$be>, "get /b/a\r\n", "multiget breakdown a");
+ is(scalar <$be>, "get /b/b\r\n", "multiget breakdown b");
+ is(scalar <$be>, "get /b/c\r\n", "multiget breakdown c");
print $be "END\r\nEND\r\nEND\r\n";
is(scalar <$ps>, "END\r\n", "final END from multiget");