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

[7.x] Deprecate Auto-Follow system indices #73237

Merged
merged 24 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9b8adc0
Add deprecation warning when a system index is followed
fcofdez May 19, 2021
f38bc7a
More precise naming
fcofdez May 25, 2021
7e8d53e
Warning only on auto-followed system indices
fcofdez Jun 1, 2021
8804d9d
Merge remote-tracking branch 'origin/7.x' into deprecate-auto-follow-…
fcofdez Jun 1, 2021
f4967e5
Fix error
fcofdez Jun 1, 2021
a48b770
Add deprecation docs
fcofdez Jun 1, 2021
69910d7
Merge remote-tracking branch 'origin/7.x' into deprecate-auto-follow-…
fcofdez Jun 1, 2021
d895be7
Fixes
fcofdez Jun 1, 2021
9066e41
Cleanup
fcofdez Jun 1, 2021
790a09f
Add proper auto follow system indices check
fcofdez Jun 8, 2021
88d5188
Merge remote-tracking branch 'origin/7.x' into deprecate-auto-follow-…
fcofdez Jun 8, 2021
19cae57
Remove unused code
fcofdez Jun 8, 2021
28918ce
Unused imports
fcofdez Jun 8, 2021
08b3641
Simplify test
fcofdez Jun 8, 2021
3a6cc0b
Include migrating behaviour
fcofdez Jun 9, 2021
3045ead
Merge remote-tracking branch 'origin/7.x' into deprecate-auto-follow-…
fcofdez Jun 9, 2021
962a747
Simplify
fcofdez Jun 9, 2021
36d81a6
Merge remote-tracking branch 'origin/7.x' into deprecate-auto-follow-…
fcofdez Jun 9, 2021
a949a91
Merge remote-tracking branch 'origin/7.x' into deprecate-auto-follow-…
fcofdez Jun 23, 2021
74c289b
Fix compilation
fcofdez Jun 23, 2021
b80e0e9
Review comments
fcofdez Jun 28, 2021
6a9edf8
Merge remote-tracking branch 'origin/7.x' into deprecate-auto-follow-…
fcofdez Jun 28, 2021
8727ce5
Rearrange docs
fcofdez Jun 28, 2021
0573217
Merge remote-tracking branch 'origin/7.x' into deprecate-auto-follow-…
fcofdez Jun 29, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions docs/reference/migration/migrate_7_14.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,35 @@ the cluster again.
====

// end::notable-breaking-changes[]

[discrete]
[[deprecated-7.14]]
=== Deprecations

The following functionality has been deprecated in {es} 7.14
and will be removed in 8.0.
While this won't have an immediate impact on your applications,
we strongly encourage you take the described steps to update your code
after upgrading to 7.14.

NOTE: Significant changes in behavior are deprecated in a minor release and
the old behavior is supported until the next major release.
To find out if you are using any deprecated functionality,
enable <<deprecation-logging, deprecation logging>>.

[discrete]
[[breaking_714_ccr_changes]]
==== CCR deprecations

[[system-indices-auto-follow-deprecation]]
.Auto-follow remote system indices is deprecated.
[%collapsible]
====
*Details* +
Currently, remote system indices matching an <<ccr-auto-follow,auto-follow pattern>>
are configured as a follower index automatically, this behavior is deprecated.

*Impact* +
In 8.0.0, remote system indices matching an <<ccr-auto-follow,auto-follow pattern>>
won't be configured as a follower index automatically.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should add a note on how to adapt to 8.0 behavior, by excluding patterns matching system indices like .tasks and .kibana-*.

====
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@
import org.apache.http.util.EntityUtils;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.client.WarningFailureException;
import org.elasticsearch.client.WarningsHandler;
import org.elasticsearch.common.CheckedRunnable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.SecureString;
Expand All @@ -23,9 +26,11 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.tasks.TaskResultsService;

import java.io.IOException;
import java.text.SimpleDateFormat;
import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Locale;
Expand All @@ -36,6 +41,7 @@
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.elasticsearch.common.xcontent.ObjectPath.eval;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.emptyOrNullString;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -823,6 +829,51 @@ public void testAutoFollowSearchableSnapshotsFails() throws Exception {
deleteAutoFollowPattern(client(), autoFollowPattern);
}

