Skip to content

Conversation

@bleskes
Copy link
Contributor

@bleskes bleskes commented Jun 20, 2017

MockTransportServices allows us to simulate network disruptions in our testing infra. Sadly it wasn't updated to the state of the art in Transport land. This PR brings it up to speed. Specifically:

  1. Opening a connection is now also blocked (before only node connections were blocked)
  2. Simplifies things using the latest connection based notification between TcpTransport and TransportService for when a disconnect happens.
  3. By 2, it fixes a race condition where we may fail to respond to a sent request when it is sent concurrently with the closing of a connection. The old code relied on a node based bridge between tcp transport and transport service. Sadly, the following doesn't work any more:
   if (transport.nodeConnected(node)) {
            // this a connected node, disconnecting from it will be up the exception
            transport.disconnectFromNode(node); <-- this may now be a noop and it doesn't mean that the transport service was notified of the disconnect between the nodeConnected check and here.
   } else {
            throw new ConnectTransportException(node, reason, e);
   }

Closes #25338

@bleskes bleskes added >bug >test Issues or PRs that are addressing/adding tests v5.5.0 v5.6.0 v6.0.0 labels Jun 20, 2017
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.

LGTM

@bleskes bleskes merged commit 7013cbd into elastic:master Jun 21, 2017
@bleskes bleskes deleted the mock_transport_disconnect_simulation branch June 21, 2017 08:28
bleskes added a commit that referenced this pull request Jun 21, 2017
MockTransportServices allows us to simulate network disruptions in our testing infra. Sadly it wasn't updated to the state of the art in Transport land. This PR brings it up to speed. Specifically:

1) Opening a connection is now also blocked (before only node connections were blocked)
2) Simplifies things using the latest connection based notification between TcpTransport and TransportService for when a disconnect happens.
3) By 2, it fixes a race condition where we may fail to respond to a sent request when it is sent concurrently with the closing of a connection. The old code relied on a node based bridge between tcp transport and transport service. Sadly, the following doesn't work any more:

```
   if (transport.nodeConnected(node)) {
            // this a connected node, disconnecting from it will be up the exception
            transport.disconnectFromNode(node); <-- this may now be a noop and it doesn't mean that the transport service was notified of the disconnect between the nodeConnected check and here.
   } else {
            throw new ConnectTransportException(node, reason, e);
   }
```
bleskes added a commit that referenced this pull request Jun 21, 2017
MockTransportServices allows us to simulate network disruptions in our testing infra. Sadly it wasn't updated to the state of the art in Transport land. This PR brings it up to speed. Specifically:

1) Opening a connection is now also blocked (before only node connections were blocked)
2) Simplifies things using the latest connection based notification between TcpTransport and TransportService for when a disconnect happens.
3) By 2, it fixes a race condition where we may fail to respond to a sent request when it is sent concurrently with the closing of a connection. The old code relied on a node based bridge between tcp transport and transport service. Sadly, the following doesn't work any more:

```
   if (transport.nodeConnected(node)) {
            // this a connected node, disconnecting from it will be up the exception
            transport.disconnectFromNode(node); <-- this may now be a noop and it doesn't mean that the transport service was notified of the disconnect between the nodeConnected check and here.
   } else {
            throw new ConnectTransportException(node, reason, e);
   }
```
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jun 22, 2017
* master: (56 commits)
  Initialize max unsafe auto ID timestamp on shrink
  Enable a long translog retention policy by default (elastic#25294)
  Remove `index.mapping.single_type=false` from core/tests (elastic#25331)
  test: single type defaults to true since alpha1 and not alpha3
  Get short path name for native controllers
  Live primary-replica resync (no rollback) (elastic#24841)
  Upgrade to lucene-7.0.0-snapshot-ad2cb77. (elastic#25349)
  percolator: Deprecate `document_type` parameter.
  [DOCS] Fixed typo.
  [rest-api-spec/indices.refresh] Remove old params
  Remove redundant and broken MD5 checksum from repository-s3 (elastic#25270)
  Initialize sequence numbers on a shrunken index
  Port most snapshot/restore static bwc tests to qa:full-cluster-restart (elastic#25296)
  Javadoc: ThreadPool doesn't reject while shutdown (elastic#23678)
  test: verify `size_to_upgrade_in_bytes` in assertBusy(...)
  Docs: Removed duplicated line in mapping docs
  Add backward compatibility indices for 5.4.2
  Update MockTransportService to the age of Transport.Connection (elastic#25320)
  Add version v5.4.2 after release
  IndexMetaData: Add internal format index setting (elastic#25292)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>test Issues or PRs that are addressing/adding tests v5.5.0 v5.6.0 v6.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] DiscoveryDisruptionIT testClusterFormingWithASlowNode fails

3 participants