-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Changes from 2 commits
9b8adc0
f38bc7a
7e8d53e
8804d9d
f4967e5
a48b770
69910d7
d895be7
9066e41
790a09f
88d5188
19cae57
28918ce
08b3641
3a6cc0b
3045ead
962a747
36d81a6
a949a91
74c289b
b80e0e9
6a9edf8
8727ce5
0573217
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,9 +27,12 @@ | |
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.IndexScopedSettings; | ||
import org.elasticsearch.common.settings.Settings; | ||
import org.elasticsearch.common.util.concurrent.AbstractRunnable; | ||
import org.elasticsearch.common.util.concurrent.ThreadContext; | ||
import org.elasticsearch.index.Index; | ||
import org.elasticsearch.index.IndexSettings; | ||
import org.elasticsearch.license.LicenseUtils; | ||
|
@@ -52,12 +55,14 @@ | |
import java.util.Locale; | ||
import java.util.Objects; | ||
import java.util.function.BiConsumer; | ||
import java.util.function.Supplier; | ||
import java.util.stream.Collectors; | ||
|
||
public final class TransportPutFollowAction | ||
extends TransportMasterNodeAction<PutFollowAction.Request, PutFollowAction.Response> { | ||
|
||
private static final Logger logger = LogManager.getLogger(TransportPutFollowAction.class); | ||
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(TransportPutFollowAction.class); | ||
|
||
private final IndexScopedSettings indexScopedSettings; | ||
private final Client client; | ||
|
@@ -135,6 +140,12 @@ private void createFollowerIndex( | |
return; | ||
} | ||
|
||
if (leaderIndexMetadata.isSystem()) { | ||
deprecationLogger.deprecate(DeprecationCategory.INDICES, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This deprecation logging will fire on any explicit system index follow, whereas the breaking change in #72815 only skips auto-following system indices. I would prefer to have the deprecation logging for auto-following only. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that's something we need to discuss. Currently there's no way to know if a followed index request come from AutoFollow or from an explicit user request. I was wondering if we should add some extra metadata in order to distinguish the following origin, in that case existing followed system indices won't show up in the deprecation API. wdyt @henningandersen? |
||
"ccr_follow_system_indices", | ||
"Following a system index " + leaderIndexMetadata.getIndex() + " will not work in the next major version" | ||
); | ||
} | ||
final Settings replicatedRequestSettings = TransportResumeFollowAction.filter(request.getSettings()); | ||
if (replicatedRequestSettings.isEmpty() == false) { | ||
final List<String> unknownKeys = | ||
|
@@ -232,18 +243,21 @@ public void onFailure(Exception e) { | |
listener = originalListener; | ||
} | ||
|
||
final Supplier<ThreadContext.StoredContext> contextSupplier = threadPool.getThreadContext().newRestorableContext(true); | ||
RestoreClusterStateListener.createAndRegisterListener(clusterService, response, listener.delegateFailure( | ||
(delegatedListener, restoreSnapshotResponse) -> { | ||
RestoreInfo restoreInfo = restoreSnapshotResponse.getRestoreInfo(); | ||
if (restoreInfo == null) { | ||
// If restoreInfo is null then it is possible there was a master failure during the | ||
// restore. | ||
delegatedListener.onResponse(new PutFollowAction.Response(true, false, false)); | ||
} else if (restoreInfo.failedShards() == 0) { | ||
initiateFollowing(clientWithHeaders, request, delegatedListener); | ||
} else { | ||
assert restoreInfo.failedShards() > 0 : "Should have failed shards"; | ||
delegatedListener.onResponse(new PutFollowAction.Response(true, false, false)); | ||
try (ThreadContext.StoredContext ctx = contextSupplier.get()) { | ||
RestoreInfo restoreInfo = restoreSnapshotResponse.getRestoreInfo(); | ||
if (restoreInfo == null) { | ||
// If restoreInfo is null then it is possible there was a master failure during the | ||
// restore. | ||
delegatedListener.onResponse(new PutFollowAction.Response(true, false, false)); | ||
} else if (restoreInfo.failedShards() == 0) { | ||
initiateFollowing(clientWithHeaders, request, delegatedListener); | ||
} else { | ||
assert restoreInfo.failedShards() > 0 : "Should have failed shards"; | ||
delegatedListener.onResponse(new PutFollowAction.Response(true, false, false)); | ||
} | ||
} | ||
})); | ||
} | ||
|
There was a problem hiding this comment.
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)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in b80e0e9