[FLINK-33859] Support OpenSearch v2 #38
Conversation
snuyanzin
force-pushed
the
flink33859
branch
2 times, most recently
from
2070c96
to
dc39a1a
Compare
snuyanzin
force-pushed
the
flink33859
branch
from
dc39a1a
to
51a81da
Compare
|
@snuyanzin I think separating v1 and v2 would bring a lot of addition maintenance work, how about trying to use multi-release jar instead? We know 100% that 2.x needs JDK-11, so we could isolate a few key classes that use breaking APIs under |
|
I'm curious whether multirelease jar supports cases when for the same Flink cluster there is a necessity to use both Opensearch v1 and OpenSearch v2 connectors and both are built with jdk11 for instance? |
It is very possible in theory, but would be very difficult to pull off in practice I believe - both clients have overlapping set of classes - and if used with same job, would need quite extensive classloader manipulations |
|
yes, I see... Then it is not clear to me what multi release jar could give us here?
Since currently (main branch) connector is building with jdk8, 11 then IIUC it doesn't solve the issue when connector both for v1 and v2 is building with jdk11. Or did I miss anything? Here in the PR I also extracted common classes into base module and in v1, v2 related modules are only opensearch api dependent classes |
Correct, hence the profiles that we touched upon the other day, jdk-8 - 1.x, jdk-11 - 2.x
I have objections to that (it is a right way to me), I am just suggesting more simpler alternative that we might consider |
I guess the issue is that so far the build/test by default is happening for jdk8 and 11, that means that ideally we need to keep it building/testing with jdk11 for v1 as well |
Fair point, but we cannot build with jdk8 for 2.x, so it is no win/win in any case sadly |
|
at least we could build/test with what it is allowed
|
Yep. it looks reasonable |
|
@reswqa do you have time to have a look here please? |
There was a problem hiding this comment.
Thanks @snuyanzin, I believe supporting OpenSearch2 is a worthwhile endeavor.
This changes overall looks good to me(just based on my knowledge of es-connector repo). 👍
I just have one question: Since the legacy sink implementation (based on the sink function) is deprecated, why did we need to include it in OpenSearch V2 still?
|
thanks for the feedback @reswqa
Here the intention was just to support both versions. The approach was same as for version 1 and for ElasticSearch connector I guess in fact we need to address this issue with deprecated Sink for both Elastic (6, 7) and OpenSearch(1, 2) connectors. I would suggest to create a dedicated follow up task for that, WDYT? |
@snuyanzin fyi, we have #5 open |
Sounds good. |
|
Can this be resolved and pushed? Is there a reason why it hasnt? @reta @reswqa @snuyanzin |
|
first this should be reviewed/approved/merged |
|
can we now continue with this @reta @reswqa @snuyanzin |
|
this is in my todo list for today/tomorrow |
snuyanzin
force-pushed
the
flink33859
branch
4 times, most recently
from
02b0345
to
58dfe14
Compare
|
Thanks for the review |
|
where can i download opensearch 2 compatible flink sql connector jar? |
|
It is in voting stage |
I mean the built Then I submitting my sql job that copies data from postgres table to opensearch index and I get following error: As I understand, something was not build well and |
if you open the email thread https://lists.apache.org/thread/by44cdpfv6p9394vwxhh1vzh3rfskzms you will see
and this is a link to the folder containing jars, however again these jars are only RC, not the release yet |
|
there is only |
|
With this jar i get another error: something changed in table configuration except connector name? |
|
@snuyanzin I'm also having this issue, what should we do? |
|
could you provide a minimum reproducing test for that? |
The PR adds support for OpenSearch v2
Since there are breaking changes introduced in OpenSearch[1], [2] there is no way for current code in main to be working for both v1 and v2.
For that reason it is now splitted in same way like it is for elastic: one jar for v1, another for v2.
Connector name for v1 is same as before -
opensearch, for v2 it isopensearch-2.Since v2 is java 11 based there is a java11 maven profile for v2 which makes opensearch connector for v2 building only in case of java 11+. There are some attempts on OpenSearch side to improve this situation, in case of success building with java8 for OpenSearch v2 could be easily added by removal of that profile.
Also PR bumps dependency for Flink to 1.18.0. The reason is incompatible changes for ArchUnit which makes the code passing archunit tests either only for 1.17 or only for 1.18., 1.19.
Also it adds support for java 17
[1] opensearch-project/OpenSearch#9082
[2] opensearch-project/OpenSearch#5902