hibernate-reactive statement-batch-size is not used

[holomekc]

Describe the bug

In hibernate-reactive the following property is ignored:

quarkus.hibernate-orm.jdbc.statement-batch-size

although it is described here. They also mention although it is a jdbc property it is used by hibernate reactive:
https://hibernate.org/reactive/documentation/1.0/reference/html_single/#_enabling_statement_batching

This is quite an important property to improve performance for example if you insert a lot of values.

Besides that I noticed that hibernate-reactive also uses the already deprecated value for batch-fetch-size and ignores the new property:

quarkus.hibernate-orm.fetch.batch-size

Expected behavior

statement-batch-size can be set and new property for fetch.batch-size can be set as well.

Actual behavior

See description of bug

To Reproduce

Enable logging and persist a collection or multiple entities of the same type and then check logs.

Configuration

quarkus.hibernate-orm.log.sql=true
quarkus.hibernate-orm.log.format-sql=true
quarkus.hibernate-orm.log.jdbc-warning=true

Screenshots

Additional context

[quarkus-bot]

/cc @DavideD, @Sanne, @gavinking

[holomekc]

Created PR:
#16646

Did I miss something or what do you think?

[holomekc]

It looks like that even more properties are not updated in hibernate-reactive to the same state as it exists in hibernate-orm. I am not really sure if I understand the configuration setup though. Are there some configuration which is shared? It is quite hard to understand which configuration should also work in hibernate-reactive.

[Sanne]

It looks like that even more properties are not updated in hibernate-reactive to the same state as it exists in hibernate-orm. I am not really sure if I understand the configuration setup though. Are there some configuration which is shared? It is quite hard to understand which configuration should also work in hibernate-reactive.

That is a very fair point. Yes the hibernate-reactive extension has been built as a POC extending the "traditional" Hibernate ORM extension, which implies it inherits all its configuration properties - and some other logic which it doesn't really need or use yet.

We'll be separating it soon in fully independent extensions, that should help to clarify which properties are actually available for use.

[Sanne]

cc/ @gsmet : when separating the extensions, we should take such issues into account: they highlight that not all configuration properties for ORM "blocking" are suitable to be exposed on HR.

[gavinking]

I'm not sure if it's related, but note that there was a bug in HR that caused this property to be ignored. It should be fixed now. @Sanne

[Sanne]

Ok good to know. Still, it would be wise to keep the configuration properties separated.

[cdmikechen]

@Sanne @gavinking @holomekc
I think the root cause of this problem is the code in hibernate-core

https://github.com/hibernate/hibernate-orm/blob/f2aa533dfc9fd502cdd90ae909c95bccb4f62493/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsBuilder.java#L479-L482

  this.jdbcBatchSize = ConfigurationHelper.getInt( STATEMENT_BATCH_SIZE, configurationSettings, 1 );
  if ( !meta.supportsBatchUpdates() ) {
    this.jdbcBatchSize = 0;
  }

In quarkus-hibernate, I found that quarkus set hibernate.temp.use_jdbc_metadata_defaults false, so that meta.supportsBatchUpdates() always return false.

[cdmikechen]

I think batch insert/update is still a very necessary requirement.
I test that it takes about 5-7 seconds to write 300 records in one request (quarkus Fruit demo). I think most people can't accept this result.

[cdmikechen]

Update my current progress.
I can successfully handle some batch situations by manually setting the batch size of the session (I had written codes in issue hibernate/hibernate-reactive#1256).
However, there are still some situations that cannot be handled. After debugging and tracking the codes, I found that there seems to be some hard coding in hibernate-reactive, which makes it impossible to start batch. I am trying to submit a new PR to solve this problem

[Sanne]

hi @cdmikechen - yes I agree fixing this would be important.

Looking forward for any PRs you might have; feel free to ping me and/or @DavideD or @gavinking on PRs of either hibernate-reactive of hibernate-orm, we work on both projects and will try to prioritize these.