AssociationValueEntry entity has id field annotated with @GeneratedValue without any parameters (AssociationValueEntry) which means that the AUTO strategy is selected. In combination with Postgres db it means that actually SEQUENCE strategy is selected. That creates a sequence with a default increment value of 50. Hibernate uses a pooled optimization strategy by default with allocationSize 50 which causes it to ask the database about nextval from the sequence and next values increment by itself until exceeds allocationSize then asks the database for a next val from the sequence again.
It is a good optimization in a standard environment but dangerous in a multitenant environment because e.g. imagine the first operation triggered by tenant1 causes retrieving nextval from tenant1 sequence then the next operation is in triggered by tenant2 and hibernate will not ask db again for nextval from sequence because not used allocationSize pool. That can cause a problem with the uniqueness of primary keys.
The other thing is that if hibernate optimization is disabled in configuration and sequence increment size is changed to 1 and the service uses schema validation then another issue can happen (if it will be changed in one service and others will remain the same).
The sequence can be invalidated in case if we have many axon services that use the same database but different schemas. Hibernate uses naive validation - gets all sequences from the database (from all schemas) and puts them to a map where a key is just a sequence name which is the same in that case in all schemas so they are overridden. So at the end, it can compare entity with sequence from different service (schema)
I think that the definition should be changed which at least should cause there will be no optimization used so all the time next val is taken from the database.
But actually, probably it would be better to not use sequence idis at all and instead use e.g. UUID
I think that the definition should be changed which at least should cause there will be no optimization used so all the time next val is taken from the database.
We fully agree with this statement, @KaeF. The AssociationValueEntry class is, however, in its current format, something all current users of Sagas depend on.
If we suddenly changed the sequence generation within a patch or minor release of either Axon Framework or the Multi-tenancy Extension, we would likely be bombarded with many new bug issues.
As such, there isn’t much we can do within Axon Framework 4.
However, I can confirm the approach will change for Axon Framework 5.
Added, although suboptimal, there are workarounds for multi-tenant-specific environments, such as overriding the generation defaults in your environment.
Note that I prefer giving this suggestion, but given our intent NOT to introduce breaking changes, it is sadly all I can do for the time being.
@Component
class MultitenantNamingStrategy(private val environment: Environment) : CamelCaseToUnderscoresNamingStrategy() {
override fun toPhysicalSequenceName(name: Identifier, context: JdbcEnvironment): Identifier {
return super.toPhysicalSequenceName(name, context)
.let { Identifier("${environment.getProperty("spring.application.name")}_${it.text}", it.isQuoted) }
}
}
alter sequence definition by
ALTER SEQUENCE association_value_entry_seq INCREMENT BY 1;
-- additionally set broken current val (this query has to be customized to reach service tenants databases )
SELECT setval('association_value_entry_seq', (SELECT max(nextval(sequence_schema ||'.' ||sequence_name)) as nextval
FROM information_schema.sequences
WHERE sequence_schema ilike '%[APP_NAME]'));
ALTER SEQUENCE association_value_entry_seq RENAME TO [APP_NAME]_association_value_entry_seq;
So disabling optimization and changing INCREMENT BY with setting proper curr value (in case it was already used) caused the next val is always retrieved from the database.
Changing name (adding service prefix) fixed schema validation - check only sequences for the current service and not all in db.
Anyway even if you cannot do any not backward compatible changes you should at least put some documentation in multitenancy how it should be configured or do a patch only for multitenant environments (providing multitenant generator/strategy).
If you do nothing then multitenancy extension is buggy and can cause a real problem for users.