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.

Sign up for GitHub

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

Jump to bottom

Introduced class reuses follow parameter code between ShardFollowTasks #38910

Merged
merged 4 commits into from
Feb 15, 2019
Merged

Introduced class reuses follow parameter code between ShardFollowTasks #38910

merged 4 commits into from
Feb 15, 2019

Conversation

martijnvg
Copy link
Member

and AutoFollowPattern classes.

The ImmutableFollowParameters (maybe there is a better name?) is like the already existing FollowParameters, but all of its fields are final.

This reduces the places where to change code in case a follow config changes from 3 to 2.
(ImmutableFollowParameters and FollowParameters classes)

@martijnvg
Introduced class reuses follow parameter code between ShardFollowTasks
a5c256b 
and AutoFollowPattern classes.

The ImmutableFollowParameters is like the already existing FollowParameters,
but all of its fields are final.
@martijnvg martijnvg added >non-issue v7.0.0 :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features v6.7.0 v8.0.0 v7.2.0 labels Feb 14, 2019
@martijnvg martijnvg requested a review from dnhatn February 14, 2019 16:44
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

dnhatn
dnhatn approved these changes Feb 14, 2019
View reviewed changes
Copy link
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

The ImmutableFollowParameters (maybe there is a better name?)

Let's go with this for now :). LGTM.

