feat(connection): Support pipelining with Memcached #2648
Conversation
| @@ -277,28 +277,6 @@ string_view Connection::PubMessage::Message() const { | |||
| return {buf.get() + channel_len, message_len}; | |||
| } | |||
|
|
|||
| struct Connection::DispatchOperations { | |||
There was a problem hiding this comment.
I just moved it down because it was in an almost random place and all the definitions are actually 100 lines below
dranikpg
force-pushed
the
mc-async-dispatch
branch
from
3ab671c
to
77de5cc
Compare
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
dranikpg
force-pushed
the
mc-async-dispatch
branch
from
77de5cc
to
ce07c64
Compare
|
Do we need any optimizations? Like the following:
|
src/server/main_service.cc
Outdated
| @@ -1397,7 +1397,7 @@ void Service::DispatchMC(const MemcacheParser::Command& cmd, std::string_view va | |||
| strcpy(cmd_name, "MGET"); | |||
| break; | |||
| case MemcacheParser::FLUSHALL: | |||
| strcpy(cmd_name, "FLUSHDB"); | |||
| strcpy(cmd_name, "FLUSHALL"); | |||
There was a problem hiding this comment.
Ah, good catch, forgot to follow up on this 😅 FLUSHDB is broken for some reason, will look at this
| DispatchCommand(consumed, tlh); | ||
| bool has_more = consumed < io_buf_.InputLen(); | ||
|
|
||
| if (tl_traffic_logger.log_file) // Log command as soon as we receive it |
There was a problem hiding this comment.
Consider keeping it for Memcache as well. Can be done later though.
There was a problem hiding this comment.
We didn't log anything for memcached before. It's also not a "one liner" because we have to serialize our parsed command back, including all the flags
src/facade/memcache_parser.cc
Outdated
| res->key = tokens[key_pos++]; | ||
|
|
||
| if (key_pos < num_tokens && base::_in(res->type, {MP::STATS, MP::FLUSHALL})) | ||
| return MP::PARSE_ERROR; // we do not support additional arguments for now. |
There was a problem hiding this comment.
The flow is broken, the line above conflicts with the if, we can't safely parse a key for STATS or FLUSHALL. I quick fixed it below
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
dranikpg
force-pushed
the
mc-async-dispatch
branch
from
70ba15d
to
1aacb93
Compare
| mi_heap_t* tlh = mi_heap_get_backing(); | ||
|
|
||
| // Re-use connection local resources to reduce allocations | ||
| RespVec& parse_args = tmp_parse_args_; |
There was a problem hiding this comment.
why do you prefer aliasing these variables and then pass them by capture?
There was a problem hiding this comment.
To possibly untie them later, maybe they can become thread locals instead, they look weird as connection members
Add support for pipelining to memcached connection interface
Update pytests to cover noreply dynamics more extensively