From c7448b12e1e1c995929d25154ea591bb77f96802 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 27 May 2019 11:28:50 +0200 Subject: [PATCH] Cleanup Redundant BlobStoreFormat Class (#42195) (#42570) * No need to have an abstract class here when there's only a single impl. --- .../blobstore/BlobStoreFormat.java | 119 ------------------ .../blobstore/BlobStoreRepository.java | 3 - .../blobstore/ChecksumBlobStoreFormat.java | 92 ++++++++++---- 3 files changed, 71 insertions(+), 143 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreFormat.java diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreFormat.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreFormat.java deleted file mode 100644 index dc9f8092e3f..00000000000 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreFormat.java +++ /dev/null @@ -1,119 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.repositories.blobstore; - -import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.common.CheckedFunction; -import org.elasticsearch.common.blobstore.BlobContainer; -import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.snapshots.SnapshotInfo; - -import java.io.IOException; -import java.util.HashMap; -import java.util.Locale; -import java.util.Map; - -/** - * Base class that handles serialization of various data structures during snapshot/restore operations. - */ -public abstract class BlobStoreFormat { - - protected final String blobNameFormat; - - protected final CheckedFunction reader; - - protected final NamedXContentRegistry namedXContentRegistry; - - // Serialization parameters to specify correct context for metadata serialization - protected static final ToXContent.Params SNAPSHOT_ONLY_FORMAT_PARAMS; - - static { - Map snapshotOnlyParams = new HashMap<>(); - // when metadata is serialized certain elements of the metadata shouldn't be included into snapshot - // exclusion of these elements is done by setting MetaData.CONTEXT_MODE_PARAM to MetaData.CONTEXT_MODE_SNAPSHOT - snapshotOnlyParams.put(MetaData.CONTEXT_MODE_PARAM, MetaData.CONTEXT_MODE_SNAPSHOT); - // serialize SnapshotInfo using the SNAPSHOT mode - snapshotOnlyParams.put(SnapshotInfo.CONTEXT_MODE_PARAM, SnapshotInfo.CONTEXT_MODE_SNAPSHOT); - SNAPSHOT_ONLY_FORMAT_PARAMS = new ToXContent.MapParams(snapshotOnlyParams); - } - - /** - * @param blobNameFormat format of the blobname in {@link String#format(Locale, String, Object...)} format - * @param reader the prototype object that can deserialize objects with type T - */ - protected BlobStoreFormat(String blobNameFormat, CheckedFunction reader, - NamedXContentRegistry namedXContentRegistry) { - this.reader = reader; - this.blobNameFormat = blobNameFormat; - this.namedXContentRegistry = namedXContentRegistry; - } - - /** - * Reads and parses the blob with given blob name. - * - * @param blobContainer blob container - * @param blobName blob name - * @return parsed blob object - */ - public abstract T readBlob(BlobContainer blobContainer, String blobName) throws IOException; - - /** - * Reads and parses the blob with given name, applying name translation using the {link #blobName} method - * - * @param blobContainer blob container - * @param name name to be translated into - * @return parsed blob object - */ - public T read(BlobContainer blobContainer, String name) throws IOException { - String blobName = blobName(name); - return readBlob(blobContainer, blobName); - } - - /** - * Deletes obj in the blob container - */ - public void delete(BlobContainer blobContainer, String name) throws IOException { - blobContainer.deleteBlob(blobName(name)); - } - - /** - * Checks obj in the blob container - */ - public boolean exists(BlobContainer blobContainer, String name) { - return blobContainer.blobExists(blobName(name)); - } - - public String blobName(String name) { - return String.format(Locale.ROOT, blobNameFormat, name); - } - - protected T read(BytesReference bytes) throws IOException { - try (XContentParser parser = XContentHelper - .createParser(namedXContentRegistry, LoggingDeprecationHandler.INSTANCE, bytes)) { - T obj = reader.apply(parser); - return obj; - } - } - -} diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 998c4f2db52..22567f1313c 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -392,9 +392,6 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp if (repositoryData.getAllSnapshotIds().stream().anyMatch(s -> s.getName().equals(snapshotName))) { throw new InvalidSnapshotNameException(metadata.name(), snapshotId.getName(), "snapshot with the same name already exists"); } - if (snapshotFormat.exists(blobContainer(), snapshotId.getUUID())) { - throw new InvalidSnapshotNameException(metadata.name(), snapshotId.getName(), "snapshot with the same name already exists"); - } // Write Global MetaData globalMetaDataFormat.write(clusterMetaData, blobContainer(), snapshotId.getUUID()); diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java index ca6ec74dc2c..16751399a18 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java @@ -23,6 +23,7 @@ import org.apache.lucene.index.CorruptIndexException; import org.apache.lucene.index.IndexFormatTooNewException; import org.apache.lucene.index.IndexFormatTooOldException; import org.apache.lucene.store.OutputStreamIndexOutput; +import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.blobstore.BlobContainer; @@ -33,24 +34,43 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.lucene.store.ByteArrayIndexInput; import org.elasticsearch.common.lucene.store.IndexOutputOutputStream; +import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.core.internal.io.Streams; import org.elasticsearch.gateway.CorruptStateException; +import org.elasticsearch.snapshots.SnapshotInfo; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; /** * Snapshot metadata file format used in v2.0 and above */ -public class ChecksumBlobStoreFormat extends BlobStoreFormat { +public final class ChecksumBlobStoreFormat { + + // Serialization parameters to specify correct context for metadata serialization + private static final ToXContent.Params SNAPSHOT_ONLY_FORMAT_PARAMS; + + static { + Map snapshotOnlyParams = new HashMap<>(); + // when metadata is serialized certain elements of the metadata shouldn't be included into snapshot + // exclusion of these elements is done by setting MetaData.CONTEXT_MODE_PARAM to MetaData.CONTEXT_MODE_SNAPSHOT + snapshotOnlyParams.put(MetaData.CONTEXT_MODE_PARAM, MetaData.CONTEXT_MODE_SNAPSHOT); + // serialize SnapshotInfo using the SNAPSHOT mode + snapshotOnlyParams.put(SnapshotInfo.CONTEXT_MODE_PARAM, SnapshotInfo.CONTEXT_MODE_SNAPSHOT); + SNAPSHOT_ONLY_FORMAT_PARAMS = new ToXContent.MapParams(snapshotOnlyParams); + } private static final XContentType DEFAULT_X_CONTENT_TYPE = XContentType.SMILE; @@ -59,12 +79,18 @@ public class ChecksumBlobStoreFormat extends BlobStoreForm private static final int BUFFER_SIZE = 4096; - protected final XContentType xContentType; + private final XContentType xContentType; - protected final boolean compress; + private final boolean compress; private final String codec; + private final String blobNameFormat; + + private final CheckedFunction reader; + + private final NamedXContentRegistry namedXContentRegistry; + /** * @param codec codec name * @param blobNameFormat format of the blobname in {@link String#format} format @@ -74,7 +100,9 @@ public class ChecksumBlobStoreFormat extends BlobStoreForm */ public ChecksumBlobStoreFormat(String codec, String blobNameFormat, CheckedFunction reader, NamedXContentRegistry namedXContentRegistry, boolean compress, XContentType xContentType) { - super(blobNameFormat, reader, namedXContentRegistry); + this.reader = reader; + this.blobNameFormat = blobNameFormat; + this.namedXContentRegistry = namedXContentRegistry; this.xContentType = xContentType; this.compress = compress; this.codec = codec; @@ -91,6 +119,29 @@ public class ChecksumBlobStoreFormat extends BlobStoreForm this(codec, blobNameFormat, reader, namedXContentRegistry, compress, DEFAULT_X_CONTENT_TYPE); } + /** + * Reads and parses the blob with given name, applying name translation using the {link #blobName} method + * + * @param blobContainer blob container + * @param name name to be translated into + * @return parsed blob object + */ + public T read(BlobContainer blobContainer, String name) throws IOException { + String blobName = blobName(name); + return readBlob(blobContainer, blobName); + } + + /** + * Deletes obj in the blob container + */ + public void delete(BlobContainer blobContainer, String name) throws IOException { + blobContainer.deleteBlob(blobName(name)); + } + + public String blobName(String name) { + return String.format(Locale.ROOT, blobNameFormat, name); + } + /** * Reads blob with specified name without resolving the blobName using using {@link #blobName} method. * @@ -108,8 +159,10 @@ public class ChecksumBlobStoreFormat extends BlobStoreForm CodecUtil.checkHeader(indexInput, codec, VERSION, VERSION); long filePointer = indexInput.getFilePointer(); long contentSize = indexInput.length() - CodecUtil.footerLength() - filePointer; - BytesReference bytesReference = new BytesArray(bytes, (int) filePointer, (int) contentSize); - return read(bytesReference); + try (XContentParser parser = XContentHelper.createParser(namedXContentRegistry, LoggingDeprecationHandler.INSTANCE, + new BytesArray(bytes, (int) filePointer, (int) contentSize))) { + return reader.apply(parser); + } } catch (CorruptIndexException | IndexFormatTooOldException | IndexFormatTooNewException ex) { // we trick this into a dedicated exception with the original stacktrace throw new CorruptStateException(ex); @@ -156,7 +209,17 @@ public class ChecksumBlobStoreFormat extends BlobStoreForm } private void writeTo(final T obj, final String blobName, final CheckedConsumer consumer) throws IOException { - final BytesReference bytes = write(obj); + final BytesReference bytes; + try (BytesStreamOutput bytesStreamOutput = new BytesStreamOutput()) { + if (compress) { + try (StreamOutput compressedStreamOutput = CompressorFactory.COMPRESSOR.streamOutput(bytesStreamOutput)) { + write(obj, compressedStreamOutput); + } + } else { + write(obj, bytesStreamOutput); + } + bytes = bytesStreamOutput.bytes(); + } try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) { final String resourceDesc = "ChecksumBlobStoreFormat.writeBlob(blob=\"" + blobName + "\")"; try (OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput(resourceDesc, blobName, outputStream, BUFFER_SIZE)) { @@ -176,20 +239,7 @@ public class ChecksumBlobStoreFormat extends BlobStoreForm } } - protected BytesReference write(T obj) throws IOException { - try (BytesStreamOutput bytesStreamOutput = new BytesStreamOutput()) { - if (compress) { - try (StreamOutput compressedStreamOutput = CompressorFactory.COMPRESSOR.streamOutput(bytesStreamOutput)) { - write(obj, compressedStreamOutput); - } - } else { - write(obj, bytesStreamOutput); - } - return bytesStreamOutput.bytes(); - } - } - - protected void write(T obj, StreamOutput streamOutput) throws IOException { + private void write(T obj, StreamOutput streamOutput) throws IOException { try (XContentBuilder builder = XContentFactory.contentBuilder(xContentType, streamOutput)) { builder.startObject(); obj.toXContent(builder, SNAPSHOT_ONLY_FORMAT_PARAMS);