clickhouse-java icon indicating copy to clipboard operation
clickhouse-java copied to clipboard

Use BND for generating of Use bnd-jar-plugin to generate a proper

Open laeubi opened this issue 3 years ago • 9 comments

This is a PR to address the issues mentioned in https://github.com/ClickHouse/clickhouse-jdbc/issues/905

  1. Use bnd-jar-plugin to generate a proper Manifest.MF file that is needed for OSGI-Bundles(jars) (from @stbischof)
  2. Implement the DatasourceFactory (tbd)

laeubi avatar Jul 25 '22 09:07 laeubi

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: stbischof
:x: Christoph Läubrich


Christoph Läubrich seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Jul 25 '22 09:07 CLAassistant

Benchmark                                (client)  (connection)  (statement)  (type)   Mode  Cnt     Score     Error  Units
Basic.insertOneRandomNumber  clickhouse-http-jdbc         reuse       normal  object  thrpt   20   279.473 ±  40.533  ops/s
Basic.insertOneRandomNumber  clickhouse-http-jdbc         reuse     prepared  object  thrpt   20   252.835 ±  23.847  ops/s
Basic.insertOneRandomNumber  clickhouse-http-jdbc           new       normal  object  thrpt   20   296.137 ±  23.783  ops/s
Basic.insertOneRandomNumber  clickhouse-http-jdbc           new     prepared  object  thrpt   20   253.625 ±  20.492  ops/s
Basic.insertOneRandomNumber  clickhouse-grpc-jdbc         reuse       normal  object  thrpt   20   291.233 ±  41.233  ops/s
Basic.insertOneRandomNumber  clickhouse-grpc-jdbc         reuse     prepared  object  thrpt   20   250.644 ±  31.116  ops/s
Basic.insertOneRandomNumber  clickhouse-grpc-jdbc           new       normal  object  thrpt   20   286.819 ±  29.839  ops/s
Basic.insertOneRandomNumber  clickhouse-grpc-jdbc           new     prepared  object  thrpt   20   258.595 ±  26.806  ops/s
Basic.selectOneRandomNumber  clickhouse-http-jdbc         reuse       normal  object  thrpt   20  1059.229 ± 112.988  ops/s
Basic.selectOneRandomNumber  clickhouse-http-jdbc         reuse     prepared  object  thrpt   20  1071.266 ± 102.861  ops/s
Basic.selectOneRandomNumber  clickhouse-http-jdbc           new       normal  object  thrpt   20  1075.900 ±  80.726  ops/s
Basic.selectOneRandomNumber  clickhouse-http-jdbc           new     prepared  object  thrpt   20  1091.310 ±  69.766  ops/s
Basic.selectOneRandomNumber  clickhouse-grpc-jdbc         reuse       normal  object  thrpt   20   991.168 ±  65.282  ops/s
Basic.selectOneRandomNumber  clickhouse-grpc-jdbc         reuse     prepared  object  thrpt   20   966.761 ±  68.666  ops/s
Basic.selectOneRandomNumber  clickhouse-grpc-jdbc           new       normal  object  thrpt   20   977.113 ±  69.037  ops/s
Basic.selectOneRandomNumber  clickhouse-grpc-jdbc           new     prepared  object  thrpt   20  1013.867 ±  68.299  ops/s

github-actions[bot] avatar Jul 25 '22 09:07 github-actions[bot]

@zhicwu Is master not the correct branch to use here? It seems the develop has some conflicts now can you explain what master vs develop is? master for lats release?

laeubi avatar Jul 25 '22 12:07 laeubi

@zhicwu Is master not the correct branch to use here? It seems the develop has some conflicts now can you explain what master vs develop is? master for lats release?

Hi @laeubi, sorry this is a bit confusing. As of now, master is for released code, while develop is for new and experimental stuff. In the near future, once we fully retire the legacy driver, develop will be dropped and master will be renamed to main.

Could you rebase your code to latest on develop? And I'm sorry that there'll be more changes in these days for transaction support, which may impact your work.

zhicwu avatar Jul 25 '22 13:07 zhicwu

@laeubi, can we keep multi-release jar and JDK 8 support? As I mentioned in the original thread, multi-release jar is required for more features like ByteUtils using Vector API(requiring JDK 16+). Have you considered to start with a new module just for OSGi bundles?

zhicwu avatar Jul 25 '22 14:07 zhicwu

@zhicwu I'll check this, I just picked up @stbischof work and got the feeling that multi-release might be dropped, but will check if we can keep this.

laeubi avatar Jul 25 '22 14:07 laeubi

Wouldn't it be a better idea to use multiple service-jars that provide a service via ServiceLoader that can be consumed instead of your multi release approach?

stbischof avatar Jul 25 '22 16:07 stbischof

Wouldn't it be a better idea to use multiple service-jars that provide a service via ServiceLoader that can be consumed instead of your multi release approach?

Thanks for the suggestion. Actually I thought about that in the beginning but we're talking about multiple JDKs here. They're actually same service but using different version of JDK. We can create multiple packages one for each JDK but given the fact that we have many packages already, are we going to multiply that by 2 or 3?

Is it impossible to generate OSGi bundles together with multi-release jar? Does it work if we create a separate module just for OSGi?

zhicwu avatar Jul 26 '22 12:07 zhicwu

OSGi support multi-release: https://blog.osgi.org/2018/02/osgi-r7-highlights-java-9-support.html Having an extra module sounds a bit superfluous, as the idea is to reuse whats there instead of adding much more on-top.

It is more that BND (a tool to automatically generate the header data) do not yet support multi-release jars but I'm trying to get this running atm.

laeubi avatar Jul 26 '22 12:07 laeubi