spring-data-opensearch icon indicating copy to clipboard operation
spring-data-opensearch copied to clipboard

[FEATURE] Add `opensearch-java` client support

Open reta opened this issue 2 years ago • 17 comments

Is your feature request related to a problem?

The only supported client at the moment is RHLC, we should also add opensearch-java support.

What solution would you like?

Support opensearch-java

What alternatives have you considered?

Keep only RHLC

Do you have any additional context?

N/A

reta avatar Oct 11 '22 14:10 reta

https://opensearch.org/docs/latest/clients/java-rest-high-level/

The OpenSearch Java high-level REST client will be deprecated starting with OpenSearch version 3.0.0 and will be removed in a future release. We recommend switching to the Java client instead.

lbenedetto avatar Jun 14 '23 15:06 lbenedetto

hi @reta it's been a while, hope you're doing well.

Was wondering your thoughts on this issue. Now that high level rest client is deprecated, do you think it makes more sense to just update this code to use opensearch-java or make both options until 3.0 fully removes the deprecated client?

I might have some cycles to look at this.

johnament avatar Oct 02 '23 14:10 johnament

Hey @johnament , great to hear from you!

Was wondering your thoughts on this issue. Now that high level rest client is deprecated, do you think it makes more sense to just update this code to use opensearch-java or make both options until 3.0 fully removes the deprecated client?

I think it makes sense to go with opensearch-java all way down since Spring Data Elasticsearch 5.2 drops the rest client support altogether, and we won't be able to update.

reta avatar Oct 02 '23 15:10 reta

@johnament it seems like the urgency for this one has elevated a bit, AFAIK Spring Data Elasticsearch in 5.2 drops the RHCL based support completely, have you started some work already? If not, I could kick it off and would appreciate your help, thank you.

reta avatar Nov 07 '23 18:11 reta

We are removing the Elasticsearch RestHighLevelClient, but your code is derived from the abstract class that is the base for the old and new client, so this removal should not affect you.

sothawo avatar Nov 14 '23 15:11 sothawo

is there a plan for when this will happen? it'd be great to get rid of the RHLC dependency when working with spring-data-opensearch

rursprung avatar Jan 05 '24 09:01 rursprung

is there a plan for when this will happen?

There is intent to do that, no timelines though, sadly

reta avatar Jan 05 '24 23:01 reta

@reta apologies. no work has started on my side as we are able to do what we need using opensearch-java directly instead of the repository pattern though I have more use cases I want to enable that support repositories and raw client usage and would see this being ideal. I am available to help out a bit but have just now started digging back in.

johnament avatar Jan 07 '24 16:01 johnament

I am available to help out a bit but have just now started digging back in.

No problem, thanks a lot for the update @johnament , I hope to pick it up shortly (at least have a draft pull request to collaborate), thank you!

reta avatar Jan 07 '24 16:01 reta

Also, I'm sure you already realize it but when cutting over I found it easier to start with something like this, which constructs the java client from the underlying rest client via transport, this way the configuration code can be cutover separately from the consuming code:

