ctubbsii opened a new issue, #5390: URL: https://github.com/apache/accumulo/issues/5390
Accumulo currently has a custom Thrift protocol created by a class called `TraceProtocolFactory`, which is used by `ThriftUtil` to construct transports for our RPCs. This class does nothing more than extend `TCompactProtocol` and override two write methods in order to produce a side-effect of capturing trace timing information via opentelemetry on the client side (if tracing is enabled). It doesn't actually modify the RPC message in any way from what `TCompactProtocol` does. This issue proposes that we *do* modify the message from what `TCompactProtocol` does, so that we include a header on every RPC message containing certain information needed on every RPC request to verify it. At a minimum, the header should include some kind of [magic number](https://en.wikipedia.org/wiki/Magic_number_(programming)) to uniquely identify the message as one recognizable by servers as the Accumulo protocol, and sufficient version information from the client side of the connection for the server side to verify that the client is a compatible Accumulo version. This header should be validated on the server side part of the protocol when the message is read, and throw an appropriate TException that informs the client that it is not using a recognized/compatible protocol. In addition, we currently pass a `TInfo` object as the first parameter of all of our RPC methods. Then we wrap this with a `Proxy` object on the server side with `TraceUtil.wrapService()` so that it can create server-side traces. This could be done in the custom protocol that this issue is proposing, instead. If a trace info object was serialized to the header of the custom protocol, then it could be deserialized on the receiving end of the RPC and all of this would be self-contained inside the custom Accumulo protocol, and we could simplify all of our RPCs dramatically. It may also be possible to serialize the RPC credentials into the header of the protocol as well, to avoid needing to pass that as a separate parameter on every method, too. But I think that bit is low-priority, and could be done as a follow-on task, if at all. So, the priority is: 1. Primarily, create a custom protocol with the versioning information stored in a header and checks on the server side to validate it, then 2. Secondarily, simplify the thrift APIs by adding tracing info to the protocol header, then lastly 3. Optionally, further simplify the thrift APIs by adding rpc credentials to the protocol header (I'm not sure if this should be done at all, though, so this should definitely be done as a follow-on, if it is done at all) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
