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

Introduce http and tcp server channels #31446

Merged
merged 11 commits into from
Jun 20, 2018

Conversation

Tim-Brooks
Copy link
Contributor

Historically in TcpTransport server channels were represented by the
same channel interface as socket channels. This was necessary as
TcpTransport was parameterized by the channel type. This commit
introduces TcpServerChannel and HttpServerChannel classes. Additionally,
it adds the implementations for the various transports. This allows
server channels to have unique functionality and not implement the
methods they do not support (such as send and getRemoteAddress).

Additionally, with the introduction of HttpServerChannel this commit
extracts some of the storing and closing channel work to the abstract
http server transport.

@Tim-Brooks Tim-Brooks added >non-issue review :Distributed/Network Http and internode communication implementations v7.0.0 labels Jun 19, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Contributor

@s1monw s1monw left a comment

Choose a reason for hiding this comment

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

I like this distinction it was always odd to have no distinction between client and server channels

@Override
protected void onException(HttpChannel channel, Exception cause) {
if (cause instanceof ReadTimeoutException) {
if (logger.isTraceEnabled()) {
logger.trace("Http read timeout {}", channel);
}
CloseableChannel.closeChannel(channel);;
CloseableChannel.closeChannel(channel);
;
Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary line?

@@ -413,4 +344,18 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E
}
}

@ChannelHandler.Sharable
private class ServerChannelExceptionHandler extends ChannelHandlerAdapter {
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be static?

@@ -999,7 +999,7 @@ protected final void doStop() {
}
}

protected void onException(TcpChannel channel, Exception e) {
public void onException(TcpChannel channel, Exception e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm why is this public?

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 is called directly by Netty4MessageChannelHandler now, instead of having a separate exception handling method in Netty4Transport.

@Tim-Brooks Tim-Brooks merged commit 9ab1325 into elastic:master Jun 20, 2018
dnhatn added a commit that referenced this pull request Jun 23, 2018
* master:
  Add get field mappings to High Level REST API Client (#31423)
  [DOCS] Updates Watcher examples for code testing (#31152)
  TEST: Add bwc recovery tests with synced-flush index
  [DOCS] Move sql to docs (#31474)
  [DOCS] Move monitoring to docs folder (#31477)
  Core: Combine doExecute methods in TransportAction (#31517)
  IndexShard should not return null stats (#31528)
  fix repository update with the same settings but different type (#31458)
  Fix Mockito trying to mock IOException that isn't thrown by method (#31433) (#31527)
  Node selector per client rather than per request (#31471)
  Core: Combine messageRecieved methods in TransportRequestHandler (#31519)
  Upgrade to Lucene 7.4.0. (#31529)
  [ML] Add ML filter update API (#31437)
  Allow multiple unicast host providers (#31509)
  Avoid deprecation warning when running the ML datafeed extractor. (#31463)
  REST high-level client: add simulate pipeline API (#31158)
  Get Mapping API to honour allow_no_indices and ignore_unavailable (#31507)
  [PkiRealm] Invalidate cache on role mappings change (#31510)
  [Security] Check auth scheme case insensitively (#31490)
  In NumberFieldType equals and hashCode, make sure that NumberType is taken into account. (#31514)
  [DOCS] Fix REST tests in SQL docs
  [DOCS] Add code snippet testing in more ML APIs (#31339)
  Core: Remove ThreadPool from base TransportAction (#31492)
  [DOCS] Remove fixed file from build.gradle
  Rename createNewTranslog to fileBasedRecovery (#31508)
  Test: Skip assertion on windows
  [DOCS] Creates field and document level security overview (#30937)
  [DOCS] Significantly improve SQL docs
  [DOCS] Move migration APIs to docs (#31473)
  Core: Convert TransportAction.execute uses to client calls (#31487)
  Return transport addresses from UnicastHostsProvider (#31426)
  Ensure local addresses aren't null (#31440)
  Remove unused generic type for client execute method (#31444)
  Introduce http and tcp server channels (#31446)
@Tim-Brooks Tim-Brooks deleted the add_server_channels branch December 18, 2019 14:44
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 >non-issue v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants