-
Notifications
You must be signed in to change notification settings - Fork 25.5k
Update RemoteClusterService skip_unavailable handling for CPS #132478
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
Update RemoteClusterService skip_unavailable handling for CPS #132478
Conversation
In stateless CPS skip_unavailable will not be used and should always be true. Also DisconnectedStrategy.RECONNECT_UNLESS_SKIP_UNAVAILABLE is rejected in CPS as a parameter value for getRemoteClusterClient(). Resolves: ES-12267
5510b9d
to
c1cd712
Compare
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
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 have a question for the validation.
server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java
Outdated
Show resolved
Hide resolved
I'll need to split this into separate PRs. We can make the skip_unavailable settings change now, but the client call change for |
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
server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/transport/RemoteClusterSettingsTests.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/transport/RemoteClusterSettingsTests.java
Outdated
Show resolved
Hide resolved
} | ||
}; | ||
|
||
private static class UnsupportedInStatelessValidator<T> extends RemoteConnectionEnabled<T> { |
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 use "serverless" rather than "stateless"? We typically use the terms "stateful" vs. "serverless", so when I see "stateless" I think have to take a moment to parse what it means since the first half of the word feels like one and the second half like the other.
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.
Oh, I see why you used it - because of the pre-existing DiscoveryNode.STATELESS_ENABLED_SETTING_NAME
. In that case, I guess it makes sense to keep using "stateless" here.
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.
Switch to CPS in the name since the RemoteClusterService
and related code all now test the CPS specific setting.
In serverless cross-project search (CPS)
skip_unavailable
will not be used and should always betrue. This PR adds a settings validator that rejects setting skip_unavailable when CPS is enabled.
Resolves: ES-12267