Skip to content

Remove obsolete Metadata.FORMAT field and usages #129519

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.elasticsearch.core.FixForMultiProject;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.gateway.MetadataStateFormat;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.index.IndexVersion;
Expand All @@ -50,7 +49,6 @@
import org.elasticsearch.xcontent.NamedObjectNotFoundException;
import org.elasticsearch.xcontent.NamedXContentRegistry;
import org.elasticsearch.xcontent.ToXContent;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;

import java.io.IOException;
Expand Down Expand Up @@ -2101,30 +2099,6 @@ static <C extends MetadataCustom<C>> void parseCustomObject(
}
}

private static final ToXContent.Params FORMAT_PARAMS;
static {
Map<String, String> params = Maps.newMapWithExpectedSize(2);
params.put("binary", "true");
params.put(Metadata.CONTEXT_MODE_PARAM, Metadata.CONTEXT_MODE_GATEWAY);
FORMAT_PARAMS = new ToXContent.MapParams(params);
}

/**
* State format for {@link Metadata} to write to and load from disk
*/
public static final MetadataStateFormat<Metadata> FORMAT = new MetadataStateFormat<>(GLOBAL_STATE_FILE_PREFIX) {

@Override
public void toXContent(XContentBuilder builder, Metadata state) throws IOException {
ChunkedToXContent.wrapAsToXContent(state).toXContent(builder, FORMAT_PARAMS);
}

@Override
public Metadata fromXContent(XContentParser parser) throws IOException {
return Builder.fromXContent(parser);
}
};

private volatile Metadata.ProjectLookup projectLookup = null;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,14 @@
import org.elasticsearch.xcontent.NamedXContentRegistry;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Predicate;

import static org.elasticsearch.cluster.metadata.Metadata.GLOBAL_STATE_FILE_PREFIX;

/**
* Handles writing and loading {@link Manifest}, {@link Metadata} and {@link IndexMetadata} as used for cluster state persistence in
* versions prior to {@link Version#V_7_6_0}, used to read this older format during an upgrade from these versions.
Expand Down Expand Up @@ -75,20 +79,29 @@ List<IndexMetadata> loadIndicesStates(Predicate<String> excludeIndexPathIdsPredi
return indexMetadataList;
}

/**
* Loads the global state, *without* index state
*/
Metadata loadGlobalState() throws IOException {
return Metadata.FORMAT.loadLatestState(logger, namedXContentRegistry, nodeEnv.nodeDataPaths());
}

/**
* Creates empty cluster state file on disk, deleting global metadata and unreferencing all index metadata
* (only used for dangling indices at that point).
*/
public void unreferenceAll() throws IOException {
Manifest.FORMAT.writeAndCleanup(Manifest.empty(), nodeEnv.nodeDataPaths()); // write empty file so that indices become unreferenced
Metadata.FORMAT.cleanupOldFiles(Long.MAX_VALUE, nodeEnv.nodeDataPaths());
cleanUpGlobalStateFiles();
}

