From 2ba6aed4afe765f8d05e2c3ac171e822e8c07ab9 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Fri, 28 Feb 2025 13:49:13 +1100 Subject: [PATCH] Optimize index lookup for single default project case We can avoid using the volatile field when there is only a single default project. Relates: #123662 --- .../cluster/metadata/Metadata.java | 53 ++++++------------- .../cluster/metadata/MetadataTests.java | 11 ++-- 2 files changed, 19 insertions(+), 45 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java index 1d762230b5df0..a00017d477450 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java @@ -1848,6 +1848,10 @@ public Metadata fromXContent(XContentParser parser) throws IOException { * Attempt to find a project for the supplied {@link Index}. */ public Optional lookupProject(Index index) { + if (isSingleProject()) { + final var project = projectMetadata.get(DEFAULT_PROJECT_ID); + return project.hasIndex(index) ? Optional.of(project) : Optional.empty(); + } return getProjectLookup().project(index); } @@ -1866,6 +1870,9 @@ public ProjectMetadata projectFor(Index index) { * @throws org.elasticsearch.index.IndexNotFoundException if the index does not exist in any project */ public IndexMetadata indexMetadata(Index index) { + if (isSingleProject()) { + return projectMetadata.get(DEFAULT_PROJECT_ID).getIndexSafe(index); + } return projectFor(index).getIndexSafe(index); } @@ -1874,6 +1881,9 @@ public IndexMetadata indexMetadata(Index index) { * throwing when either the project or the index is not found. */ public Optional findIndex(Index index) { + if (isSingleProject()) { + return Optional.ofNullable(projectMetadata.get(DEFAULT_PROJECT_ID).index(index)); + } return lookupProject(index).map(projectMetadata -> projectMetadata.index(index)); } @@ -1883,16 +1893,13 @@ ProjectLookup getProjectLookup() { * That means it is possible that we will generate multiple lookup objects if there are multiple concurrent callers * Those lookup objects will be identical, and the double assignment will be safe, but there is the cost of building the lookup * more than once. - * In the single project case building the lookup is cheap, and synchronization would be costly. + * The single default project case has special handling and does not go through the project lookup. * In the multiple project case, it might be cheaper to synchronize, but the long term solution is to maintain the lookup table * as projects/indices are added/removed rather than rebuild it each time the cluster-state/metadata object changes. */ if (this.projectLookup == null) { - if (this.isSingleProject()) { - projectLookup = new SingleProjectLookup(getSingleProject()); - } else { - projectLookup = new MultiProjectLookup(); - } + assert isSingleProject() == false; + projectLookup = new ProjectLookup(); } return projectLookup; } @@ -1900,39 +1907,10 @@ ProjectLookup getProjectLookup() { /** * A lookup table from {@link Index} to {@link ProjectId} */ - interface ProjectLookup { - /** - * Return the {@link ProjectId} for the provided {@link Index}, if it exists - */ - Optional project(Index index); - } - - /** - * An implementation of {@link ProjectLookup} that is optimized for the case where there is a single project. - * - */ - static class SingleProjectLookup implements ProjectLookup { - - private final ProjectMetadata project; - - SingleProjectLookup(ProjectMetadata project) { - this.project = project; - } - - @Override - public Optional project(Index index) { - if (project.hasIndex(index)) { - return Optional.of(project); - } else { - return Optional.empty(); - } - } - } - - class MultiProjectLookup implements ProjectLookup { + class ProjectLookup { private final Map lookup; - private MultiProjectLookup() { + private ProjectLookup() { this.lookup = Maps.newMapWithExpectedSize(Metadata.this.getTotalNumberOfIndices()); for (var project : projectMetadata.values()) { for (var indexMetadata : project) { @@ -1947,7 +1925,6 @@ private MultiProjectLookup() { } } - @Override public Optional project(Index index) { final ProjectMetadata project = lookup.get(index.getUUID()); if (project != null && project.hasIndex(index)) { diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java index 300005bf7d1d1..84e1cf32c20ca 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java @@ -76,7 +76,6 @@ import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xcontent.json.JsonXContent; -import org.hamcrest.Matchers; import java.io.IOException; import java.lang.reflect.Field; @@ -3037,17 +3036,16 @@ public void testProjectLookupWithSingleProject() { projectBuilder.put(indexMetadata, false); } - final Metadata.ProjectLookup lookup = Metadata.builder().put(projectBuilder).build().getProjectLookup(); - assertThat(lookup, Matchers.instanceOf(Metadata.SingleProjectLookup.class)); + final Metadata lookup = Metadata.builder().put(projectBuilder).build(); indices.forEach(im -> { final Index index = im.getIndex(); - assertThat(lookup.project(index).map(ProjectMetadata::id), isPresentWith(projectId)); + assertThat(lookup.lookupProject(index).map(ProjectMetadata::id), isPresentWith(projectId)); Index alt1 = new Index(index.getName(), randomValueOtherThan(im.getIndexUUID(), ESTestCase::randomUUID)); - assertThat(lookup.project(alt1), isEmpty()); + assertThat(lookup.lookupProject(alt1), isEmpty()); Index alt2 = new Index(randomAlphaOfLength(8), im.getIndexUUID()); - assertThat(lookup.project(alt2), isEmpty()); + assertThat(lookup.lookupProject(alt2), isEmpty()); }); } @@ -3076,7 +3074,6 @@ public void testProjectLookupWithMultipleProjects() { } final Metadata.ProjectLookup lookup = metadataBuilder.build().getProjectLookup(); - assertThat(lookup, Matchers.instanceOf(Metadata.MultiProjectLookup.class)); indices.forEach((project, ix) -> ix.forEach(im -> { final Index index = im.getIndex();