Skip to content

More Compact Serialization of Metadata #82608

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 14, 2022
Merged

More Compact Serialization of Metadata #82608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,15 @@ public IndexMetadata apply(IndexMetadata part) {
}

public static IndexMetadata readFrom(StreamInput in) throws IOException {
return readFrom(in, null);
}

/**
* @param mappingLookup optional lookup function that translates mapping metadata hashes into concrete instances. If specified we
* assume that the stream contains only mapping metadata hashes but not fully serialized instances of mapping
* metadata.
*/
public static IndexMetadata readFrom(StreamInput in, @Nullable Function<String, MappingMetadata> mappingLookup) throws IOException {
Builder builder = new Builder(in.readString());
builder.version(in.readLong());
builder.mappingVersion(in.readVLong());
Expand All @@ -1063,9 +1072,15 @@ public static IndexMetadata readFrom(StreamInput in) throws IOException {
builder.settings(readSettingsFromStream(in));
builder.primaryTerms(in.readVLongArray());
int mappingsSize = in.readVInt();
for (int i = 0; i < mappingsSize; i++) {
MappingMetadata mappingMd = new MappingMetadata(in);
builder.putMapping(mappingMd);
if (mappingsSize == 1) {
if (mappingLookup != null && in.getVersion().onOrAfter(Metadata.MAPPINGS_AS_HASH_VERSION)) {
final String mappingHash = in.readString();
final MappingMetadata metadata = mappingLookup.apply(mappingHash);
assert metadata != null : "failed to find mapping [" + mappingHash + "] for [" + builder.index + "]";
builder.putMapping(metadata);
} else {
builder.putMapping(new MappingMetadata(in));
}
}
int aliasesSize = in.readVInt();
for (int i = 0; i < aliasesSize; i++) {
Expand Down Expand Up @@ -1095,8 +1110,10 @@ public static IndexMetadata readFrom(StreamInput in) throws IOException {
return builder.build();
}

@Override
public void writeTo(StreamOutput out) throws IOException {
/**
* @param mappingsAsHash whether to serialize {@link MappingMetadata} in full or just its hash {@link MappingMetadata#getSha256()}
*/
public void writeTo(StreamOutput out, boolean mappingsAsHash) throws IOException {
out.writeString(index.getName()); // uuid will come as part of settings
out.writeLong(version);
out.writeVLong(mappingVersion);
Expand All @@ -1113,7 +1130,11 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeVInt(0);
} else {
out.writeVInt(1);
mapping.writeTo(out);
if (mappingsAsHash && out.getVersion().onOrAfter(Metadata.MAPPINGS_AS_HASH_VERSION)) {
out.writeString(mapping.getSha256());
} else {
mapping.writeTo(out);
}
}
out.writeVInt(aliases.size());
for (AliasMetadata aliasMetadata : aliases.values()) {
Expand All @@ -1139,6 +1160,11 @@ public void writeTo(StreamOutput out) throws IOException {
timestampRange.writeTo(out);
}

@Override
public void writeTo(StreamOutput out) throws IOException {
writeTo(out, false);
}

public boolean isSystem() {
return isSystem;
}
Expand Down
28 changes: 26 additions & 2 deletions server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,8 @@ public Metadata apply(Metadata part) {
}
}

public static final Version MAPPINGS_AS_HASH_VERSION = Version.V_8_1_0;

public static Metadata readFrom(StreamInput in) throws IOException {
Builder builder = new Builder();
builder.version = in.readLong();
Expand All @@ -1030,9 +1032,25 @@ public static Metadata readFrom(StreamInput in) throws IOException {
if (in.getVersion().onOrAfter(Version.V_7_3_0)) {
builder.hashesOfConsistentSettings(DiffableStringMap.readFrom(in));
}
final Function<String, MappingMetadata> mappingLookup;
if (in.getVersion().onOrAfter(MAPPINGS_AS_HASH_VERSION)) {
final int mappings = in.readVInt();
if (mappings > 0) {
final Map<String, MappingMetadata> mappingMetadataMap = new HashMap<>(mappings);
Copy link
Contributor

@arteam arteam Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The HashMap constructors accepts the capacity, not the expected amount of elements. It needs to be sized a bit higher than mappings, otherwise it will need to be resized/rehashed.

See https://github.com/google/guava/blob/master/guava/src/com/google/common/collect/Maps.java#L273

Copy link
Contributor Author

@original-brownbear original-brownbear Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, though I guess it might be worthwhile to have a general fix to this. We seem to always pre-size capacity == element count in deserialization. Technically, we probably could move to accounting for the load factor, but I wouldn't expect too much from it (especially when the key's hashcode is essentially free).

for (int i = 0; i < mappings; i++) {
final MappingMetadata m = new MappingMetadata(in);
mappingMetadataMap.put(m.getSha256(), m);
}
mappingLookup = mappingMetadataMap::get;
} else {
mappingLookup = null;
}
} else {
mappingLookup = null;
}
int size = in.readVInt();
for (int i = 0; i < size; i++) {
builder.put(IndexMetadata.readFrom(in), false);
builder.put(IndexMetadata.readFrom(in, mappingLookup), false);
}
size = in.readVInt();
for (int i = 0; i < size; i++) {
Expand All @@ -1057,9 +1075,15 @@ public void writeTo(StreamOutput out) throws IOException {
if (out.getVersion().onOrAfter(Version.V_7_3_0)) {
hashesOfConsistentSettings.writeTo(out);
}
// Starting in #MAPPINGS_AS_HASH_VERSION we write the mapping metadata first and then write the indices without metadata so that
// we avoid writing duplicate mappings twice
if (out.getVersion().onOrAfter(MAPPINGS_AS_HASH_VERSION)) {
out.writeCollection(mappingsByHash.values());
}
out.writeVInt(indices.size());
final boolean writeMappingsHash = out.getVersion().onOrAfter(MAPPINGS_AS_HASH_VERSION);
for (IndexMetadata indexMetadata : this) {
indexMetadata.writeTo(out);
indexMetadata.writeTo(out, writeMappingsHash);
}
out.writeVInt(templates.size());
for (IndexTemplateMetadata template : templates.values()) {
Expand Down