When processing the sentinel response, twemproxy should not make assumptions about field ordering and line number #527
Description
Making assumptions about line numbers makes the code brittle, and may break in new (or patched) redis sentinel versions
Compared to redis operations (GET/SET), sentinel operations are infrequent, and don't need to be optimized.
I'm seeing errors such as the below right now (may have different line numbers)
[2017-06-19 11:45:05.479] nc_mbuf.c:300 mbuf read string failed, split char :
[2017-06-19 11:45:05.479] nc_sentinel.c:457 sentinel's response error, close sentinel conn.
[2017-06-19 11:45:05.479] nc_mbuf.c:300 mbuf read string failed, split char :
[2017-06-19 11:45:05.479] nc_sentinel.c:457 sentinel's response error, close sentinel conn.
[2017-06-19 11:45:20.119] nc_mbuf.c:300 mbuf read string failed, split char =
[2017-06-19 11:45:20.119] nc_sentinel.c:173 skip server name prefix failed.
[2017-06-19 11:45:20.119] nc_sentinel.c:457 sentinel's response error, close sentinel conn.
[2017-06-19 11:45:20.119] nc_mbuf.c:300 mbuf read string failed, split char :
[2017-06-19 11:45:20.119] nc_sentinel.c:457 sentinel's response error, close sentinel conn
this is happening with redis sentinel version 3.2.6 (Not 100% sure if new fields are added, or if it's due to a patch to sentinel. Will update this once I figure it out)
Also, it seems like the sentinel response parsing code (sentinel_proc_sentinel_info) doesn't pay attention to how many entries redis server sends back? (From the first glance at the code)
line_buf = mbuf_get();
if (line_buf == NULL) {
goto error;
}
/* get sentinel master num at line 3 */
msg_read_line(msg, line_buf, 3); // Is this skipping the line with the number of entries in the response?
if (mbuf_length(line_buf) == 0) {
log_error("read line failed from sentinel ack info when skip line not used.");
goto error;
}