Skip to content
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

[DISCUSS] Client implementation (Rest High LevelClient vs OpenSearch Java client) for Extensibility #294

Closed
owaiskazi19 opened this issue Dec 21, 2022 · 11 comments
Labels

Comments

@owaiskazi19
Copy link
Member

owaiskazi19 commented Dec 21, 2022

What/Why

What are you proposing?

As we are working towards converting AnomalyDetector plugin into AnomalyDetector(AD) Extension, we are using OpenSearch Java client to make REST API calls from AD Extension to OpenSearch. We have finished the first feature of AD i.e. Create Detector using the same.
The AD plugin uses High Level Rest client and the whole plugin is written based on a parser model (XContent) whereas Java client uses JSON model. Migrating this model to Java Client causes issue such as: opensearch-project/opensearch-java#297 and opensearch-project/opensearch-java#257. These issues are blocking us from proceeding with the next set of features of AD.

This issue will talk about the pros/cons of both the clients to move forward for Extensibility:

High Level Rest client (present in OpenSearch):
Pros:

  1. Easier to use this for converting any plugin to extension, as currently the plugin presents are using the same client.
  2. Less pain in migration or less code changes, which will eventually help any plugin owner to convert their plugin into extension.
  3. Request/Response models are already present in OpenSearch for the use cases.

Cons:

  1. Will bring OpenSearch dependency. (SDK also has OpenSearch dependency)
  2. Available only in Java. For multi-language SDK, this can be an issue
  3. Open proposal for deprecating REST Client: [META] Deprecate REST client OpenSearch#5424

OpenSearch Java Client:
Pros:

  1. Avoids dependency on OpenSearch and provides isolation.
  2. Available in multi-language. Will help in multi-language SDK.
  3. Eventually in 4.0 plugins owners will move to this client.

Cons:

  1. Difficult for converting plugins into extension as the model is different parser vs builder respectively.
  2. Pain in migration as the whole plugin needs to be written from scratch.
  3. Request/Response model should be match with java client requirements.
  4. Doesn't have a good support for all the request, builders right now which will impact Extensibility work.

If we move forward with Rest High level client, the migration would be much easier but eventually in the future we need to migrate to Java Client. Until then Java Client will be in the good shape to incorporate the above issues which is blocking the current work of AD Extension.

@owaiskazi19 owaiskazi19 changed the title [PROPOSAL] Best client (High Level Rest client vs OpenSearch Java client) for Extensibility [DISCUSS] Best client (High Level Rest client vs OpenSearch Java client) for Extensibility Dec 21, 2022
@dbwiddis
Copy link
Member

A full reading of opensearch-project/opensearch-java#262 should also be conducted before considering the above options.

@owaiskazi19
Copy link
Member Author

owaiskazi19 commented Dec 21, 2022

@dblock @reta @dbwiddis @saratvemulapalli need your input here. Thanks!

@joshpalis
Copy link
Member

If we frame this decision based on what would be the right path forward, rather than the easier path forward, I would be partial to java-client. The genesis of extensibility was to provide isolation for extensions, and thus bringing in OpenSearch as a dependency for extensions would be antithetical to our original goal. Additionally, the eventuality for other plugins to migrate over to java-client and for the rest-client to be deprecated later on would require us to later migrate over anyways.

@reta
Copy link
Contributor

reta commented Dec 21, 2022

Agree with @joshpalis, we also have a quorum on 2 things:

  • recommend opensearch-java as the way to communicate with OpenSearch from JVM applications
  • deprecate RHCL client

The picture at large: the opensearch-java would drop dependency on OpenSearch core (it has now) and would become significantly more lightweight (the work is ongoing in this direction).

@harshavamsi
Copy link

Agree with @joshpalis and @reta. java client should be the way to go. HLRC should be deprecated and dependency on core should ideally be avoided.

@dbwiddis
Copy link
Member

I don't disagree with the previous replies, but I think the initial question is asking about the "near term" way forward for plugin migration:

If we move forward with Rest High level client, the migration would be much easier but eventually in the future we need to migrate to Java Client.

I think the agreement is unanimous that we will eventually move to opensearch-java.

Background of this discussion

There are multiple simultaneous work streams going on with developing extensions.

  • Implement internal capabilities to support extensions (configuration, hot swapping, dependencies, communication between dependencies and native plugins, etc.)
  • Create the SDK and implement extension points, and document it so the community can build new plugins
  • Migrate existing plugins, some of which have very complex dependencies on HLRC

As far as the migration of plugins, there are two distinct portions of that:

  • Change the interface/SDK implementation from plugin to extension (extension points)
  • Change every RestHandler implementation from the existing implementation (HLRC) to the Java Client.

We have been taking great pains to try to keep the extension point implementation as similar as possible (same-named methods, etc.) to maximize re-use of existing code.

However, changing the client in the Rest Handlers is not permitting very much code re-use, and the refactored code looks very different.

We chose to migrate the anomaly-detection extension first because it implemented many of the extension points we needed to develop. However, it is proving extremely challenging as the Request/Response model used is completely different. Further, as noted in the initial comment on this issue, there are some very hard incompatibilities with existing HLRC implementations that simply do not have an equivalent in Java Client.

Further, the work is incredibly inefficient, both now and (I foresee) in the future. We have only a few people trying to understand the inner workings of code on a plugin supported by a different team; migrate them to a new client (which is complex to do) and then we are expected to hand it back to this other team to maintain the migrated code that they didn't write, leading to more inefficiencies.

Current status

HLRC is on a path to deprecation, but a longer path, per opensearch-project/OpenSearch#5424 where @reta and @dblock seemed to concur:

  • Deprecate a large portion of/all org.opensearch.client namespace in 2.x
  • Keep it in 3.0.0 but make the respective transport optional in opensearch-java (giving a time to migrate since it could be a short notice, might be a good start for 2.0 but we are at 2.4 already)
  • Remove it in 4.0.0