public void testFollowSystemIndexTriggersDeprecationWarnings() throws Exception {
final String testPrefix = getTestName().toLowerCase(Locale.ROOT);
final String autoFollowPatternName = "pattern-" + testPrefix;

try {
if ("leader".equals(targetCluster)) {
final Request request = new Request("POST", "/" + TaskResultsService.TASK_INDEX + "/_doc/123");
XContentBuilder document = jsonBuilder();
document.startObject();
document.field("completed", true);
document.endObject();

// Avoid throwing a warning exception since we're writing into a system index
request.setOptions(RequestOptions.DEFAULT.toBuilder().setWarningsHandler(warnings -> false));
request.setJsonEntity(Strings.toString(document));
assertOK(adminClient().performRequest(request));
ensureGreen(TaskResultsService.TASK_INDEX);
} else {
final WarningFailureException exception = expectThrows(WarningFailureException.class,
() -> createAutoFollowPattern(client(),
autoFollowPatternName,
TaskResultsService.TASK_INDEX,
"leader_cluster",
WarningsHandler.STRICT));

final List<String> warnings = exception.getResponse().getWarnings();
boolean expectedWarningMessageFound = warnings.stream()
.anyMatch(warning ->
warning.startsWith("Auto following a system index [" + TaskResultsService.TASK_INDEX) &&
warning.endsWith("will not work in the next major version"));
assertThat(
"Expected to find a warning about auto following system indices but the warnings are: " + warnings,
expectedWarningMessageFound,
equalTo(true)
);
}
} finally {
cleanUpFollower(
Collections.emptyList(),
Collections.emptyList(),
singletonList(autoFollowPatternName)
);
}
}