private void cleanUpGlobalStateFiles() throws IOException {
for (Path location : nodeEnv.nodeDataPaths()) {
logger.trace("cleanupOldFiles: cleaning up {} for global state files", location);
final Path stateLocation = location.resolve(MetadataStateFormat.STATE_DIR_NAME);
try (var paths = Files.list(stateLocation)) {
paths.filter(file -> file.getFileName().toString().startsWith(GLOBAL_STATE_FILE_PREFIX)).forEach(file -> {
Copy link
Member Author

Choose a reason for hiding this comment

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

This no longer filters based on Long.MAX_VALUE anymore. I think in practice, it is the same as deleting all global-* files. But please let me know if you prefer to keep it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer this, the Long.MAX_VALUE filter was kind of a hack to make it delete everything.

try {
Files.deleteIfExists(file);
} catch (IOException e) {
logger.trace("failed to delete global state file: {}", file);
}
});
}
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ public void testXContentWithIndexGraveyard() throws IOException {
final Metadata originalMeta = Metadata.builder().put(ProjectMetadata.builder(projectId).indexGraveyard(graveyard)).build();
final XContentBuilder builder = JsonXContent.contentBuilder();
builder.startObject();
Metadata.FORMAT.toXContent(builder, originalMeta);
ChunkedToXContent.wrapAsToXContent(originalMeta).toXContent(builder, formatParams());
builder.endObject();
try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) {
final Metadata fromXContentMeta = Metadata.fromXContent(parser);
Expand All @@ -647,7 +647,7 @@ public void testXContentClusterUUID() throws IOException {
.build();
final XContentBuilder builder = JsonXContent.contentBuilder();
builder.startObject();
Metadata.FORMAT.toXContent(builder, originalMeta);
ChunkedToXContent.wrapAsToXContent(originalMeta).toXContent(builder, formatParams());
builder.endObject();
try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) {
final Metadata fromXContentMeta = Metadata.fromXContent(parser);
Expand Down Expand Up @@ -732,7 +732,7 @@ public void testXContentWithCoordinationMetadata() throws IOException {

final XContentBuilder builder = JsonXContent.contentBuilder();
builder.startObject();
Metadata.FORMAT.toXContent(builder, metadata);
ChunkedToXContent.wrapAsToXContent(metadata).toXContent(builder, formatParams());
builder.endObject();

try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) {
Expand Down Expand Up @@ -3345,6 +3345,10 @@ private static CreateIndexResult createIndices(int numIndices, int numBackingInd
return new CreateIndexResult(indices, backingIndices, b.build());
}

private static ToXContent.Params formatParams() {
return new ToXContent.MapParams(Map.of("binary", "true", Metadata.CONTEXT_MODE_PARAM, Metadata.CONTEXT_MODE_GATEWAY));
}

private static class CreateIndexResult {
final List<Index> indices;
final List<Index> backingIndices;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,11 @@ public void testSimpleJsonFromAndTo() throws IOException {

XContentBuilder builder = JsonXContent.contentBuilder();
builder.startObject();
Metadata.FORMAT.toXContent(builder, metadata);
ChunkedToXContent.wrapAsToXContent(metadata)
.toXContent(
builder,
new ToXContent.MapParams(Map.of("binary", "true", Metadata.CONTEXT_MODE_PARAM, Metadata.CONTEXT_MODE_GATEWAY))
);
builder.endObject();

Metadata parsedMetadata;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@
package org.elasticsearch.gateway;

import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexVersion;
Expand Down Expand Up @@ -53,20 +51,4 @@ public void testWriteLoadIndex() throws Exception {
public void testLoadMissingIndex() throws Exception {
assertThat(metaStateService.loadIndexState(new Index("test1", "test1UUID")), nullValue());
}

public void testWriteLoadGlobal() throws Exception {
Metadata metadata = Metadata.builder().persistentSettings(Settings.builder().put("test1", "value1").build()).build();
MetaStateWriterUtils.writeGlobalState(env, "test_write", metadata);
assertThat(metaStateService.loadGlobalState().persistentSettings(), equalTo(metadata.persistentSettings()));
}

public void testWriteGlobalStateWithIndexAndNoIndexIsLoaded() throws Exception {
Metadata metadata = Metadata.builder().persistentSettings(Settings.builder().put("test1", "value1").build()).build();
IndexMetadata index = indexMetadata("test1");
Metadata metadataWithIndex = Metadata.builder(metadata).put(index, true).build();

MetaStateWriterUtils.writeGlobalState(env, "test_write", metadataWithIndex);
assertThat(metaStateService.loadGlobalState().persistentSettings(), equalTo(metadata.persistentSettings()));
assertThat(metaStateService.loadGlobalState().getProject().hasIndex("test1"), equalTo(false));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
import org.apache.logging.log4j.Logger;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.Manifest;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.index.Index;

Expand All @@ -29,22 +27,6 @@ private MetaStateWriterUtils() {
throw new AssertionError("static methods only");
}

/**
* Writes manifest file (represented by {@link Manifest}) to disk and performs cleanup of old manifest state file if
* the write succeeds or newly created manifest state if the write fails.
*
* @throws WriteStateException if exception when writing state occurs. See also {@link WriteStateException#isDirty()}
*/
public static void writeManifestAndCleanup(NodeEnvironment nodeEnv, String reason, Manifest manifest) throws WriteStateException {
Copy link
Member Author

Choose a reason for hiding this comment

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

This method is not used before this PR. So I simply removing it here.

logger.trace("[_meta] writing state, reason [{}]", reason);
try {
long generation = Manifest.FORMAT.writeAndCleanup(manifest, nodeEnv.nodeDataPaths());
logger.trace("[_meta] state written (generation: {})", generation);
} catch (WriteStateException ex) {
throw new WriteStateException(ex.isDirty(), "[_meta]: failed to write meta state", ex);
}
}

/**
* Writes the index state.
* <p>
Expand All @@ -65,21 +47,4 @@ public static long writeIndex(NodeEnvironment nodeEnv, String reason, IndexMetad
}
}

/**
* Writes the global state, *without* the indices states.
*
* @throws WriteStateException if exception when writing state occurs. {@link WriteStateException#isDirty()} will always return
* false, because new global state file is not yet referenced by manifest file.
*/
static long writeGlobalState(NodeEnvironment nodeEnv, String reason, Metadata metadata) throws WriteStateException {
logger.trace("[_global] writing state, reason [{}]", reason);
try {
long generation = Metadata.FORMAT.write(metadata, nodeEnv.nodeDataPaths());
logger.trace("[_global] state written");
return generation;
} catch (WriteStateException ex) {
throw new WriteStateException(false, "[_global]: failed to write global state", ex);
}
}

}