This is also not just a "let's wait". There are capabilities that do not yet exist in the Java Client. From this comment by @dlvenable:

It appears that this project just needs a new implementation of OpenSearchTransport which does not depend on opensearch-rest-client. An initial implementation could possibly just copy-and-paste the code from that project and abstract it appropriately (package protected).

The opensearch-java project could possibly retain the existing RestClientTransport and make opensearch-rest-client an optional dependency. This could help with migration.

My thoughts

While I think migrating the main plugin interface and extension points is well within the capability of the developers focusing on opensearch-sdk-java, the Rest Handler migration between clients is probably best handled by the teams already familiar with (and still actively developing) the plugins.

There is certainly value in the opensearch-sdk-java team doing some of the migration work alongside plugin teams. Identifying the obstacles and challenges and use of the extension points helps the entire project be more robust. But doing the migration without the participation of the plugin maintainers is more likely to be a net negative from a technical debt perspective.

My strong preference is:

  • Work on extension interface/SDK integration and extension points
  • Migrate existing Rest Handlers to use the new extension points but keep the existing client code
  • Let maintainers of actively supported plugins which use HLRC migrate to the Java Client independently of extension development. Work together with them to show examples, but keep them involved in all the new code development.

@owaiskazi19
Copy link
Member Author

owaiskazi19 commented Dec 22, 2022

Agree with @dbwiddis and thanks everyone for putting a fair point for the clients above.

For extensibility our main aim is to provide plugin owners with minimal changes to convert the plugin into an extension (Basically, using the extension points API from SDK rather overriden extension points from OpenSearch). If we have a hard requirement of using a java client instead of RHCL, it will create more questions for the migration.
We can provide both the clients i.e. java client and RHCL as a part of SDK. It should be a decision of plugin owner to move forward with the client of their choice.

Coming back to the deprecation of RHCL, it will be best if plugin owners handles the migration from RHCL to Java Client as the teams are already familiar with the functionalities. We can work together with them to help with the migration. Whenever the migration is complete instead of using RHCL they will be able to use SDKClient(Java Client) from SDK and thus converting the plugin back to extension with minimal changes.

@saratvemulapalli
Copy link
Member

saratvemulapalli commented Dec 23, 2022

+1 I think we are are in quorum that opensearch-java will be the client longer term.
The challenges we are seeing with extensions is we are moving over extension points to APIs and migrate plugins to use opensearch-java client is multiplying the effort.

I like @dbwiddis's suggestion, lets move on with High Level Rest Client in the SDK as one of the client options.
We could offer multiple clients from the SDK and eventually start deprecating HLRC overtime.

In the meantime though the opensearch-java client supports the uses cases we are looking for, its not a drop in replacement of existing plugins code which is causing the friction/effort to consume it. @wbeckler @harshavamsi
If we could make these migrations easier, we could move on existing devs from plugins/applications to opensearch-java.

TL;DR My Thoughts

  • Continue with HLRC for Extensions, this is strategically important to have minimal changes for plugins to migrate and avoid friction.
  • We are seeing gaps in the clients, though the functionality exists we must invest to make it easier to migrate from HLRC which enables deprecating HLRC.
  • With SDK we should start deprecating HLRC interfaces overtime and help extension authors migrate to opensearch-java/other language clients.

@dbwiddis
Copy link
Member

Thanks @saratvemulapalli ... TLDR of all the above, there are essentially three directions forward:

  1. Continue down the path of converting to java client while implementing. I really really don't like this because I think we are creating more technical debt
  2. Go down the path of "drop in replacement" using HLRC syntax at the minimum, using the actual HLRC. I really like this as a faster way to migrate now and change clients later.
  3. Go down the path of "drop in replacement" but create a new SDK client that does the translation between HLRC syntax and Java Client implementation. This is a possibility to get the best of both worlds by keeping syntax but implementing java client. This would be a large effort but could also speed the overall migration of all plugins. This may be the best final end state anyway, so we could do a "2 now, 3 later" approach as well.

@dblock
Copy link
Member

dblock commented Jan 10, 2023

Sorry for jumping late here. I'll throw my 0.02c :)

For a project like extensions I would do whatever hard work is required to not take a dependency on OpenSearch code earlier rather than later. This probably means picking up some of the work in opensearch-java for the folks working on extensions. Finally, I think it's acceptable to take a dependency on OpenSearch for a demo, debatable/borderline to do it for an experimental feature, and I rather strongly believe is a hard no for a GA release.

@dbwiddis dbwiddis reopened this Jan 10, 2023
@dbwiddis dbwiddis changed the title [DISCUSS] Best client (High Level Rest client vs OpenSearch Java client) for Extensibility [DISCUSS] Client implementation (Rest High LevelClient vs OpenSearch Java client) for Extensibility Jan 11, 2023
@dbwiddis
Copy link
Member

Re-opened this based on @dblock comment above and wanted to update on where we're going with this:

  1. I recognize the "GA Release" concern. We need to balance that with the fact that extensions are already "GA".
  2. I believe we have a nice "middle ground" with Create SDKRestClient wrapper class around RestHighLevelClient #367 where I've created a SDKRestClient class:
    • It is already marked deprecated and marked not to be used for new development
    • It doesn't expose the entire RestHighLevelClient, only needed methods used by existing plugins
    • It makes migrating existing plugins as easy as changing imports (in 95%+ of cases) without having to (immediately) change code
    • Since it's marked deprecated, the "unchanged" code now produces deprecation warnings that give the plugin maintainer teams clear indications of code they need to migrate in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants