Skip to content

Conversation

@Tim-Brooks
Copy link
Contributor

We have various assertions that check we never block on transport
threads. This commit adds the thread names for the NioTransport to
these assertions.

With this change I had to fix two places where we were calling blocking
methods from the transport threads.

We have various assertions that check we never block on transport
threads. This commit adds the thread names for the nio transport to
these assertions.

With this change I had to fix two places where we were calling blocking
methods from the transport threads.
@Tim-Brooks Tim-Brooks added :Distributed Coordination/Network Http and internode communication implementations review >test Issues or PRs that are addressing/adding tests v6.0.0 labels Jun 29, 2017
@Tim-Brooks Tim-Brooks requested review from jasontedor and s1monw June 29, 2017 16:30
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.

Left one comment LGTM otherwise

public static final String TEST_MOCK_TRANSPORT_THREAD_PREFIX = "__mock_network_thread";

public static final String NIO_TRANSPORT_WORKER_THREAD_NAME_PREFIX = "nio_transport_worker";
public static final String NIO_TRANSPORT_ACCEPTOR_THREAD_NAME_PREFIX = "nio_transport_acceptor";
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we prefix them with es?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Prefix the constant or the thread name? Or both?

@Tim-Brooks Tim-Brooks merged commit cac2eec into elastic:master Jun 29, 2017
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM too.

jasontedor added a commit that referenced this pull request Jun 30, 2017
* master: (129 commits)
  Add doc note regarding explicit publish host
  Fix typo in name of test
  Add additional test for sequence-number recovery
  WrapperQueryBuilder should also rewrite the parsed query.
  Remove dead code and stale Javadoc
  Update defaults in documentation (#25483)
  [DOCS] Add docs-dir to Painless (#25482)
  Add concurrent deprecation logger test
  [DOCS] Update shared attributes for Elasticsearch (#25479)
  Use LRU set to reduce repeat deprecation messages
  Add NioTransport threads to thread name checks (#25477)
  Add shortcut for AbstractQueryBuilder.parseInnerQueryBuilder to QueryShardContext
  Prevent channel enqueue after selector close (#25478)
  Fix Java 9 compilation issue
  Remove unregistered `transport.netty.*` settings (#25476)
  Handle ping correctly in NioTransport (#25462)
  Tests: Remove platform specific assertion in NioSocketChannelTests
  Remove QueryParseContext from parsing QueryBuilders (#25448)
  Promote replica on the highest version node (#25277)
  test: added not null assertion
  ...
@Tim-Brooks Tim-Brooks deleted the add_nio_transport_thread_names_to_assertions branch November 14, 2018 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Network Http and internode communication implementations >test Issues or PRs that are addressing/adding tests v6.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants