Projects reserved state is moved to ProjectStateRegistry #132332
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,7 +61,6 @@ | |
| import java.util.Objects; | ||
| import java.util.Optional; | ||
| import java.util.Set; | ||
| import java.util.function.BiConsumer; | ||
| import java.util.function.BiPredicate; | ||
| import java.util.function.Consumer; | ||
|
|
@@ -799,12 +798,6 @@ private Iterator<? extends ToXContent> toXContentChunkedWithSingleProjectFormat( | |
| @FixForMultiProject | ||
| final ProjectMetadata project = projectMetadata.values().iterator().next(); | ||
|
|
||
|
Comment on lines
-802
to
-806
There was a problem hiding this comment. The intention of this code is keep the output from the GetClusterState API identical in non-MP and MP (when the API is invoked via a project URL, i.e. single project context). Removing it without any replace means the responses will now differ, i.e. vs Maybe it's OK for them to be different. But we need to make an explicit decision here. Also, there is an existing issue that we don't filter There was a problem hiding this comment. On a second thought, the difference in ClusterState API output format between non-MP and MP setups can also be addressed in future PRs. There should not be any BWC concern to change the MP output in future. So nothing is needed in this PR other than maybe raise a JIRA issue so that we don't forget about it. There was a problem hiding this comment. Thank you for highlighting this. I'll address the project filtering issue in a separate PR since it's straightforward, and I don't want to leave it for later. There was a problem hiding this comment.
👍
Yes it is not an issue for existing clusters (non-MP). My question is about MP cluster. We have a general principle that the API output for a single project should be the same regardless whether the cluster is non-MP or MP. For the GetClusterState API, non-MP already sets the convention that reserved state is printed under There was a problem hiding this comment. PR for the filtering issue: #133401 There was a problem hiding this comment.
I see now, thanks for the clarification. I'm a bit concerned we're going to make a mistake in one of these many branches eventually, but I'll support the convention. However, I'd prefer to handle it at the transport action level. I think that's cleaner, and I believe we don't want to store project's reserved state metadata to disk There was a problem hiding this comment. I'll do it in a separate PR, because turned out that I need changes from both this PR and #133401 😕 There was a problem hiding this comment.
That might be an option as well. I don't have strong opinion.
Yeah that's what I meant. No need for this PR. Since it affects only MP cluster, there is no real BWC either. |
||
| // Similarly, combine cluster and project persistent tasks and report them under a single key | ||
| Iterator<ToXContent> customs = Iterators.flatMap(customs().entrySet().iterator(), entry -> { | ||
| if (entry.getValue().context().contains(context) && ClusterPersistentTasksCustomMetadata.TYPE.equals(entry.getKey()) == false) { | ||
|
|
@@ -824,13 +817,20 @@ private Iterator<? extends ToXContent> toXContentChunkedWithSingleProjectFormat( | |
| ); | ||
| } | ||
|
|
||
| // make order deterministic | ||
| Iterator<ReservedStateMetadata> reservedStateMetadataIterator = reservedStateMetadata.entrySet() | ||
| .stream() | ||
| .sorted(Map.Entry.comparingByKey()) | ||
| .map(Map.Entry::getValue) | ||
| .iterator(); | ||
|
|
||
| return Iterators.concat( | ||
| start, | ||
| clusterCoordination, | ||
| persistentSettings, | ||
| project.toXContentChunked(p), | ||
| customs, | ||
| ChunkedToXContentHelper.object("reserved_state", reservedStateMetadataIterator), | ||
| ChunkedToXContentHelper.endObject() | ||
| ); | ||
| } | ||
|
|
@@ -845,6 +845,7 @@ private static class MetadataDiff implements Diff<Metadata> { | |
| private final Settings persistentSettings; | ||
| private final Diff<DiffableStringMap> hashesOfConsistentSettings; | ||
| private final ProjectMetadata.ProjectMetadataDiff singleProject; | ||
|
|
||
| private final MapDiff<ProjectId, ProjectMetadata, Map<ProjectId, ProjectMetadata>> multiProject; | ||
| private final MapDiff<String, ClusterCustom, ImmutableOpenMap<String, ClusterCustom>> clusterCustoms; | ||
| private final MapDiff<String, ReservedStateMetadata, ImmutableOpenMap<String, ReservedStateMetadata>> reservedStateMetadata; | ||
|
|
@@ -981,7 +982,7 @@ private MetadataDiff(StreamInput in) throws IOException { | |
| RESERVED_DIFF_VALUE_READER | ||
| ); | ||
|
|
||
| singleProject = new ProjectMetadata.ProjectMetadataDiff(indices, templates, projectCustoms); | ||
| multiProject = null; | ||
| } else { | ||
| fromNodeBeforeMultiProjectsSupport = false; | ||
|
|
@@ -1048,7 +1049,7 @@ public void writeTo(StreamOutput out) throws IOException { | |
| singleProject.indices().writeTo(out); | ||
| singleProject.templates().writeTo(out); | ||
| buildUnifiedCustomDiff().writeTo(out); | ||
| reservedStateMetadata.writeTo(out); | ||
| } else { | ||
| clusterCustoms.writeTo(out); | ||
| reservedStateMetadata.writeTo(out); | ||
|
|
@@ -1094,15 +1095,6 @@ public void writeTo(StreamOutput out) throws IOException { | |
| } | ||
| } | ||
|
|
||
| @Override | ||
| public Metadata apply(Metadata part) { | ||
| if (empty) { | ||
|
|
@@ -1304,12 +1296,7 @@ public void writeTo(StreamOutput out) throws IOException { | |
| ); | ||
| VersionedNamedWriteable.writeVersionedWriteables(out, combinedCustoms); | ||
|
|
||
| out.writeCollection(reservedStateMetadata.values()); | ||
| } else { | ||
| VersionedNamedWriteable.writeVersionedWriteables(out, customs.values()); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a convenience method to get
reservedStateMetadatatoProjectStateas we've done for settings so this could just beprojectResolver.getProjectState(state).reservedStateMetadata()?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not strongly oppose it, but I don't think reserved state metadata is needed in a project state - for example,
Settingscan be used by many components, but there are not so many things that are interested in the reserved state