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

Side-Channel/Control Plane Information #38

Open
sirupsen opened this issue Feb 26, 2020 · 6 comments
Open

Side-Channel/Control Plane Information #38

sirupsen opened this issue Feb 26, 2020 · 6 comments

Comments

@sirupsen
Copy link

sirupsen commented Feb 26, 2020

Hey Redis Team!

I've looked through the RESP3 spec, it looks great!

One thing that sprung to mind is passing metadata in the request/response cycle. In MySQL, for example, we tend to pass a comment in each query with where the query came from (path, request id, ..). This is very helpful in the slow query log, and for instrumenting with e.g. BPF:

Account Load (0.3ms)  SELECT `accounts`.* FROM `accounts` 
WHERE `accounts`.`queenbee_id` = 1234567890 
LIMIT 1 
/*application:xx,controller:project_imports,action:show,request_id:x, ..*/

In the response cycle, it's possible to attach metadata as well (continuing to use MySQL as an example). We anticipate we'll soon through a plugin we will pass back information such as the CPU, Network and IO cost of a query, as well as the utilization of these resources. This will help throttle further up the stack, e.g. shed load the load-balancers or prioritize queries. We think this could be useful for Redis too, longer-term.

As far as I can understand this is not something RESP3 in its current form will support. The closest I can see is Push, which is out-of-band data, that will not be tied to a query.

Curious what you think, or whether I missed something! 👀

@kristoff-it
Copy link

For attaching metadata to a reply, take a look at the Attribute type.

@sirupsen
Copy link
Author

I can't believe I missed that, thank you!

@sirupsen sirupsen reopened this Feb 27, 2020
@sirupsen
Copy link
Author

I closed this too quickly. A lovely co-worker pointed out that it's only for the response, this doesn't attach information to the query itself for observability purposes.

@sirupsen sirupsen changed the title Out of Band Information Side-Channel/Control Plane Information Mar 2, 2020
@kristoff-it
Copy link

For commands the protocol will most probably never add extra syntax for metadata, but you can take a look at the Redis Module API (redismodule.h) and see how to create a module that can capture all commands sent to Redis and rewrite them before they get executed.

This way you could say that if the last argument of a command starts with trace:, then it gets logged and removed before the command gets executed.

This would allow you then to send commands as follows:

SET key "hello world" "trace: request123 ..."
GET key "trace: ..."

...and so on.

@sirupsen
Copy link
Author

sirupsen commented Mar 4, 2020

Clever work-around :)

For commands the protocol will most probably never add extra syntax for metadata

Can I ask why? 👂

@AngusP
Copy link
Contributor

AngusP commented May 4, 2020

A lovely co-worker pointed out that it's only for the response, this doesn't attach information to the query itself for observability purposes.

According to the spec:

The RESP3 protocol can be used asymmetrically, as it is in Redis: only a subset can be sent by the client to the server, [...] different applications willing to use RESP3 for other goals may just allow the protocol to be used in a "full duplex" fashion where both the ends can use the full set of types available.

So Redis doesn't implement the attribute type in a query, but the spec allows it.

There may be an argument to be had that Redis should allow this kind of thing because a module may need it, but a working example for why one might need to send an attribute in a query would probably be needed for that kind of proposal to be implemented 😄

If it's for some kind of sidecar to log/track performance, as RESP3 is line based you could pass every query to your monitoring application first with an extra attribute line, have it strip that attribute line out, and then pass the modified query along to redis. Might be tricky to get working with a client, but maybe worth a try?

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

3 participants