Ensure changes requests return the latest mapping version #37633

dnhatn · 2019-01-19T23:06:40Z

Today we keep the mapping on the follower in sync with the leader's using the mapping version from changes requests. There are two rare cases where the mapping on the follower is not synced properly:

The returned mapping version (from ClusterService) is outdated than the actual mapping. This happens because we expose the latest cluster state in ClusterService after applying it to IndexService.
It's possible for the FollowTask to receive an outdated mapping than the min_required_mapping. In that case, it should fetch the mapping again; otherwise, the follower won't have the right mapping.

Relates #31140 (comment)

elasticmachine · 2019-01-19T23:06:42Z

Pinging @elastic/es-distributed

martijnvg

👍 lgtm, good stuff.

x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/FollowerFailOverIT.java

martijnvg · 2019-01-21T08:07:56Z

...ck/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java

                Index leaderIndex = params.getLeaderShardId().getIndex();
                Index followIndex = params.getFollowShardId().getIndex();

                ClusterStateRequest clusterStateRequest = CcrRequests.metaDataRequest(leaderIndex.getName());
                CheckedConsumer<ClusterStateResponse, Exception> onResponse = clusterStateResponse -> {
                    IndexMetaData indexMetaData = clusterStateResponse.getState().metaData().getIndexSafe(leaderIndex);
+                    // the returned mapping is outdated - retry again
+                    if (indexMetaData.getMappingVersion() < minRequiredMappingVersion) {


if we had the metadata version (which is also updated whenever index metadata / mapping changes), we could just do a waitForMetaDataVersion? This would avoid a possible busyloop.

I will remove this retry in 7.0 after backporting to 6.x

martijnvg · 2019-01-21T08:12:27Z

Maybe also backport this to the 6.6 branch? As this fixes replication issues that also exist in that branch.

ywelsch

Thanks for this PR @dnhatn. I've suggested a small change (which might have BWC implications though) that will avoid a busy loop.

ywelsch · 2019-01-21T09:24:40Z

...ck/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java

                Index leaderIndex = params.getLeaderShardId().getIndex();
                Index followIndex = params.getFollowShardId().getIndex();

                ClusterStateRequest clusterStateRequest = CcrRequests.metaDataRequest(leaderIndex.getName());
                CheckedConsumer<ClusterStateResponse, Exception> onResponse = clusterStateResponse -> {
                    IndexMetaData indexMetaData = clusterStateResponse.getState().metaData().getIndexSafe(leaderIndex);
+                    // the returned mapping is outdated - retry again
+                    if (indexMetaData.getMappingVersion() < minRequiredMappingVersion) {


if we had the metadata version (which is also updated whenever index metadata / mapping changes), we could just do a waitForMetaDataVersion? This would avoid a possible busyloop.

dnhatn · 2019-01-21T17:53:11Z

@martijnvg @ywelsch Thanks for looking. I have updated to use waitForMetaDataVersion. Would you please have another look?

server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java

...ck/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java

This reverts commit 4c26292.

dnhatn · 2019-01-21T22:02:44Z

@ywelsch I've addressed your comment. Can you please give this a go? Thank you!

ywelsch

LGTM

ywelsch · 2019-01-22T09:57:45Z

...ck/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java

+            remoteClient(params).admin().cluster().state(clusterStateRequest, ActionListener.wrap(
+                r -> {
+                    // if wait_for_metadata_version timeout, the response is empty
+                    if (r.getState() == null) {


what odd behavior

If the indices of a ClusterStateRequest are specified, we fail to include the cluster state metadata version in the response. Relates #37633

Today we keep the mapping on the follower in sync with the leader's using the mapping version from changes requests. There are two rare cases where the mapping on the follower is not synced properly: 1. The returned mapping version (from ClusterService) is outdated than the actual mapping. This happens because we expose the latest cluster state in ClusterService after applying it to IndexService. 2. It's possible for the FollowTask to receive an outdated mapping than the min_required_mapping. In that case, it should fetch the mapping again; otherwise, the follower won't have the right mapping. Relates to #31140

…ead-de-duplication * elastic/master: Use explicit version for build-tools in example plugin integ tests (elastic#37792) Change `rational` to `saturation` in script_score (elastic#37766) Deprecate types in get field mapping API (elastic#37667) Add ability to listen to group of affix settings (elastic#37679) Ensure changes requests return the latest mapping version (elastic#37633) Make Minio Setup more Reliable (elastic#37747)

* elastic/master: (85 commits) Use explicit version for build-tools in example plugin integ tests (elastic#37792) Change `rational` to `saturation` in script_score (elastic#37766) Deprecate types in get field mapping API (elastic#37667) Add ability to listen to group of affix settings (elastic#37679) Ensure changes requests return the latest mapping version (elastic#37633) Make Minio Setup more Reliable (elastic#37747) Liberalize StreamOutput#writeStringList (elastic#37768) Add PersistentTasksClusterService::unassignPersistentTask method (elastic#37576) Tests: disable testRandomGeoCollectionQuery on tiny polygons (elastic#37579) Use ILM for Watcher history deletion (elastic#37443) Make sure PutMappingRequest accepts content types other than JSON. (elastic#37720) Retry ILM steps that fail due to SnapshotInProgressException (elastic#37624) Use disassociate in preference to deassociate (elastic#37704) Delete Redundant RoutingServiceTests (elastic#37750) Always return metadata version if metadata is requested (elastic#37674) [TEST] Mute MlMappingsUpgradeIT testMappingsUpgrade Streamline skip_unavailable handling (elastic#37672) Only bootstrap and elect node in current voting configuration (elastic#37712) Ensure either success or failure path for SearchOperationListener is called (elastic#37467) Target only specific index in update settings test ...

If the indices of a ClusterStateRequest are specified, we fail to include the cluster state metadata version in the response. Relates #37633

Ensure changes requests return latest mapping version

ba0df59

dnhatn added >bug v7.0.0 :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features v6.7.0 labels Jan 19, 2019

dnhatn requested review from martijnvg, ywelsch and jasontedor January 19, 2019 23:06

martijnvg approved these changes Jan 21, 2019

View reviewed changes

ywelsch suggested changes Jan 21, 2019

View reviewed changes

dnhatn added 5 commits January 21, 2019 11:56

return metadata_version

4c26292

wait for metadata version

7565875

comment

64aa5a3

Merge branch 'master' into mapping-version

e19d242

increase timeout in test

9c4a595

dnhatn requested a review from ywelsch January 21, 2019 17:53

dnhatn added the v6.6.1 label Jan 21, 2019

ywelsch suggested changes Jan 21, 2019

View reviewed changes

server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java Outdated Show resolved Hide resolved

...ck/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java Outdated Show resolved Hide resolved

dnhatn added 2 commits January 21, 2019 13:40

Revert "return metadata_version"

14c0642

This reverts commit 4c26292.

retry with the higher metadata version

249a612

dnhatn requested a review from ywelsch January 21, 2019 22:02

ywelsch approved these changes Jan 22, 2019

View reviewed changes

dnhatn mentioned this pull request Jan 22, 2019

Always return metadata version if metadata is requested #37674

Merged

Merge branch 'master' into mapping-version

68efd37

dnhatn added a commit that referenced this pull request Jan 23, 2019

Always return metadata version if metadata is requested (#37674)

6a98383

If the indices of a ClusterStateRequest are specified, we fail to include the cluster state metadata version in the response. Relates #37633

dnhatn added a commit that referenced this pull request Jan 23, 2019

Always return metadata version if metadata is requested (#37674)

6551e5d

If the indices of a ClusterStateRequest are specified, we fail to include the cluster state metadata version in the response. Relates #37633

dnhatn merged commit 0096f1b into elastic:master Jan 23, 2019

dnhatn deleted the mapping-version branch January 23, 2019 18:41

dnhatn added the backport pending label Jan 23, 2019

dnhatn mentioned this pull request Jan 25, 2019

Wait for mapping ready in testReadRequestsReturnLatestMappingVersion #37886

Merged

dnhatn added a commit that referenced this pull request Jan 26, 2019

Always return metadata version if metadata is requested (#37674)

ec243f2

If the indices of a ClusterStateRequest are specified, we fail to include the cluster state metadata version in the response. Relates #37633

dnhatn added v6.6.1 and removed backport pending v6.6.1 labels Jan 31, 2019

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure changes requests return the latest mapping version #37633

Ensure changes requests return the latest mapping version #37633

dnhatn commented Jan 19, 2019 •

edited

Loading

elasticmachine commented Jan 19, 2019

martijnvg left a comment

martijnvg Jan 21, 2019

ywelsch Jan 21, 2019

dnhatn Jan 21, 2019

martijnvg commented Jan 21, 2019

ywelsch left a comment

ywelsch Jan 21, 2019

dnhatn commented Jan 21, 2019

dnhatn commented Jan 21, 2019

ywelsch left a comment

ywelsch Jan 22, 2019

Ensure changes requests return the latest mapping version #37633

Ensure changes requests return the latest mapping version #37633

Conversation

dnhatn commented Jan 19, 2019 • edited Loading

elasticmachine commented Jan 19, 2019

martijnvg left a comment

Choose a reason for hiding this comment

martijnvg Jan 21, 2019

Choose a reason for hiding this comment

ywelsch Jan 21, 2019

Choose a reason for hiding this comment

dnhatn Jan 21, 2019

Choose a reason for hiding this comment

martijnvg commented Jan 21, 2019

ywelsch left a comment

Choose a reason for hiding this comment

ywelsch Jan 21, 2019

Choose a reason for hiding this comment

dnhatn commented Jan 21, 2019

dnhatn commented Jan 21, 2019

ywelsch left a comment

Choose a reason for hiding this comment

ywelsch Jan 22, 2019

Choose a reason for hiding this comment

dnhatn commented Jan 19, 2019 •

edited

Loading