Skip to content

Add apache http client #1146

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

Merged
merged 12 commits into from
Dec 8, 2022
Merged

Add apache http client #1146

merged 12 commits into from
Dec 8, 2022

Conversation

JackyWoo
Copy link
Contributor

@JackyWoo JackyWoo commented Dec 4, 2022

This pr will close #1139 .

change log

  1. Add apache http client
  2. who support configuring socket option.
  3. Make ClickHouseHttpClientTest parameterized

@zhicwu
Copy link
Contributor

zhicwu commented Dec 4, 2022

Thank you @JackyWoo for the pull request. Just added a few immediate feedback. Apart from that, we also need to update pom to ensure Apache libraries will be only included in -all jar. Will run benchmark tomorrow to understand more.

@zhicwu
Copy link
Contributor

zhicwu commented Dec 5, 2022

The performance does not look good somehow.

Benchmark                                     (client)  (connection)  (statement)   (type)   Mode  Cnt    Score   Error  Units
Query.selectInt8    clickhouse-apache-http-client-jdbc         reuse     prepared  default  thrpt   20   95.181 ± 3.783  ops/s
Query.selectInt8                  clickhouse-http-jdbc         reuse     prepared  default  thrpt   20  112.334 ± 7.949  ops/s
Query.selectString  clickhouse-apache-http-client-jdbc         reuse     prepared  default  thrpt   20   27.539 ± 0.925  ops/s
Query.selectString                clickhouse-http-jdbc         reuse     prepared  default  thrpt   20   35.449 ± 2.106  ops/s
Query.selectUInt64  clickhouse-apache-http-client-jdbc         reuse     prepared  default  thrpt   20   45.539 ± 1.620  ops/s
Query.selectUInt64                clickhouse-http-jdbc         reuse     prepared  default  thrpt   20   62.749 ± 4.282  ops/s

@JackyWoo
Copy link
Contributor Author

JackyWoo commented Dec 5, 2022

May I ask how to configure benchmark client to clickhouse-apache-http-client-jdbc?

@zhicwu
Copy link
Contributor

zhicwu commented Dec 5, 2022

May I ask how to configure benchmark client to clickhouse-apache-http-client-jdbc?

Please pull latest code fron your branch and follow instructions below.

cd clickhouse-benchmark
mvn -Drelease clean package
java -jar target/benchmarks.jar -p client=clickhouse-apache-http-client-jdbc -p type=default -p connection=reuse Query.selectInt8

@JackyWoo
Copy link
Contributor Author

JackyWoo commented Dec 5, 2022

Optimize apache http client by reuse connection. Below is benchmark in my local machine.

It seems a little better than http url client.

Benchmark               (client)  (connection)  (statement)   (type)   Mode  Cnt    Score   Error  Units
Query.selectInt8  ClickhouseJdbc         reuse     prepared  default  thrpt   20   96.870 ± 5.932  ops/s
Query.selectInt8  ClickhouseJdbc         reuse     prepared   string  thrpt   20   96.102 ± 7.897  ops/s
Query.selectInt8  ClickhouseJdbc         reuse     prepared   object  thrpt   20  100.323 ± 8.398  ops/s

Benchmark                               (client)  (connection)  (statement)   (type)   Mode  Cnt    Score   Error  Units
Query.selectInt8  ClickhouseApacheHttpClientJdbc         reuse     prepared  default  thrpt   20  107.038 ± 4.379  ops/s
Query.selectInt8  ClickhouseApacheHttpClientJdbc         reuse     prepared   string  thrpt   20  107.169 ± 3.687  ops/s
Query.selectInt8  ClickhouseApacheHttpClientJdbc         reuse     prepared   object  thrpt   20  109.726 ± 2.098  ops/s

```

@JackyWoo
Copy link
Contributor Author

JackyWoo commented Dec 7, 2022

@zhicwu Please review the PR, when you have a changce.

@zhicwu
Copy link
Contributor

zhicwu commented Dec 8, 2022

Thank you for the contribution @JackyWoo. I was waiting the build failure got fixed :)

The benchmark makes sense as they should be similar, and the result could be slightly different when using single thread vs. multi-thread for the benchmark.

Anyway, just document my concerns and may change the code after merging this PR.

  • Is ByteArraysInputStream really needed?
  • Consider CloseableHttpAsyncClient
  • Packing - change httpclient5 to provided?
  • Stress test to understand if we still have "no response" error

Lastly, while I'm working on TCP client, if we need better control of Socket, we probably need non-blocking SocketChannel based implementation as well.

@zhicwu zhicwu merged commit 925fde5 into ClickHouse:develop Dec 8, 2022
@JackyWoo
Copy link
Contributor Author

JackyWoo commented Dec 8, 2022

@zhicwu Thank you, may I know when 0.33 will release?

@JackyWoo JackyWoo mentioned this pull request Dec 11, 2022
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants