-
Notifications
You must be signed in to change notification settings - Fork 204
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
How to save transport context? #392
Comments
Hi, thanks for your questions!
|
Yes, basically that is what I am asking, "wrapping" the packet can be done to include the response topic and the correlation data information, and on the other side the transport can unpackage keeping only the actual client request, however after the service has processed it and calculated the response, I would need access to that data to "wrap" the response with the same information that the request was "wrapped" with. And this is causing my headache, I'm not sure how I can save some contextual data during the lifecycle of the request on the server side, between the time the request arrives at the stream, and the response is written to the sink. |
Can you link me to an overview of the MQTTv5 protocol? I'd like to understand what's contained in an MQTTv5 packet. Is there any differentiation between stream ID and message ID? |
This is an explanation of the request response flow in mqtt v5. The correlation id has roughly the same purpose as the message id, the response topic doesnt have an equivalent, and each client will want to listen to responses on another topic, so they only see their own |
The MQTT specifies the correlation ID as basically anything the cilent wants, and the server needs to send it back unmodified. The protocol also allows a response topic to be sent, to tell the server where the client expects the response, which may be different from client to client, or even for each request made by a single client. My first idea was to encode the request id in the correlation data, but I'm reluctant to add this constraint, because if another client (written in another language for ex) comes around that simply assumes that it can send any data there (or has its own conflicting assumptions on what to use it for), that would cause issues. Basically we'd make it part of the tarpc protocol that not any data can be encoded there, and some clients may or may not be okay with that, so I'm more reassured if it handle it as an opaque value. Either way the response topic still needs to be kept throughout the request's lifecycle somehow, because there could be multiple clients sending requests to the server and they should obviously not see the responses the server sends to one another, and they may have different permissions to see one topic or another. A hack would be to compile the response topic from the request id, such as I'll release a repo shortly with the implementation of the MQTT transport, just need to clean the code up a bit. |
https://github.com/axos88/tarpc-mqtt-transport - still needs some work |
Ah one more thought - Having this context info saved would make it possible for tarpc to work over UDP as well, since the source IP/port can be saved in the context and the response could be sent to the correct place. The two problems are somewhat analogous. |
@tikue ^ |
Sorry for the delay, @axos88! I'll try to prioritize this in the next few weeks. |
Ok, I have now taken a very quick look at MQTT and understand a little better what you're trying to do! I have a few clarifying questions:
|
Thanks for looking into this.
What I'm trying to say is that if we want to be compliant with the spec, we need to implement a way to let the client define the request topic, and the correlation data not the transport. |
But I also think that having a context being produced and consumed by the transport could be useful for other purposes as well. I'm not sure it's possible to supply that to the context in the requests, but that would be awesome. Currently I sent in an auth field in each request, and parse the jwt token from the request itself, but it feels quirky and unnatural. What I'd imagine doing is something like With this feature the requests themselves would not need to include an auth field, and the transport could read and check auth from the place that is most natural or as described in their own spec. Authorization header for http, Mqtt property for Mqtt. |
Hm, I'm a little confused about the issue around compliance with the spec. From my perspective, the tarpc transport is the MQTT client? MQTT is being used to implement a tarpc transport; from the tarpc user's perspective, it's an implementation detail. MQTT is not being exposed directly to the tarpc user. I agree it would be cool to be able to stash arbitrary metadata in the context! Auth is the example I always think about, too. There is data that pretty much every RPC implementation needs, and it's orthogonal to the application itself. I've experimented with context extensions before, but I haven't landed on a design that I really like. The most straightforward solution is to add some kind of an AnyMap to the context, but I don't love that it's not type safe—e.g. it would be nice if a service could specify a bound on the context requiring that a username be present, but you can't really do that with a simple AnyMap. But maybe I'm overthinking it; something is better than nothing, after all. |
So the MQTTv5 spec defined how a request-response should look like over MQTT. So if we want our MqttTrasport for TARPC to be compliant with the MQTTv5 Request-Response spec, we cannot say that "the client needs to send this or that (the client Id, the request ID) in the CorrelationData", since the server needs to simply parrot back the CD without making any assumptions about its contents. It MAY do that, but the server cannot rely on the fact. If a random client comes along (such as my TS client), and sends a differently generated CD, we need to be able to respond to that client as well as long as the message payload is a valid TARPC request. Yep, I was also thinking about an anymap, and was wondering as well if we could make this more typesafe. Actix does something like that I guess with its extractors, but I'm not sure if we'd be able to make use of that code for this purpose. All in all something IS better than nothing, but I don't have such strong negative feelings about the use of an AnyMap either. I wouldn't say it's typeless, you just can't reason something being there at compile time, the same way you can't reason about the presence of a key in a HashMap. As far as I remember, an older version of actix (not sure, maybe it was another fw) also used an anymap internally (for extensions of an HttpRequest), and failed the requests if the data the handler was looking for was not there. Before running the handler, the framework would look at the anymap, and if it could not extract either one of the requested context, it would fail the request without running the handler. If successful, the handler would be handed the extracted context params, and would not actually see the raw AnyMap. It was handled magically by some macro I think. |
To answer your question. Yes, the tarpc transport IS the mqtt client, on both sides of the transport, and in a perfect world where everyone is using rust and this library, it would be an implementation detail. (Even in that case I'd prefer staying compliant with the MQTT spec), but this becomes even more desirable if we admit that some clients may be generating requests from code that is not written in rust or using our library. Then the way we send messages on the mqtt broker is not an implementation detail anymore, since this other piece of software needs to be compatible with it. IF they use an MQTT library that has its own opinion on how to generate a CD (because the spec lets them do that as they desire), they may have to fight the library before being able to generate valid messages understood by the tarpc server, which is to be avioded IMHO. |
Now I'm confused :) Is the MQTT library in control of the correlation data, or is the MQTT client (who is using the MQTT library) in control of the correlation data?
I'm confused, how do other clients affect what's done by the rust client? A UID should be globally unique, so I don't think other MQTT clients connecting to the same server would interfere. Unless you're saying you want the tarpc client to receive messages from other clients? (but then that would not really be implementing the tarpc spec, which is request/response between two peers). |
Sorry, it was pretty late, may have not answered clearly. For the second point. The confusion may come from mixing up terminologies. In the Mqtt world both the tarpc server and the tarpc client act as mqtt clients, and talk to each other via the mqtt broker. So the server is a client..... The tarpc server itself should be able to receive messages from a tarpc client using this library, but should also receive messages from a tarpc client written in typescript, or assembly as long as they both send the correct message format. |
Ok, got it, thanks! Here is the way I'm envisioning this working.
The tarpc client transport has two layers:
The tarpc server transport is a little bit simpler; it shouldn't need a spawned task to manage anything. It would need one piece of state besides the MQTT transport, a correlation_map.insert(request_id, msg.properties().get_binary(PropertyCode::CorrelationData)); The tarpc server transport wouldn't have to inspect the correlation data at all—it'd still be entirely opaque. And the tarpc server itself would not even know about the correlation data. When the tarpc server sends a |
Yeah, I think this approach could work, with minor modifications - besides the CD, the server needs to track the response topic as well, so it knows which topic to send the response to, but it can be part of the same Map<RequestId, (CorrelationData, ResponseTopic)>. I don't follow why you'd want to have two layers on the client side. I think this approach can be implemented without having an in-memory transport in between. Anyways, when I was working on the transport, I tried to avoid saving the Map<RequestId (CD, RT)> state in the transport itself, because you'd then need to do some housekeeping on both sides of the channel in case some requests are never answered for example, which complicate things. (When to do the housekeeping? How would that be triggered? What to do if an entry for a RequestId is not found? How does that fit in with request cancellations? Etc.) Adding the (CD, RT) state to the transport request itself on the server side would avoid this problem, since they'd be cleaned up when the request itself is cleaned up, and the compiler would guarantee it's always there. With all the context now available, can you take another look at tikue#10? Definately needs cleanup, but it should give you an idea of what I was trying to achieve (and it works) Something similar could be used for saving authentication/whatever context as well I think. |
You were mentioning wanting to have multiple clients using the same mqtt transport, right? How does that work exactly? from a quick glance, I thought the mqtt transport would only allow you to set up one AsyncReceiver. But if you can have one AsyncReceiver per client, all sharing the same underlying mqtt transport, then I don't think any multiplexing would be needed. I trust that you know how paho_mqtt works better than I do :)
Ahh, yes, this is indeed a problem! I agree, the context would be a nice place to store this mapping.
Yes, I'll take a look once it's cleaned up! I kind of implied it earlier, but to be explicit, I think the only modification needed is to add some kind of
|
The request sequencer was added to avoid collisions between the request IDs (two different clients sending two requests with the same request id would confuse the server since the incoming channel is shared), although it is a half assed solution - I agree. Now that I'm thinking about it ( I'm writing from mobile,) but I think an approach where each client would send to a unique topic
|
Re 2: that sounds like trace::Context? context::Context says "A request context that carries request-scoped information like deadlines and trace information." I think this wording applies to the use case we're considering. |
Regarding the server-side request ID issue, I think there shouldn't be a need to change the request ID at all, though I realize now what I fleshed out earlier would be insufficient. It can be done, though, with a similar setup to what I described for the client multiplexing.
|
I'm trying to implement an MQTTv5 transport, and I have it more-or-less up and running, with fixed request and response topics.
There are two challanges I'm facing:
Basically I'd need a way to save a transport context through the lifecycle of the request, so that it can be accessed when sending the response through the MQTT broker.
The text was updated successfully, but these errors were encountered: