`Database.deleteAll()` Skips Bean due to Hash Collision

[raphaelNguyen]

Versions

java: Temurin 17.0.15
ebean: 15.11.0

Description

Calling Database.deleteAll() occassionally skips over bean when executing. Looking at io.ebean.SQL log, I can observe that one element of the list passed into Database.deleteAll() is missing from the list of parameters being bound to the delete query.

Investigation

Investigating this, I noticed that the been is being skipped because JdbcTransaction.isRegisteredDeleteBean(JdbcTransaction.java:321) is returning true when the missing bean is being queued up for deletion (call stack below).

Further debugging shows that another bean in the collection passed into Database.deleteAll gives the same hash when run through PersistRequestBean.beanHash(PersistRequestBean.java:481). Since JdbcTransaction stores the bean hash directly in a HashSet<Integer> instead of storing a HashSet of the beans, the hash collision wasn't double-checked by an equality check and therefore producing a false positive result. This causes DefaultPersister to think the second of the 2 hash-collided beans were already queued for deletion when instead it was not, leading to the skipping of the bean in the delete query.

Given that the JdbcTransaction.persistingBeans collection, which seems to serve a similar purpose but for save instead of delete, is already implemented as a collection of bean, perhaps reimplementing JdbcTransaction.deletingBeansHash as a hashed collection of beans will resolve this issue? What are you thoughts on this?

Call Stack
io.ebeaninternal.server.transaction.JdbcTransaction.isRegisteredDeleteBean(JdbcTransaction.java:321)
io.ebeaninternal.api.SpiTransactionProxy.isRegisteredDeleteBean(SpiTransactionProxy.java:176)
io.ebeaninternal.server.core.PersistRequestBean.isRegisteredForDeleteBean(PersistRequestBean.java:503)
io.ebeaninternal.server.persist.DefaultPersister.deleteRequest(DefaultPersister.java:349)
io.ebeaninternal.server.persist.DefaultPersister.deleteRequest(DefaultPersister.java:341)
io.ebeaninternal.server.persist.DefaultPersister.delete(DefaultPersister.java:334)
io.ebeaninternal.server.core.DefaultServer.lambda$deleteAllInternal$12(DefaultServer.java:1726)
io.ebeaninternal.server.core.DefaultServer.executeInTrans(DefaultServer.java:1936)
io.ebeaninternal.server.core.DefaultServer.deleteAllInternal(DefaultServer.java:1722)
io.ebeaninternal.server.core.DefaultServer.deleteAll(DefaultServer.java:1704)

[rbygrave]

Great investigation and decription - thanks!!

instead of storing a HashSet of the beans

Rather than beans, we need register / detect by Id values. So the "fix" is to have a HashSet of Id values mapped by type.

[raphaelNguyen]

Thank you for getting to this so quickly, @rbygrave. I'm looking forward to the release.

Your fix makes sense. When I made the suggestion, I hadn't considered the possibility of different instances of the same object (same Id) being requested for deletion in the same transaction.