Issues

Select view

Select search mode

 
18 of 18

Metamodel imports cache increases indefinitely for dynamically generated HQL aliases eventually leading to an OOM

Fixed

Description

https://hibernate.atlassian.net/browse/HHH-12485#icft=HHH-12485 caches failed imports (with an empty string instance) in MetamodelImpl with the intention of avoiding exceptions every time an unknown class name reaches MetamodelImpl#getImportedClassName.

My experience with Hibernate, however, shows that the target entity being joined with is also sent to said method.

For example, in the following HQL query:

from Entity as e join e.relatedEntity as related

e.relatedEntity will be passed to getImportedClass as process will evaluate to true (e is a valid Java identifier and the surrounding tokens also pass the validation). That’s probably not an issue for hard-coded queries for which the join target will always remain e.relatedEntity and which would lead to adding just this one additional e.relatedEntity --> ”” node to the map.

The problem for us, however, is that our application dynamically generates random aliases in HQL queries (due to these queries being dynamically built). Such dynamically generated entity aliases end up being added to the imports map with an empty string value. As a result, the imports map in MetamodelImpl constantly grows, eventually leading to an OOM for our applications as this isn't session-specific cache and as I have not been able to find any way of clearing the failed imports.

I could not find any information in the documentation stating that our approach with dynamic aliases is incorrect and decided to raise a ticket considering the severity of the issue for us. I would assume that either failed imports need not be cached after all (for memory reasons) or this method need not be invoked for them.

PR: https://github.com/hibernate/hibernate-orm/pull/4411

Created December 2, 2021 at 10:23 AM
Updated August 19, 2022 at 1:07 PM
Resolved December 13, 2021 at 9:32 PM

Activity

Show:

Réda Housni Alaoui August 19, 2022 at 1:07 PM

I was already aware of the explanation of https://github.com/hibernate/hibernate-orm/pull/4411#issuecomment-989304639 . It still did not make sense to me. What was the cost of adding at least a configuration? Why not keep the default value for 5.6.x ? Why not adding this to the release note as a breaking change with all the blinking lights?
I am sure, the use case reported here is only representative of a small minority of Hibernate users.

And if people ask nicely

If I appeared not nice, it is probably because I think the decision derived from the explanation is a mistake.
https://hibernate.atlassian.net/browse/HHH-15100 was created by someone else on march and received zero answer. Moreover, I am not among the kind of people who asks OSS maintainers then wait. I immediatly created a pull request yesterday for https://hibernate.atlassian.net/browse/HHH-15100 .

Generally speaking, I would suggest that if your map is growing so much, it would be interesting to try to avoid the root cause. Are you generating many queries dynamicaly via string manipulation? Why are all these tokens different? Or are you able to trigger this in another way?

The map contains 1318 entries on application startup before any query. The keys are either JPA entities simple name (like “product” for “Product”) or full qualified name (like “org.example.Product”). We don’t use any query string. But we have a massive usage of JPA CriteriaQuery generating tokens like “from” or “generatedAlias0.product”. Since the current fix, those false positives never make it to the map because of the 1000 entries limit. So each false positive generate one call to Class.forName().

Given the knowledge we had on December 2021 and potential performance implication on huge applications (the ones precisely needing performance optimization), I will keep saying that this ticket was very badly managed. This happens to everybody so please do not hold a grudge against me if you don’t agree.

Sanne Grinovero August 19, 2022 at 10:32 AM

The explanation is here: https://github.com/hibernate/hibernate-orm/pull/4411#issuecomment-989304639 . As mentioned, we could introduce a configurable threshold, if there’s evidence of this being necessary. And if people ask nicely.

Generally speaking, I would suggest that if your map is growing so much, it would be interesting to try to avoid the root cause. Are you generating many queries dynamicaly via string manipulation? Why are all these tokens different? Or are you able to trigger this in another way?

Réda Housni Alaoui August 18, 2022 at 4:56 PM

For information our imports map contains 1318 entries without any negative entry. So this fix pretty much disabled the negative cache.

Réda Housni Alaoui August 18, 2022 at 4:54 PM

The added limit caused a critical and hard to trace performance issue on our major application.
I can’t fathom how this kind of breaking change could have been added without a configurable limit and without waiting for a major version …

Ivaylo Mitrev December 9, 2021 at 9:03 AM

Thank you! I’ve now also submitted the requested changes in the PR.