diff --git a/settings.gradle.kts b/settings.gradle.kts
index 2180e9b..c1df11d 100644
--- a/settings.gradle.kts
+++ b/settings.gradle.kts
@@ -31,6 +31,7 @@ dependencyResolutionManagement {
     
     create("opensearchLibs") {
       version("opensearch", "2.11.0")
+      library("java", "org.opensearch.client:opensearch-java:2.8.1")
       library("client", "org.opensearch.client", "opensearch-rest-client").versionRef("opensearch")
       library("high-level-client", "org.opensearch.client", "opensearch-rest-high-level-client").versionRef("opensearch")
       library("sniffer", "org.opensearch.client", "opensearch-rest-client-sniffer").versionRef("opensearch")
diff --git a/spring-data-opensearch/build.gradle.kts b/spring-data-opensearch/build.gradle.kts
index a981afe..24217e0 100644
--- a/spring-data-opensearch/build.gradle.kts
+++ b/spring-data-opensearch/build.gradle.kts
@@ -26,6 +26,7 @@ dependencies {
   api(opensearchLibs.high.level.client) {
     exclude("commons-logging", "commons-logging")
   }
+  api(opensearchLibs.java)
   
   implementation(jacksonLibs.core)
   implementation(jacksonLibs.databind)
diff --git a/spring-data-opensearch/src/main/java/org/opensearch/data/client/orhlc/RestClients.java b/spring-data-opensearch/src/main/java/org/opensearch/data/client/orhlc/RestClients.java
index 5a84f7a..95c0003 100644
--- a/spring-data-opensearch/src/main/java/org/opensearch/data/client/orhlc/RestClients.java
+++ b/spring-data-opensearch/src/main/java/org/opensearch/data/client/orhlc/RestClients.java
@@ -30,6 +30,10 @@ import org.apache.http.protocol.HttpContext;
 import org.opensearch.client.RestClient;
 import org.opensearch.client.RestClientBuilder;
 import org.opensearch.client.RestHighLevelClient;
+import org.opensearch.client.json.jackson.JacksonJsonpMapper;
+import org.opensearch.client.opensearch.OpenSearchClient;
+import org.opensearch.client.transport.OpenSearchTransport;
+import org.opensearch.client.transport.rest_client.RestClientTransport;
 import org.springframework.data.elasticsearch.support.HttpHeaders;
 import org.springframework.util.Assert;
 
@@ -139,6 +143,11 @@ public final class RestClients {
             return rest().getLowLevelClient();
         }
 
+        default OpenSearchClient javaClient() {
+            final OpenSearchTransport transport = new RestClientTransport(lowLevelRest(), new JacksonJsonpMapper());
+            return new OpenSearchClient(transport);
+        }
+
         @Override
         default void close() throws IOException {
             rest().close();

johnament avatar Jan 07 '24 17:01 johnament

Also, I'm sure you already realize it but when cutting over I found it easier to start with something like this, which constructs the java client from the underlying rest client via transport, this way the configuration code can be cutover separately from the consuming code:

Thanks @johnament , to be fair I have not considered this option (primarily because it keeps dependency on opensearch-rest-client whereas opensearch-java does not need it with Apache HttpClient 5 based transport), but this is certainly one of the possible route to explore.

reta avatar Jan 07 '24 19:01 reta

but this is certainly one of the possible route to explore.

So yeah, I had incorrectly assumed that the OpenSearchRestClient was used more than it actually is, and the real problem here is the mapping to/from the internal Spring Data Elasticsearch classes.

Now that there's starting to be bigger forking behaviors here, is that still the right approach to think of? For many of the older models they just maintained arbitrary maps. The new java client now uses more concrete classes.

johnament avatar Jan 07 '24 20:01 johnament

Now that there's starting to be bigger forking behaviors here, is that still the right approach to think of? For many of the older models they just maintained arbitrary maps. The new java client now uses more concrete classes.

Correct, the idea was to follow the similar approach Spring Data Elasticsearch took with 2 independent clients for Elasticsearch: RestClient and elasticsearch-java client. So yeah, it is larger change.

reta avatar Jan 07 '24 21:01 reta

So how would you see handling something like this?

In Opensearch, Alias has 3 different routing strings - https://github.com/opensearch-project/opensearch-java/blob/main/java-client/src/main/java/org/opensearch/client/opensearch/indices/Alias.java#L52-L68 (the extra field has a different semantic per https://opensearch.org/docs/latest/api-reference/index-apis/alias/ ) But in Elastcisearch (and Spring Data ES) there's only two: https://github.com/spring-projects/spring-data-elasticsearch/blob/main/src/main/java/org/springframework/data/elasticsearch/core/index/AliasData.java#L27-L32

Basically my fundamental question is should Spring Data Opensearch continue to be derived from Spring Data Elasticsearch or should it be its own independent module (so that incorrect classes don't leak).

johnament avatar Jan 07 '24 21:01 johnament

Basically my fundamental question is should Spring Data Opensearch continue to be derived from Spring Data Elasticsearch or should it be its own independent module (so that incorrect classes don't leak).

This is very good question, the abstractions provided by Spring Data Elasticsearch allowed us to cut a lot of corners, but the key to that was the fact that ES and OS APIs didn't diverge too much (if at all). I think the vision for a long term is that Spring Data Opensearch will become independent.

reta avatar Jan 07 '24 21:01 reta

Hello. @reta My team has recently been thinking about adopting Opensearch and have been writing some simple code. However, we realized that RestHighLevelClient is deprecated, so we decided to develop using only opensearch-java

But it seems that only RestHighLevelClient is still supported in spring-data-opensearch, so we are having a hard time developing with Opensearch. If this can be resolved quickly, I would like to proceed with development using both RestHighLevelClient and opensearch-java for now.

AntCode97 avatar Mar 14 '24 09:03 AntCode97

Hey @AntCode97

However, we realized that RestHighLevelClient is deprecated, so we decided to develop using only opensearch-java

👍

But it seems that only RestHighLevelClient is still supported in spring-data-opensearch,

That is correct at the moment, the work is ongoing [1], there are some changes on opensearch-java side that have to go in (to complement Spring Data featureset), so no date here sadly.

so we are having a hard time developing with Opensearch.

Using 2 clients is indeed a bit annoying however if you use spring-data-opensearch, in your application you could use opensearch-java with RestClientTransport that uses the same RestClient as spring-data-opensearch, I believe that should expose only opensearch-java to all other parts of the application

[1] https://github.com/opensearch-project/spring-data-opensearch/pull/227

reta avatar Mar 14 '24 11:03 reta