Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When processing the sentinel response, twemproxy should not make assumptions about field ordering and line number #527

Closed
TysonAndre opened this issue Jun 19, 2017 · 1 comment

Comments

@TysonAndre
Copy link
Collaborator

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;
    }   
@TysonAndre
Copy link
Collaborator Author

Sorry, this issue refers to code in the unmerged PR #324

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant