Proposal: lookup persistence context for ID-only queries

[vladimirfx]

In analogy to the currently working bean cache (L2) lookup. So queries like below do not access DB for the same entities within the transaction.
This always going to DB event if all entities in with given ids loaded to persistence context:

QPhoneEntity(database)
.setIdIn(ids)
.findSet()

L2 cache can't be used because it is not transactional. In most OLTP systems is a hard constraint.

Use case:

We have a complex OLTP processing platform decoupled with domain events. These events may travel by network or be processed in-process by sync or async listeners. So events can hold only IDs of affected entities which are later used to load entities themselves. Typical event interface:

interface BusinessDataEvent<E : BusinessData> {
    val entityId: Id<E>
    val actualTimestamp: Instant
}

About 30% of our by ID lookups becomes to in-process and in-transaction lookups. And these lookups are batched and implemented as in the query above.

Another unwanted effect of current behavior is earlier flush (flushBeforeQuery) that also negatively affects performance. If the query is fully populated from cache there is no reason to flash.

We are migrated to Ebean in one of our projects (that uses a common platform) and spot a significant raise in SQL queries count that hit DB. After some investigation, we found that most of the query count increases are SELECT ID IN(...). We develop some ugly workarounds for this like fetch entities with find in the loop (one by one). But it is not obvious what is worse - many SELECT ID IN or even more SELECT WHERE id=? .

IMO is strange behavior - check L2 cache but ignore transaction cache (persistence context).

Suggested solution:

When detected ID-only query check persistence context first then proceed as is. Initially implement this as experimental feature with a feature flag.

Alternative solution:

Provide overloaded find methods with vararg + Collection parameters.

Personally, I prefer the first approach because ID-only query detection logic can be improved in feature and hit caches for more complex queries like this:

QPhoneEntity(database)
  .or()
  .id.isIn(ids)
  .externalId.isIn(extIds)
  .findSet()

Where extIds is empty. I.e. other parts of the query are statically resolved without database access.

Plus the first solution helps in cased when a query is dynamically constructed and for some conditions turns in ID-only.

[vladimirfx]

If the idea is useful I can provide the PR.

[rbygrave]

I'd say it is a bug that the below queries don't hit the persistence context. So yes they should hit the PersistenceContext and yes we should fix it. At this stage I don't think we need to feature toggle / feature enable this but can treat it like a bug in that it currently executes the query against the DB but when loading would hit the PersistenceContext (use the beans already loaded in the PC).

QPhoneEntity(database)
.setIdIn(ids)
.findSet()

QPhoneEntity(database)
.setIdIn(ids)
.findList()

QPhoneEntity(database)
.id.in(ids) // plus when the id property is known to be the `@Id` (like it does wrt L2 cache hits now in master) 
.findList()

• We can probably look to reuse the CacheIdLookup<T> idLookup = query.cacheIdLookup(); to hit the persistence context.
• DefaultServer findIdCheckPersistenceContextAndCache() gives us an outline of hitting the persistence context with a single id (but isn't exactly want we want)

[vladimirfx]

Does this #2927 fixes the issue?

[rbygrave]

Not unless the desire is to additionally hit the L2 cache and I don't think that is the original desire here. I think the desire here was to only use the persistence context.

So no, it doesn't fix this issue.