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.

Sign up for GitHub

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

[feat][io] Upgarde ClickHouse driver to support loadbalance policy #18774

Merged
merged 2 commits into from
Dec 21, 2022
Merged

[feat][io] Upgarde ClickHouse driver to support loadbalance policy #18774

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf7bb1a
[feat][io] Upgarde ClickHouse driver to support loadbalance policy
tisonkun Dec 7, 2022
909e870
Update pom.xml
tisonkun Dec 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ flexible messaging model and an intuitive client API.</description>
<sqlite-jdbc.version>3.36.0.3</sqlite-jdbc.version>
<mysql-jdbc.version>8.0.11</mysql-jdbc.version>
<postgresql-jdbc.version>42.4.1</postgresql-jdbc.version>
<clickhouse-jdbc.version>0.3.2</clickhouse-jdbc.version>
<clickhouse-jdbc.version>0.3.2-patch11</clickhouse-jdbc.version>
<mariadb-jdbc.version>2.7.5</mariadb-jdbc.version>
<openmldb-jdbc.version>0.4.4-hotfix1</openmldb-jdbc.version>
<hdfs-offload-version3>3.3.3</hdfs-offload-version3>
Expand Down
2 changes: 1 addition & 1 deletion pulsar-io/jdbc/clickhouse/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>ru.yandex.clickhouse</groupId>
<groupId>com.clickhouse</groupId>
Copy link

Choose a reason for hiding this comment

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

Perhaps it's better to use shaded jar?

com.clickhouse clickhouse-jdbc 0.3.2-patch11 all * *

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your review @zhicwu!

What's the most common dependency conflict that can be avoided by using the shaded jar?

I can see using the basic JAR, the final connector NAR is in size 9.7M, and after switching to the shaded jar, it grows to 46M.

If no user reports the current solution's issue, I tend to keep it as is.

Copy link

Choose a reason for hiding this comment

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

What's the most common dependency conflict that can be avoided by using the shaded jar?

LZ4 and Gson. Put native library aside, if there's custom classloader for isolation, it should not be a problem.

I can see using the basic JAR, the final connector NAR is in size 9.7M, and after switching to the shaded jar, it grows to 46M.

Did you exclude all dependencies when specifying classifier? I'm not familiar with the Nifi maven plugin, but the JDBC driver with only http implementation is ~1MB. -all contains more like gRPC etc., so it's 20+ MB.

Copy link
Member Author

Choose a reason for hiding this comment

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

Did you exclude all dependencies when specifying classifier?

No. I'm unsure what dependencies should be excluded XD.

Copy link

Choose a reason for hiding this comment

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

All of them because you only need the shaded jar :)

<dependency>
    <groupId>com.clickhouse</groupId>
    <artifactId>clickhouse-jdbc</artifactId>
    <version>0.3.2-patch11</version>
    <classifier>all</classifier>
    <exclusions>
        <exclusion>
            <groupId>*</groupId>
            <artifactId>*</artifactId>
        </exclusion>
    </exclusions>
</dependency>

Copy link
Member Author

Choose a reason for hiding this comment

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

@zhicwu That's it!

Copy link
Member Author

Choose a reason for hiding this comment

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

Now it reduces to 25M

<artifactId>clickhouse-jdbc</artifactId>
<version>${clickhouse-jdbc.version}</version>
<scope>runtime</scope>
Expand Down