Ignite's HibernateNonStrictAccessStrategy does not update the cache with latest changes when transaction contains multiple flushes

["Prigoreanu, Alexandru" <pr...@anteash.com>]

Hello everyone! thank you for your time.

- we have an entity PlaceImpl annotated @Cache(usage =
CacheConcurrencyStrategy.NONSTRICT_READ_WRITE)
- we use Ignite's L2 Hibernate cache implementation through the application
property
spring.jpa.properties.hibernate.cache.region.factory_class=org.apache.ignite.cache.hibernate.HibernateRegionFactory
- we use ignite 2.14.0, ignite-hibernate-ext 5.3.0, hibernate 5.4.33
- we have a complex transaction that creates a new PlaceImpl, saves it in
the database, and updates it.
- when the PlaceImpl is returned from the level 2 cache it does not contain
the latest data for some fields. let's say we expect that
PlaceImpl.description is not null while PlaceImpl.description is null.

after a bit of debugging we got the following data:
- during the transaction the changes to PlaceImpl are flushed twice or
more: one in the middle of the transaction and the second one before the
transaction is committed.
- given the CacheConcurrencyStrategy.NONSTRICT_READ_WRITE, this results in
2 EntityUpdateAction for our PlaceImpl that was created (we don't talk
about inserts here) added to the collection of processes in
AfterTransactionCompletionProcessQueue.processes
- after the transaction is completed, on the processes of the
aforementioned list hibernate invokes doAfterTransactionCompletion

public void afterTransactionCompletion(boolean success) {
  while ( !processes.isEmpty() ) {
    try {
      processes.poll().doAfterTransactionCompletion( success, session );
    }

- the first EntityUpdateAction contains an incomplete PlaceImpl that does
not yet have all the fields set
- the second EntityUpdateAction contains the complete PlaceImpl with all
the fields set

- the first EntityUpdateAction.doAfterTransactionCompletion gets to execute
HibernateNonStrictAccessStrategy.afterUpdate: here ctx is not null and the
if ctx != null branch is executed and the incomplete PlaceImpl is put in
the level 2 cache

 @Override public boolean afterUpdate(Object key, Object val) {
     WriteContext ctx = writeCtx.get();


     if (log.isDebugEnabled())
         log.debug("Put after update [cache=" + cache.name() + ", key=" +
key + ", val=" + val + ']');


     if (ctx != null) {
         ctx.updated(key, val);


         unlock(key);


         return true;
     }


     return false;
 }

which invokes also unlock(key);

 @Override public void unlock(Object key) {
     try {
         WriteContext ctx = writeCtx.get();


         if (ctx != null && ctx.unlocked(key)) {
             writeCtx.remove();


             ctx.updateCache(cache);
         }
     }

     catch (IgniteCheckedException e) {
         throw convertException(e);
     }
 }

that removes writeCtx from the current thread with writeCtx.remove();

- the second EntityUpdateAction.doAfterTransactionCompletion gets to
execute HibernateNonStrictAccessStrategy.afterUpdate: here ctx is null and
the if ctx != null branch is not executed, so the level 2 cache is never
updated with the latest changes in the PlaceImpl entity.

we were able to have a minimal dummy text example of the transaction that
creates a PlaceImpl

 @Test
 public void testPlaceImplCacheWorksWithFlush() throws Exception {
     long[] placeId = new long [] {0L};
     doInTransaction(() -> {
         PlaceImpl place = new PlaceImpl();
         entityManager.persist(place);
         placeId[0] = place.getId();
         entityManager.flush();
         place.setName("NAME"); //set some place properties
         entityManager.flush();
         place.setDescription("description"); //set some other place
properties
         assertThat(place.getDescription(), Matchers.is("description"));
     });
     //load place from the cache
     Place place = placeImplRepository.findOne(placeId[0]);
     assertThat(place.getName(), Matchers.is("NAME"));
     //the following assertion fails
     assertThat(place.getDescription(), Matchers.is("description"));
 }

we are aware the given example should not manually invoke flushes but in
our real transaction the flush is not manual, our code provokes
inadvertently autoFlushIfRequired that happens to flush also updates to our
new PlaceImpl entity

what are your thoughts on the matter?
could this be a bug?
should we not use Ignite's hibernate level 2 cache implementation
HibernateRegionFactory when transactions update entities with multiple
flushes?
if you have any pointers on a solution we could also try to provide you a
pull request with the implementation.

thank you for your time!

Alex