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] Support multi-table sink feature for paimon #5652 #6449

Merged
merged 8 commits into from
Mar 19, 2024

Conversation

dailai
Copy link
Contributor

@dailai dailai commented Mar 5, 2024

Purpose of this pull request

[Feature][Paimon] Support cdc write of paimon sink #6427
[Feature][Paimon] Support the streaming mode for sink #6342
[Feature][Paimon] Support save mode
[Feature][Paimon] Support multi-table sink feature #5652

Does this PR introduce any user-facing change?

no

How was this patch tested?

add new test.

Check list

@EricJoy2048 EricJoy2048 closed this Mar 5, 2024
@EricJoy2048 EricJoy2048 reopened this Mar 5, 2024
@Hisoka-X
Copy link
Member

Hisoka-X commented Mar 5, 2024

Please open CI on your fork repository.

@dailai dailai force-pushed the dev-support-cdc-on-paimon-connector branch from 2f7ccda to 013d68e Compare March 5, 2024 11:24
@dailai
Copy link
Contributor Author

dailai commented Mar 5, 2024

Please open CI on your fork repository.

done

@dailai dailai force-pushed the dev-support-cdc-on-paimon-connector branch from 5e3fe5e to 7650e3f Compare March 6, 2024 03:35
Comment on lines 29 to 32
permissions:
packages: write
name: Run
uses: ./.github/workflows/backend.yml
uses: ./.github/notify_test_workflow.yml
Copy link
Member

Choose a reason for hiding this comment

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

Hi @dailai , please do not change the workflow file. It should work fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it was a mistake

@dailai dailai force-pushed the dev-support-cdc-on-paimon-connector branch from 7650e3f to 23dbea9 Compare March 6, 2024 04:01
@dailai dailai requested a review from Hisoka-X March 6, 2024 04:12
@Hisoka-X
Copy link
Member

Hisoka-X commented Mar 6, 2024

Please try merge from dev to retrigger CI.

@Hisoka-X Hisoka-X added the feature New feature label Mar 6, 2024

import java.util.concurrent.atomic.AtomicInteger;

public class PaimonTypeMapper {
Copy link
Member

Choose a reason for hiding this comment

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

Please extends TypeConverter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please extends TypeConverter.

ok

Comment on lines 107 to 112
// Auto create if not exists the database and table for paimon
catalog.createDatabase(database, true);
TableSchema tableSchema = catalogTable.getTableSchema();
this.seaTunnelRowType = tableSchema.toPhysicalRowDataType();
Schema paimonTableSchema = SchemaUtil.toPaimonSchema(tableSchema);
catalog.createTable(identifier, paimonTableSchema, true);
Copy link
Member

Choose a reason for hiding this comment

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

Please move create table logic to SaveModeHandler part. You can refer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please move create table logic to SaveModeHandler part. You can refer

done, please review!

Comment on lines +50 to +54
<dependency>
<groupId>org.xerial.snappy</groupId>
<artifactId>snappy-java</artifactId>
<version>1.1.10.4</version>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for add this?

Copy link
Contributor Author

@dailai dailai Mar 6, 2024

Choose a reason for hiding this comment

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

Any reason for add this?
The version of snnapy in seatunnel-shade/seatunnel-hadoop3-3.1.4-uber is confict with the version of sannpy built info flink-dis(flink1.16). If you submit tasks through flink mode, may cause this error.

企业微信截图_3ac783f8-740b-4616-a89f-a7cf8c1285b0

Copy link
Member

Choose a reason for hiding this comment

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

I think we shoud not put seatunnel-hadoop3-3.1.4-uber into flink libs. It only used for zeta. With flink, we should use their own hadoop jar.

Copy link
Contributor Author

@dailai dailai Mar 6, 2024

Choose a reason for hiding this comment

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

I think we shoud not put seatunnel-hadoop3-3.1.4-uber into flink libs. It only used for zeta. With flink, we should use their own hadoop jar.

I did not put the seatunnel-hadoop3-3.1.4-uber into flink libs. If we submit a job by the seatunnel's flink script , the flink will load the seatunnel-hadoop3-3.1.4-uber in classpath. For example, exec "start-seatunnel-flink-15-connector-v2.sh --config xxxx.job"(The source is mysql-cdc and sink is paimon )

Copy link
Member

Choose a reason for hiding this comment

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

@Hisoka-X please recheck

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for ping me. I will check it this weekend.

@dailai dailai force-pushed the dev-support-cdc-on-paimon-connector branch from 23dbea9 to d1925e8 Compare March 6, 2024 08:12
@dailai dailai requested a review from Hisoka-X March 7, 2024 06:00
@dailai dailai force-pushed the dev-support-cdc-on-paimon-connector branch from 2e0943b to 6d44826 Compare March 7, 2024 09:12
@dailai dailai force-pushed the dev-support-cdc-on-paimon-connector branch from a003e52 to cd7c21a Compare March 8, 2024 02:26
@dailai dailai force-pushed the dev-support-cdc-on-paimon-connector branch from cd7c21a to 5b32488 Compare March 8, 2024 02:44
@dailai dailai changed the title [Feature][Paimon] Support the streaming mode for sink #6342 [Feature][Connector-V2] Support multi-table sink feature #5652 Mar 11, 2024
@dailai dailai changed the title [Feature][Connector-V2] Support multi-table sink feature #5652 [Feature][Connector-V2] Support multi-table sink feature for paimon #5652 Mar 11, 2024
}

