[Fix][Connector-V2] Fixed clickhouse connectors cannot stop under multiple parallelism #7921

YOMO-Lee · 2024-10-26T16:04:38Z

Fixed the bug where clickhouse connectors cannot stop properly under multiple parallelism

Supplement and optimize the description of the LocalFile connector on filtering files [(#7887)](#7887)

1、When the ClickHouse connector is set to multi parallelism, the task extraction is completed but cannot be stopped normally [(#7897)](#7897) 2、Added E2E test cases for this issue [(#7897)](#7897) 3、Local developers want to observe **Job Progress Information** in a timely manner, Need to modify the following configuration.The configuration in config is invalid ``` seatunnel engine/seatunnel-engineer-common/src/main/resources/seatunnely.yaml ```

Continue to optimize the document about filtering files and add some examples [(#7887)](#7887)

Change to English log [(#7887)](#7887)

This reverts commit 42e5919.

This reverts commit baa7cb8.

corgy-w · 2024-10-26T18:56:13Z

seatunnel-engine/seatunnel-engine-common/src/main/resources/seatunnel.yaml

@@ -20,6 +20,7 @@ seatunnel:
        backup-count: 1
        queue-type: blockingqueue
        print-execution-info-interval: 60
+        print-job-metrics-info-interval: 60


Default value of print-job-metrics-info-interval is 60. This is valid without change

I know that the default value is also used if it is not changed here, but here is a parameter to let you know where you can change it.
Otherwise, if you run an example in the project and want to change the parameters, the results will not work

Default value of print-job-metrics-info-interval is 60. This is valid without change

I think this parameter can be added here, at least adding the default value will not cause any other bad effects
But it worked for me, at first I wanted to change the configuration, I didn't know where to change, I searched a lot of places in the project, and it didn't take effect after the change, and then it was someone else who pointed out that I knew to change here

You can configure it in your local environment. But do not commit it. Please revert it.

Hisoka-X · 2024-10-28T01:58:21Z

...use-e2e/src/test/java/org/apache/seatunnel/connectors/seatunnel/clickhouse/ClickhouseIT.java

@@ -101,6 +101,13 @@ public void testClickhouse(TestContainer container) throws Exception {
        clearSinkTable();
    }

+    @TestTemplate
+    public void testSourceParallelism(TestContainer container) throws Exception {
+        LOG.info("=========Multi parallelism testing begins===========");


Suggested change

LOG.info("=========Multi parallelism testing begins===========");

Hisoka-X · 2024-10-28T02:01:36Z

seatunnel-engine/seatunnel-engine-common/src/main/resources/seatunnel.yaml

@@ -20,6 +20,7 @@ seatunnel:
        backup-count: 1
        queue-type: blockingqueue
        print-execution-info-interval: 60
+        print-job-metrics-info-interval: 60


You can configure it in your local environment. But do not commit it. Please revert it.

Hisoka-X · 2024-10-28T02:02:01Z

...apache/seatunnel/connectors/seatunnel/clickhouse/source/ClickhouseSourceSplitEnumerator.java

        if (assigned < 0) {
            assigned = subtaskId;
            context.assignSplit(subtaskId, new ClickhouseSourceSplit());
+        } else {
+            context.signalNoMoreSplits(subtaskId);
        }


Suggested change

if (assigned < 0) {

assigned = subtaskId;

context.assignSplit(subtaskId, new ClickhouseSourceSplit());

} else {

context.signalNoMoreSplits(subtaskId);

}

if (assigned < 0) {

assigned = subtaskId;

context.assignSplit(subtaskId, new ClickhouseSourceSplit());

context.signalNoMoreSplits(subtaskId);

} else {

context.signalNoMoreSplits(subtaskId);

}

Write it like this?

if (assigned < 0) { assigned = subtaskId; context.assignSplit(subtaskId, new ClickhouseSourceSplit()); } context.signalNoMoreSplits(subtaskId);

Write it like this?

if (assigned < 0) { assigned = subtaskId; context.assignSplit(subtaskId, new ClickhouseSourceSplit()); } context.signalNoMoreSplits(subtaskId);

+1

Okay, I didn't notice this

Hisoka-X · 2024-10-28T02:02:44Z

...java/org/apache/seatunnel/connectors/seatunnel/clickhouse/source/ClickhouseSourceReader.java

            this.readerContext.signalNoMoreElement();
            this.splits.clear();
+        } else if (noMoreSplit
+                && splits.isEmpty()
+                && Boundedness.BOUNDED.equals(readerContext.getBoundedness())) {
+            log.info("Closed the bounded ClickHouse source");
+            this.readerContext.signalNoMoreElement();
+            this.splits.clear();
        }


Suggested change

this.readerContext.signalNoMoreElement();

this.splits.clear();

} else if (noMoreSplit

&& splits.isEmpty()

&& Boundedness.BOUNDED.equals(readerContext.getBoundedness())) {

log.info("Closed the bounded ClickHouse source");

this.readerContext.signalNoMoreElement();

this.splits.clear();

}

this.splits.clear();

}

if (noMoreSplit

&& splits.isEmpty()

&& Boundedness.BOUNDED.equals(readerContext.getBoundedness())) {

log.info("Closed the bounded ClickHouse source");

this.readerContext.signalNoMoreElement();

this.splits.clear();

}

Boundedness.BOUNDED.equals(readerContext.getBoundedness())
This can be removed I think because clickhouse only supports batch mode reading
cc @Hisoka-X

Keep it for now. The purpose of keeping it is to unify the processing logic of most connectors, to facilitate reference for other developers and reduce the chance of bugs.

Keep it for now. The purpose of keeping it is to unify the processing logic of most connectors, to facilitate reference for other developers and reduce the chance of bugs.

What if the user uses processing mode to write STREAMING? It's unnecessary code and I think it's better to get rid of it Developers adding features are certainly familiar with their read patterns

What if the user uses processing mode to write STREAMING?

Sorry, I don get it. Could you share more details? You meaning set job.mode to STREAMING?

What if the user uses processing mode to write STREAMING?

Sorry, I don get it. Could you share more details? You meaning set job.mode to STREAMING?

env { job.mode = "STREAMING" parallelism = 2 } source { # This is a example source plugin **only for test and demonstrate the feature source plugin** Clickhouse { host = "127.0.0.1:8123" database = "default" sql = "select * from students" username = "default" password = "" } # If you would like to get more information about how to configure seatunnel and see full list of source plugins, # please go to https://seatunnel.apache.org/docs/connector-v2/source/ClickhouseSource } sink { Console { } # If you would like to get more information about how to configure seatunnel and see full list of sink plugins, # please go to https://seatunnel.apache.org/docs/connector-v2/sink }

So for example in this case the user will never exit the program, essentially no matter how you configure the read mode clickhouse only supports batch mode, right

If users set to STREAMING, but source only supported BATCH mode. The job will execute as BATCH mode. It depends on

seatunnel/seatunnel-connectors-v2/connector-clickhouse/src/main/java/org/apache/seatunnel/connectors/seatunnel/clickhouse/source/ClickhouseSource.java

Line 160 in a8d0d4c

return Boundedness.BOUNDED;

Understood, but an extra judgment is a redundant operation

Hisoka-X

Please add checkpoint lock when read spilt. Please refer

seatunnel/seatunnel-connectors-v2/connector-doris/src/main/java/org/apache/seatunnel/connectors/doris/source/reader/DorisSourceReader.java

Line 76 in 50113e7

synchronized (output.getCheckpointLock()) {

Hisoka-X · 2024-10-28T02:57:20Z

cc @zhilinli123 as well.

YOMO-Lee · 2024-10-28T09:39:30Z

cc @zhilinli123 as well.
1.Thanks for ping me, I think pom needs to be added


   <dependency>
            <groupId>org.apache.seatunnel</groupId>
            <artifactId>seatunnel-api</artifactId>
            <version>${project.version}</version>
        </dependency>

I don't quite understand where this should be added and where it needs to be used. Currently, I haven't found any places where it needs to be used

zhilinli123 · 2024-10-28T12:51:27Z

Waiting for the start of the current pr merger modify this code: #7529

…tiple parallelism Adjust the bug regarding the Clickhouse connector not being able to stop properly

zhilinli123 · 2024-10-29T02:59:23Z

...java/org/apache/seatunnel/connectors/seatunnel/clickhouse/source/ClickhouseSourceReader.java

+                }
+                this.readerContext.signalNoMoreElement();
+                this.splits.clear();
+            }


Extract clickhouse Close the task method

this.readerContext.signalNoMoreElement(); this.splits.clear();

zhilinli123 · 2024-10-29T02:59:41Z

...java/org/apache/seatunnel/connectors/seatunnel/clickhouse/source/ClickhouseSourceReader.java

+            }
+            if (noMoreSplit && splits.isEmpty()) {
+                log.info("Closed the bounded ClickHouse source");
+                this.readerContext.signalNoMoreElement();


zhilinli123 · 2024-10-29T03:00:29Z

...apache/seatunnel/connectors/seatunnel/clickhouse/source/ClickhouseSourceSplitEnumerator.java

@@ -77,6 +77,9 @@ public void registerReader(int subtaskId) {
        if (assigned < 0) {
            assigned = subtaskId;
            context.assignSplit(subtaskId, new ClickhouseSourceSplit());
+            context.signalNoMoreSplits(subtaskId);
+        } else {


if (assigned < 0) { assigned = subtaskId; context.assignSplit(subtaskId, new ClickhouseSourceSplit()); } context.signalNoMoreSplits(subtaskId);

…tiple parallelism Optimize code structure

…tiple parallelism format code

zhilinli123 · 2024-10-29T06:20:56Z

Wait for CI pass

Hisoka-X

LGTM if ci passes. Thanks @YOMO-Lee and @zhilinli123 @corgy-w for review!

zhilinli123 · 2024-10-29T07:40:05Z

LGTM if ci passes. Thanks @YOMO-Lee and @zhilinli123 @corgy-w for review!

+1

hailin0 · 2024-10-29T08:08:38Z

waiting for ci passed

YOMO-Lee and others added 13 commits October 22, 2024 17:28

[Fix][Doc] Fix LocalFile doc (#7887)

baa7cb8

Supplement and optimize the description of the LocalFile connector on filtering files [(#7887)](#7887)

Merge branch 'apache:dev' into dev

415a510

Merge branch 'apache:dev' into dev

9088a0d

[Fix][Doc] Fix LocalFile doc (#7887)

42e5919

Continue to optimize the document about filtering files and add some examples [(#7887)](#7887)

Merge branch 'apache:dev' into dev

8b13c97

[Fix][Doc] Fix LocalFile doc (#7887)

2e9162d

Change to English log [(#7887)](#7887)

Merge remote-tracking branch 'origin/dev' into dev

f5073f6

Merge branch 'apache:dev' into dev

1dcf78e

Revert "[Fix][Doc] Fix LocalFile doc (#7887)"

be22d93

This reverts commit 42e5919.

Revert "[Fix][Doc] Fix LocalFile doc (#7887)"

64fa711

This reverts commit baa7cb8.

github-actions bot added Zeta connectors-v2 e2e clickhouse labels Oct 26, 2024

corgy-w reviewed Oct 26, 2024

View reviewed changes

Hisoka-X changed the title ~~[Fix] ClickHouse Connector BUG (#7897)~~ [Fix][Connector-V2] Fixed clickhouse connectors cannot stop under multiple parallelism Oct 28, 2024

Hisoka-X added the First-time contributor First-time contributor label Oct 28, 2024

Hisoka-X reviewed Oct 28, 2024

View reviewed changes

Hisoka-X linked an issue Oct 28, 2024 that may be closed by this pull request

[Bug] [clickhouse] CK连接器任务抽取完毕后无法正常停止 #7897

Closed

3 tasks

[Fix][Connector-V2] Fixed clickhouse connectors cannot stop under mul…

9109024

…tiple parallelism Adjust the bug regarding the Clickhouse connector not being able to stop properly

zhilinli123 reviewed Oct 29, 2024

View reviewed changes

[Fix][Connector-V2] Fixed clickhouse connectors cannot stop under mul…

ace9fb0

…tiple parallelism Optimize code structure

YOMO-Lee requested review from zhilinli123, Hisoka-X and corgy-w October 29, 2024 04:03

[Fix][Connector-V2] Fixed clickhouse connectors cannot stop under mul…

802f1f9

…tiple parallelism format code

Hisoka-X approved these changes Oct 29, 2024

View reviewed changes

github-actions bot added approved reviewed labels Oct 29, 2024

zhilinli123 approved these changes Oct 29, 2024

View reviewed changes

hailin0 approved these changes Oct 29, 2024

View reviewed changes

hailin0 merged commit 8d9c6a3 into apache:dev Oct 29, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix][Connector-V2] Fixed clickhouse connectors cannot stop under multiple parallelism #7921

[Fix][Connector-V2] Fixed clickhouse connectors cannot stop under multiple parallelism #7921

YOMO-Lee commented Oct 26, 2024

corgy-w Oct 26, 2024

YOMO-Lee Oct 27, 2024

YOMO-Lee Oct 27, 2024

corgy-w Oct 27, 2024

Hisoka-X Oct 28, 2024

Hisoka-X Oct 28, 2024

Hisoka-X Oct 28, 2024

Hisoka-X Oct 28, 2024

zhilinli123 Oct 28, 2024

Hisoka-X Oct 29, 2024

YOMO-Lee Oct 29, 2024

Hisoka-X Oct 28, 2024

zhilinli123 Oct 29, 2024

Hisoka-X Oct 29, 2024

zhilinli123 Oct 29, 2024

Hisoka-X Oct 29, 2024 •

edited

Loading

zhilinli123 Oct 29, 2024

Hisoka-X Oct 29, 2024

zhilinli123 Oct 29, 2024

Hisoka-X left a comment

Hisoka-X commented Oct 28, 2024

YOMO-Lee commented Oct 28, 2024

zhilinli123 commented Oct 28, 2024

zhilinli123 Oct 29, 2024

zhilinli123 Oct 29, 2024

YOMO-Lee Oct 29, 2024

zhilinli123 Oct 29, 2024

zhilinli123 commented Oct 29, 2024

Hisoka-X left a comment

zhilinli123 commented Oct 29, 2024

hailin0 commented Oct 29, 2024

[Fix][Connector-V2] Fixed clickhouse connectors cannot stop under multiple parallelism #7921

[Fix][Connector-V2] Fixed clickhouse connectors cannot stop under multiple parallelism #7921

Conversation

YOMO-Lee commented Oct 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hisoka-X Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hisoka-X left a comment

Choose a reason for hiding this comment

Hisoka-X commented Oct 28, 2024

YOMO-Lee commented Oct 28, 2024

zhilinli123 commented Oct 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhilinli123 commented Oct 29, 2024

Hisoka-X left a comment

Choose a reason for hiding this comment

zhilinli123 commented Oct 29, 2024

hailin0 commented Oct 29, 2024

Hisoka-X Oct 29, 2024 •

edited

Loading