Skip to content

Rewritten commands are logged as their original command#8006

Merged
madolson merged 4 commits intoredis:unstablefrom
madolson:slowlow-rewrite
Nov 10, 2020
Merged

Rewritten commands are logged as their original command#8006
madolson merged 4 commits intoredis:unstablefrom
madolson:slowlow-rewrite

Conversation

@madolson
Copy link
Contributor

@madolson madolson commented Nov 2, 2020

Slowlog emits whatever is on the client argv, which can be rewritten for replication. Note: They are rewritten even when there is no replica, so this occurs on standalone redis instances as well.

Copy link
Member

@oranagra oranagra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@madolson i think you need to add this to rewriteClientCommandArgument too, and in that case, maybe add test coverage using INCRBYFLOAT.

speaking of coverage, replaceClientCommandVector isn't covered by this test.
maybe add SET GET to that test, which will make it covered once #7957 is merged.

@madolson
Copy link
Contributor Author

I had the mental model there was two functions that rewrote client arguments, and just didn't realize the third one. Will also test the other cases as well!

@madolson
Copy link
Contributor Author

@oranagra Note, I refactored the code for rewriteClientVector since they do the same thing. The code is verbatim the same, it's just a little confusing since they do it differently.

@madolson madolson merged commit 3feff7d into redis:unstable Nov 10, 2020
oranagra pushed a commit that referenced this pull request Nov 11, 2020
(this is a fixup for #8006)

Co-authored-by: Madelyn Olson <[email protected]>
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Nov 19, 2020
* Rewritten commands are logged as their original command

Co-authored-by: Madelyn Olson <[email protected]>
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Nov 19, 2020
(this is a fixup for redis#8006)

Co-authored-by: Madelyn Olson <[email protected]>
@oranagra oranagra mentioned this pull request Jan 13, 2021
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

Successfully merging this pull request may close these issues.

incrbyfloat/hincrbyfloat should appear in the slow log EVALSHA is logged as EVAL in SLOWLOG

2 participants