-
Notifications
You must be signed in to change notification settings - Fork 58
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
[FEATURE] Replace calls to SDKClusterService state() with more targeted calls #674
Comments
This issue will address the performance for AD Extension as well.
IMO, if we are able to find all the alternative APIs for the usage of ClusterState then moving with the 1st option makes sense. |
Other common case I see is checking for blocks (e.g., |
It's possible the ClusterService call gives information on system indices not available via the Index API, so there may be a need for some middle ground not currently covered by the APIs. |
Needed methods: String[] concreteIndices = indexNameExpressionResolver
.concreteIndexNames(
sdkClusterService.state(),
IndicesOptions.lenientExpandOpen(),
sourceIndices.toArray(new String[0])
);
|
More research notes:
|
Did some digging into the different ways AD Extension is using the cluster state to determine if an index exists, currently there are three :
From this exploration, it seems that there is no difference in using the |
I'm curious if that differentiates between index and alias? i.e., if an index exists and has an alias, does Other APIs looks pretty simple...
|
I don't think this is the case, as the metadata class also has an |
Next set of cluster state calls that I shall handle focuses on retrieving index level metadata/routing tables : The following both return the
The following returns a list of
The following call returns the
TLDR : Can replace calls to the cluster state to retrieve |
Moving onto the following cluster state calls :
@dbwiddis rather than retrieving the cluster name from another call to the
|
Is your feature request related to a problem?
In the AD plugin, the process of creating a detector makes frequent use of the
ClusterService.state()
method, to:There are several more usages associated with Job Scheduler.
In OpenSearch, these repeated calls aren't a performance issue because the methods simply return the same (up to date) object every time.
In the SDK, every single request sends a transport call and since it's unfiltered, returns the entire cluster state of OpenSearch over transport, even if we only need one bit of information.
This is like turning on a fire hose to get a sip of water.
Eight times.
What solution would you like?
Don't use
SDKClusterService.state()
unless you really really really mean it. And you probably don't.We should audit our usage of state in AD (as in the first comment) and update the migration guide to identify use cases and provide suggested alternatives.
We may want to just consider marking the
SDKClusterService
state()
method as deprecated to give a clear signal to developers that this is not the droid they are looking for.What alternatives have you considered?
Status quo: fetch the cluster state every time
Maintain a local state, similar to the way we do for Settings, with update consumers:
Do you have any additional context?
Hat tip to @dblock for pointing out the obvious in this comment. Reading all the comments on that PR is also educational.
The text was updated successfully, but these errors were encountered: