Dockstore: Hibernate flushing entities for readonly transactions

[coverbeck]

Describe the bug
When invoking an endpoint annotated with @UnitOfWork(readOnly = true), you'll see in the server logs that Hibernate is flushing entities. This should be a read-only transaction, with read-only entities, so no flushing should be going on, I think.

To Reproduce
Steps to reproduce the behavior:

1. curl localhost:8080/organizations
2. That will hit this code, that has readOnly annotation.
3. See message server-side that includes: flushing a total of 17 entities and 68 collections

Expected behavior
No entities should be flushed.

Additional context

• Webservice version: on develop branch for sure (testing locally), probably in prod as well.

┆Issue is synchronized with this Jira Story
┆Issue Type: Story
┆Fix Versions: Dockstore 1.9
┆Sprint: Sprint 37 Lobster
┆Issue Number: DOCK-1393

[coverbeck]

Initial findings, seems like we need to add an explicit flushMode to readonly transactions.

    @UnitOfWork(readOnly = true, flushMode = FlushMode.MANUAL)

In org.hibernate.internal.SessionImpl#flushBeforeTransactionCompletion, there is:

        final boolean doFlush = isTransactionFlushable()
                && getHibernateFlushMode() != FlushMode.MANUAL;

I thought I saw somewhere that setting a Hibernate transaction to readonly should override flush mode, but it doesn't seem to be the case.

[coverbeck]

Our read-only transactions don't seem to be read-only. I was able to save a change to the database from a readOnly=true annotated method. Confusing...

[coverbeck]

I've managed to convince myself this isn't too bad by debugging through it.

1. The entities that are loaded in the transaction do have their status set to read-only. When Hibernate flushes, it checks the status of the entity, and does not flush it if its status is read-only. Even though it counts it in the statistics.
2. If I make a change to an entity created in a read-only transaction, it is ignored, i.e., not persisted to the database. This contradicts my previous comment, but I think what happened there was either:
   
   
   
   1. I was just plain wrong and misread things, because I can longer reproduce.
   
   
   2. More likely, I think I made a change to the User object, which is injected via SimpleAuthenticator from outside the transaction, i.e., not constrained by the read-only attribute.
   
   

Although the dirty-check is being short-circuited by the read-only status, Hibernate is still iterating through all entities, if only to see they don't need to be persisted.

1. This probably partially explains why DTOs are faster -- this is pattern we've been moving to.
2. We could add FlushMode.MANUAL to the read-only transactions, but I'd rather not as we near a release.
3. Our GA4GH apis do not have read-only attribute. Created #3582

Probably going to close this as is, but going to mull over it, and if others any have thoughts...

[denis-yuen]

This probably partially explains why DTOs are faster -- this is pattern we've been moving to.

Probably my preference long term (that and the GA4GH api change)