private int getNumberOfSuccessfulFollowedIndices() throws IOException {
return getNumberOfSuccessfulFollowedIndices(client());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
import org.apache.http.HttpHost;
import org.apache.http.util.EntityUtils;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.client.WarningsHandler;
import org.elasticsearch.cluster.metadata.DataStream;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -329,6 +331,11 @@ protected static void verifyDataStream(final RestClient client,
}

protected static void createAutoFollowPattern(RestClient client, String name, String pattern, String remoteCluster) throws IOException {
createAutoFollowPattern(client, name, pattern, remoteCluster, WarningsHandler.PERMISSIVE);
}

protected static void createAutoFollowPattern(RestClient client, String name, String pattern, String remoteCluster,
WarningsHandler warningsHandler) throws IOException {
Request request = new Request("PUT", "/_ccr/auto_follow/" + name);
try (XContentBuilder bodyBuilder = JsonXContent.contentBuilder()) {
bodyBuilder.startObject();
Expand All @@ -343,6 +350,8 @@ protected static void createAutoFollowPattern(RestClient client, String name, St
bodyBuilder.endObject();
request.setJsonEntity(Strings.toString(bodyBuilder));
}

request.setOptions(RequestOptions.DEFAULT.toBuilder().setWarningsHandler(warningsHandler));
assertOK(client.performRequest(request));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.xpack.CcrIntegTestCase;
import org.elasticsearch.xpack.core.ccr.AutoFollowMetadata;
import org.elasticsearch.xpack.core.ccr.AutoFollowStats;
import org.elasticsearch.xpack.core.ccr.CcrConstants;
import org.elasticsearch.xpack.core.ccr.action.ActivateAutoFollowPatternAction;
import org.elasticsearch.xpack.core.ccr.action.CcrStatsAction;
import org.elasticsearch.xpack.core.ccr.action.DeleteAutoFollowPatternAction;
Expand Down Expand Up @@ -117,8 +118,8 @@ public void testCleanFollowedLeaderIndexUUIDs() throws Exception {

Metadata metadata = getFollowerCluster().clusterService().state().metadata();
String leaderIndexUUID = metadata.index("copy-logs-201901")
.getCustomData(Ccr.CCR_CUSTOM_METADATA_KEY)
.get(Ccr.CCR_CUSTOM_METADATA_LEADER_INDEX_UUID_KEY);
.getCustomData(CcrConstants.CCR_CUSTOM_METADATA_KEY)
.get(CcrConstants.CCR_CUSTOM_METADATA_LEADER_INDEX_UUID_KEY);
AutoFollowMetadata autoFollowMetadata = metadata.custom(AutoFollowMetadata.TYPE);
assertThat(autoFollowMetadata, notNullValue());
List<String> followedLeaderIndixUUIDs = autoFollowMetadata.getFollowedLeaderIndexUUIDs().get("my-pattern");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.elasticsearch.xpack.ccr.action.repositories.PutCcrRestoreSessionAction;
import org.elasticsearch.xpack.ccr.repository.CcrRepository;
import org.elasticsearch.xpack.ccr.repository.CcrRestoreSourceService;
import org.elasticsearch.xpack.core.ccr.CcrConstants;
import org.elasticsearch.xpack.core.ccr.action.PutFollowAction;

import java.io.IOException;
Expand Down Expand Up @@ -185,9 +186,9 @@ public void testThatRepositoryRecoversEmptyIndexBasedOnLeaderSettings() throws I
IndexMetadata leaderMetadata = leaderState.getState().metadata().index(leaderIndex);
IndexMetadata followerMetadata = followerState.getState().metadata().index(followerIndex);
assertEquals(leaderMetadata.getNumberOfShards(), followerMetadata.getNumberOfShards());
Map<String, String> ccrMetadata = followerMetadata.getCustomData(Ccr.CCR_CUSTOM_METADATA_KEY);
Map<String, String> ccrMetadata = followerMetadata.getCustomData(CcrConstants.CCR_CUSTOM_METADATA_KEY);
assertEquals(leaderIndex, ccrMetadata.get(Ccr.CCR_CUSTOM_METADATA_LEADER_INDEX_NAME_KEY));
assertEquals(leaderMetadata.getIndexUUID(), ccrMetadata.get(Ccr.CCR_CUSTOM_METADATA_LEADER_INDEX_UUID_KEY));
assertEquals(leaderMetadata.getIndexUUID(), ccrMetadata.get(CcrConstants.CCR_CUSTOM_METADATA_LEADER_INDEX_UUID_KEY));
assertEquals("leader_cluster", ccrMetadata.get(Ccr.CCR_CUSTOM_METADATA_REMOTE_CLUSTER_NAME_KEY));
assertEquals(followerIndex, followerMetadata.getSettings().get(IndexMetadata.SETTING_INDEX_PROVIDED_NAME));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,7 @@
public class Ccr extends Plugin implements ActionPlugin, PersistentTaskPlugin, EnginePlugin, RepositoryPlugin, ClusterPlugin {

public static final String CCR_THREAD_POOL_NAME = "ccr";
public static final String CCR_CUSTOM_METADATA_KEY = "ccr";
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps leave a comment here, stating that these moved to CcrConstants in 7.x (since this targets 7.x only)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in b80e0e9

public static final String CCR_CUSTOM_METADATA_LEADER_INDEX_SHARD_HISTORY_UUIDS = "leader_index_shard_history_uuids";
public static final String CCR_CUSTOM_METADATA_LEADER_INDEX_UUID_KEY = "leader_index_uuid";
public static final String CCR_CUSTOM_METADATA_LEADER_INDEX_NAME_KEY = "leader_index_name";
public static final String CCR_CUSTOM_METADATA_REMOTE_CLUSTER_NAME_KEY = "remote_cluster_name";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.license.LicenseUtils;
import org.elasticsearch.transport.NoSuchRemoteClusterException;
import org.elasticsearch.xpack.ccr.Ccr;
import org.elasticsearch.xpack.ccr.CcrLicenseChecker;
import org.elasticsearch.xpack.ccr.CcrSettings;
import org.elasticsearch.xpack.core.ccr.AutoFollowMetadata;
import org.elasticsearch.xpack.core.ccr.AutoFollowMetadata.AutoFollowPattern;
import org.elasticsearch.xpack.core.ccr.AutoFollowStats;
import org.elasticsearch.xpack.core.ccr.CcrConstants;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should add a check to checkAutoFollowPattern to see if the leader index is a system index and output a deprecation warning if so?

(not related to the line the comment is on).

import org.elasticsearch.xpack.core.ccr.action.PutFollowAction;
import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants;

Expand Down Expand Up @@ -555,9 +555,9 @@ private static boolean leaderIndexAlreadyFollowed(AutoFollowPattern autoFollowPa
// we should let the auto follower attempt to auto follow it, so it can fail later and
// it is then visible in the auto follow stats. For example a cluster can just happen to have
// an index with the same name as the new follower index.
Map<String, String> customData = indexMetadata.getCustomData(Ccr.CCR_CUSTOM_METADATA_KEY);
Map<String, String> customData = indexMetadata.getCustomData(CcrConstants.CCR_CUSTOM_METADATA_KEY);
if (customData != null) {
String recordedLeaderIndexUUID = customData.get(Ccr.CCR_CUSTOM_METADATA_LEADER_INDEX_UUID_KEY);
String recordedLeaderIndexUUID = customData.get(CcrConstants.CCR_CUSTOM_METADATA_LEADER_INDEX_UUID_KEY);
return leaderIndex.getUUID().equals(recordedLeaderIndexUUID);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
import org.elasticsearch.xpack.ccr.action.bulk.BulkShardOperationsAction;
import org.elasticsearch.xpack.ccr.action.bulk.BulkShardOperationsRequest;
import org.elasticsearch.xpack.ccr.action.bulk.BulkShardOperationsResponse;
import org.elasticsearch.xpack.core.ccr.CcrConstants;
import org.elasticsearch.xpack.core.ccr.action.ShardFollowTask;

import java.util.ArrayList;
Expand Down Expand Up @@ -521,7 +522,7 @@ private void logRetentionLeaseFailure(final String retentionLeaseId, final Throw

private String getLeaderShardHistoryUUID(ShardFollowTask params) {
IndexMetadata followIndexMetadata = clusterService.state().metadata().index(params.getFollowShardId().getIndex());
Map<String, String> ccrIndexMetadata = followIndexMetadata.getCustomData(Ccr.CCR_CUSTOM_METADATA_KEY);
Map<String, String> ccrIndexMetadata = followIndexMetadata.getCustomData(CcrConstants.CCR_CUSTOM_METADATA_KEY);
String[] recordedLeaderShardHistoryUUIDs = extractLeaderShardHistoryUUIDs(ccrIndexMetadata);
return recordedLeaderShardHistoryUUIDs[params.getLeaderShardId().id()];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.ccr.Ccr;
import org.elasticsearch.xpack.core.ccr.CcrConstants;
import org.elasticsearch.xpack.core.ccr.action.FollowInfoAction;
import org.elasticsearch.xpack.core.ccr.action.FollowInfoAction.Response.FollowerInfo;
import org.elasticsearch.xpack.core.ccr.action.FollowInfoAction.Response.Status;
Expand Down Expand Up @@ -65,7 +66,7 @@ static List<FollowerInfo> getFollowInfos(List<String> concreteFollowerIndices, C

for (String index : concreteFollowerIndices) {
IndexMetadata indexMetadata = state.metadata().index(index);
Map<String, String> ccrCustomData = indexMetadata.getCustomData(Ccr.CCR_CUSTOM_METADATA_KEY);
Map<String, String> ccrCustomData = indexMetadata.getCustomData(CcrConstants.CCR_CUSTOM_METADATA_KEY);
if (ccrCustomData != null) {
Optional<ShardFollowTask> result;
if (persistentTasks != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import org.elasticsearch.persistent.PersistentTasksService;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.ccr.Ccr;
import org.elasticsearch.xpack.core.ccr.CcrConstants;
import org.elasticsearch.xpack.core.ccr.action.PauseFollowAction;
import org.elasticsearch.xpack.core.ccr.action.ShardFollowTask;

Expand Down Expand Up @@ -56,7 +56,7 @@ protected void masterOperation(PauseFollowAction.Request request,
listener.onFailure(new IndexNotFoundException(request.getFollowIndex()));
return;
}
if (followerIMD.getCustomData(Ccr.CCR_CUSTOM_METADATA_KEY) == null) {
if (followerIMD.getCustomData(CcrConstants.CCR_CUSTOM_METADATA_KEY) == null) {
listener.onFailure(new IllegalArgumentException("index [" + request.getFollowIndex() + "] is not a follower index"));
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.logging.DeprecationCategory;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.LicenseUtils;
import org.elasticsearch.threadpool.ThreadPool;
Expand All @@ -44,6 +46,8 @@

public class TransportPutAutoFollowPatternAction extends AcknowledgedTransportMasterNodeAction<PutAutoFollowPatternAction.Request> {

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(TransportPutAutoFollowPatternAction.class);

private final Client client;
private final CcrLicenseChecker ccrLicenseChecker;

Expand Down Expand Up @@ -196,6 +200,12 @@ private static void markExistingIndicesAsAutoFollowed(
for (final IndexMetadata indexMetadata : leaderMetadata) {
IndexAbstraction indexAbstraction = leaderMetadata.getIndicesLookup().get(indexMetadata.getIndex().getName());
if (AutoFollowPattern.match(patterns, indexAbstraction)) {
if (indexAbstraction.isSystem()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am torn on this. I think the logic here ensures that we do not follow these existing system indices anyway and therefore it hardly makes sense to have the deprecation warning here? I would opt to remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thinking here was that we'll deprecate this behaviour in 7.14 but keep auto following system indices until 8.0. is that a misunderstanding on my side?

deprecationLogger.deprecate(DeprecationCategory.INDICES,
"ccr_auto_follow_system_indices",
"Auto following a system index " + indexMetadata.getIndex() + " will not work in the next major version"
);
}
followedIndexUUIDS.add(indexMetadata.getIndexUUID());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.elasticsearch.xpack.ccr.CcrLicenseChecker;
import org.elasticsearch.xpack.ccr.CcrSettings;
import org.elasticsearch.xpack.core.ClientHelper;
import org.elasticsearch.xpack.core.ccr.CcrConstants;
import org.elasticsearch.xpack.core.ccr.action.FollowParameters;
import org.elasticsearch.xpack.core.ccr.action.ResumeFollowAction;
import org.elasticsearch.xpack.core.ccr.action.ShardFollowTask;
Expand Down Expand Up @@ -121,7 +122,7 @@ protected void masterOperation(final ResumeFollowAction.Request request,
return;
}

final Map<String, String> ccrMetadata = followerIndexMetadata.getCustomData(Ccr.CCR_CUSTOM_METADATA_KEY);
final Map<String, String> ccrMetadata = followerIndexMetadata.getCustomData(CcrConstants.CCR_CUSTOM_METADATA_KEY);
if (ccrMetadata == null) {
throw new IllegalArgumentException("follow index ["+ request.getFollowerIndex() + "] does not have ccr metadata");
}
Expand Down Expand Up @@ -183,12 +184,12 @@ static void validate(
final MapperService followerMapperService) {
FollowParameters parameters = request.getParameters();

Map<String, String> ccrIndexMetadata = followIndex.getCustomData(Ccr.CCR_CUSTOM_METADATA_KEY);
Map<String, String> ccrIndexMetadata = followIndex.getCustomData(CcrConstants.CCR_CUSTOM_METADATA_KEY);
if (ccrIndexMetadata == null) {
throw new IllegalArgumentException("follow index ["+ followIndex.getIndex().getName() + "] does not have ccr metadata");
}
String leaderIndexUUID = leaderIndex.getIndex().getUUID();
String recordedLeaderIndexUUID = ccrIndexMetadata.get(Ccr.CCR_CUSTOM_METADATA_LEADER_INDEX_UUID_KEY);
String recordedLeaderIndexUUID = ccrIndexMetadata.get(CcrConstants.CCR_CUSTOM_METADATA_LEADER_INDEX_UUID_KEY);
if (leaderIndexUUID.equals(recordedLeaderIndexUUID) == false) {
throw new IllegalArgumentException("follow index [" + request.getFollowerIndex() + "] should reference [" +
leaderIndexUUID + "] as leader index but instead reference [" + recordedLeaderIndexUUID + "] as leader index");
Expand Down
Loading