Conversation
|
huangwei03 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
TysonAndre
left a comment
There was a problem hiding this comment.
Seems promising and should have low impact when monitors are not used, but I had some review comments.
I'm also not that familiar with the redis monitor protocol and the edge cases, so I'll probably have more comments when I do look at that.
E.g. twemproxy should probably disconnect a client and assume the client is misbehaving if a command is sent by a client (with c->monitor true) before the client signals that the monitor should be stopped.
Co-authored-by: Tyson Andre <tyson.andre@uwaterloo.ca>
|
@TysonAndre |
Also, it would be reasonable to have it be off by default in the pool config (nc_conf) - if nobody's actively using it or deciding to enable it the performance impact would be surprising when someone tried |
|
@TysonAndre please review |
There was a problem hiding this comment.
i review again, please give me some suggestion, thanks. @TysonAndre
implents #652