diff options
author | dormando <dormando@rydia.net> | 2023-03-13 16:57:23 -0700 |
---|---|---|
committer | dormando <dormando@rydia.net> | 2023-03-26 16:48:37 -0700 |
commit | db0e5b33b0d1043bb51346e7d7cf9bd317a64921 (patch) | |
tree | 194b4865ec600ae0ddabaa3bc5b85a30cc84c2ae | |
parent | 6c80728209acdb46629db8db3868d59d627ec33e (diff) | |
download | memcached-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.c | 7 | ||||
-rw-r--r-- | t/proxyunits.t | 16 |
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"); |