-
Notifications
You must be signed in to change notification settings - Fork 25.5k
Fix single project GetClusterState API response for multiproject #133936
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
Fix single project GetClusterState API response for multiproject #133936
Conversation
… API for single project MP clusters
Pinging @elastic/es-core-infra (Team:Core/Infra) |
@@ -39,6 +39,7 @@ public class ClusterStateRequest extends LocalClusterStateRequest implements Ind | |||
private TimeValue waitForTimeout = DEFAULT_WAIT_FOR_NODE_TIMEOUT; | |||
private String[] indices = Strings.EMPTY_ARRAY; | |||
private IndicesOptions indicesOptions = IndicesOptions.lenientExpandOpen(); | |||
private boolean multiproject = false; |
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.
ClusterStateRequest
is LocalClusterStateRequest
, so doesn't require a new transport version
…er state is expected
test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java
Show resolved
Hide resolved
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.
Sorry for the delay here. I have one question about filtering cluster state.
.../src/main/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateAction.java
Outdated
Show resolved
Hide resolved
test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java
Show resolved
Hide resolved
params = Map.of(Metadata.CONTEXT_MODE_PARAM, Metadata.CONTEXT_MODE_API, "multi-project", "true"); | ||
clusterStateRequest.multiproject(true); |
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.
Now we have the new field, it feels an opportunity to remove the use of XContent parameter here as suggest by the above @FixForMultiProject
annotation. It does not need to be this PR.
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.
Great work. I had only some more minor comments.
.../src/main/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateAction.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateAction.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateAction.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateAction.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateAction.java
Outdated
Show resolved
Hide resolved
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.
LGTM
As it was noted in #132332 (comment) we have a general principle that API output for a single project should be the same regardless of whether the cluster is non-MP or MP. (However, the reserved metadata merge logic was never properly implemented to support this)
As a result of changes in that PR, response formats diverged. This PR makes the single project GetClusterState API response for MP clusters match the non-MP format