@martijnvg martijnvg merged commit c02aa40 into elastic:master Feb 15, 2019
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 15, 2019
@jasontedor
Merge remote-tracking branch 'elastic/master' into retention-lease-ccr
fa7b4df 
* elastic/master:
  Avoid double term construction in DfsPhase (elastic#38716)
  Fix typo in DateRange docs (yyy → yyyy) (elastic#38883)
  Introduced class reuses follow parameter code between ShardFollowTasks (elastic#38910)
  Ensure random timestamps are within search boundary (elastic#38753)
  [CI] Muting  method testFollowIndex in IndexFollowingIT
  Update Lucene snapshot repo for 7.0.0-beta1 (elastic#38946)
  SQL: Doc on syntax (identifiers in particular) (elastic#38662)
  Upgrade to Gradle 5.2.1 (elastic#38880)
  Tie break search shard iterator comparisons on cluster alias (elastic#38853)
  Also mmap cfs files for hybridfs (elastic#38940)
  Build: Fix issue with test status logging (elastic#38799)
  Adapt FullClusterRestartIT on master (elastic#38856)
  Fix testAutoFollowing test to use createLeaderIndex() helper method.
  Migrate muted auto follow rolling upgrade test and unmute this test (elastic#38900)
  ShardBulkAction ignore primary response on primary (elastic#38901)
  Recover peers from translog, ignoring soft deletes (elastic#38904)
  Fix NPE on Stale Index in IndicesService (elastic#38891)
  Smarter CCR concurrent file chunk fetching (elastic#38841)
  Fix intermittent failure in ApiKeyIntegTests (elastic#38627)
  re-enable SmokeTestWatcherWithSecurityIT (elastic#38814)
martijnvg added a commit that referenced this pull request Feb 15, 2019
@martijnvg
Fix serialization bug in ShardFollowTask after cutting this class ove…
9339fc8 
…r to

extend from ImmutableFollowParameters.

Prior to #38910 readVInt and writeVInt was used, #38910 accidentally
changed ShardFollowTask to use readOptionalVInt / writeVOptionalVInt.
This changes changes ShardFollowTask back to use vint.
martijnvg added a commit that referenced this pull request Feb 15, 2019
@martijnvg
Introduced class reuses follow parameter code between ShardFollowTasks (
03b67b3 
#38910)

and AutoFollowPattern classes.

The ImmutableFollowParameters is like the already existing FollowParameters,
but all of its fields are final.
martijnvg added a commit that referenced this pull request Feb 15, 2019
@martijnvg
Introduced class reuses follow parameter code between ShardFollowTasks (
9269d0c 
#38910)

and AutoFollowPattern classes.

The ImmutableFollowParameters is like the already existing FollowParameters,
but all of its fields are final.
martijnvg added a commit that referenced this pull request Feb 15, 2019
@martijnvg
Introduced class reuses follow parameter code between ShardFollowTasks (
f3fd81c 
#38910)

and AutoFollowPattern classes.

The ImmutableFollowParameters is like the already existing FollowParameters,
but all of its fields are final.
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Feb 19, 2019
@martijnvg
Change ShardFollowTask to reuse common serialization logic
b518bb4 
Initially in elastic#38910, ShardFollowTask was reusing ImmutableFollowParameters'
serialization logic. After merging, bwc tests failed sometimes and
the binary serialization that ShardFollowTask was originally was using
was added back. ImmutableFollowParameters is using optional fields (optional vint)
while ShardFollowTask was not (vint).

This PR is against 6.7 branch. Prior to merging this pr, bwc tests in 7.0,
7.x and master branch will need to be disabled.

This change then will need to be forwardported to 7.0, 7.x and master
branches (but without the version check and pre 6.7 serialization).
@martijnvg martijnvg mentioned this pull request Feb 19, 2019
Merged
martijnvg added a commit that referenced this pull request Feb 21, 2019
@martijnvg
Change ShardFollowTask to reuse common serialization logic (#39094)
2cbccc7 
Initially in #38910, ShardFollowTask was reusing ImmutableFollowParameters'
serialization logic. After merging, bwc tests failed sometimes and
the binary serialization that ShardFollowTask was originally was using
was added back. ImmutableFollowParameters is using optional fields (optional vint)
while ShardFollowTask was not (vint).

This PR is against 6.7 branch. Prior to merging this pr, bwc tests in 7.0,
7.x and master branch will need to be disabled.

This change then will need to be forwardported to 7.0, 7.x and master
branches (but without the version check and pre 6.7 serialization).
martijnvg added a commit that referenced this pull request Feb 21, 2019
@martijnvg
Change ShardFollowTask to reuse common serialization logic (#39094)
d5de7c4 
Initially in #38910, ShardFollowTask was reusing ImmutableFollowParameters'
serialization logic. After merging, bwc tests failed sometimes and
the binary serialization that ShardFollowTask was originally was using
was added back. ImmutableFollowParameters is using optional fields (optional vint)
while ShardFollowTask was not (vint).

This PR is against 6.7 branch. Prior to merging this pr, bwc tests in 7.0,
7.x and master branch will need to be disabled.

This change then will need to be forwardported to 7.0, 7.x and master
branches (but without the version check and pre 6.7 serialization).
martijnvg added a commit that referenced this pull request Feb 21, 2019
@martijnvg
Change ShardFollowTask to reuse common serialization logic (#39094)
f40139c 
Initially in #38910, ShardFollowTask was reusing ImmutableFollowParameters'
serialization logic. After merging, bwc tests failed sometimes and
the binary serialization that ShardFollowTask was originally was using
was added back. ImmutableFollowParameters is using optional fields (optional vint)
while ShardFollowTask was not (vint).
martijnvg added a commit that referenced this pull request Feb 21, 2019
@martijnvg
Change ShardFollowTask to reuse common serialization logic (#39094)
b69df5d 
Initially in #38910, ShardFollowTask was reusing ImmutableFollowParameters'
serialization logic. After merging, bwc tests failed sometimes and
the binary serialization that ShardFollowTask was originally was using
was added back. ImmutableFollowParameters is using optional fields (optional vint)
while ShardFollowTask was not (vint).
weizijun pushed a commit to weizijun/elasticsearch that referenced this pull request Feb 22, 2019
@martijnvg @weizijun
Change ShardFollowTask to reuse common serialization logic (elastic#3…
3c7f6e7 
…9094)

Initially in elastic#38910, ShardFollowTask was reusing ImmutableFollowParameters'
serialization logic. After merging, bwc tests failed sometimes and
the binary serialization that ShardFollowTask was originally was using
was added back. ImmutableFollowParameters is using optional fields (optional vint)
while ShardFollowTask was not (vint).
weizijun pushed a commit to weizijun/elasticsearch that referenced this pull request Feb 22, 2019
@martijnvg @weizijun
Change ShardFollowTask to reuse common serialization logic (elastic#3…
a50a55d 
…9094)

Initially in elastic#38910, ShardFollowTask was reusing ImmutableFollowParameters'
serialization logic. After merging, bwc tests failed sometimes and
the binary serialization that ShardFollowTask was originally was using
was added back. ImmutableFollowParameters is using optional fields (optional vint)
while ShardFollowTask was not (vint).
@jakelandis jakelandis removed the v7.0.0 label Apr 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dnhatn dnhatn dnhatn approved these changes

Assignees
No one assigned
Labels
:Distributed Indexing/CCR Issues around the Cross Cluster State Replication features >non-issue v6.7.0 v7.0.0-rc2 v7.2.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@martijnvg @elasticmachine @dnhatn @jakelandis