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

[Connector-V2][JDBC-connector] support Jdbc dm #2377

Merged
merged 10 commits into from
Sep 9, 2022

Conversation

laglangyue
Copy link
Contributor

@laglangyue laglangyue commented Aug 6, 2022

Purpose of this pull request

Check list

@laglangyue laglangyue changed the title Jdbc dm [Connector-V2][JDBC-connector] support Jdbc dm Aug 6, 2022
@laglangyue
Copy link
Contributor Author

I imitate MySqlCatalog to DMCatalog, but I don't know it work.
and the workflows is failed, I need some time to solve it,and I try to learn know how to do it rencently.

@CalvinKirs
Copy link
Member

If necessary, please update the documentation to describe the new feature. https://github.com/apache/incubator-seatunnel/tree/dev/docs

@CalvinKirs
Copy link
Member

@ic4y PTAL,thx

@ic4y
Copy link
Contributor

ic4y commented Aug 8, 2022

I imitate MySqlCatalog to DMCatalog, but I don't know it work. and the workflows is failed, I need some time to solve it,and I try to learn know how to do it rencently.

Ask any questions at any time.

@laglangyue
Copy link
Contributor Author

If necessary, please update the documentation to describe the new feature. https://github.com/apache/incubator-seatunnel/tree/dev/docs

ok,I will lately

@laglangyue
Copy link
Contributor Author

I imitate MySqlCatalog to DMCatalog, but I don't know it work. and the workflows is failed, I need some time to solve it,and I try to learn know how to do it rencently.

Ask any questions at any time.

TypeMapper is in the method
org.apache.seatunnel.connectors.seatunnel.jdbc.catalog.MySqlCatalog#fromJdbcType

but also in this method
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.mysql.MySqlTypeMapper

Any difference?

@laglangyue
Copy link
Contributor Author

another question, Mysql has the catalog named MysqlCatalog ,and why does PostgreSQL have no catalog such as PostGresqlCatalog?

@ic4y
Copy link
Contributor

ic4y commented Aug 9, 2022

I imitate MySqlCatalog to DMCatalog, but I don't know it work. and the workflows is failed, I need some time to solve it,and I try to learn know how to do it rencently.

Ask any questions at any time.

TypeMapper is in the method org.apache.seatunnel.connectors.seatunnel.jdbc.catalog.MySqlCatalog#fromJdbcType

but also in this method org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.mysql.MySqlTypeMapper

Any difference?

There is really no difference between the two, This is indeed repeated in the implementation of the catalog. However, the catalog has not been started yet, so it is not necessary to implement the catalog related things.

Refer to the implementation under the jdbc.internal.dialect package.

@ic4y
Copy link
Contributor

ic4y commented Aug 9, 2022

another question, Mysql has the catalog named MysqlCatalog ,and why does PostgreSQL have no catalog such as PostGresqlCatalog?

Because only the catalog of mysql is currently implemented, pg has not yet been implemented. DM can also temporarily not implement catalog

@laglangyue
Copy link
Contributor Author

DM can also temporarily not implement catalog

thx,I get , I will modify my code this night

@laglangyue laglangyue force-pushed the jdbc_dm branch 2 times, most recently from df82462 to 2b2ce89 Compare August 10, 2022 14:11
@ic4y
Copy link
Contributor

ic4y commented Aug 11, 2022

Have you tested XA-Transaction?, This will be used in exactly-once Sink. And add how to configure exactly-once sink in the document

@laglangyue
Copy link
Contributor Author

Have you tested XA-Transaction?, This will be used in exactly-once Sink. And add how to configure exactly-once sink in the document

Thx for your guide, I will do it this weekend.

@ic4y
Copy link
Contributor

ic4y commented Aug 15, 2022

@laglangyue Wait for the JDBC e2e test (#2321) to merge. You need to add test for DM in jdbc e2e test

@laglangyue
Copy link
Contributor Author

laglangyue commented Aug 20, 2022

I have encountered a difficulty. The PostgreSQL driver is directly introduced and introduced into the Lib of connector-v2. I am afraid this is not appropriate.

In the E2E test, I need a driver in the docker. I can manually copy it to the Lib folder of connector-v2-dist, but it will fail if in the CI environment or someone else.

Therefore, I wonder whether an additional module, similar to e2e-driver-jar-dist, is required to be provided to E2E test and copied to docker.

@hailin0
Copy link
Member

hailin0 commented Aug 21, 2022

The PostgreSQL driver is directly introduced and introduced into the Lib of connector-v2. I am afraid this is not appropriate.

@CalvinKirs Do you have any suggestions

@laglangyue
Copy link
Contributor Author

laglangyue commented Aug 23, 2022

image
I have 2 ideas to solve it.

  1. maven-dependency-plugin generate the dir such lib include all dirver-jar, and copy it to container, like above.
  2. Run the command in container wget https://repo1.maven.org/maven2/com/dameng/DmJdbcDriver18/8.1.2.141/DmJdbcDriver18-8.1.2.141.jar,and copy it to lib.

In our pom just need to provided scope
@CalvinKirs

@CalvinKirs
Copy link
Member

In fact, we could have a dist directory dedicated to these jars if we really needed to. (if necessary), but it seems that they are all jdbc driven, as a temporary solution, you can also use the second one. Of course, how you do it is up to you.

@laglangyue
Copy link
Contributor Author

image

@laglangyue
Copy link
Contributor Author

Please fixed checkstyle problem and conflct

done

pom.xml Outdated Show resolved Hide resolved
@Hisoka-X
Copy link
Member

Hisoka-X commented Sep 6, 2022

@ic4y Hi, PTAL

@laglangyue
Copy link
Contributor Author

@ic4y @Hisoka-X @CalvinKirs I has rebased it to dev, and change dependencies,Please review again.

tools/dependencies/known-dependencies.txt Outdated Show resolved Hide resolved
seatunnel-dist/release-docs/licenses/LICENSE-DMjdbc.txt Outdated Show resolved Hide resolved
assertHasData(SOURCE_TABLE);
Container.ExecResult execResult = executeSeaTunnelFlinkJob("/jdbc/jdbc_dm_source_and_sink.conf");
Assertions.assertEquals(0, execResult.getExitCode());
assertHasData(SINK_TABLE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest check whether the data in the two tables are the same

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I Ihave debugged to check. Verifying the data consistency between sink and source is a public Issue.. I hope to discuss with you in another PR, such as how to preset data and assert verification. I think this is required for all E2E use cases.
Currently, only DM has preset SQL for all fields, and other E2E has only a few fields

@ic4y
Copy link
Contributor

ic4y commented Sep 8, 2022

LGTM

Copy link
Member

@EricJoy2048 EricJoy2048 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@EricJoy2048 EricJoy2048 merged commit 7278209 into apache:dev Sep 9, 2022
laglangyue added a commit to laglangyue/seatunnel that referenced this pull request Sep 11, 2022
* [Connector-V2][JDBC-connector] Add DM source and sink connector
@laglangyue laglangyue deleted the jdbc_dm branch September 11, 2022 06:01
lhyundeadsoul pushed a commit to lhyundeadsoul/incubator-seatunnel that referenced this pull request Sep 13, 2022
* [Connector-V2][JDBC-connector] Add DM source and sink connector
MRYOG pushed a commit to MRYOG/incubator-seatunnel that referenced this pull request Sep 16, 2022
* [Connector-V2][JDBC-connector] Add DM source and sink connector
TyrantLucifer pushed a commit to TyrantLucifer/incubator-seatunnel that referenced this pull request Sep 18, 2022
* [Connector-V2][JDBC-connector] Add DM source and sink connector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants