-
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
[Profiling] Support index migrations #97773
[Profiling] Support index migrations #97773
Conversation
With this commit we add the required infrastructure to detect whether indices need to be migrated and define migrations to update mappings or dynamic index settings.
Pinging @elastic/profiling (Team:Universal Profiling) |
Hi @danielmitterdorfer, I've created a changelog YAML for you. |
Testing this in action is a bit tricky because index and index template versions are defined in code. I've tested this as follows:
diff --git a/x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/ProfilingIndexManager.java b/x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/ProfilingIndexManager.java
index 6c649836877..bfa79847cba 100644
--- a/x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/ProfilingIndexManager.java
+++ b/x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/ProfilingIndexManager.java
@@ -19,8 +19,10 @@ import org.elasticsearch.client.internal.Client;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.core.TimeValue;
+import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xpack.core.ClientHelper;
@@ -45,7 +47,13 @@ public class ProfilingIndexManager extends AbstractProfilingPersistenceManager<P
ProfilingIndex.regular(
"profiling-returnpads-private",
ProfilingIndexTemplateRegistry.PROFILING_RETURNPADS_PRIVATE_VERSION,
- OnVersionBump.KEEP_OLD
+ OnVersionBump.KEEP_OLD,
+ new Migration.Builder().migrateToIndexTemplateVersion(2)
+ .addProperty("new-property", "keyword")
+ .migrateToIndexTemplateVersion(3)
+ .dynamicSettings(
+ Settings.builder().put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), TimeValue.timeValueSeconds(30)).build()
+ )
),
ProfilingIndex.regular(
"profiling-sq-executables",
diff --git a/x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/ProfilingIndexTemplateRegistry.java b/x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/ProfilingIndexTemplateRegistry.java
index f2b9d0cedd9..69490f347ef 100644
--- a/x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/ProfilingIndexTemplateRegistry.java
+++ b/x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/ProfilingIndexTemplateRegistry.java
@@ -40,7 +40,7 @@ public class ProfilingIndexTemplateRegistry extends IndexTemplateRegistry {
private static final Logger logger = LogManager.getLogger(ProfilingIndexTemplateRegistry.class);
// history (please add a comment why you increased the version here)
// version 1: initial
- public static final int INDEX_TEMPLATE_VERSION = 1;
+ public static final int INDEX_TEMPLATE_VERSION = 3;
// history for individual indices / index templates. Only bump these for breaking changes that require to create a new index
public static final int PROFILING_EVENTS_VERSION = 1;
Note: The second PUT mapping call is injected by the migration infrastructure to ensure that To check mappings and settings, use:
|
x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/Migration.java
Outdated
Show resolved
Hide resolved
I can see the
Shouldn't there be a |
No, this is exactly working as intended. Only the index template version is bumped but not the index version. This helps us upgrade indices gradually without too much disruption in the cluster unless it is truly required (e.g. changing a static index setting requires a version bump, but not adding a property) |
* @param version The index template version that the targeted index will reach after this migration has been applied. | ||
* @return <code>this</code> to allow for method chaining. | ||
*/ | ||
public Builder migrateToIndexTemplateVersion(int version) { |
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.
Just for my curiosity, why not have a constructor taking the version to avoid the checkVersionSet()
function (and also migrateToIndexTemplateVersion()
?
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.
We could probably do that for the initial version but we allow for migrations across multiple versions and thus we need to be able to call it multiple times. But there's definitely multiple ways to approach this. :)
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.
LGTM
With this commit we add the required infrastructure to detect whether indices need to be migrated and define migrations to update mappings or dynamic index settings.