فهرست منبع

Make Parsing SnapshotInfo more Efficient (#74005)

Flatting the logic for parsing `SnapshotInfo` to go field by field like we do for `RepositoryData`
which is both easier to read and also faster (mostly when moving to batch multiple of these blobs into one
and doing on-the-fly filtering in an upcoming PR where the approach allows for more tricks).
Also, simplified/deduplicated parsing out (mostly/often) empty lists in the deserialization code
and used the new utility in a few more spots as well to save empty lists.
Armin Braun 4 سال پیش
والد
کامیت
d4e6e4c155

+ 1 - 1
server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotResponse.java

@@ -54,7 +54,7 @@ public class CreateSnapshotResponse extends ActionResponse implements ToXContent
 
     public CreateSnapshotResponse(StreamInput in) throws IOException {
         super(in);
-        snapshotInfo = in.readOptionalWriteable(SnapshotInfo::new);
+        snapshotInfo = in.readOptionalWriteable(SnapshotInfo::readFrom);
     }
 
     private void setSnapshotInfoFromBuilder(SnapshotInfoBuilder snapshotInfoBuilder) {

+ 2 - 2
server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java

@@ -50,12 +50,12 @@ public class GetSnapshotsResponse extends ActionResponse implements ToXContentOb
 
     public GetSnapshotsResponse(StreamInput in) throws IOException {
         if (in.getVersion().onOrAfter(GetSnapshotsRequest.MULTIPLE_REPOSITORIES_SUPPORT_ADDED)) {
-            Map<String, List<SnapshotInfo>> successfulResponses = in.readMapOfLists(StreamInput::readString, SnapshotInfo::new);
+            Map<String, List<SnapshotInfo>> successfulResponses = in.readMapOfLists(StreamInput::readString, SnapshotInfo::readFrom);
             Map<String, ElasticsearchException> failedResponses = in.readMap(StreamInput::readString, StreamInput::readException);
             this.successfulResponses = Collections.unmodifiableMap(successfulResponses);
             this.failedResponses = Collections.unmodifiableMap(failedResponses);
         } else {
-            this.successfulResponses = Collections.singletonMap("unknown", in.readList(SnapshotInfo::new));
+            this.successfulResponses = Collections.singletonMap("unknown", in.readList(SnapshotInfo::readFrom));
             this.failedResponses = Collections.emptyMap();
         }
     }

+ 6 - 13
server/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -27,10 +27,10 @@ import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.XContentParserUtils;
 import org.elasticsearch.common.xcontent.XContentType;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.EnumSet;
@@ -1520,18 +1520,11 @@ public class Setting<T> implements ToXContentObject {
         // fromXContent doesn't use named xcontent or deprecation.
         try (XContentParser xContentParser = XContentType.JSON.xContent()
                 .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, parsableString)) {
-            XContentParser.Token token = xContentParser.nextToken();
-            if (token != XContentParser.Token.START_ARRAY) {
-                throw new IllegalArgumentException("expected START_ARRAY but got " + token);
-            }
-            ArrayList<String> list = new ArrayList<>();
-            while ((token = xContentParser.nextToken()) != XContentParser.Token.END_ARRAY) {
-                if (token != XContentParser.Token.VALUE_STRING) {
-                    throw new IllegalArgumentException("expected VALUE_STRING but got " + token);
-                }
-                list.add(xContentParser.text());
-            }
-            return list;
+            xContentParser.nextToken();
+            return XContentParserUtils.parseList(xContentParser, p -> {
+                XContentParserUtils.ensureExpectedToken(XContentParser.Token.VALUE_STRING, p.currentToken(), p);
+                return p.text();
+            });
         } catch (IOException e) {
             throw new IllegalArgumentException("failed to parse array", e);
         }

+ 24 - 0
server/src/main/java/org/elasticsearch/common/xcontent/XContentParserUtils.java

@@ -12,8 +12,11 @@ import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.xcontent.XContentParser.Token;
+import org.elasticsearch.core.CheckedFunction;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Locale;
 import java.util.function.Consumer;
 
@@ -151,4 +154,25 @@ public final class XContentParserUtils {
             throw new ParsingException(parser.getTokenLocation(), "Failed to parse object: empty key");
         }
     }
+
+    /**
+     * Parses a list of a given type from the given {@code parser}. Assumes that the parser is currently positioned on a
+     * {@link Token#START_ARRAY} token and will fail if it is not. The returned list may or may not be mutable.
+     *
+     * @param parser      x-content parser
+     * @param valueParser parser for expected list value type
+     * @return list parsed from parser
+     */
+    public static <T> List<T> parseList(XContentParser parser,
+                                        CheckedFunction<XContentParser, T, IOException> valueParser) throws IOException {
+        XContentParserUtils.ensureExpectedToken(Token.START_ARRAY, parser.currentToken(), parser);
+        if (parser.nextToken() == Token.END_ARRAY) {
+            return List.of();
+        }
+        final ArrayList<T> list = new ArrayList<>();
+        do {
+            list.add(valueParser.apply(parser));
+        } while (parser.nextToken() != Token.END_ARRAY);
+        return list;
+    }
 }

