Skip to content

Remove allocation id from replica replication response #25488

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
Merged
merged 1 commit into from
Jul 1, 2017
Merged

Remove allocation id from replica replication response #25488

merged 1 commit into from
Jul 1, 2017

Conversation

@ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Jun 30, 2017 •
edited
Loading

The replica replication response object has an extra allocationId field that contains the allocation id of the replica on which the request was executed. As we are sending the allocation id with the actual replica replication request, and check when executing the replica replication action AsyncReplicaAction.doRun() (see

elasticsearch/core/src/main/java/org/elasticsearch/action/support/replication/TransportReplicationAction.java

Lines 593 to 597 in 8ae61c0

final String actualAllocationId = this.replica.routingEntry().allocationId().getId();
if (actualAllocationId.equals(targetAllocationID) == false) {
throw new ShardNotFoundException(this.replica.shardId(), "expected aID [{}] but found [{}]", targetAllocationID,
actualAllocationId);
}
) that the allocation of the replica shard is what we expect, there is no need to communicate back the allocation id as part of the response object.

@ywelsch ywelsch requested a review from jasontedor June 30, 2017 09:18
jasontedor
jasontedor approved these changes Jun 30, 2017
View reviewed changes
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.

@ywelsch ywelsch merged commit bb23d3b into elastic:master Jul 1, 2017
@ywelsch
Copy link
Contributor Author

ywelsch commented Jul 1, 2017

Thanks @jasontedor

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jul 4, 2017
@jasontedor
Merge branch 'master' into local-checkpoint-throwback
813032a 
* master: (52 commits)
  Include shared/attributes.asciidoc from docs master
  Fixed page breaks for ICU Collation Keyword Fields
  Remove QueryParseContext (elastic#25486)
  [Test] Use a common testing class for all XContent filtering tests (elastic#25491)
  Tests fix - Significant terms/text aggs  (elastic#25499)
  [DOCS] add docs for REST high level client index method (elastic#25501)
  Tests: Add Debian 9 (Stretch) to the packaging tests
  test: Run flush before upgrade and refresh after upgrade.
  Fix third party audit for repository-hdfs
  [TEST] Expect nodes getting disconnected quickly
  testPrimaryFailureIncreasesTerm should use assertBusy to wait for yellow
  Cleanup network / transport related settings (elastic#25489)
  Fix repository-hdfs plugin packaging test
  Remove allocation id from replica replication response (elastic#25488)
  Adjust BWC version on bad allocation request test
  Upgrading HDFS Repository Plugin to use HDFS 2.8.1 Client (elastic#25497)
  Adjust status on bad allocation explain requests
  Preliminary support for ARM
  Add doc note regarding explicit publish host
  Fix typo in name of test
  ...
@clintongormley clintongormley added :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. and removed :Sequence IDs labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasontedor jasontedor jasontedor approved these changes

Assignees

No one assigned

Labels

:Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >non-issue v6.0.0-beta1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ywelsch @jasontedor @clintongormley @colings86