diff --git a/server/src/main/java/org/elasticsearch/cluster/DiffableUtils.java b/server/src/main/java/org/elasticsearch/cluster/DiffableUtils.java index 8749397323fdf..f5fb4e797aa58 100644 --- a/server/src/main/java/org/elasticsearch/cluster/DiffableUtils.java +++ b/server/src/main/java/org/elasticsearch/cluster/DiffableUtils.java @@ -21,13 +21,14 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Function; public final class DiffableUtils { private DiffableUtils() {} - private static final MapDiff EMPTY = new MapDiff<>(null, null, List.of(), List.of(), List.of()) { + private static final MapDiff EMPTY = new MapDiff<>(null, null, List.of(), List.of(), List.of(), null) { @Override - public Object apply(Object part) { + public Map apply(Map part) { return part; } }; @@ -53,62 +54,31 @@ public static KeySerializer getVIntKeySerializer() { return VIntKeySerializer.INSTANCE; } - /** - * Calculates diff between two ImmutableOpenMaps of Diffable objects - */ - public static > MapDiff> diff( - ImmutableOpenMap before, - ImmutableOpenMap after, - KeySerializer keySerializer - ) { - assert after != null && before != null; - return before.equals(after) - ? emptyDiff() - : ImmutableOpenMapDiff.create(before, after, keySerializer, DiffableValueSerializer.getWriteOnlyInstance()); - } - - /** - * Calculates diff between two ImmutableOpenMaps of non-diffable objects - */ - public static MapDiff> diff( - ImmutableOpenMap before, - ImmutableOpenMap after, - KeySerializer keySerializer, - ValueSerializer valueSerializer - ) { - assert after != null && before != null; - return before.equals(after) ? emptyDiff() : ImmutableOpenMapDiff.create(before, after, keySerializer, valueSerializer); - } - /** * Calculates diff between two Maps of Diffable objects. */ - public static > MapDiff> diff( - Map before, - Map after, - KeySerializer keySerializer - ) { + public static , M extends Map> MapDiff diff(M before, M after, KeySerializer keySerializer) { assert after != null && before != null; return before.equals(after) ? emptyDiff() - : JdkMapDiff.create(before, after, keySerializer, DiffableValueSerializer.getWriteOnlyInstance()); + : createDiff(before, after, keySerializer, DiffableValueSerializer.getWriteOnlyInstance()); } /** * Calculates diff between two Maps of non-diffable objects */ - public static MapDiff> diff( - Map before, - Map after, + public static > MapDiff diff( + M before, + M after, KeySerializer keySerializer, ValueSerializer valueSerializer ) { assert after != null && before != null; - return before.equals(after) ? emptyDiff() : JdkMapDiff.create(before, after, keySerializer, valueSerializer); + return before.equals(after) ? emptyDiff() : createDiff(before, after, keySerializer, valueSerializer); } @SuppressWarnings("unchecked") - private static MapDiff emptyDiff() { + public static > MapDiff emptyDiff() { return (MapDiff) EMPTY; } @@ -120,7 +90,7 @@ public static MapDiff> readImmutableOpenMapD KeySerializer keySerializer, ValueSerializer valueSerializer ) throws IOException { - return diffOrEmpty(new ImmutableOpenMapDiff<>(in, keySerializer, valueSerializer)); + return diffOrEmpty(new MapDiff<>(in, keySerializer, valueSerializer, ImmutableOpenMapBuilder::new)); } /** @@ -131,7 +101,7 @@ public static MapDiff> readJdkMapDiff( KeySerializer keySerializer, ValueSerializer valueSerializer ) throws IOException { - return diffOrEmpty(new JdkMapDiff<>(in, keySerializer, valueSerializer)); + return diffOrEmpty(new MapDiff<>(in, keySerializer, valueSerializer, JdkMapBuilder::new)); } /** @@ -142,7 +112,7 @@ public static > MapDiff> r KeySerializer keySerializer, DiffableValueReader diffableValueReader ) throws IOException { - return diffOrEmpty(new ImmutableOpenMapDiff<>(in, keySerializer, diffableValueReader)); + return diffOrEmpty(new MapDiff<>(in, keySerializer, diffableValueReader, ImmutableOpenMapBuilder::new)); } /** @@ -154,10 +124,10 @@ public static > MapDiff> readJdkMapDiff Reader reader, Reader> diffReader ) throws IOException { - return diffOrEmpty(new JdkMapDiff<>(in, keySerializer, new DiffableValueReader<>(reader, diffReader))); + return diffOrEmpty(new MapDiff<>(in, keySerializer, new DiffableValueReader<>(reader, diffReader), JdkMapBuilder::new)); } - private static MapDiff diffOrEmpty(MapDiff diff) { + private static > MapDiff diffOrEmpty(MapDiff diff) { // TODO: refactor map diff reading to avoid having to construct empty diffs before throwing them away here if (diff.getUpserts().isEmpty() && diff.getDiffs().isEmpty() && diff.getDeletes().isEmpty()) { return emptyDiff(); @@ -165,166 +135,127 @@ private static MapDiff diffOrEmpty(MapDiff diff) { return diff; } - /** - * Represents differences between two Maps of (possibly diffable) objects. - * - * @param the diffable object - */ - private static class JdkMapDiff extends MapDiff> { - - private JdkMapDiff( - KeySerializer keySerializer, - ValueSerializer valueSerializer, - List deletes, - List>> diffs, - List> upserts - ) { - super(keySerializer, valueSerializer, deletes, diffs, upserts); - } - - private JdkMapDiff(StreamInput in, KeySerializer keySerializer, ValueSerializer valueSerializer) throws IOException { - super(in, keySerializer, valueSerializer); - } + private static > MapDiff createDiff( + M before, + M after, + KeySerializer keySerializer, + ValueSerializer valueSerializer + ) { + assert after != null && before != null; - private static JdkMapDiff create( - Map before, - Map after, - KeySerializer keySerializer, - ValueSerializer valueSerializer - ) { - assert after != null && before != null; - - int inserts = 0; - var upserts = new ArrayList>(); - var diffs = new ArrayList>>(); - for (Map.Entry entry : after.entrySet()) { - T previousValue = before.get(entry.getKey()); - if (previousValue == null) { + int inserts = 0; + var upserts = new ArrayList>(); + var diffs = new ArrayList>>(); + for (Map.Entry entry : after.entrySet()) { + T previousValue = before.get(entry.getKey()); + if (previousValue == null) { + upserts.add(entry); + inserts++; + } else if (entry.getValue().equals(previousValue) == false) { + if (valueSerializer.supportsDiffableValues()) { + diffs.add(Map.entry(entry.getKey(), valueSerializer.diff(entry.getValue(), previousValue))); + } else { upserts.add(entry); - inserts++; - } else if (entry.getValue().equals(previousValue) == false) { - if (valueSerializer.supportsDiffableValues()) { - diffs.add(Map.entry(entry.getKey(), valueSerializer.diff(entry.getValue(), previousValue))); - } else { - upserts.add(entry); - } } } + } - int expectedDeletes = before.size() + inserts - after.size(); - var deletes = new ArrayList(expectedDeletes); - if (expectedDeletes > 0) { - for (K key : before.keySet()) { - if (after.containsKey(key) == false) { - deletes.add(key); - if (--expectedDeletes == 0) { - break; - } + int expectedDeletes = before.size() + inserts - after.size(); + var deletes = new ArrayList(expectedDeletes); + if (expectedDeletes > 0) { + for (K key : before.keySet()) { + if (after.containsKey(key) == false) { + deletes.add(key); + if (--expectedDeletes == 0) { + break; } } } + } - return new JdkMapDiff<>(keySerializer, valueSerializer, deletes, diffs, upserts); + Function> builderCtor; + if (before instanceof ImmutableOpenMap) { + builderCtor = DiffableUtils::createImmutableMapBuilder; + } else { + builderCtor = DiffableUtils::createJdkMapBuilder; } - @Override - public Map apply(Map map) { - Map builder = new HashMap<>(map); + return new MapDiff<>(keySerializer, valueSerializer, deletes, diffs, upserts, builderCtor); + } - for (K part : deletes) { - builder.remove(part); - } + @SuppressWarnings("unchecked") + private static > MapBuilder createImmutableMapBuilder(Map m) { + assert m instanceof ImmutableOpenMap; + return (MapBuilder) new ImmutableOpenMapBuilder<>((ImmutableOpenMap) m); + } - for (Map.Entry> diff : diffs) { - builder.put(diff.getKey(), diff.getValue().apply(builder.get(diff.getKey()))); - } + @SuppressWarnings("unchecked") + private static > MapBuilder createJdkMapBuilder(Map m) { + return (MapBuilder) new JdkMapBuilder<>(m); + } - for (Map.Entry upsert : upserts) { - builder.put(upsert.getKey(), upsert.getValue()); - } + private interface MapBuilder> { + void remove(K key); - return Collections.unmodifiableMap(builder); - } + T get(K key); + + void put(K key, T value); + + M build(); } - /** - * Represents differences between two ImmutableOpenMap of (possibly diffable) objects - * - * @param the object type - */ - public static class ImmutableOpenMapDiff extends MapDiff> { + private static class JdkMapBuilder implements MapBuilder> { + private final Map map; - private ImmutableOpenMapDiff( - KeySerializer keySerializer, - ValueSerializer valueSerializer, - List deletes, - List>> diffs, - List> upserts - ) { - super(keySerializer, valueSerializer, deletes, diffs, upserts); + JdkMapBuilder(Map map) { + this.map = new HashMap<>(map); } - private ImmutableOpenMapDiff(StreamInput in, KeySerializer keySerializer, ValueSerializer valueSerializer) - throws IOException { - super(in, keySerializer, valueSerializer); + @Override + public void remove(K key) { + map.remove(key); } - private static ImmutableOpenMapDiff create( - ImmutableOpenMap before, - ImmutableOpenMap after, - KeySerializer keySerializer, - ValueSerializer valueSerializer - ) { - assert after != null && before != null; - - int inserts = 0; - var upserts = new ArrayList>(); - var diffs = new ArrayList>>(); - for (Map.Entry entry : after.entrySet()) { - T beforeValue = before.get(entry.getKey()); - if (beforeValue == null) { - upserts.add(entry); - inserts++; - } else if (entry.getValue().equals(beforeValue) == false) { - if (valueSerializer.supportsDiffableValues()) { - diffs.add(Map.entry(entry.getKey(), valueSerializer.diff(entry.getValue(), beforeValue))); - } else { - upserts.add(entry); - } - } - } + @Override + public T get(K key) { + return map.get(key); + } - int expectedDeletes = before.size() + inserts - after.size(); - var deletes = new ArrayList(expectedDeletes); - if (expectedDeletes > 0) { - for (Map.Entry key : before.entrySet()) { - if (after.containsKey(key.getKey()) == false) { - deletes.add(key.getKey()); - if (--expectedDeletes == 0) { - break; - } - } - } - } + @Override + public void put(K key, T value) { + map.put(key, value); + } + + @Override + public Map build() { + return Collections.unmodifiableMap(map); + } + } - return new ImmutableOpenMapDiff<>(keySerializer, valueSerializer, deletes, diffs, upserts); + private static class ImmutableOpenMapBuilder implements MapBuilder> { + private final ImmutableOpenMap.Builder builder; + + ImmutableOpenMapBuilder(ImmutableOpenMap map) { + this.builder = ImmutableOpenMap.builder(map); } @Override - public ImmutableOpenMap apply(ImmutableOpenMap map) { - ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(map); + public void remove(K key) { + builder.remove(key); + } - for (K part : deletes) { - builder.remove(part); - } + @Override + public T get(K key) { + return builder.get(key); + } - for (Map.Entry> diff : diffs) { - builder.put(diff.getKey(), diff.getValue().apply(builder.get(diff.getKey()))); - } + @Override + public void put(K key, T value) { + builder.put(key, value); + } - for (Map.Entry upsert : upserts) { - builder.put(upsert.getKey(), upsert.getValue()); - } + @Override + public ImmutableOpenMap build() { return builder.build(); } } @@ -336,31 +267,38 @@ public ImmutableOpenMap apply(ImmutableOpenMap map) { * * @param the type of map keys * @param the type of map values - * @param the map implementation type */ - public abstract static class MapDiff implements Diff { + public static class MapDiff> implements Diff { protected final List deletes; protected final List>> diffs; // incremental updates protected final List> upserts; // additions or full updates protected final KeySerializer keySerializer; protected final ValueSerializer valueSerializer; + private final Function> builderCtor; - protected MapDiff( + private MapDiff( KeySerializer keySerializer, ValueSerializer valueSerializer, List deletes, List>> diffs, - List> upserts + List> upserts, + Function> builderCtor ) { this.keySerializer = keySerializer; this.valueSerializer = valueSerializer; this.deletes = deletes; this.diffs = diffs; this.upserts = upserts; + this.builderCtor = builderCtor; } - protected MapDiff(StreamInput in, KeySerializer keySerializer, ValueSerializer valueSerializer) throws IOException { + private MapDiff( + StreamInput in, + KeySerializer keySerializer, + ValueSerializer valueSerializer, + Function> builderCtor + ) throws IOException { this.keySerializer = keySerializer; this.valueSerializer = valueSerializer; deletes = in.readList(keySerializer::readKey); @@ -378,6 +316,25 @@ protected MapDiff(StreamInput in, KeySerializer keySerializer, ValueSerialize T newValue = valueSerializer.read(in, key); upserts.add(Map.entry(key, newValue)); } + this.builderCtor = builderCtor; + } + + @Override + public M apply(M map) { + MapBuilder builder = builderCtor.apply(map); + + for (K part : deletes) { + builder.remove(part); + } + + for (Map.Entry> diff : diffs) { + builder.put(diff.getKey(), diff.getValue().apply(builder.get(diff.getKey()))); + } + + for (Map.Entry upsert : upserts) { + builder.put(upsert.getKey(), upsert.getValue()); + } + return builder.build(); } /** diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/ImmutableStateMetadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/ImmutableStateMetadata.java index f876cf1328c47..22d8197aad8b3 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/ImmutableStateMetadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/ImmutableStateMetadata.java @@ -9,7 +9,6 @@ package org.elasticsearch.cluster.metadata; import org.elasticsearch.cluster.Diff; -import org.elasticsearch.cluster.DiffableUtils; import org.elasticsearch.cluster.SimpleDiffable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -126,17 +125,6 @@ public static Diff readDiffFrom(StreamInput in) throws I return SimpleDiffable.readDiffFrom(ImmutableStateMetadata::readFrom, in); } - /** - * Empty {@link org.elasticsearch.cluster.DiffableUtils.MapDiff} helper for metadata backwards compatibility. - */ - public static final DiffableUtils.MapDiff> EMPTY_DIFF = - new DiffableUtils.MapDiff<>(null, null, List.of(), List.of(), List.of()) { - @Override - public Map apply(Map part) { - return part; - } - }; - /** * Convenience method for creating a {@link Builder} for {@link ImmutableStateMetadata} * 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 b7dea61814dc5..ac94180de6eaa 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java @@ -1175,7 +1175,7 @@ private static class MetadataDiff implements Diff { IMMUTABLE_DIFF_VALUE_READER ); } else { - immutableStateMetadata = ImmutableStateMetadata.EMPTY_DIFF; + immutableStateMetadata = DiffableUtils.emptyDiff(); } } diff --git a/server/src/test/java/org/elasticsearch/cluster/serialization/DiffableTests.java b/server/src/test/java/org/elasticsearch/cluster/serialization/DiffableTests.java index da8eb24251b78..64dee2014f353 100644 --- a/server/src/test/java/org/elasticsearch/cluster/serialization/DiffableTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/serialization/DiffableTests.java @@ -149,7 +149,7 @@ protected MapDiff> readDiff(S * @param map type * @param value type */ - public abstract class MapDriver { + public abstract class MapDriver, V> { protected final Set keys = randomPositiveIntSet(); protected final Set keysToRemove = new HashSet<>(randomSubsetOf(randomInt(keys.size()), keys.toArray(new Integer[0]))); protected final Set keysThatAreNotRemoved = Sets.difference(keys, keysToRemove);