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

Buffered transport for binary protocol #541

Open
aawilson opened this issue Mar 19, 2021 · 2 comments
Open

Buffered transport for binary protocol #541

aawilson opened this issue Mar 19, 2021 · 2 comments

Comments

@aawilson
Copy link

Currently, the only client/server implementations over the binary protocol are with framed transport. It would be swell if elixir-thrift could also generate implementations that use "unframed" (as described in the section "Framed" vs "Unframed" of the doc here) (I've also seen it referred to as "buffered", as in the thriftpy2 implementation here, although I don't know if their use of terminology makes sense given the salient difference seems to be not buffering incoming frames, but that could be my misunderstanding) transport, instead.

I looked at implementing this, but for the life of me, I can't see where thrift/framed/protocol_handler.ex is handling the framing, meaning I either just have poor elixir reading comprehension (which is a fair cop), or it's happening somewhere in the :gen_tcp module. I'm happy to keep staring at this in my free time, but pointers would be appreciated if available.

@pguillory
Copy link
Collaborator

The framing is done by setting the {:packet, 4} option on the socket in Thrift.Binary.Framed.ProtocolHandler. It's built in to gen_tcp and conveniently does the right thing for this purpose. See the docs here.

However the tricky part for implemented unframed transport will probably involve how the generated deserialization code assumes the entire message is available, which was both for simplicity and performance. When it fails, it just returns :error. It would need to be modified to instead return some kind of continuation that could be used to resume deserialization when more data arrives.

@aawilson
Copy link
Author

Thanks for the pointer, I was suspicious about how convenient that 4 was in the options there but couldn't find the documentation in gen_tcp that pulled it together, so I appreciate it. I've got some good models for how other projects handle this lying around so hopefully I can take a stab at something passable.

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

2 participants