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

Use the underlying connection version for CCS connections #28093

Merged
merged 4 commits into from
Jan 8, 2018

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Jan 5, 2018

Previously this would default to the version of the remote Node.
However, if the remote cluster was mixed-version (e.g. it was part way through a
rolling upgrade), then the TransportService may have negotiated a connection
version that is not identical to connected Node's version.

This mismatch would cause the Stream and the (Remote)Connection to report
different version numbers, which could cause data to be sent over the wire
using an incorrect serialization version.

Previously this would default to the version of the remote Node.
However, if the remote cluster was mixed-version (e.g. it was part way through a
rolling upgrade), then the TransportService may have negotiated a connection
version that is not identical to connected Node's version.

This mismatch would cause the Stream and the (Remote)Connection to report
different version numbers, which could cause data to be sent over the wire
using an incorrect serialization version.
@tvernum tvernum added :Search/Search Search-related issues that do not fall into other categories review v6.2.0 v7.0.0 labels Jan 5, 2018
@tvernum tvernum requested review from s1monw and jaymode January 5, 2018 05:03
@tvernum
Copy link
Contributor Author

tvernum commented Jan 5, 2018

Since I don't normally deal with MockTransportService, and no other tests seemed to call getConnection() I had to work out how to set everything up for this test.
If there's a simpler way, then I'd love to know about it.

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

@s1monw
Copy link
Contributor

s1monw commented Jan 5, 2018

@tvernum this looks like a bug should we backport to 5.6 as well?

Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

LGTM. We should also backport this to 5.6

@tvernum tvernum merged commit db18631 into elastic:master Jan 8, 2018
tvernum added a commit that referenced this pull request Jan 8, 2018
Previously this would default to the version of the remote Node.
However, if the remote cluster was mixed-version (e.g. it was part way through a
rolling upgrade), then the TransportService may have negotiated a connection
version that is not identical to connected Node's version.

This mismatch would cause the Stream and the (Remote)Connection to report
different version numbers, which could cause data to be sent over the wire
using an incorrect serialization version.
tvernum added a commit that referenced this pull request Jan 8, 2018
Previously this would default to the version of the remote Node.
However, if the remote cluster was mixed-version (e.g. it was part way through a
rolling upgrade), then the TransportService may have negotiated a connection
version that is not identical to connected Node's version.

This mismatch would cause the Stream and the (Remote)Connection to report
different version numbers, which could cause data to be sent over the wire
using an incorrect serialization version.
tvernum added a commit that referenced this pull request Jan 8, 2018
Previously this would default to the version of the remote Node.
However, if the remote cluster was mixed-version (e.g. it was part way through a
rolling upgrade), then the TransportService may have negotiated a connection
version that is not identical to connected Node's version.

This mismatch would cause the Stream and the (Remote)Connection to report
different version numbers, which could cause data to be sent over the wire
using an incorrect serialization version.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 9, 2018
* master:
  Use Gradle wrapper when building BWC
  Painless: Add a simple cache for whitelist methods and fields. (elastic#28142)
  Fix upgrading indices which use a custom similarity plugin. (elastic#26985)
  Fix Licenses values for CDDL and Custom URL (elastic#27999)
  Cleanup TcpChannelFactory and remove classes (elastic#28102)
  Fix expected plugins test for transport-nio
  [Docs] Fix Date Math example descriptions (elastic#28125)
  Fail rollover if duplicated alias found in template (elastic#28110)
  Avoid concurrent snapshot finalizations when deleting an INIT snapshot (elastic#28078)
  Deprecate `isShardsAcked()` in favour of `isShardsAcknowledged()` (elastic#27819)
  [TEST] Wait for replicas to be allocated before shrinking
  Use the underlying connection version for CCS connections  (elastic#28093)
  test: do not use asn fields
  Test: Add assumeFalse for test that cannot pass on windows
  Clarify reproduce info on Windows
  Remove out-of-date projectile file
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 9, 2018
* master: (27 commits)
  Declare empty package dirs as output dirs
  Consistent updates of IndexShardSnapshotStatus (elastic#28130)
  Fix Gradle wrapper usage on Windows when building BWC (elastic#28146)
  [Docs] Fix some typos in comments (elastic#28098)
  Use Gradle wrapper when building BWC
  Painless: Add a simple cache for whitelist methods and fields. (elastic#28142)
  Fix upgrading indices which use a custom similarity plugin. (elastic#26985)
  Fix Licenses values for CDDL and Custom URL (elastic#27999)
  Cleanup TcpChannelFactory and remove classes (elastic#28102)
  Fix expected plugins test for transport-nio
  [Docs] Fix Date Math example descriptions (elastic#28125)
  Fail rollover if duplicated alias found in template (elastic#28110)
  Avoid concurrent snapshot finalizations when deleting an INIT snapshot (elastic#28078)
  Deprecate `isShardsAcked()` in favour of `isShardsAcknowledged()` (elastic#27819)
  [TEST] Wait for replicas to be allocated before shrinking
  Use the underlying connection version for CCS connections  (elastic#28093)
  test: do not use asn fields
  Test: Add assumeFalse for test that cannot pass on windows
  Clarify reproduce info on Windows
  Remove out-of-date projectile file
  ...
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories v5.6.6 v6.1.2 v6.2.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants