From 5cc352d9361268566cd3ecadbe1d54487369a038 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 29 Jul 2022 18:30:46 +0200 Subject: [PATCH] Make Settings Diffable (#88815) Making settings diffable so that index metadata diffs are smaller whenever the metadata changes without a setting change as well as to make index setting updates over a wider number of indices faster. This saves about 3% of CPU time on master and about half that on data nodes that is just burnt for writing setting strings when bootstrapping many shards benchmarks benchmarks to 50k indices. --- .../admin/cluster/node/info/NodeInfo.java | 2 +- .../put/PutRepositoryRequest.java | 3 +- .../settings/ClusterGetSettingsAction.java | 6 +- .../ClusterUpdateSettingsRequest.java | 5 +- .../ClusterUpdateSettingsResponse.java | 4 +- .../create/CreateSnapshotRequest.java | 3 +- .../restore/RestoreSnapshotRequest.java | 3 +- .../indices/create/CreateIndexRequest.java | 3 +- .../admin/indices/get/GetIndexResponse.java | 4 +- .../settings/get/GetSettingsResponse.java | 4 +- .../settings/put/UpdateSettingsRequest.java | 3 +- .../template/put/PutIndexTemplateRequest.java | 3 +- .../elasticsearch/bootstrap/ServerArgs.java | 2 +- .../elasticsearch/cluster/DiffableUtils.java | 25 +++++- .../cluster/metadata/DesiredNode.java | 2 +- .../cluster/metadata/IndexMetadata.java | 33 +++++-- .../metadata/IndexTemplateMetadata.java | 2 +- .../cluster/metadata/Metadata.java | 9 +- .../cluster/metadata/RepositoryMetadata.java | 2 +- .../cluster/metadata/Template.java | 2 +- .../common/settings/Settings.java | 85 +++++++++++++++---- .../index/analysis/NameOrDefinition.java | 2 +- .../common/settings/SettingsTests.java | 34 +++++++- .../action/PutAutoscalingPolicyAction.java | 2 +- .../autoscaling/policy/AutoscalingPolicy.java | 2 +- .../xpack/core/ccr/AutoFollowMetadata.java | 2 +- .../action/PutAutoFollowPatternAction.java | 2 +- .../core/ccr/action/PutFollowAction.java | 2 +- .../MountSearchableSnapshotRequest.java | 3 +- .../TransformDestIndexSettings.java | 2 +- .../xpack/ml/autoscaling/MlScalingReason.java | 2 +- 31 files changed, 188 insertions(+), 70 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodeInfo.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodeInfo.java index 1f323bfbf9766..b803a627207da 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodeInfo.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodeInfo.java @@ -183,7 +183,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeBoolean(false); } else { out.writeBoolean(true); - Settings.writeSettingsToStream(settings, out); + settings.writeTo(out); } out.writeOptionalWriteable(getInfo(OsInfo.class)); out.writeOptionalWriteable(getInfo(ProcessInfo.class)); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutRepositoryRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutRepositoryRequest.java index 00acf3d04b83d..4402519c4eacd 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutRepositoryRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutRepositoryRequest.java @@ -22,7 +22,6 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; -import static org.elasticsearch.common.settings.Settings.writeSettingsToStream; /** * Register repository request. @@ -207,7 +206,7 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeString(name); out.writeString(type); - writeSettingsToStream(settings, out); + settings.writeTo(out); out.writeBoolean(verify); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsAction.java index 71767728e57bc..15af362fe4a46 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterGetSettingsAction.java @@ -92,9 +92,9 @@ public Response(Settings persistentSettings, Settings transientSettings, Setting @Override public void writeTo(StreamOutput out) throws IOException { assert out.getVersion().onOrAfter(Version.V_8_3_0); - Settings.writeSettingsToStream(persistentSettings, out); - Settings.writeSettingsToStream(transientSettings, out); - Settings.writeSettingsToStream(settings, out); + persistentSettings.writeTo(out); + transientSettings.writeTo(out); + settings.writeTo(out); } public Settings persistentSettings() { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequest.java index 359f9fe976862..23348716ffcca 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequest.java @@ -25,7 +25,6 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; -import static org.elasticsearch.common.settings.Settings.writeSettingsToStream; /** * Request for an update cluster settings action @@ -162,8 +161,8 @@ public ClusterUpdateSettingsRequest persistentSettings(Map source) { @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - writeSettingsToStream(transientSettings, out); - writeSettingsToStream(persistentSettings, out); + transientSettings.writeTo(out); + persistentSettings.writeTo(out); } @Override diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponse.java index d661f2a1e8bbd..0891de0c5f970 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponse.java @@ -67,8 +67,8 @@ public Settings getPersistentSettings() { @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - Settings.writeSettingsToStream(transientSettings, out); - Settings.writeSettingsToStream(persistentSettings, out); + transientSettings.writeTo(out); + persistentSettings.writeTo(out); } @Override diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java index 251bf6fb17645..04acdb7a3fa40 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java @@ -33,7 +33,6 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.common.Strings.EMPTY_ARRAY; import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; -import static org.elasticsearch.common.settings.Settings.writeSettingsToStream; import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBooleanValue; /** @@ -115,7 +114,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeStringArray(indices); indicesOptions.writeIndicesOptions(out); if (out.getVersion().before(SETTINGS_IN_REQUEST_VERSION)) { - writeSettingsToStream(Settings.EMPTY, out); + Settings.EMPTY.writeTo(out); } out.writeStringArray(featureStates); out.writeBoolean(includeGlobalState); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index c383b2d610e5f..a561ac48ed793 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -30,7 +30,6 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; -import static org.elasticsearch.common.settings.Settings.writeSettingsToStream; import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBooleanValue; /** @@ -113,7 +112,7 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(VERSION_SUPPORTING_QUIET_PARAMETER)) { out.writeBoolean(quiet); } - writeSettingsToStream(indexSettings, out); + indexSettings.writeTo(out); out.writeStringArray(ignoreIndexSettings); out.writeOptionalString(snapshotUuid); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java index aa65352e50468..985115cca3776 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java @@ -43,7 +43,6 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; -import static org.elasticsearch.common.settings.Settings.writeSettingsToStream; /** * A request to create an index. Best created with {@link org.elasticsearch.client.internal.Requests#createIndexRequest(String)}. @@ -453,7 +452,7 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeString(cause); out.writeString(index); - writeSettingsToStream(settings, out); + settings.writeTo(out); if (out.getVersion().before(Version.V_8_0_0)) { if ("{}".equals(mappings)) { out.writeVInt(0); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/get/GetIndexResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/get/GetIndexResponse.java index 5d9ede276bd3b..4f8f24ea5b72f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/get/GetIndexResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/get/GetIndexResponse.java @@ -172,8 +172,8 @@ public void writeTo(StreamOutput out) throws IOException { out.writeStringArray(indices); MappingMetadata.writeMappingMetadata(out, mappings); out.writeMap(aliases, StreamOutput::writeString, StreamOutput::writeList); - out.writeMap(settings, StreamOutput::writeString, (o, v) -> Settings.writeSettingsToStream(v, o)); - out.writeMap(defaultSettings, StreamOutput::writeString, (o, v) -> Settings.writeSettingsToStream(v, o)); + out.writeMap(settings, StreamOutput::writeString, (o, v) -> v.writeTo(o)); + out.writeMap(defaultSettings, StreamOutput::writeString, (o, v) -> v.writeTo(o)); out.writeMap(dataStreams, StreamOutput::writeString, StreamOutput::writeOptionalString); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java index ae90b405b9106..0305f123bba11 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsResponse.java @@ -90,8 +90,8 @@ public String getSetting(String index, String setting) { @Override public void writeTo(StreamOutput out) throws IOException { - out.writeMap(indexToSettings, StreamOutput::writeString, (o, s) -> Settings.writeSettingsToStream(s, o)); - out.writeMap(indexToDefaultSettings, StreamOutput::writeString, (o, s) -> Settings.writeSettingsToStream(s, o)); + out.writeMap(indexToSettings, StreamOutput::writeString, (o, v) -> v.writeTo(o)); + out.writeMap(indexToDefaultSettings, StreamOutput::writeString, (o, v) -> v.writeTo(o)); } private static void parseSettingsField( diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java index f4fe5224927af..6e443744b8835 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java @@ -31,7 +31,6 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; -import static org.elasticsearch.common.settings.Settings.writeSettingsToStream; /** * Request for an update index settings action @@ -182,7 +181,7 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeStringArrayNullable(indices); indicesOptions.writeIndicesOptions(out); - writeSettingsToStream(settings, out); + settings.writeTo(out); out.writeBoolean(preserveExisting); if (out.getVersion().onOrAfter(Version.V_7_12_0)) { out.writeString(origin); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequest.java index 66deeaa64df36..a332b57cd475f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequest.java @@ -43,7 +43,6 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; -import static org.elasticsearch.common.settings.Settings.writeSettingsToStream; /** * A request to create an index template. @@ -446,7 +445,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeStringCollection(indexPatterns); out.writeInt(order); out.writeBoolean(create); - writeSettingsToStream(settings, out); + settings.writeTo(out); if (out.getVersion().before(Version.V_8_0_0)) { out.writeVInt(mappings == null ? 0 : 1); if (mappings != null) { diff --git a/server/src/main/java/org/elasticsearch/bootstrap/ServerArgs.java b/server/src/main/java/org/elasticsearch/bootstrap/ServerArgs.java index cc67a0b742d80..c324370573fce 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/ServerArgs.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/ServerArgs.java @@ -82,7 +82,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeBoolean(quiet); out.writeOptionalString(pidFile == null ? null : pidFile.toString()); out.writeSecureString(keystorePassword); - Settings.writeSettingsToStream(nodeSettings, out); + nodeSettings.writeTo(out); out.writeString(configDir.toString()); } } diff --git a/server/src/main/java/org/elasticsearch/cluster/DiffableUtils.java b/server/src/main/java/org/elasticsearch/cluster/DiffableUtils.java index f5fb4e797aa58..bcb9222e384ae 100644 --- a/server/src/main/java/org/elasticsearch/cluster/DiffableUtils.java +++ b/server/src/main/java/org/elasticsearch/cluster/DiffableUtils.java @@ -153,7 +153,7 @@ private static > MapDiff createDiff( inserts++; } else if (entry.getValue().equals(previousValue) == false) { if (valueSerializer.supportsDiffableValues()) { - diffs.add(Map.entry(entry.getKey(), valueSerializer.diff(entry.getValue(), previousValue))); + diffs.add(mapEntry(entry.getKey(), valueSerializer.diff(entry.getValue(), previousValue))); } else { upserts.add(entry); } @@ -307,14 +307,14 @@ private MapDiff( for (int i = 0; i < diffsCount; i++) { K key = keySerializer.readKey(in); Diff diff = valueSerializer.readDiff(in, key); - diffs.add(Map.entry(key, diff)); + diffs.add(mapEntry(key, diff)); } int upsertsCount = in.readVInt(); upserts = upsertsCount == 0 ? List.of() : new ArrayList<>(upsertsCount); for (int i = 0; i < upsertsCount; i++) { K key = keySerializer.readKey(in); T newValue = valueSerializer.read(in, key); - upserts.add(Map.entry(key, newValue)); + upserts.add(mapEntry(key, newValue)); } this.builderCtor = builderCtor; } @@ -402,6 +402,25 @@ public void writeTo(StreamOutput out) throws IOException { } } + private static Map.Entry mapEntry(K key, T newValue) { + return new Map.Entry<>() { + @Override + public K getKey() { + return key; + } + + @Override + public T getValue() { + return newValue; + } + + @Override + public T setValue(T value) { + throw new UnsupportedOperationException(); + } + }; + } + /** * Provides read and write operations to serialize keys of map * @param type of key diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/DesiredNode.java b/server/src/main/java/org/elasticsearch/cluster/metadata/DesiredNode.java index cf8440f569527..1cb1552e5afa3 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/DesiredNode.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/DesiredNode.java @@ -188,7 +188,7 @@ public static DesiredNode readFrom(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { - Settings.writeSettingsToStream(settings, out); + settings.writeTo(out); if (out.getVersion().onOrAfter(RANGE_FLOAT_PROCESSORS_SUPPORT_VERSION)) { out.writeOptionalFloat(processors); out.writeOptionalWriteable(processorsRange); diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java index 032a5acc90c90..412f4cc3ae475 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java @@ -77,7 +77,6 @@ import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.OpType.OR; import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.validateIpValue; import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; -import static org.elasticsearch.common.settings.Settings.writeSettingsToStream; import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOT_PARTIAL_SETTING_KEY; public class IndexMetadata implements Diffable, ToXContentFragment { @@ -1314,6 +1313,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } + private static final Version SETTING_DIFF_VERSION = Version.V_8_5_0; + private static class IndexMetadataDiff implements Diff { private final String index; @@ -1324,7 +1325,12 @@ private static class IndexMetadataDiff implements Diff { private final long aliasesVersion; private final long[] primaryTerms; private final State state; + + // used for BwC when this instance was written by an older version node that does not diff settings yet + @Nullable private final Settings settings; + @Nullable + private final Diff settingsDiff; private final Diff> mappings; private final Diff> aliases; private final Diff> customData; @@ -1342,6 +1348,7 @@ private static class IndexMetadataDiff implements Diff { routingNumShards = after.routingNumShards; state = after.state; settings = after.settings; + settingsDiff = after.settings.diff(before.settings); primaryTerms = after.primaryTerms; // TODO: find a nicer way to do BwC here and just work with Diff here and in networking mappings = DiffableUtils.diff( @@ -1387,7 +1394,13 @@ private static class IndexMetadataDiff implements Diff { aliasesVersion = 1; } state = State.fromId(in.readByte()); - settings = Settings.readSettingsFromStream(in); + if (in.getVersion().onOrAfter(SETTING_DIFF_VERSION)) { + settings = null; + settingsDiff = Settings.readSettingsDiffFromStream(in); + } else { + settings = Settings.readSettingsFromStream(in); + settingsDiff = null; + } primaryTerms = in.readVLongArray(); mappings = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), MAPPING_DIFF_VALUE_READER); aliases = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), ALIAS_METADATA_DIFF_VALUE_READER); @@ -1421,7 +1434,13 @@ public void writeTo(StreamOutput out) throws IOException { out.writeVLong(aliasesVersion); } out.writeByte(state.id); - Settings.writeSettingsToStream(settings, out); + assert settings != null + : "settings should always be non-null since this instance is not expected to have been read from another node"; + if (out.getVersion().onOrAfter(SETTING_DIFF_VERSION)) { + settingsDiff.writeTo(out); + } else { + settings.writeTo(out); + } out.writeVLongArray(primaryTerms); mappings.writeTo(out); aliases.writeTo(out); @@ -1443,7 +1462,11 @@ public IndexMetadata apply(IndexMetadata part) { builder.aliasesVersion(aliasesVersion); builder.setRoutingNumShards(routingNumShards); builder.state(state); - builder.settings(settings); + if (settingsDiff == null) { + builder.settings(settings); + } else { + builder.settings(settingsDiff.apply(part.settings)); + } builder.primaryTerms(primaryTerms); builder.mapping = mappings.apply( ImmutableOpenMap.builder(1).fPut(MapperService.SINGLE_MAPPING_NAME, part.mapping).build() @@ -1531,7 +1554,7 @@ public void writeTo(StreamOutput out, boolean mappingsAsHash) throws IOException } out.writeInt(routingNumShards); out.writeByte(state.id()); - writeSettingsToStream(settings, out); + settings.writeTo(out); out.writeVLongArray(primaryTerms); // TODO: adjust serialization format to using an optional writable if (mapping == null) { diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateMetadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateMetadata.java index 4e9cfa5a5083c..551cda35eb753 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateMetadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateMetadata.java @@ -206,7 +206,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(name); out.writeInt(order); out.writeStringCollection(patterns); - Settings.writeSettingsToStream(settings, out); + settings.writeTo(out); out.writeMap(mappings, StreamOutput::writeString, (o, v) -> v.writeTo(o)); out.writeCollection(aliases.values()); out.writeOptionalVInt(version); diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java index 506581c7ad5cf..dbbbba70ee067 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java @@ -81,7 +81,6 @@ import static org.elasticsearch.cluster.metadata.LifecycleExecutionState.ILM_CUSTOM_METADATA_KEY; import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; -import static org.elasticsearch.common.settings.Settings.writeSettingsToStream; /** * {@link Metadata} is the part of the {@link ClusterState} which persists across restarts. This persistence is XContent-based, so a @@ -1228,8 +1227,8 @@ public void writeTo(StreamOutput out) throws IOException { out.writeBoolean(clusterUUIDCommitted); out.writeLong(version); coordinationMetadata.writeTo(out); - Settings.writeSettingsToStream(transientSettings, out); - Settings.writeSettingsToStream(persistentSettings, out); + transientSettings.writeTo(out); + persistentSettings.writeTo(out); if (out.getVersion().onOrAfter(Version.V_7_3_0)) { hashesOfConsistentSettings.writeTo(out); } @@ -1314,8 +1313,8 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(clusterUUID); out.writeBoolean(clusterUUIDCommitted); coordinationMetadata.writeTo(out); - writeSettingsToStream(transientSettings, out); - writeSettingsToStream(persistentSettings, out); + transientSettings.writeTo(out); + persistentSettings.writeTo(out); if (out.getVersion().onOrAfter(Version.V_7_3_0)) { hashesOfConsistentSettings.writeTo(out); } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/RepositoryMetadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/RepositoryMetadata.java index de40fc641e710..27bdd94a231f4 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/RepositoryMetadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/RepositoryMetadata.java @@ -152,7 +152,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(uuid); } out.writeString(type); - Settings.writeSettingsToStream(settings, out); + settings.writeTo(out); out.writeLong(generation); out.writeLong(pendingGeneration); } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/Template.java b/server/src/main/java/org/elasticsearch/cluster/metadata/Template.java index 84a5f4c8f6d5e..89115dfe9e51c 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/Template.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/Template.java @@ -126,7 +126,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeBoolean(false); } else { out.writeBoolean(true); - Settings.writeSettingsToStream(this.settings, out); + this.settings.writeTo(out); } if (this.mappings == null) { out.writeBoolean(false); diff --git a/server/src/main/java/org/elasticsearch/common/settings/Settings.java b/server/src/main/java/org/elasticsearch/common/settings/Settings.java index 5260dabee66ab..3ae5994ed1ac2 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -13,10 +13,14 @@ import org.elasticsearch.ElasticsearchGenerationException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; +import org.elasticsearch.cluster.Diff; +import org.elasticsearch.cluster.Diffable; +import org.elasticsearch.cluster.DiffableUtils; import org.elasticsearch.common.Numbers; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.LogConfigurator; import org.elasticsearch.common.unit.ByteSizeUnit; @@ -71,7 +75,7 @@ /** * An immutable settings implementation. */ -public final class Settings implements ToXContentFragment { +public final class Settings implements ToXContentFragment, Writeable, Diffable { public static final Settings EMPTY = new Settings(Map.of(), null); @@ -598,21 +602,54 @@ public static Settings readSettingsFromStream(StreamInput in) throws IOException return builder.build(); } - public static void writeSettingsToStream(Settings settings, StreamOutput out) throws IOException { - // pull settings to exclude secure settings in size() - out.writeMap(settings.settings, StreamOutput::writeString, (streamOutput, value) -> { - if (value instanceof String) { - streamOutput.writeGenericString((String) value); - } else if (value instanceof List) { - @SuppressWarnings("unchecked") - // exploit the fact that we know all lists to be string lists - final List stringList = (List) value; - streamOutput.writeGenericList(stringList, StreamOutput::writeGenericString); - } else { - assert value == null : "unexpected value [" + value + "]"; - streamOutput.writeGenericNull(); + private static final DiffableUtils.ValueSerializer DIFF_VALUE_SERIALIZER = + new DiffableUtils.NonDiffableValueSerializer<>() { + @Override + public void write(Object value, StreamOutput out) throws IOException { + writeSettingValue(out, value); + } + + @Override + public Object read(StreamInput in, String key) throws IOException { + return in.readGenericValue(); } - }); + }; + + public static Diff readSettingsDiffFromStream(StreamInput in) throws IOException { + return new SettingsDiff(DiffableUtils.readJdkMapDiff(in, DiffableUtils.getStringKeySerializer(), DIFF_VALUE_SERIALIZER)); + } + + @Override + public Diff diff(Settings previousState) { + final DiffableUtils.MapDiff> mapDiff = DiffableUtils.diff( + previousState.settings, + settings, + DiffableUtils.getStringKeySerializer(), + DIFF_VALUE_SERIALIZER + ); + return new SettingsDiff(mapDiff); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + // pull settings to exclude secure settings in size() + out.writeMap(settings, StreamOutput::writeString, Settings::writeSettingValue); + } + + private static void writeSettingValue(StreamOutput streamOutput, Object value) throws IOException { + // we only have strings, lists of strings or null values so as an optimization we can dispatch those directly instead of going + // through the much slower StreamOutput#writeGenericValue that would write the same format + if (value instanceof String) { + streamOutput.writeGenericString((String) value); + } else if (value instanceof List) { + @SuppressWarnings("unchecked") + // exploit the fact that we know all lists to be string lists + final List stringList = (List) value; + streamOutput.writeGenericList(stringList, StreamOutput::writeGenericString); + } else { + assert value == null : "unexpected value [" + value + "]"; + streamOutput.writeGenericNull(); + } } /** @@ -1517,4 +1554,22 @@ private static String toString(Object o) { static String internKeyOrValue(String s) { return settingLiteralDeduplicator.deduplicate(s); } + + private record SettingsDiff(DiffableUtils.MapDiff> mapDiff) implements Diff { + + @Override + public Settings apply(Settings part) { + final var updated = mapDiff.apply(part.settings); + if (updated == part.settings) { + // noop map diff, no change to the settings + return part; + } + return Settings.of(updated, null); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + mapDiff.writeTo(out); + } + } } diff --git a/server/src/main/java/org/elasticsearch/index/analysis/NameOrDefinition.java b/server/src/main/java/org/elasticsearch/index/analysis/NameOrDefinition.java index e4e68c2f673c3..ca6fb800b6cb7 100644 --- a/server/src/main/java/org/elasticsearch/index/analysis/NameOrDefinition.java +++ b/server/src/main/java/org/elasticsearch/index/analysis/NameOrDefinition.java @@ -56,7 +56,7 @@ public void writeTo(StreamOutput out) throws IOException { boolean isNotNullDefinition = this.definition != null; out.writeBoolean(isNotNullDefinition); if (isNotNullDefinition) { - Settings.writeSettingsToStream(definition, out); + definition.writeTo(out); } } diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java index 64b7153d2a4ff..2cc11e68f9c5e 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java @@ -10,6 +10,7 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; +import org.elasticsearch.cluster.Diff; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; @@ -31,6 +32,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Set; @@ -431,7 +433,7 @@ public void testWriteSettingsToStream() throws IOException { builder.putList("test.key4.foo", "1", "2"); builder.setSecureSettings(secureSettings); assertEquals(7, builder.build().size()); - Settings.writeSettingsToStream(builder.build(), out); + builder.build().writeTo(out); StreamInput in = StreamInput.wrap(out.bytes().toBytesRef().bytes); Settings settings = Settings.readSettingsFromStream(in); assertEquals(3, settings.size()); @@ -441,6 +443,34 @@ public void testWriteSettingsToStream() throws IOException { assertEquals(Arrays.asList("1", "2"), settings.getAsList("test.key4.foo")); } + public void testDiff() throws IOException { + final Settings before = Settings.builder().put("foo", "bar").put("setting", "value").build(); + { + final Settings after = Settings.builder() + .put("foo", "bar") + .putNull("null_setting") + .putList("list_setting", List.of("a", "bbb", "ccc")) + .put("added_setting", "added") + .build(); + final Diff diff = after.diff(before); + BytesStreamOutput out = new BytesStreamOutput(); + diff.writeTo(out); + final Diff diffRead = Settings.readSettingsDiffFromStream(out.bytes().streamInput()); + final Settings afterFromDiff = diffRead.apply(before); + assertEquals(after, afterFromDiff); + } + + { + final Settings afterSameAsBefore = Settings.builder().put(before).build(); + final Diff diff = afterSameAsBefore.diff(before); + BytesStreamOutput out = new BytesStreamOutput(); + diff.writeTo(out); + final Diff diffRead = Settings.readSettingsDiffFromStream(out.bytes().streamInput()); + assertSame(before, diff.apply(before)); + assertSame(before, diffRead.apply(before)); + } + } + public void testSecureSettingConflict() { Setting setting = SecureSetting.secureString("something.secure", null); Settings settings = Settings.builder().put("something.secure", "notreallysecure").build(); @@ -596,7 +626,7 @@ public void testReadWriteArray() throws IOException { BytesStreamOutput output = new BytesStreamOutput(); output.setVersion(randomFrom(Version.CURRENT, Version.V_7_0_0)); Settings settings = Settings.builder().putList("foo.bar", "0", "1", "2", "3").put("foo.bar.baz", "baz").build(); - Settings.writeSettingsToStream(settings, output); + settings.writeTo(output); StreamInput in = StreamInput.wrap(BytesReference.toBytes(output.bytes())); Settings build = Settings.readSettingsFromStream(in); assertEquals(2, build.size()); diff --git a/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/action/PutAutoscalingPolicyAction.java b/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/action/PutAutoscalingPolicyAction.java index 64b29e1d83eaa..3a8f09f021d1b 100644 --- a/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/action/PutAutoscalingPolicyAction.java +++ b/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/action/PutAutoscalingPolicyAction.java @@ -118,7 +118,7 @@ public void writeTo(final StreamOutput out) throws IOException { out.writeInt(deciders.size()); for (Map.Entry entry : deciders.entrySet()) { out.writeString(entry.getKey()); - Settings.writeSettingsToStream(entry.getValue(), out); + entry.getValue().writeTo(out); } } else { out.writeBoolean(false); diff --git a/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/policy/AutoscalingPolicy.java b/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/policy/AutoscalingPolicy.java index a2c31c124c747..64a50687acba1 100644 --- a/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/policy/AutoscalingPolicy.java +++ b/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/policy/AutoscalingPolicy.java @@ -103,7 +103,7 @@ public void writeTo(final StreamOutput out) throws IOException { out.writeInt(deciders.size()); for (Map.Entry entry : deciders.entrySet()) { out.writeString(entry.getKey()); - Settings.writeSettingsToStream(entry.getValue(), out); + entry.getValue().writeTo(out); } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/AutoFollowMetadata.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/AutoFollowMetadata.java index fb127b2547883..d74be00ef18f3 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/AutoFollowMetadata.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/AutoFollowMetadata.java @@ -367,7 +367,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeStringCollection(leaderIndexPatterns); out.writeOptionalString(followIndexPattern); if (out.getVersion().onOrAfter(Version.V_7_9_0)) { - Settings.writeSettingsToStream(settings, out); + settings.writeTo(out); } super.writeTo(out); if (out.getVersion().onOrAfter(Version.V_7_5_0)) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/PutAutoFollowPatternAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/PutAutoFollowPatternAction.java index a3154cfec7925..a7080131001ea 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/PutAutoFollowPatternAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/PutAutoFollowPatternAction.java @@ -202,7 +202,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeStringCollection(leaderIndexPatterns); out.writeOptionalString(followIndexNamePattern); if (out.getVersion().onOrAfter(Version.V_7_9_0)) { - Settings.writeSettingsToStream(settings, out); + settings.writeTo(out); } parameters.writeTo(out); if (out.getVersion().onOrAfter(Version.V_7_14_0)) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/PutFollowAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/PutFollowAction.java index cf4846b761041..910cf956c5dac 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/PutFollowAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/PutFollowAction.java @@ -188,7 +188,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(leaderIndex); out.writeString(followerIndex); if (out.getVersion().onOrAfter(Version.V_7_9_0)) { - Settings.writeSettingsToStream(settings, out); + settings.writeTo(out); } parameters.writeTo(out); waitForActiveShards.writeTo(out); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/searchablesnapshots/MountSearchableSnapshotRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/searchablesnapshots/MountSearchableSnapshotRequest.java index 0c2a28e576556..88bad8d1e20db 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/searchablesnapshots/MountSearchableSnapshotRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/searchablesnapshots/MountSearchableSnapshotRequest.java @@ -31,7 +31,6 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; -import static org.elasticsearch.common.settings.Settings.writeSettingsToStream; import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg; import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg; @@ -129,7 +128,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(repositoryName); out.writeString(snapshotName); out.writeString(snapshotIndexName); - writeSettingsToStream(indexSettings, out); + indexSettings.writeTo(out); out.writeStringArray(ignoredIndexSettings); out.writeBoolean(waitForCompletion); if (out.getVersion().onOrAfter(SHARED_CACHE_VERSION)) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/TransformDestIndexSettings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/TransformDestIndexSettings.java index adda7102d6393..c967c2177a819 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/TransformDestIndexSettings.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/TransformDestIndexSettings.java @@ -118,7 +118,7 @@ public static TransformDestIndexSettings fromXContent(final XContentParser parse @Override public void writeTo(StreamOutput out) throws IOException { out.writeGenericMap(mappings); - Settings.writeSettingsToStream(settings, out); + settings.writeTo(out); out.writeCollection(aliases); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlScalingReason.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlScalingReason.java index c6d2962b3a3d1..c42f3dc83e52f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlScalingReason.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlScalingReason.java @@ -139,7 +139,7 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_8_0_0)) { out.writeStringCollection(this.waitingModels); } - Settings.writeSettingsToStream(this.passedConfiguration, out); + this.passedConfiguration.writeTo(out); this.currentMlCapacity.writeTo(out); out.writeOptionalWriteable(this.requiredCapacity); out.writeOptionalVLong(largestWaitingAnalyticsJob);