source {
MySQL-CDC {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hailin0
Copy link
Member

hailin0 commented Mar 11, 2024

Please change the docs

@dailai
Copy link
Contributor Author

dailai commented Mar 11, 2024

Please change the docs
done

@dailai dailai requested a review from hailin0 March 11, 2024 08:50
@dailai dailai force-pushed the dev-support-cdc-on-paimon-connector branch from 317712e to bf918e4 Compare March 11, 2024 08:57
@@ -0,0 +1,65 @@
#
Copy link
Member

Choose a reason for hiding this comment

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

remove this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,24 @@
--
Copy link
Member

Choose a reason for hiding this comment

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

remove this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

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
cc @TyrantLucifer

hailin0
hailin0 previously approved these changes Mar 11, 2024
import static org.apache.seatunnel.connectors.seatunnel.paimon.config.PaimonConfig.HDFS_SITE_PATH;
import static org.apache.seatunnel.connectors.seatunnel.paimon.config.PaimonConfig.TABLE;
import static org.apache.seatunnel.connectors.seatunnel.paimon.config.PaimonConfig.WAREHOUSE;
import static org.apache.seatunnel.api.table.factory.FactoryUtil.discoverFactory;

@AutoService(SeaTunnelSink.class)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to remove this annotation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need to remove this annotation?

I have removed it.

<dependencies>
<dependency>
<groupId>org.apache.seatunnel</groupId>
<artifactId>connector-jdbc</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

Can this dependency be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

"sh", "-c", "cd " + CATALOG_ROOT_DIR + " && tar -zxvf " + NAMESPACE_TAR);
try {
Process process = processBuilder.start();
// 等待命令执行完成
Copy link
Member

Choose a reason for hiding this comment

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

Please use English comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

- [x] [exactly-once](../../concept/connector-v2-features.md)

## 连接器选项

Copy link
Member

Choose a reason for hiding this comment

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

There seem to be some problems with the format

Copy link
Contributor Author

@dailai dailai Mar 12, 2024

Choose a reason for hiding this comment

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

There seem to be some problems with the format

I've already previewed it and there don't seem to be any formatting issues here.

截屏2024-03-12 13 37 46

@dailai dailai force-pushed the dev-support-cdc-on-paimon-connector branch 6 times, most recently from 8a1d736 to b5474a9 Compare March 12, 2024 07:35
@dailai dailai force-pushed the dev-support-cdc-on-paimon-connector branch from b5474a9 to 408784f Compare March 12, 2024 08:09
Copy link
Member

@Carl-Zhou-CN Carl-Zhou-CN left a comment

Choose a reason for hiding this comment

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

LGTM

@Hisoka-X Hisoka-X merged commit b0abbd2 into apache:dev Mar 19, 2024
8 checks passed
@dailai dailai deleted the dev-support-cdc-on-paimon-connector branch March 19, 2024 06:42
chaorongzhi pushed a commit to chaorongzhi/seatunnel that referenced this pull request Aug 21, 2024
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.

5 participants