[FEATURE] Let an extension query OpenSearch for all initialized extensions

[dbwiddis]

Is your feature request related to a problem?

#108 will require extensions to be aware of what other extensions are initialized.

#102 (resolving #52 and #93) changed the initialization to only send one extension rather than the whole list. This is good because that list didn't have any status of the other extensions.

We need to be able to have an extension query OpenSearch to either obtain a list of initialized extensions or determine whether a particular extension is initialized.

What solution would you like?

This should be a simple Request/Response transport call fetching the values in the List<DiscoveryExtension> extensionsInitializedList; and returning it.

Alternately (see #151) accept a list of uniqueIds and return a shortened list containing just those extensions.

Bonus points: do both.

What alternatives have you considered?

The ExtensionsOrchestrator could broadcast the list to all extensions after initialization is complete. The transport would essentially be the same with request/response reversed. However, in the case of dependent extensions, an extension may not fully respond to the initialization request until after its dependency has been initialized.

Do you have any additional context?

Eventually this feature can be embedded as part of initialization, with a polling feature to enforce initialization order of dependencies. And hope there's no circular dependency loop!

[dbwiddis]

Implementation should be similar to sendClusterSettingsRequest except the response would be a List<DiscoveryExtension> object rather than a Settings object.

[mloufra]

• Review the Meta Issue and its sub issue ([FEATURE] Update DiscoveryExtension to include dependency information #151 and [FEATURE] Relay Request/Response actions between extensions #152) to get the idea about context.
• Think about any possible use case will happen.
• Establish a completion priority for each sub issue ([FEATURE] Update DiscoveryExtension to include dependency information #151 -> [FEATURE] Let an extension query OpenSearch for all initialized extensions #149 -> [FEATURE] Relay Request/Response actions between extensions #152)

[mloufra]

• Establish a completion priority for each sub issue ([FEATURE] Update DiscoveryExtension to include dependency information #151 -> [FEATURE] Let an extension query OpenSearch for all initialized extensions #149 -> [FEATURE] Relay Request/Response actions between extensions #152)

The reason why should start #151 first is because the purpose of #149 is to get initialization information based on extension's dependency, before that we should make sure the extension's dependency information which is #151 do.

[dbwiddis]

The reason why should start #151 first is because the purpose of #149 is to get initialization information based on extension's dependency, before that we should make sure the extension's dependency information which is #151 do.

I like this line of thinking! When I originally wrote the issues I tried to make them independent of each other, but doing them in the order you propose will reduce the re-writing of code.

[mloufra]

The purpose of this issue is to generate a request for dependency information, and send it from SDK to Opensearch thought TransportService. After the Opensearch receive the request, it will use extensionIdMap to find the List<ExtensionDependency> based on uniqueId, then generate a response with the List<ExtensionDependency> and send the response to SDK though TransportService. After we received the response from Opensearch, we transfer the response to List.

Opensearch-sdk-java

Add Request class ExtensionDependencyRequest for generate a request.
Add Response class ExtensionDependencyResponse for transfer the response from Opensearch to the List List<ExtensionDependency>
Add Handler class ExtensionDepdenceRequestHandler to prove logic process for sending dependency information request to Opensearch and receive dependency information response from Opensearch.
Register the request though TransportService by RegisterRequestHandler in ExtensionRunner.

• Add sendExtensionDependencyRequest method in ExtensionRunner.java like sendEnvironmentSettingsRequest

Opensearch (feature/extensions)

• Add new RequestType REQUEST_EXTENSION_DEPENDENCY_INFORMATION in ExtensionsManager
  Register the request though TransportService by RegisterRequestHandler in ExtensionsManager
  Add case for extension dependency information request in handleExtensionRequest method in ExtensionsManager

• Add Response class ExtensionDependencyResponse to generate response

• Add logic process for using extensionIdMap to get the List<ExtensionDiscoveryNode> in ExtensionsManager

• Add handleResponse method for DependencyInformationResponse to use countdown latches to delay the handler return until the response is available, like ExtensionManager.initializeExtension()

• Modify ExtensionRequest by adding new class variable and constructor.

[dbwiddis]

Add Request class ExtensionDependencyRequest for generate a request

If you create a new class for this, yes. If you use the ExtensionRequest as indicated later, then you don't need a new one; but see comments below.

Add Handler class ExtensionDepdenceRequestHandler

typo :) Also similar to the above comment, if you use the ExtensionRequest the handler already exists and you just need to add a case to it.

Register the request though TransportService by RegisterRequestHandler in ExtensionRunner

I think the handler needs to be registered in OpenSearch, since that is the node responding to the request. On the ExtensionsRunner side (probably in the ExtensionInitRequestHandler you will create a send....Request() method to do the transport sending and awaiting response.

Add new RequestType REQUEST_EXTENSION_DEPENDENCY_INFORMATION in ExtensionsManager

You will need both a String with a discovery prefix, and add to the enum.

Add case for extension dependency information request in handleExtensionRequest method in ExtensionsManager

The problem with using ExtensionRequest for this is that it currently only passes an enum value in the request which is used as a case in the handler. I see two alternatives here:

• Create your own request type and handler as the first two bullets, and create a separate handler for them
• Modify ExtensionRequest to add an Optional<String> parameter for the uniqueId. When it is needed, extensions can provide it; otherwise they can omit it. @joshpalis interested in your thoughts here.

Add handleResponse method for DependencyInformationResponse to use countdown latches to delay the handler return until the response is available

This may already be taken care of inside the existing handleExtensionRequest.
Two ways of doing this:

• countdown latch method is used in ExtensionRunner.sendEnvironmentSettingsRequest(). The environmentSettingsResponseHandler has the latch internally and an awaitResponse() method. This probably overcomplicates things if you use the ExtensionRequest.
• completable futures can be used as well. ExtensionManager.initializeExtension() method shows an example.

[joshpalis]

@dbwiddis I agree with your first alternative of modifying ExtensionRequest and adding an Optional parameter. I think it would be best to reuse more of our existing architecture rather than add a completely new request just for the unique ID.

[dbwiddis]

OK, so we'd want to modify ExtensionRequest to:

• add a new class variable private Optional<String> uniqueId = null; (default)
• add as second constructor that takes a String id parameter and assigns it this.uniqueId = Optional.of(id);
• add a getter for uniqueId that does a null check (return null) or returns the optional orElse(null)
• add writeable readers and writers to the stream constructor and writeTo(). The writeOptionalString() and readOptionalString() methods already exist.

[dbwiddis]

Also @mloufra note in the original issue we can enable returning the whole list (if we pass null instead of the uniqueID, that unfiltered list should be the default) or a filtered list, so that can work into your logic process.

[dbwiddis]

Looks good with two tweaks:

• Try to make the sendWhateverNameRequest() method and WhateverNameResponse class have the same name.
• THe list returned should be a List<DiscoveryExtensionNode>. It is currently the ExtensionManager field extension on line 88. You will either return the whole list (if no uniqueID supplied so it's null in the request) or return the list filtered to only dependencies.

[dbwiddis]

@joshpalis @mloufra I've been thinking about this a bit more and I think restricting to a single String overcomplicates this.

Consider that on initialization, the ExtensionsManager has already sent the ExtensionsRunner a copy of its own DiscoveryExtensionNode, which is stored in its own class variable extensionNode. This means that the extension already knows the uniqueId values of its dependencies.

I'm thinking rather than adding an Optional<String> to the ExtensionRequest, we can send a List<String>. This simplifies things:

• I'm always iffy about passing null around and this avoids that by allowing an empty list to be passed instead (and the input/output streams already have methods for lists of strings.
• In the case where a future implementation of ExtensionRequest wants a single string, we can always pass a list with size 1.
• But in this case, we can pass a list of strings, specifically a list of uniqueID of extensions we want to know are initialized. So the logic would simplify to:
  • start with the list of initialized extensions
  • if the extension request string list is empty, just return that list
  • if the extension request string list is not empty, filter the list such that stringList.contains(extension.getUniqueID()).

[dbwiddis]

After more thought, I realized we need more than the strings; we'd need the whole object including ID and versions and potentially other information.

This information is already in a map on the ExtensionsManager object so there is really no need to spend bandwidth sending it over transport; just send the uniqueID string as a key to get exactly this information from a map. So ignore the previous comment... I think that Optional<String> uniqueId implementation is still the best option.

[mloufra]

Last Version of the Plan

• Create a String with a discovery prefix in ExtensionManager and add this prefix in enum RequestType
• Modify ExtensionRequest for Dependency request

• add a new class variable private Optional uniqueId = null; (default)
• add as second constructor that takes a String id parameter and assigns it this.uniqueId = Optional.of(id);
• add a getter for uniqueId that does a null check (return null) or returns the optional orElse(null)
• add writeable readers and writers to the stream constructor and writeTo(). The writeOptionalString() and
  readOptionalString() methods already exist.

• Add send...Request method in ExtensionRunner (which need a handler class to handle the response form Opensearch)
• Send Asynchrom request to Opensearch threadpool. (which need to create a task for Dependency Request in thread pool)
• handleExtensionRequest method will handle the request. (which need add case for Dependency prefix and need a new response class with extension, extensionIdMap, uniqueId as argument to generate response to Extension(SDK))

[dbwiddis]

Looks good, @mloufra! Only minor clarification is that the constant names you're calling "prefix" are two separate but same-named things. One is a string constant and one is an enum. It might not have been the best idea for us to use the same names for both, but you're fine to continue this pattern.

[dbwiddis]

Reopening as opensearch-project/OpenSearch#5658 was only part of the code. Needs #300 to be complete.