Update index mappings when ccr restore complete #36879

Tim-Brooks · 2018-12-20T04:27:23Z

This is related to #35975. When the shard restore process is complete,
the index mappings need to be updated to ensure that the data in the
files restores is compatible with the follower mappings. This commit
implements a mapping update as the final step in a shard restore.

elasticmachine · 2018-12-20T04:27:25Z

Pinging @elastic/es-distributed

Tim-Brooks · 2018-12-20T04:28:16Z

This is an attempt to continue to split the full file restore into smaller pieces. Especially, as we continue to work out how to handle the remote requests to specific nodes.

ywelsch

@martijnvg can you have a look as well please?

ywelsch · 2018-12-20T11:06:48Z

x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java

    }

    @Override
    public IndexShardSnapshotStatus getShardSnapshotStatus(SnapshotId snapshotId, Version version, IndexId indexId, ShardId leaderShardId) {
        throw new UnsupportedOperationException("Unsupported for repository of type: " + TYPE);
    }

+    private void maybeUpdateMappings(Client localClient, Client remoteClient, Index leaderIndex, IndexSettings followerIndexSettings) {


@martijnvg would it make sense to share this code with the one in ShardFollowTasksExecutor?

Conceptually this code is doing the same as the update mapping code in shard follow task, but the update mapping code is tightly coupled with ShardFollowTasksExecutor and ShardFollowNodeTask. Also the mapping update code is asynchronous over there and here it is synchronous.

What I think is possible, is that factory methods for both ClusterStateRequest and PutMappingRequest are introduced that is then used here and in ShardFollowTasksExecutor.

Tim-Brooks · 2018-12-20T17:16:46Z

@martijnvg @ywelsch I made some changes to share the request creation.

x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/CcrRequests.java

x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java

Tim-Brooks · 2018-12-20T18:27:57Z

@ywelsch Made your changes.

ywelsch

LGTM

This is related to elastic#35975. When the shard restore process is complete, the index mappings need to be updated to ensure that the data in the files restores is compatible with the follower mappings. This commit implements a mapping update as the final step in a shard restore.

This is related to #35975. When the shard restore process is complete, the index mappings need to be updated to ensure that the data in the files restores is compatible with the follower mappings. This commit implements a mapping update as the final step in a shard restore.

martijnvg · 2018-12-21T06:52:17Z

x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/CcrRequests.java

+import org.elasticsearch.cluster.metadata.MappingMetaData;
+import org.elasticsearch.common.xcontent.XContentType;
+
+public final class CcrRequests {


* elastic/master: (539 commits) SQL: documentation improvements and updates (elastic#36918) [DOCS] Merges list of discovery and cluster formation settings (elastic#36909) Only compress responses if request was compressed (elastic#36867) Remove duplicate paragraph (elastic#36942) Fix URI to cluster stats endpoint on specific nodes (elastic#36784) Fix typo in unitTest task (elastic#36930) RecoveryMonitor#lastSeenAccessTime should be volatile (elastic#36781) [CCR] Add `ccr.auto_follow_coordinator.wait_for_timeout` setting (elastic#36714) Scripting: Remove deprecated params.ctx (elastic#36848) Refactor the REST actions to clarify what endpoints are deprecated. (elastic#36869) Add JDK 12 to CI rotation (elastic#36915) Improve error message for 6.x style realm settings (elastic#36876) Send clear session as routable remote request (elastic#36805) [DOCS] Remove redundant ILM attributes (elastic#36808) SQL: Fix bug regarding histograms usage in scripting (elastic#36866) Update index mappings when ccr restore complete (elastic#36879) Docs: Bump version to alpha2 after release Enable IPv6 URIs in reindex from remote (elastic#36874) Watcher: Remove unused local variable in doExecute (elastic#36655) [DOCS] Synchs titles of X-Pack APIs ...

* master: (31 commits) Move ingest-geoip default databases out of config (elastic#36949) [ILM][DOCS] add extra scenario to policy update docs (elastic#36871) [Painless] Add String Casting Tests (elastic#36945) SQL: documentation improvements and updates (elastic#36918) [DOCS] Merges list of discovery and cluster formation settings (elastic#36909) Only compress responses if request was compressed (elastic#36867) Remove duplicate paragraph (elastic#36942) Fix URI to cluster stats endpoint on specific nodes (elastic#36784) Fix typo in unitTest task (elastic#36930) RecoveryMonitor#lastSeenAccessTime should be volatile (elastic#36781) [CCR] Add `ccr.auto_follow_coordinator.wait_for_timeout` setting (elastic#36714) Scripting: Remove deprecated params.ctx (elastic#36848) Refactor the REST actions to clarify what endpoints are deprecated. (elastic#36869) Add JDK 12 to CI rotation (elastic#36915) Improve error message for 6.x style realm settings (elastic#36876) Send clear session as routable remote request (elastic#36805) [DOCS] Remove redundant ILM attributes (elastic#36808) SQL: Fix bug regarding histograms usage in scripting (elastic#36866) Update index mappings when ccr restore complete (elastic#36879) Docs: Bump version to alpha2 after release ...

There are two issues regarding the way that we sync mapping from leader to follower when a ccr restore is completed: 1. The returned mapping from a cluster service might not be up to date as the mapping of the restored index commit. 2. We should not compare the mapping version of the follower and the leader. They are not related to one another. Moreover, I think we should only ensure that once the restore is done, the mapping on the follower should be at least the mapping of the copied index commit. We don't have to sync the mapping which is updated after we have opened a session. Relates #36879 Closes #37887

There are two issues regarding the way that we sync mapping from leader to follower when a ccr restore is completed: 1. The returned mapping from a cluster service might not be up to date as the mapping of the restored index commit. 2. We should not compare the mapping version of the follower and the leader. They are not related to one another. Moreover, I think we should only ensure that once the restore is done, the mapping on the follower should be at least the mapping of the copied index commit. We don't have to sync the mapping which is updated after we have opened a session. Relates elastic#36879 Closes elastic#37887

There are two issues regarding the way that we sync mapping from leader to follower when a ccr restore is completed: 1. The returned mapping from a cluster service might not be up to date as the mapping of the restored index commit. 2. We should not compare the mapping version of the follower and the leader. They are not related to one another. Moreover, I think we should only ensure that once the restore is done, the mapping on the follower should be at least the mapping of the copied index commit. We don't have to sync the mapping which is updated after we have opened a session. Relates #36879 Closes #37887 * Change

Tim-Brooks added 3 commits December 18, 2018 18:06

WIP

2d6a755

Merge remote-tracking branch 'upstream/master' into mappings_update

9cdf844

WIP

ad23578

Tim-Brooks added >non-issue v7.0.0 :Distributed/CCR Issues around the Cross Cluster State Replication features v6.7.0 labels Dec 20, 2018

Tim-Brooks requested a review from ywelsch December 20, 2018 04:27

ywelsch approved these changes Dec 20, 2018

View reviewed changes

Changes

e865fb2

ywelsch suggested changes Dec 20, 2018

View reviewed changes

x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/CcrRequests.java Outdated Show resolved Hide resolved

x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/repository/CcrRepository.java Outdated Show resolved Hide resolved

Changes

7f23deb

ywelsch approved these changes Dec 20, 2018

View reviewed changes

Merge remote-tracking branch 'upstream/master' into mappings_update

636b042

Tim-Brooks merged commit 4cd5705 into elastic:master Dec 20, 2018

Tim-Brooks added the backport pending label Dec 20, 2018

Tim-Brooks mentioned this pull request Dec 21, 2018

Implement CCR bootstrap from remote #35975

Closed

32 tasks

Tim-Brooks removed the backport pending label Dec 21, 2018

martijnvg reviewed Dec 21, 2018

View reviewed changes

dnhatn mentioned this pull request Jan 31, 2019

Tighten mapping syncing in ccr remote restore #38071

Merged

Tim-Brooks mentioned this pull request Feb 5, 2019

Tighten mapping syncing in ccr remote restore #38459

Merged

jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Tim-Brooks deleted the mappings_update branch April 30, 2020 18:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update index mappings when ccr restore complete #36879

Update index mappings when ccr restore complete #36879

Tim-Brooks commented Dec 20, 2018

elasticmachine commented Dec 20, 2018

Tim-Brooks commented Dec 20, 2018

ywelsch left a comment

ywelsch Dec 20, 2018

martijnvg Dec 20, 2018

Tim-Brooks commented Dec 20, 2018

Tim-Brooks commented Dec 20, 2018

ywelsch left a comment

martijnvg Dec 21, 2018

Update index mappings when ccr restore complete #36879

Update index mappings when ccr restore complete #36879

Conversation

Tim-Brooks commented Dec 20, 2018

elasticmachine commented Dec 20, 2018

Tim-Brooks commented Dec 20, 2018

ywelsch left a comment

Choose a reason for hiding this comment

ywelsch Dec 20, 2018

Choose a reason for hiding this comment

martijnvg Dec 20, 2018

Choose a reason for hiding this comment

Tim-Brooks commented Dec 20, 2018

Tim-Brooks commented Dec 20, 2018

ywelsch left a comment

Choose a reason for hiding this comment

martijnvg Dec 21, 2018

Choose a reason for hiding this comment