james-project
james-project copied to clipboard
apache
JAMES-3771 Migrate opensearch high level rest client to new java client
A much more reworked cleaner version of https://github.com/apache/james-project/pull/1051
Still a few things remain:
- [x] the ssl tests on the backends to debug and re-enable
- [x] the sort issue with null fields... there was a workaround in the POC (that I personally don't like but I don't think we can do better)... I was hoping that the PR to fix on the opensearch client would be merged with a bit of change but.... nope (disabled the test for now, might put back the workaround though)
- [x] debugging what's left
Do we have a JIRA for this?
I reused JAMES-3771 but as we don't upgrade to es 8 anymore... should we create a new one or just update that task on the jira explaining it changed to this?
I reused JAMES-3771 but as we don't upgrade to es 8 anymore... should we create a new one or just update that task on the jira explaining it changed to this?
No that's OK
However you should...
- [ ] Link the ML conversations on the JIRA ticket
- [ ] Clearly explain that we switch to OpenSearch and adopt the Java client in there too.
I think something rather obscure is happening on the builds of this PR... will investigate
Now this work becomes based on #1078
We can then probably migrate faster to opensearch when waiting for this to be more stable
Exception in thread "OpenSearch-driver-0" java.lang.OutOfMemoryError: unable to create native thread: possibly out of memory or process/resource limits reached
at java.base/java.lang.Thread.start0(Native Method)
at java.base/java.lang.Thread.start(Thread.java:798)
at org.apache.http.impl.nio.reactor.AbstractMultiworkerIOReactor.execute(AbstractMultiworkerIOReactor.java:337)
at org.apache.http.impl.nio.conn.PoolingNHttpClientConnectionManager.execute(PoolingNHttpClientConnectionManager.java:221)
at org.apache.http.impl.nio.client.CloseableHttpAsyncClientBase$1.run(CloseableHttpAsyncClientBase.java:64)
at java.base/java.lang.Thread.run(Thread.java:829)
Well I'm gonna give OpenSearch a bit more mem then...
Could you please give us the output of jstack?
You get this error in James tests, or in the container output?
No No that's the CI... That's the problem with local... I can't reproduce anything wrong at all, it runs fine all the time :(
@chibenwa I must admit I did not have the motivation to switch back to master and recompile everything to test, but introducing the cleanup strategy on the alias and starting the opensearch docker container on a beforeAll for OpenSearchIntegrationTests... Was it really faster?
What I can observe locally is that if I revert this (see last commit) I earn 2 minutes back on the test suite regarding OpenSearchIntegrationTest in this PR so... I'm rather surprised :)
Amazing, that's :green_apple:
Can we measure performance for this (IMAP benches?)
Can we also compare the docker image size before / after ?
Can we measure performance for this (IMAP benches?)
Will take a look
Can we also compare the docker image size before / after ?
Of James you mean? Ok
Of James you mean? Ok
Yes it would allow us to measure the gains in dependency cleaning <3
So I generated the distributed apache james image with that PR and without (so master) and I can see a 20MB size reduction with this new java client: from 453 to 433MB
I think it's because you still need to use the rest client as a dependency and it still includes a bunch... I think from what I remember they were supposed to get rid of it and implement directly the needed methods in the opensearch java client (like in ES8) but seems they still didn't do
07:07:16,935 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.2:check (check-style) on project apache-james-backends-opensearch: You have 12 Checkstyle violations. -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.2:check (check-style) on project apache-james-backends-opensearch: You have 12 Checkstyle violations.