From 6a296e3e3034b08676e95e58d5439b71c3372a74 Mon Sep 17 00:00:00 2001 From: Pooya Salehi Date: Fri, 27 Jun 2025 17:21:36 +0200 Subject: [PATCH 1/4] Record project deletions in ProjectStateRegistry --- .../org/elasticsearch/TransportVersions.java | 1 + .../cluster/project/ProjectStateRegistry.java | 87 +++++++++++++++++- .../cluster/ClusterStateTests.java | 17 +++- ...rojectStateRegistrySerializationTests.java | 89 +++++++++++++++++++ .../project/ProjectStateRegistryTests.java | 47 ++++++++++ 5 files changed, 234 insertions(+), 7 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/cluster/project/ProjectStateRegistrySerializationTests.java create mode 100644 server/src/test/java/org/elasticsearch/cluster/project/ProjectStateRegistryTests.java diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 07bf5e3d50267..3c5e1cb2e5ddc 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -325,6 +325,7 @@ static TransportVersion def(int id) { public static final TransportVersion ML_INFERENCE_COHERE_API_VERSION = def(9_110_0_00); public static final TransportVersion ESQL_PROFILE_INCLUDE_PLAN = def(9_111_0_00); public static final TransportVersion MAPPINGS_IN_DATA_STREAMS = def(9_112_0_00); + public static final TransportVersion PROJECT_STATE_REGISTRY_RECORDS_DELETIONS = def(9_113_0_00); /* * STOP! READ THIS FIRST! No, really, diff --git a/server/src/main/java/org/elasticsearch/cluster/project/ProjectStateRegistry.java b/server/src/main/java/org/elasticsearch/cluster/project/ProjectStateRegistry.java index 1ae81b8daa0e9..516952bea6167 100644 --- a/server/src/main/java/org/elasticsearch/cluster/project/ProjectStateRegistry.java +++ b/server/src/main/java/org/elasticsearch/cluster/project/ProjectStateRegistry.java @@ -20,28 +20,49 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.xcontent.ToXContent; import java.io.IOException; import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; import java.util.Map; +import java.util.Objects; +import java.util.Set; /** * Represents a registry for managing and retrieving project-specific state in the cluster state. */ public class ProjectStateRegistry extends AbstractNamedDiffable implements ClusterState.Custom { public static final String TYPE = "projects_registry"; - public static final ProjectStateRegistry EMPTY = new ProjectStateRegistry(Collections.emptyMap()); + public static final ProjectStateRegistry EMPTY = new ProjectStateRegistry(Collections.emptyMap(), Collections.emptySet(), 0); private final Map projectsSettings; + // Projects that have been marked for deletion based on their file-based setting + private final Set projectsMarkedForDeletion; + // A counter that is incremented each time one or more projects are marked for deletion. + private final long projectsMarkedForDeletionGeneration; public ProjectStateRegistry(StreamInput in) throws IOException { projectsSettings = in.readMap(ProjectId::readFrom, Settings::readSettingsFromStream); + if (in.getTransportVersion().onOrAfter(TransportVersions.PROJECT_STATE_REGISTRY_RECORDS_DELETIONS)) { + projectsMarkedForDeletion = in.readCollectionAsSet(ProjectId::readFrom); + projectsMarkedForDeletionGeneration = in.readZLong(); + } else { + projectsMarkedForDeletion = Collections.emptySet(); + projectsMarkedForDeletionGeneration = 0; + } } - private ProjectStateRegistry(Map projectsSettings) { + private ProjectStateRegistry( + Map projectsSettings, + Set projectsMarkedForDeletion, + long projectsMarkedForDeletionGeneration + ) { this.projectsSettings = projectsSettings; + this.projectsMarkedForDeletion = projectsMarkedForDeletion; + this.projectsMarkedForDeletionGeneration = projectsMarkedForDeletionGeneration; } /** @@ -72,9 +93,11 @@ public Iterator toXContentChunked(ToXContent.Params params builder.startObject("settings"); entry.getValue().toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("flat_settings", "true"))); builder.endObject(); + builder.field("marked_for_deletion", projectsMarkedForDeletion.contains(entry.getKey())); return builder.endObject(); }), - Iterators.single((builder, p) -> builder.endArray()) + Iterators.single((builder, p) -> builder.endArray()), + Iterators.single((builder, p) -> builder.field("projects_marked_for_deletion_generation", projectsMarkedForDeletionGeneration)) ); } @@ -95,12 +118,44 @@ public TransportVersion getMinimalSupportedVersion() { @Override public void writeTo(StreamOutput out) throws IOException { out.writeMap(projectsSettings); + if (out.getTransportVersion().onOrAfter(TransportVersions.PROJECT_STATE_REGISTRY_RECORDS_DELETIONS)) { + out.writeCollection(projectsMarkedForDeletion); + out.writeZLong(projectsMarkedForDeletionGeneration); + } else { + // There should be no deletion unless all MP nodes are at or after PROJECT_STATE_REGISTRY_RECORDS_DELETIONS + assert projectsMarkedForDeletion.isEmpty(); + assert projectsMarkedForDeletionGeneration == 0; + } } public int size() { return projectsSettings.size(); } + public long getProjectsMarkedForDeletionGeneration() { + return projectsMarkedForDeletionGeneration; + } + + // visible for testing + Map getProjectsSettings() { + return Collections.unmodifiableMap(projectsSettings); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o instanceof ProjectStateRegistry == false) return false; + ProjectStateRegistry that = (ProjectStateRegistry) o; + return projectsMarkedForDeletionGeneration == that.projectsMarkedForDeletionGeneration + && Objects.equals(projectsSettings, that.projectsSettings) + && Objects.equals(projectsMarkedForDeletion, that.projectsMarkedForDeletion); + } + + @Override + public int hashCode() { + return Objects.hash(projectsSettings, projectsMarkedForDeletion, projectsMarkedForDeletionGeneration); + } + public static Builder builder(ClusterState original) { ProjectStateRegistry projectRegistry = original.custom(TYPE, EMPTY); return builder(projectRegistry); @@ -116,13 +171,20 @@ public static Builder builder() { public static class Builder { private final ImmutableOpenMap.Builder projectsSettings; + private final Set projectsMarkedForDeletion; + private final long projectsMarkedForDeletionGeneration; + private boolean newProjectMarkedForDeletion = false; private Builder() { this.projectsSettings = ImmutableOpenMap.builder(); + projectsMarkedForDeletion = new HashSet<>(); + projectsMarkedForDeletionGeneration = 0; } private Builder(ProjectStateRegistry original) { this.projectsSettings = ImmutableOpenMap.builder(original.projectsSettings); + this.projectsMarkedForDeletion = new HashSet<>(original.projectsMarkedForDeletion); + this.projectsMarkedForDeletionGeneration = original.projectsMarkedForDeletionGeneration; } public Builder putProjectSettings(ProjectId projectId, Settings settings) { @@ -130,8 +192,25 @@ public Builder putProjectSettings(ProjectId projectId, Settings settings) { return this; } + public Builder markProjectForDeletion(ProjectId projectId) { + if (projectsMarkedForDeletion.add(projectId)) { + newProjectMarkedForDeletion = true; + } + return this; + } + public ProjectStateRegistry build() { - return new ProjectStateRegistry(projectsSettings.build()); + final var unknownButUnderDeletion = Sets.difference(projectsMarkedForDeletion, projectsSettings.keys()); + if (unknownButUnderDeletion.isEmpty() == false) { + throw new IllegalArgumentException( + "Cannot mark projects for deletion that are not in the registry: " + unknownButUnderDeletion + ); + } + return new ProjectStateRegistry( + projectsSettings.build(), + projectsMarkedForDeletion, + newProjectMarkedForDeletion ? projectsMarkedForDeletionGeneration + 1 : projectsMarkedForDeletionGeneration + ); } } } diff --git a/server/src/test/java/org/elasticsearch/cluster/ClusterStateTests.java b/server/src/test/java/org/elasticsearch/cluster/ClusterStateTests.java index 9d9a3c17e286d..738f234e87b36 100644 --- a/server/src/test/java/org/elasticsearch/cluster/ClusterStateTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/ClusterStateTests.java @@ -809,9 +809,18 @@ public void testToXContentWithMultipleProjects() throws IOException { "settings": { "project.setting": "42", "project.setting2": "43" - } + }, + "marked_for_deletion": false + }, + { + "id": "tb5W0bx765nDVIwqJPw92G", + "settings": { + "project.setting": "44" + }, + "marked_for_deletion": true } - ] + ], + "projects_marked_for_deletion_generation": 1 } } """, @@ -927,6 +936,8 @@ private static ClusterState buildMultiProjectClusterState(DiscoveryNode... nodes projectId1, Settings.builder().put(PROJECT_SETTING.getKey(), 42).put(PROJECT_SETTING2.getKey(), 43).build() ) + .putProjectSettings(projectId2, Settings.builder().put(PROJECT_SETTING.getKey(), 44).build()) + .markProjectForDeletion(projectId2) .build() ) .blocks( @@ -2226,7 +2237,7 @@ public static int expectedChunkCount(ToXContent.Params params, ClusterState clus } else if (custom instanceof SnapshotsInProgress snapshotsInProgress) { chunkCount += 2 + snapshotsInProgress.asStream().count(); } else if (custom instanceof ProjectStateRegistry projectStateRegistry) { - chunkCount += 2 + projectStateRegistry.size(); + chunkCount += 3 + projectStateRegistry.size(); } else { // could be anything, we have to just try it chunkCount += Iterables.size( diff --git a/server/src/test/java/org/elasticsearch/cluster/project/ProjectStateRegistrySerializationTests.java b/server/src/test/java/org/elasticsearch/cluster/project/ProjectStateRegistrySerializationTests.java new file mode 100644 index 0000000000000..97b8b86ea6a85 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/cluster/project/ProjectStateRegistrySerializationTests.java @@ -0,0 +1,89 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.cluster.project; + +import org.elasticsearch.cluster.ClusterModule; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.Diff; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.SimpleDiffableWireSerializationTestCase; + +import java.io.IOException; +import java.util.stream.IntStream; + +public class ProjectStateRegistrySerializationTests extends SimpleDiffableWireSerializationTestCase { + + @Override + protected ClusterState.Custom makeTestChanges(ClusterState.Custom testInstance) { + return mutate((ProjectStateRegistry) testInstance); + } + + @Override + protected Writeable.Reader> diffReader() { + return ProjectStateRegistry::readDiffFrom; + } + + @Override + protected NamedWriteableRegistry getNamedWriteableRegistry() { + return new NamedWriteableRegistry(ClusterModule.getNamedWriteables()); + } + + @Override + protected Writeable.Reader instanceReader() { + return ProjectStateRegistry::new; + } + + @Override + protected ClusterState.Custom createTestInstance() { + return randomProjectStateRegistry(); + } + + @Override + protected ClusterState.Custom mutateInstance(ClusterState.Custom instance) throws IOException { + return mutate((ProjectStateRegistry) instance); + } + + private ProjectStateRegistry mutate(ProjectStateRegistry instance) { + if (randomBoolean() && instance.size() > 0) { + // Remove or mutate a project's settings or deletion flag + var projectId = randomFrom(instance.getProjectsSettings().keySet()); + var builder = ProjectStateRegistry.builder(instance); + builder.putProjectSettings(projectId, randomSettings()); + if (randomBoolean()) { + // mark for deletion + builder.markProjectForDeletion(projectId); + } + return builder.build(); + } else { + // add a new project + return ProjectStateRegistry.builder(instance).putProjectSettings(randomUniqueProjectId(), randomSettings()).build(); + } + } + + private static ProjectStateRegistry randomProjectStateRegistry() { + final var projects = randomSet(1, 5, ESTestCase::randomUniqueProjectId); + final var projectsUnderDeletion = randomSet(0, 5, ESTestCase::randomUniqueProjectId); + var builder = ProjectStateRegistry.builder(); + projects.forEach(projectId -> builder.putProjectSettings(projectId, randomSettings())); + projectsUnderDeletion.forEach( + projectId -> builder.putProjectSettings(projectId, randomSettings()).markProjectForDeletion(projectId) + ); + return builder.build(); + } + + public static Settings randomSettings() { + var builder = Settings.builder(); + IntStream.range(0, randomIntBetween(1, 5)).forEach(i -> builder.put(randomIdentifier(), randomIdentifier())); + return builder.build(); + } +} diff --git a/server/src/test/java/org/elasticsearch/cluster/project/ProjectStateRegistryTests.java b/server/src/test/java/org/elasticsearch/cluster/project/ProjectStateRegistryTests.java new file mode 100644 index 0000000000000..a4d4dd6f2b154 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/cluster/project/ProjectStateRegistryTests.java @@ -0,0 +1,47 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.cluster.project; + +import org.elasticsearch.test.ESTestCase; +import org.hamcrest.Matchers; + +import static org.elasticsearch.cluster.project.ProjectStateRegistrySerializationTests.randomSettings; + +public class ProjectStateRegistryTests extends ESTestCase { + + public void testBuilder() { + final var projects = randomSet(1, 5, ESTestCase::randomUniqueProjectId); + final var projectsUnderDeletion = randomSet(0, 5, ESTestCase::randomUniqueProjectId); + var builder = ProjectStateRegistry.builder(); + projects.forEach(projectId -> builder.putProjectSettings(projectId, randomSettings())); + projectsUnderDeletion.forEach( + projectId -> builder.putProjectSettings(projectId, randomSettings()).markProjectForDeletion(projectId) + ); + var projectStateRegistry = builder.build(); + var gen1 = projectStateRegistry.getProjectsMarkedForDeletionGeneration(); + assertThat(gen1, Matchers.equalTo(projectsUnderDeletion.isEmpty() ? 0L : 1L)); + + projectStateRegistry = ProjectStateRegistry.builder(projectStateRegistry).markProjectForDeletion(randomFrom(projects)).build(); + var gen2 = projectStateRegistry.getProjectsMarkedForDeletionGeneration(); + assertThat(gen2, Matchers.equalTo(gen1 + 1)); + + if (projectsUnderDeletion.isEmpty() == false) { + // re-adding the same projectId should not change the generation + projectStateRegistry = ProjectStateRegistry.builder(projectStateRegistry) + .markProjectForDeletion(randomFrom(projectsUnderDeletion)) + .build(); + assertThat(projectStateRegistry.getProjectsMarkedForDeletionGeneration(), Matchers.equalTo(gen2)); + } + + var unknownProjectId = randomUniqueProjectId(); + var throwingBuilder = ProjectStateRegistry.builder(projectStateRegistry).markProjectForDeletion(unknownProjectId); + assertThrows(IllegalArgumentException.class, throwingBuilder::build); + } +} From 63293013e621d8931ed1091e3f93893ea00b68c0 Mon Sep 17 00:00:00 2001 From: Pooya Salehi Date: Tue, 1 Jul 2025 12:21:05 +0200 Subject: [PATCH 2/4] address review comments --- .../cluster/project/ProjectStateRegistry.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/project/ProjectStateRegistry.java b/server/src/main/java/org/elasticsearch/cluster/project/ProjectStateRegistry.java index 516952bea6167..4a31fdb78b4f1 100644 --- a/server/src/main/java/org/elasticsearch/cluster/project/ProjectStateRegistry.java +++ b/server/src/main/java/org/elasticsearch/cluster/project/ProjectStateRegistry.java @@ -47,8 +47,8 @@ public class ProjectStateRegistry extends AbstractNamedDiffable Date: Wed, 2 Jul 2025 10:36:57 +0200 Subject: [PATCH 3/4] remove assertion --- .../org/elasticsearch/cluster/project/ProjectStateRegistry.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/project/ProjectStateRegistry.java b/server/src/main/java/org/elasticsearch/cluster/project/ProjectStateRegistry.java index 4a31fdb78b4f1..5009cd6688665 100644 --- a/server/src/main/java/org/elasticsearch/cluster/project/ProjectStateRegistry.java +++ b/server/src/main/java/org/elasticsearch/cluster/project/ProjectStateRegistry.java @@ -202,7 +202,6 @@ public Builder markProjectForDeletion(ProjectId projectId) { public ProjectStateRegistry build() { final var unknownButUnderDeletion = Sets.difference(projectsMarkedForDeletion, projectsSettings.keys()); if (unknownButUnderDeletion.isEmpty() == false) { - assert unknownButUnderDeletion.stream().noneMatch(projectsSettings::containsKey); throw new IllegalArgumentException( "Cannot mark projects for deletion that are not in the registry: " + unknownButUnderDeletion ); From bd57bde90389ac9bd2b36c6263cd920d41118782 Mon Sep 17 00:00:00 2001 From: Pooya Salehi Date: Wed, 2 Jul 2025 18:33:12 +0200 Subject: [PATCH 4/4] minor test simplification --- .../org/elasticsearch/cluster/ClusterStateTests.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/ClusterStateTests.java b/server/src/test/java/org/elasticsearch/cluster/ClusterStateTests.java index 738f234e87b36..78623dd3f2738 100644 --- a/server/src/test/java/org/elasticsearch/cluster/ClusterStateTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/ClusterStateTests.java @@ -810,13 +810,6 @@ public void testToXContentWithMultipleProjects() throws IOException { "project.setting": "42", "project.setting2": "43" }, - "marked_for_deletion": false - }, - { - "id": "tb5W0bx765nDVIwqJPw92G", - "settings": { - "project.setting": "44" - }, "marked_for_deletion": true } ], @@ -936,8 +929,7 @@ private static ClusterState buildMultiProjectClusterState(DiscoveryNode... nodes projectId1, Settings.builder().put(PROJECT_SETTING.getKey(), 42).put(PROJECT_SETTING2.getKey(), 43).build() ) - .putProjectSettings(projectId2, Settings.builder().put(PROJECT_SETTING.getKey(), 44).build()) - .markProjectForDeletion(projectId2) + .markProjectForDeletion(projectId1) .build() ) .blocks(