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

Simplify TcpTransport interface by reducing send code to a single send method #19223

Merged
merged 13 commits into from
Jul 5, 2016

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Jul 1, 2016

Due to some optimization on the netty layer we had quite some code / cruft
added to the TcpTransport to allow for those optimizations. After cleaning
up BytesReference we can now move this optimization into TcpTransport and
have a simple send method on the implementation layer instead. This commit
adds a CompositeBytesReference that also allows message headers to be written
separately which simplify the header code as well since no skips are needed
anymore.

@s1monw s1monw added >enhancement review :Distributed/Network Http and internode communication implementations v5.0.0-alpha5 labels Jul 1, 2016
@s1monw
Copy link
Contributor Author

s1monw commented Jul 1, 2016

@jasontedor can you take a look

offset += reference.length();
ramBytesUsed += reference.ramBytesUsed();
}
this.ramBytesUsed = ramBytesUsed + Integer.BYTES * offsets.length;
Copy link
Member

@jasontedor jasontedor Jul 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plus the object references in BytesReference[], plus the length field, plus the ramBytesUsed field?

@jasontedor
Copy link
Member

It looks good @s1monw; I left some minor comments.

@s1monw
Copy link
Contributor Author

s1monw commented Jul 4, 2016

@jasontedor I pushed updates

@@ -62,7 +62,6 @@

@ClusterScope(scope = Scope.TEST, numDataNodes = 0)
@ESIntegTestCase.SuppressLocalMode
@TestLogging("_root:DEBUG,cluster.service:TRACE,discovery.zen:TRACE")
Copy link
Member

@jasontedor jasontedor Jul 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why this is being removed; can we keep it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's pretty usless if you have that on a test forever? can we add it back when we need it?

@jasontedor
Copy link
Member

It still looks good, I left a couple more minor comments.

s1monw added 13 commits July 5, 2016 08:26
…d method

Due to some optimization on the netty layer we hade quite some code / cruft
added to the TcpTransport to allow for those optimizations. After cleaning
up BytesReference we can now move this optimization into TcpTransport and
have a simple send method on the implemenation layer instead. This commit
adds a CompositeBytesReference that also allows message headers to be written
seperately which simplify the header code as well since no skips are needed
anymore.
@s1monw s1monw merged commit 44ccf67 into elastic:master Jul 5, 2016
@s1monw s1monw deleted the unoptimize_netty branch July 5, 2016 06:33
jasontedor added a commit that referenced this pull request Jul 6, 2016
* master: (192 commits)
  [TEST] Fix rare OBOE in AbstractBytesReferenceTestCase
  Reindex from remote
  Rename writeThrowable to writeException
  Start transport client round-robin randomly
  Reword Refresh API reference (#19270)
  Update fielddata.asciidoc
  Fix stored_fields message
  Add missing footer notes in mapper size docs
  Remote BucketStreams
  Add doc values support to the _size field in the mapper-size plugin
  Bump version to 5.0.0-alpha5.
  Update refresh.asciidoc
  Update shrink-index.asciidoc
  Change Debian repository for Vagrant debian-8 box
  [TEST] fix test to account for internal empyt reference optimization
  Upgrade to netty 3.10.6.Final (#19235)
  [TEST] fix histogram test when extended bounds overlaps data
  Remove redundant modifier
  Simplify TcpTransport interface by reducing send code to a single send method (#19223)
  Fix style violation in InstallPluginCommand.java
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Network Http and internode communication implementations >enhancement v5.0.0-alpha5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants