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

[Feature][Connector-V2] add tdengine source #2832

Merged
merged 15 commits into from
Jan 10, 2023

Conversation

lhyundeadsoul
Copy link
Contributor

Purpose of this pull request

refer to #2671

Check list

@EricJoy2048 EricJoy2048 changed the title add tdengine source [Feature][Connector-V2] add tdengine source Sep 23, 2022
@Hisoka-X
Copy link
Member

Hi, please solve ci problem and conflict. Thanks!

Copy link
Member

@hailin0 hailin0 left a comment

Choose a reason for hiding this comment

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

docs/en/connector-v2/source/TDengine.md Outdated Show resolved Hide resolved
docs/en/connector-v2/source/TDengine.md Outdated Show resolved Hide resolved
docs/en/connector-v2/source/TDengine.md Outdated Show resolved Hide resolved
docs/en/connector-v2/source/TDengine.md Show resolved Hide resolved
seatunnel-connectors-v2-dist/pom.xml Outdated Show resolved Hide resolved
@lhyundeadsoul lhyundeadsoul force-pushed the td_engine_source branch 2 times, most recently from 081314e to 0048e00 Compare September 30, 2022 09:19
Copy link
Contributor Author

@lhyundeadsoul lhyundeadsoul left a comment

Choose a reason for hiding this comment

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

pre-version is PIW version. Now It's completed.

@EricJoy2048
Copy link
Member

Please fix the code style.

docs/en/connector-v2/source/TDengine.md Outdated Show resolved Hide resolved
docs/en/connector-v2/source/TDengine.md Outdated Show resolved Hide resolved
seatunnel-connectors-v2-dist/pom.xml Outdated Show resolved Hide resolved
@lhyundeadsoul lhyundeadsoul force-pushed the td_engine_source branch 2 times, most recently from 130e0f4 to f37d682 Compare October 21, 2022 10:31
seatunnel-connectors-v2-dist/pom.xml Outdated Show resolved Hide resolved
+ " ) VALUES ("
+ StringUtils.join(convertDataType(metrics), ",")
+ ");";
final int rowCount = statement.executeUpdate(sql);
Copy link
Member

Choose a reason for hiding this comment

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

Can you use batch flush for jdbc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

org.apache.seatunnel.connectors.seatunnel.tdengine.sink.TDengineSinkWriter#write method has only one SeaTunnelRow param, so I think batch flush will be used when we have write(List<SeaTunnelRow>) api.

Copy link
Contributor

Choose a reason for hiding this comment

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

Whether to support write(List<SeaTunnelRow>) needs to be discussed. Currently ST only supports write (SeaTunnelRow), if you need better performance, you need to implement batch flush yourself.

Copy link
Member

@hailin0 hailin0 left a comment

Choose a reason for hiding this comment

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

  1. add sink docs
  2. revert example module changes
  3. add e2e testcase

@EricJoy2048 EricJoy2048 closed this Nov 1, 2022
@EricJoy2048 EricJoy2048 reopened this Nov 1, 2022
@EricJoy2048
Copy link
Member

Retry CI

@lhyundeadsoul
Copy link
Contributor Author

lhyundeadsoul commented Dec 29, 2022

@EricJoy2048 @hailin0 @Hisoka-X @TyrantLucifer @CalvinKirs
Hey guys, please help to give me a CR if you are free. Thx!

@CalvinKirs
Copy link
Member

I can't find the previous context, why can't we implement this in jdbc-connector?

@lhyundeadsoul
Copy link
Contributor Author

I can't find the previous context, why can't we implement this in jdbc-connector?

The reason why we implement tdengine connector separately is that there are some distinctive concepts in TDengine that do not belong to JDBC, such as subtable, tags, etc.

Copy link
Member

@hailin0 hailin0 left a comment

Choose a reason for hiding this comment

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

hailin0
hailin0 previously approved these changes Jan 7, 2023
Copy link
Member

@hailin0 hailin0 left a comment

Choose a reason for hiding this comment

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

LGTM

docs/en/connector-v2/sink/TDengine.md Outdated Show resolved Hide resolved
docs/en/connector-v2/source/TDengine.md Outdated Show resolved Hide resolved
+ " ) VALUES ("
+ StringUtils.join(convertDataType(metrics), ",")
+ ");";
final int rowCount = statement.executeUpdate(sql);
Copy link
Contributor

Choose a reason for hiding this comment

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

Whether to support write(List<SeaTunnelRow>) needs to be discussed. Currently ST only supports write (SeaTunnelRow), if you need better performance, you need to implement batch flush yourself.

Hisoka-X
Hisoka-X previously approved these changes Jan 8, 2023
ic4y
ic4y previously approved these changes Jan 9, 2023
Copy link
Contributor

@ic4y ic4y left a comment

Choose a reason for hiding this comment

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

LGTM

@lhyundeadsoul lhyundeadsoul dismissed stale reviews from ic4y and Hisoka-X via 80da544 January 9, 2023 06:31
TyrantLucifer
TyrantLucifer previously approved these changes Jan 9, 2023
Copy link
Member

@TyrantLucifer TyrantLucifer left a comment

Choose a reason for hiding this comment

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

@CalvinKirs CalvinKirs merged commit acf4d5b into apache:dev Jan 10, 2023
@lhyundeadsoul lhyundeadsoul deleted the td_engine_source branch January 10, 2023 07:40
harveyyue pushed a commit to harveyyue/seatunnel that referenced this pull request Feb 27, 2023
* [Feature][Connector-V2]add tdengine source and sink
1. add ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY to statement
2. add tdengine e2e module

* SourceSplitEnumerator.Context

Co-authored-by: bjyflihongyu <lihongyuinfo@jd.com>
Co-authored-by: tyrantlucifer <tyrantlucifer@gmail.com>
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.

7 participants