Skip to content

Support for TCompactProtocol in Thrift protocol #573

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.

Sign up for GitHub

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

Jump to bottom
Closed
maximpashuk opened this issue Apr 9, 2014 · 2 comments
Closed

Support for TCompactProtocol in Thrift protocol #573

maximpashuk opened this issue Apr 9, 2014 · 2 comments

Comments

@maximpashuk
Copy link

As you can see in sources of thrift plugin for elasticsearch:

https://github.com/elasticsearch/elasticsearch-transport-thrift/blob/master/src/main/java/org/elasticsearch/thrift/ThriftServer.java#L82-L86

Supported protocols are TBinaryProtocol and TCompactProtocol (binary y default).

As I can see in sources on ThriftConnetion

https://github.com/elasticsearch/elasticsearch-net/blob/d5a743d4f3b6863a20e0c4b3e95342b8a6025e60/src/Connections/Elasticsearch.Net.Connection.Thrift/ThriftConnection.cs#L41

Only TBinaryProtocol is supported.
Please add support of TCompactProtocol, it is more effective that TBinaryProtocol.
@Mpdreamz Mpdreamz added this to the NEST 1.1 milestone Jun 16, 2014
@gmarz gmarz removed this from the NEST 1.1 milestone Sep 10, 2014
@maximpashuk maximpashuk removed this from the NEST 1.1 milestone Sep 10, 2014
@robertlyson
Copy link
Contributor

I did some tests, and it looks like TCompactProtocol is more effective for bigger messages.

I'll create PR for this.

@Mpdreamz
Copy link
Member

this has been implemented by @robertlyson as per #1157

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Mpdreamz @gmarz @robertlyson @maximpashuk