honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Save Some Allocations when Working with ClusterState (#62060) (#62303) 

Just a number of obvious spots where we were allocating
duplicate empty structures or otherwise inefficient that I
found while investigating snapshot cluster state update performance.
This commit is contained in:
Armin Braun 2020-09-14 15:09:54 +02:00 committed by GitHub
parent 9e38dd0254
commit 95766da345
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 76 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
server/src/main/java/org/elasticsearch/cluster/AbstractDiffable.java
View File

@ -31,17 +31,24 @@ import java.io.IOException;
*/
public abstract class AbstractDiffable<T extends Diffable<T>> implements Diffable<T> {
private static final Diff<?> EMPTY = new CompleteDiff<>();
@SuppressWarnings("unchecked")
@Override
public Diff<T> diff(T previousState) {
if (this.get().equals(previousState)) {
return new CompleteDiff<>();
if (this.equals(previousState)) {
return (Diff<T>) EMPTY;
} else {
return new CompleteDiff<>(get());
return new CompleteDiff<>((T) this);
}
}
@SuppressWarnings("unchecked")
public static <T extends Diffable<T>> Diff<T> readDiffFrom(Reader<T> reader, StreamInput in) throws IOException {
return new CompleteDiff<T>(reader, in);
if (in.readBoolean()) {
return new CompleteDiff<>(reader.read(in));
}
return (Diff<T>) EMPTY;
}
private static class CompleteDiff<T extends Diffable<T>> implements Diff<T> {
@ -63,17 +70,6 @@ public abstract class AbstractDiffable<T extends Diffable<T>> implements Diffabl
this.part = null;
}
/**
* Read simple diff from the stream
*/
CompleteDiff(Reader<T> reader, StreamInput in) throws IOException {
if (in.readBoolean()) {
this.part = reader.read(in);
} else {
this.part = null;
}
}
@Override
public void writeTo(StreamOutput out) throws IOException {
if (part != null) {
@ -93,10 +89,5 @@ public abstract class AbstractDiffable<T extends Diffable<T>> implements Diffabl
}
}
}
@SuppressWarnings("unchecked")
public T get() {
return (T) this;
}
}

2
server/src/main/java/org/elasticsearch/cluster/ClusterState.java
View File

@ -705,7 +705,7 @@ public class ClusterState implements ToXContentFragment, Diffable<ClusterState>
builder.metadata = Metadata.readFrom(in);
builder.routingTable = RoutingTable.readFrom(in);
builder.nodes = DiscoveryNodes.readFrom(in, localNode);
builder.blocks = new ClusterBlocks(in);
builder.blocks = ClusterBlocks.readFrom(in);
int customSize = in.readVInt();
for (int i = 0; i < customSize; i++) {
Custom customIndexMetadata = in.readNamedWriteable(Custom.class);

21
server/src/main/java/org/elasticsearch/cluster/DiffableUtils.java
View File

@ -147,8 +147,8 @@ public final class DiffableUtils {
* Loads an object that represents difference between two ImmutableOpenMaps of Diffable objects using Diffable proto object
*/
public static <K, T extends Diffable<T>> MapDiff<K, T, ImmutableOpenMap<K, T>> readImmutableOpenMapDiff(StreamInput in,
KeySerializer<K> keySerializer, Reader<T> reader, Reader<Diff<T>> diffReader) throws IOException {
return new ImmutableOpenMapDiff<>(in, keySerializer, new DiffableValueReader<>(reader, diffReader));
KeySerializer<K> keySerializer, DiffableValueReader<K, T> diffableValueReader) throws IOException {
return new ImmutableOpenMapDiff<>(in, keySerializer, diffableValueReader);
}
/**
@ -393,20 +393,16 @@ public final class DiffableUtils {
protected MapDiff(StreamInput in, KeySerializer<K> keySerializer, ValueSerializer<K, T> valueSerializer) throws IOException {
this.keySerializer = keySerializer;
this.valueSerializer = valueSerializer;
deletes = new ArrayList<>();
diffs = new HashMap<>();
upserts = new HashMap<>();
int deletesCount = in.readVInt();
for (int i = 0; i < deletesCount; i++) {
deletes.add(keySerializer.readKey(in));
}
deletes = in.readList(keySerializer::readKey);
int diffsCount = in.readVInt();
diffs = diffsCount == 0 ? Collections.emptyMap() : new HashMap<>(diffsCount);
for (int i = 0; i < diffsCount; i++) {
K key = keySerializer.readKey(in);
Diff<T> diff = valueSerializer.readDiff(in, key);
diffs.put(key, diff);
}
int upsertsCount = in.readVInt();
upserts = upsertsCount == 0 ? Collections.emptyMap() : new HashMap<>(upsertsCount);
for (int i = 0; i < upsertsCount; i++) {
K key = keySerializer.readKey(in);
T newValue = valueSerializer.read(in, key);
@ -446,10 +442,7 @@ public final class DiffableUtils {
@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeVInt(deletes.size());
for (K delete : deletes) {
keySerializer.writeKey(delete, out);
}
out.writeCollection(deletes, (o, v) -> keySerializer.writeKey(v, o));
Version version = out.getVersion();
// filter out custom states not supported by the other node
int diffCount = 0;
@ -715,7 +708,7 @@ public final class DiffableUtils {
@Override
public void write(Set<String> value, StreamOutput out) throws IOException {
out.writeStringArray(value.toArray(new String[value.size()]));
out.writeStringCollection(value);
}
@Override

25
server/src/main/java/org/elasticsearch/cluster/block/ClusterBlocks.java
View File

@ -283,23 +283,23 @@ public class ClusterBlocks extends AbstractDiffable<ClusterBlocks> {
out.writeCollection(blocks);
}
public ClusterBlocks(StreamInput in) throws IOException {
this.global = readBlockSet(in);
this.indicesBlocks = in.readImmutableMap(i -> i.readString().intern(), ClusterBlocks::readBlockSet);
levelHolders = generateLevelHolders(global, indicesBlocks);
public static ClusterBlocks readFrom(StreamInput in) throws IOException {
final Set<ClusterBlock> global = readBlockSet(in);
ImmutableOpenMap<String, Set<ClusterBlock>> indicesBlocks =
in.readImmutableMap(i -> i.readString().intern(), ClusterBlocks::readBlockSet);
if (global.isEmpty() && indicesBlocks.isEmpty()) {
return EMPTY_CLUSTER_BLOCK;
}
return new ClusterBlocks(global, indicesBlocks);
}
private static Set<ClusterBlock> readBlockSet(StreamInput in) throws IOException {
int totalBlocks = in.readVInt();
Set<ClusterBlock> blocks = new HashSet<>(totalBlocks);
for (int i = 0; i < totalBlocks;i++) {
blocks.add(new ClusterBlock(in));
}
return unmodifiableSet(blocks);
final Set<ClusterBlock> blocks = in.readSet(ClusterBlock::new);
return blocks.isEmpty() ? blocks : unmodifiableSet(blocks);
}
public static Diff<ClusterBlocks> readDiffFrom(StreamInput in) throws IOException {
return AbstractDiffable.readDiffFrom(ClusterBlocks::new, in);
return AbstractDiffable.readDiffFrom(ClusterBlocks::readFrom, in);
}
static class ImmutableLevelHolder {
@ -434,6 +434,9 @@ public class ClusterBlocks extends AbstractDiffable<ClusterBlocks> {
}
public ClusterBlocks build() {
if (indices.isEmpty() && global.isEmpty()) {
return EMPTY_CLUSTER_BLOCK;
}
// We copy the block sets here in case of the builder is modified after build is called
ImmutableOpenMap.Builder<String, Set<ClusterBlock>> indicesBuilder = ImmutableOpenMap.builder(indices.size());
for (Map.Entry<String, Set<ClusterBlock>> entry : indices.entrySet()) {

11
server/src/main/java/org/elasticsearch/cluster/metadata/DiffableStringMap.java
View File

@ -43,13 +43,14 @@ public class DiffableStringMap extends AbstractMap<String, String> implements Di
private final Map<String, String> innerMap;
DiffableStringMap(final Map<String, String> map) {
this.innerMap = Collections.unmodifiableMap(map);
@SuppressWarnings("unchecked")
public static DiffableStringMap readFrom(StreamInput in) throws IOException {
final Map<String, String> map = (Map) in.readMap();
return map.isEmpty() ? EMPTY : new DiffableStringMap(map);
}
@SuppressWarnings("unchecked")
DiffableStringMap(final StreamInput in) throws IOException {
this((Map<String, String>) (Map) in.readMap());
DiffableStringMap(final Map<String, String> map) {
this.innerMap = Collections.unmodifiableMap(map);
}
@Override

44
server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java
View File

@ -790,6 +790,15 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
isSystem = after.isSystem;
}
private static final DiffableUtils.DiffableValueReader<String, AliasMetadata> ALIAS_METADATA_DIFF_VALUE_READER =
new DiffableUtils.DiffableValueReader<>(AliasMetadata::new, AliasMetadata::readDiffFrom);
private static final DiffableUtils.DiffableValueReader<String, MappingMetadata> MAPPING_DIFF_VALUE_READER =
new DiffableUtils.DiffableValueReader<>(MappingMetadata::new, MappingMetadata::readDiffFrom);
private static final DiffableUtils.DiffableValueReader<String, DiffableStringMap> CUSTOM_DIFF_VALUE_READER =
new DiffableUtils.DiffableValueReader<>(DiffableStringMap::readFrom, DiffableStringMap::readDiffFrom);
private static final DiffableUtils.DiffableValueReader<String, RolloverInfo> ROLLOVER_INFO_DIFF_VALUE_READER =
new DiffableUtils.DiffableValueReader<>(RolloverInfo::new, RolloverInfo::readDiffFrom);
IndexMetadataDiff(StreamInput in) throws IOException {
index = in.readString();
routingNumShards = in.readInt();
@ -812,21 +821,13 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
state = State.fromId(in.readByte());
settings = Settings.readSettingsFromStream(in);
primaryTerms = in.readVLongArray();
mappings = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), MappingMetadata::new,
MappingMetadata::readDiffFrom);
aliases = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), AliasMetadata::new,
AliasMetadata::readDiffFrom);
customData = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), DiffableStringMap::new,
DiffableStringMap::readDiffFrom);
mappings = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), MAPPING_DIFF_VALUE_READER);
aliases = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), ALIAS_METADATA_DIFF_VALUE_READER);
customData = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), CUSTOM_DIFF_VALUE_READER);
inSyncAllocationIds = DiffableUtils.readImmutableOpenIntMapDiff(in, DiffableUtils.getVIntKeySerializer(),
DiffableUtils.StringSetValueSerializer.getInstance());
if (in.getVersion().onOrAfter(Version.V_6_4_0)) {
rolloverInfos = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), RolloverInfo::new,
RolloverInfo::readDiffFrom);
} else {
ImmutableOpenMap<String, RolloverInfo> emptyMap = ImmutableOpenMap.of();
rolloverInfos = DiffableUtils.diff(emptyMap, emptyMap, DiffableUtils.getStringKeySerializer());
}
DiffableUtils.StringSetValueSerializer.getInstance());
rolloverInfos =
DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), ROLLOVER_INFO_DIFF_VALUE_READER);
if (in.getVersion().onOrAfter(SYSTEM_INDEX_FLAG_ADDED)) {
isSystem = in.readBoolean();
} else {
@ -917,17 +918,10 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
builder.putAlias(aliasMd);
}
int customSize = in.readVInt();
if (in.getVersion().onOrAfter(Version.V_6_5_0)) {
for (int i = 0; i < customSize; i++) {
String key = in.readString();
DiffableStringMap custom = new DiffableStringMap(in);
builder.putCustom(key, custom);
}
} else {
assert customSize == 0 : "expected no custom index metadata";
if (customSize > 0) {
throw new IllegalStateException("unexpected custom metadata when none is supported");
}
for (int i = 0; i < customSize; i++) {
String key = in.readString();
DiffableStringMap custom = DiffableStringMap.readFrom(in);
builder.putCustom(key, custom);
}
int inSyncAllocationIdsSize = in.readVInt();
for (int i = 0; i < inSyncAllocationIdsSize; i++) {

13
server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java
View File

@ -921,6 +921,11 @@ public class Metadata implements Iterable<IndexMetadata>, Diffable<Metadata>, To
customs = DiffableUtils.diff(before.customs, after.customs, DiffableUtils.getStringKeySerializer(), CUSTOM_VALUE_SERIALIZER);
}
private static final DiffableUtils.DiffableValueReader<String, IndexMetadata> INDEX_METADATA_DIFF_VALUE_READER =
new DiffableUtils.DiffableValueReader<>(IndexMetadata::readFrom, IndexMetadata::readDiffFrom);
private static final DiffableUtils.DiffableValueReader<String, IndexTemplateMetadata> TEMPLATES_DIFF_VALUE_READER =
new DiffableUtils.DiffableValueReader<>(IndexTemplateMetadata::readFrom, IndexTemplateMetadata::readDiffFrom);
MetadataDiff(StreamInput in) throws IOException {
clusterUUID = in.readString();
if (in.getVersion().onOrAfter(Version.V_7_0_0)) {
@ -939,10 +944,8 @@ public class Metadata implements Iterable<IndexMetadata>, Diffable<Metadata>, To
} else {
hashesOfConsistentSettings = DiffableStringMap.DiffableStringMapDiff.EMPTY;
}
indices = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), IndexMetadata::readFrom,
IndexMetadata::readDiffFrom);
templates = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), IndexTemplateMetadata::readFrom,
IndexTemplateMetadata::readDiffFrom);
indices = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), INDEX_METADATA_DIFF_VALUE_READER);
templates = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), TEMPLATES_DIFF_VALUE_READER);
customs = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), CUSTOM_VALUE_SERIALIZER);
}
@ -996,7 +999,7 @@ public class Metadata implements Iterable<IndexMetadata>, Diffable<Metadata>, To
builder.transientSettings(readSettingsFromStream(in));
builder.persistentSettings(readSettingsFromStream(in));
if (in.getVersion().onOrAfter(Version.V_7_3_0)) {
builder.hashesOfConsistentSettings(new DiffableStringMap(in));
builder.hashesOfConsistentSettings(DiffableStringMap.readFrom(in));
}
int size = in.readVInt();
for (int i = 0; i < size; i++) {

6
server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java
View File

@ -365,10 +365,12 @@ public class RoutingTable implements Iterable<IndexRoutingTable>, Diffable<Routi
indicesRouting = DiffableUtils.diff(before.indicesRouting, after.indicesRouting, DiffableUtils.getStringKeySerializer());
}
private static final DiffableUtils.DiffableValueReader<String, IndexRoutingTable> DIFF_VALUE_READER =
new DiffableUtils.DiffableValueReader<>(IndexRoutingTable::readFrom, IndexRoutingTable::readDiffFrom);
RoutingTableDiff(StreamInput in) throws IOException {
version = in.readLong();
indicesRouting = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), IndexRoutingTable::readFrom,
IndexRoutingTable::readDiffFrom);
indicesRouting = DiffableUtils.readImmutableOpenMapDiff(in, DiffableUtils.getStringKeySerializer(), DIFF_VALUE_READER);
}
@Override

2
server/src/test/java/org/elasticsearch/cluster/metadata/DiffableStringMapTests.java
View File

@ -97,7 +97,7 @@ public class DiffableStringMapTests extends ESTestCase {
BytesStreamOutput bso = new BytesStreamOutput();
dsm.writeTo(bso);
DiffableStringMap deserialized = new DiffableStringMap(bso.bytes().streamInput());
DiffableStringMap deserialized = DiffableStringMap.readFrom(bso.bytes().streamInput());
assertThat(deserialized, equalTo(dsm));
}
}

3
server/src/test/java/org/elasticsearch/cluster/serialization/DiffableTests.java
View File

@ -113,7 +113,8 @@ public class DiffableTests extends ESTestCase {
@Override
protected MapDiff readDiff(StreamInput in) throws IOException {
return useProtoForDiffableSerialization
? DiffableUtils.readImmutableOpenMapDiff(in, keySerializer, TestDiffable::readFrom, TestDiffable::readDiffFrom)
? DiffableUtils.readImmutableOpenMapDiff(in, keySerializer,
new DiffableUtils.DiffableValueReader<>(TestDiffable::readFrom, TestDiffable::readDiffFrom))
: DiffableUtils.readImmutableOpenMapDiff(in, keySerializer, diffableValueSerializer());
}
}.execute();

2
server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java
View File

@ -2872,7 +2872,7 @@ public class IndexShardTests extends IndexShardTestCase {
}
assertThat(requestedMappingUpdates, hasKey("_doc"));
assertThat(requestedMappingUpdates.get("_doc").get().source().string(),
assertThat(requestedMappingUpdates.get("_doc").source().string(),
equalTo("{\"properties\":{\"foo\":{\"type\":\"text\"}}}"));
closeShards(sourceShard, targetShard);

4
x-pack/plugin/stack/src/test/java/org/elasticsearch/xpack/stack/StackTemplateRegistryTests.java
View File

@ -181,7 +181,7 @@ public class StackTemplateRegistryTests extends ESTestCase {
.map(policyConfig -> policyConfig.load(xContentRegistry))
.collect(Collectors.toList());
assertThat(policies, hasSize(2));
policies.forEach(p -> policyMap.put(p.getName(), p.get()));
policies.forEach(p -> policyMap.put(p.getName(), p));
client.setVerifier((action, request, listener) -> {
if (action instanceof PutComponentTemplateAction) {
@ -210,7 +210,7 @@ public class StackTemplateRegistryTests extends ESTestCase {
.map(policyConfig -> policyConfig.load(xContentRegistry))
.collect(Collectors.toList());
assertThat(policies, hasSize(2));
policies.forEach(p -> policyMap.put(p.getName(), p.get()));
policies.forEach(p -> policyMap.put(p.getName(), p));
client.setVerifier((action, request, listener) -> {
if (action instanceof PutComponentTemplateAction) {