+ 3 - 7
server/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java

@@ -22,8 +22,6 @@ import org.elasticsearch.common.xcontent.XContentParserUtils;
 import org.elasticsearch.index.store.StoreFileMetadata;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.stream.IntStream;
 
@@ -514,7 +512,7 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
         int incrementalFileCount = 0;
         long incrementalSize = 0;
 
-        List<FileInfo> indexFiles = new ArrayList<>();
+        List<FileInfo> indexFiles = null;
         if (parser.currentToken() == null) { // fresh parser? move to the first token
             parser.nextToken();
         }
@@ -543,9 +541,7 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
                 }
             } else if (token == XContentParser.Token.START_ARRAY) {
                 if (PARSE_FILES.match(currentFieldName, parser.getDeprecationHandler())) {
-                    while ((parser.nextToken()) != XContentParser.Token.END_ARRAY) {
-                        indexFiles.add(FileInfo.fromXContent(parser));
-                    }
+                    indexFiles = XContentParserUtils.parseList(parser, FileInfo::fromXContent);
                 } else {
                     XContentParserUtils.throwUnknownField(currentFieldName, parser.getTokenLocation());
                 }
@@ -557,7 +553,7 @@ public class BlobStoreIndexShardSnapshot implements ToXContentFragment {
         return new BlobStoreIndexShardSnapshot(
             snapshot,
             indexVersion,
-            Collections.unmodifiableList(indexFiles),
+            indexFiles == null ? List.of() : indexFiles,
             startTime,
             time,
             incrementalFileCount,

+ 1 - 5
server/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshots.java

@@ -263,11 +263,7 @@ public class BlobStoreIndexShardSnapshots implements Iterable<SnapshotFiles>, To
                             currentFieldName = parser.currentName();
                             if (ParseFields.FILES.match(currentFieldName, parser.getDeprecationHandler())
                                 && parser.nextToken() == XContentParser.Token.START_ARRAY) {
-                                List<String> fileNames = new ArrayList<>();
-                                while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
-                                    fileNames.add(parser.text());
-                                }
-                                snapshotsMap.put(snapshot, fileNames);
+                                snapshotsMap.put(snapshot, XContentParserUtils.parseList(parser, XContentParser::text));
                             } else if (ParseFields.SHARD_STATE_ID.match(currentFieldName, parser.getDeprecationHandler())) {
                                 parser.nextToken();
                                 historyUUIDs.put(snapshot, parser.text());

+ 100 - 85
server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java

@@ -430,9 +430,9 @@ public final class SnapshotInfo implements Comparable<SnapshotInfo>, ToXContent,
         Map<String, IndexSnapshotDetails> indexSnapshotDetails
     ) {
         this.snapshotId = Objects.requireNonNull(snapshotId);
-        this.indices = Collections.unmodifiableList(Objects.requireNonNull(indices));
-        this.dataStreams = Collections.unmodifiableList(Objects.requireNonNull(dataStreams));
-        this.featureStates = Collections.unmodifiableList(Objects.requireNonNull(featureStates));
+        this.indices = List.copyOf(indices);
+        this.dataStreams = List.copyOf(dataStreams);
+        this.featureStates = List.copyOf(featureStates);
         this.state = state;
         this.reason = reason;
         this.version = version;
@@ -440,31 +440,48 @@ public final class SnapshotInfo implements Comparable<SnapshotInfo>, ToXContent,
         this.endTime = endTime;
         this.totalShards = totalShards;
         this.successfulShards = successfulShards;
-        this.shardFailures = Objects.requireNonNull(shardFailures);
+        this.shardFailures = List.copyOf(shardFailures);
         this.includeGlobalState = includeGlobalState;
         this.userMetadata = userMetadata;
-        this.indexSnapshotDetails = Collections.unmodifiableMap(Objects.requireNonNull(indexSnapshotDetails));
+        this.indexSnapshotDetails = Map.copyOf(indexSnapshotDetails);
     }
 
     /**
      * Constructs snapshot information from stream input
      */
-    public SnapshotInfo(final StreamInput in) throws IOException {
-        snapshotId = new SnapshotId(in);
-        indices = Collections.unmodifiableList(in.readStringList());
-        state = in.readBoolean() ? SnapshotState.fromValue(in.readByte()) : null;
-        reason = in.readOptionalString();
-        startTime = in.readVLong();
-        endTime = in.readVLong();
-        totalShards = in.readVInt();
-        successfulShards = in.readVInt();
-        shardFailures = Collections.unmodifiableList(in.readList(SnapshotShardFailure::new));
-        version = in.readBoolean() ? Version.readVersion(in) : null;
-        includeGlobalState = in.readOptionalBoolean();
-        userMetadata = in.readMap();
-        dataStreams = in.readStringList();
-        featureStates = Collections.unmodifiableList(in.readList(SnapshotFeatureInfo::new));
-        indexSnapshotDetails = in.readMap(StreamInput::readString, IndexSnapshotDetails::new);
+    public static SnapshotInfo readFrom(final StreamInput in) throws IOException {
+        final SnapshotId snapshotId = new SnapshotId(in);
+        final List<String> indices = in.readStringList();
+        final SnapshotState state = in.readBoolean() ? SnapshotState.fromValue(in.readByte()) : null;
+        final String reason = in.readOptionalString();
+        final long startTime = in.readVLong();
+        final long endTime = in.readVLong();
+        final int totalShards = in.readVInt();
+        final int successfulShards = in.readVInt();
+        final List<SnapshotShardFailure> shardFailures = in.readList(SnapshotShardFailure::new);
+        final Version version = in.readBoolean() ? Version.readVersion(in) : null;
+        final Boolean includeGlobalState = in.readOptionalBoolean();
+        final Map<String, Object> userMetadata = in.readMap();
+        final List<String> dataStreams = in.readStringList();
+        final List<SnapshotFeatureInfo> featureStates = in.readList(SnapshotFeatureInfo::new);
+        final Map<String, IndexSnapshotDetails> indexSnapshotDetails = in.readMap(StreamInput::readString, IndexSnapshotDetails::new);
+        return new SnapshotInfo(
+            snapshotId,
+            indices,
+            dataStreams,
+            featureStates,
+            reason,
+            version,
+            startTime,
+            endTime,
+            totalShards,
+            successfulShards,
+            shardFailures,
+            includeGlobalState,
+            userMetadata,
+            state,
+            indexSnapshotDetails
+        );
     }
 
     /**
@@ -780,7 +797,9 @@ public final class SnapshotInfo implements Comparable<SnapshotInfo>, ToXContent,
         if (includeGlobalState != null) {
             builder.field(INCLUDE_GLOBAL_STATE, includeGlobalState);
         }
-        builder.field(USER_METADATA, userMetadata);
+        if (userMetadata != null) {
+            builder.field(USER_METADATA, userMetadata);
+        }
         builder.field(START_TIME, startTime);
         builder.field(END_TIME, endTime);
         builder.field(TOTAL_SHARDS, totalShards);
@@ -835,73 +854,69 @@ public final class SnapshotInfo implements Comparable<SnapshotInfo>, ToXContent,
         if (parser.currentToken() == XContentParser.Token.START_OBJECT) {  // on a start object move to next token
             parser.nextToken();
         }
-        XContentParser.Token token;
         XContentParserUtils.ensureFieldName(parser, parser.currentToken(), SNAPSHOT);
         XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser);
-        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
-            if (token == XContentParser.Token.FIELD_NAME) {
-                final String currentFieldName = parser.currentName();
-                token = parser.nextToken();
-                if (token.isValue()) {
-                    if (NAME.equals(currentFieldName)) {
-                        name = parser.text();
-                    } else if (UUID.equals(currentFieldName)) {
-                        uuid = parser.text();
-                    } else if (STATE.equals(currentFieldName)) {
-                        state = SnapshotState.valueOf(parser.text());
-                    } else if (REASON.equals(currentFieldName)) {
-                        reason = parser.text();
-                    } else if (START_TIME.equals(currentFieldName)) {
-                        startTime = parser.longValue();
-                    } else if (END_TIME.equals(currentFieldName)) {
-                        endTime = parser.longValue();
-                    } else if (TOTAL_SHARDS.equals(currentFieldName)) {
-                        totalShards = parser.intValue();
-                    } else if (SUCCESSFUL_SHARDS.equals(currentFieldName)) {
-                        successfulShards = parser.intValue();
-                    } else if (VERSION_ID.equals(currentFieldName)) {
-                        version = Version.fromId(parser.intValue());
-                    } else if (INCLUDE_GLOBAL_STATE.equals(currentFieldName)) {
-                        includeGlobalState = parser.booleanValue();
-                    }
-                } else if (token == XContentParser.Token.START_ARRAY) {
-                    if (DATA_STREAMS.equals(currentFieldName)) {
-                        dataStreams = new ArrayList<>();
-                        while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
-                            dataStreams.add(parser.text());
-                        }
-                    } else if (INDICES.equals(currentFieldName)) {
-                        ArrayList<String> indicesArray = new ArrayList<>();
-                        while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
-                            indicesArray.add(parser.text());
-                        }
-                        indices = Collections.unmodifiableList(indicesArray);
-                    } else if (FAILURES.equals(currentFieldName)) {
-                        ArrayList<SnapshotShardFailure> shardFailureArrayList = new ArrayList<>();
-                        while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
-                            shardFailureArrayList.add(SnapshotShardFailure.fromXContent(parser));
-                        }
-                        shardFailures = Collections.unmodifiableList(shardFailureArrayList);
-                    } else if (FEATURE_STATES.equals(currentFieldName)) {
-                        ArrayList<SnapshotFeatureInfo> snapshotFeatureInfoArrayList = new ArrayList<>();
-                        while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
-                            snapshotFeatureInfoArrayList.add(SnapshotFeatureInfo.fromXContent(parser));
-                        }
-                        featureStates = Collections.unmodifiableList(snapshotFeatureInfoArrayList);
-                    } else {
-                        // It was probably created by newer version - ignoring
-                        parser.skipChildren();
-                    }
-                } else if (token == XContentParser.Token.START_OBJECT) {
-                    if (USER_METADATA.equals(currentFieldName)) {
+        while (parser.nextToken() == XContentParser.Token.FIELD_NAME) {
+            final String currentFieldName = parser.currentName();
+            final XContentParser.Token token = parser.nextToken();
+            switch (currentFieldName) {
+                case NAME:
+                    name = parser.text();
+                    break;
+                case UUID:
+                    uuid = parser.text();
+                    break;
+                case STATE:
+                    state = SnapshotState.valueOf(parser.text());
+                    break;
+                case REASON:
+                    reason = parser.text();
+                    break;
+                case START_TIME:
+                    startTime = parser.longValue();
+                    break;
+                case END_TIME:
+                    endTime = parser.longValue();
+                    break;
+                case TOTAL_SHARDS:
+                    totalShards = parser.intValue();
+                    break;
+                case SUCCESSFUL_SHARDS:
+                    successfulShards = parser.intValue();
+                    break;
+                case VERSION_ID:
+                    version = Version.fromId(parser.intValue());
+                    break;
+                case INCLUDE_GLOBAL_STATE:
+                    includeGlobalState = parser.booleanValue();
+                    break;
+                case DATA_STREAMS:
+                    dataStreams = XContentParserUtils.parseList(parser, XContentParser::text);
+                    break;
+                case INDICES:
+                    indices = XContentParserUtils.parseList(parser, XContentParser::text);
+                    break;
+                case FAILURES:
+                    shardFailures = XContentParserUtils.parseList(parser, SnapshotShardFailure::fromXContent);
+                    break;
+                case FEATURE_STATES:
+                    featureStates = XContentParserUtils.parseList(parser, SnapshotFeatureInfo::fromXContent);
+                    break;
+                case USER_METADATA:
+                    if (token != XContentParser.Token.VALUE_NULL) {
+                        // some older versions a redundant null value for this field
+                        XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, token, parser);
                         userMetadata = parser.map();
-                    } else if (INDEX_DETAILS.equals(currentFieldName)) {
-                        indexSnapshotDetails = parser.map(HashMap::new, p -> IndexSnapshotDetails.PARSER.parse(p, null));
-                    } else {
-                        // It was probably created by newer version - ignoring
-                        parser.skipChildren();
                     }
-                }
+                    break;
+                case INDEX_DETAILS:
+                    XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, token, parser);
+                    indexSnapshotDetails = parser.map(HashMap::new, p -> IndexSnapshotDetails.PARSER.parse(p, null));
+                    break;
+                default:
+                    // It was probably created by newer version - ignoring
+                    parser.skipChildren();
+                    break;
             }
         }
         // closing bracket of the object containing the "snapshot" field should be there

+ 1 - 1
server/src/test/java/org/elasticsearch/snapshots/SnapshotInfoWriteableTests.java

@@ -23,7 +23,7 @@ public class SnapshotInfoWriteableTests extends AbstractWireSerializingTestCase<
 
     @Override
     protected Writeable.Reader<SnapshotInfo> instanceReader() {
-        return SnapshotInfo::new;
+        return SnapshotInfo::readFrom;
     }
 
     @Override