Repository navigation
Replies: 1 comment 1 reply
|
HI @ahus1 Thanks for putting this together. Identifying read-only code sectionsI do like the idea of identifying areas which are performing "read-only" operations. Using manual flush during read-only code sectionsI think that setting Hibernate to manual flush won't cut it, because all write-enabled methods are still accessible. A first example lives around an explicit flush: a manager method adding an entity for persistency and triggering an explicit flush would actually perform the dirty check, defeating the purpose of avoid the flush entirely. A second example is adding an entity to another's entity collection (like a So I believe that enforcing a fail-fast behavior is required to avoid getting into these tight situations. In addition to this, the Hibernate internal list of managed entities still grows very large, and when leaving the read-only section, the next dirty checking may take ages anyway: this means that we save the (potential) multiple dirty checks during the read-only section, but we still have the cost of it when leaving. Note about read-only Hibernate flag: as explained in https://thorben-janssen.com/read-only-query-hint/ , you can leverage the read-only flag to indicate to Hibernate that resulting entities are supposed to be read-only, which triggers internal Hibernate optimisations: it saves on memory (it does not keep the dehydrated form of such entities) and skips these from dirty checking. => The only two ways I'm aware of to deal with this are actually listed in the alternatives: using projection DTOs, or detach entities aggressively. Using Projection DTOsUsing projection DTOs is a good way to explicitly handle this situation, because you can bake this into the model, and even get compile-time warnings that something is wrong (because of type checking). This can be achieved either by:
From the model standpoint, there are 3 options that I know of:
Modeling a read-only projection DTO as a standalone POJOThis is the straightforward way of doing this, and has the benefit of allowing to make it as lightweight and purpose-built as required. The drawback is obviously that this is a totally new kind of beast and cannot be used anywhere in the existing code without change. Modeling a read-only projection DTO as the base class for the Hibernate/JPA mapped entity classThe read-only DTO has class members and getters, but not setters or write-oriented operations, and entities built on top add the Hibernate mapping annotations and write-oriented methods. This has a major drawback: this forces to rely on method-based annotations, so this is often not very practical unless you're already in this situation. Modeling a read-only projection DTO as a derived class from the Hibernate/JPA mapped entity classThe read-only DTO is a class derived from the regular Hibernate-mapped entity, overriding all write-oriented methods so they throw an exception (preferably unchecked, such as a Still, this entity can be used when invoking managers (example: to persist such an instance), so making sure that such an invocation fails implies that the read-only DTO is not Hibernate annotated. Note that it's been a long time since I did not try this, so checking that passing such a not-annotated read-only DTO derived from a normal entity to an entity manager actually fails is probably required. Using detached entitiesThe first thing to mention here is that any internal/intermediate, non-exposed (read: not in the returned set of objects) managed entity should be detached as soon as not useful any more. This is obvious but frequently overlooked, btw. Now talking about dealing with read-only operations, using unconditionally detached entity may only work in a single case: when the entire code path is known to be read-only (for example a read-only API endpoint querying data and returning them). When dealing with code paths mixing both read-write and read-only sections is more difficult, because:
Case 2 is somehow an expected behavior along the fail-fast way of thinking (all entities retrieved during a read-only section remain read-only), but it is counter-intuitive, as plain JPA/Hibernate entities are returned: leaving the read-only section would intuitively allow write operations again, which fails in this case. Assuming that case 2 is acceptable, tackling case 1 requires either that: I'm actually unaware of any way if doing a). The b) approach is feasible (demonstrated in #11074, see https://github.com/davoustp/keycloak/blob/KEYCLOAK-4593-investigations/model/jpa/src/main/java/org/keycloak/storage/jpa/resources/HibernateContextAbsencePredicate.java ). Note that properly detaching entities also means that the |
|
Hi @davoustp - reading the options I see some things that are part of the new store already: We already use read-only projection DTOs that (by using a facade) switch to full entities once attributes are read that are not part of the projection, or when the caller wants to write to them. I am not a fan of detached entities as it hands a big responsibility to the caller. About the "fail fast" you mentioned in "Identifying read-only code sections": I planned to do without such a fail-fast check, and use this only where we are very sure this works without problems (again, handing a lot of responsibility to the caller). About read-only entities: I see a big potential here, unfortunately I had to work around a bug there, see https://hibernate.atlassian.net/browse/HHH-15317. I also asked in the Hibernate Zulip chat about it https://hibernate.zulipchat.com/#narrow/stream/132096-hibernate-user/topic/Problem.20with.20user.20defined.20types.20.2B.20read-only With all that input, I'll abandon this idea for the old store, and will pick it read-only entities in Hibernate session for the new store at a later time: I'll first evaluation solution where one would not read the data from the database, see #12332. |
Uh oh!
There was an error while loading. Please reload this page.
Problem statement
When loading data iteratively from a database with JPA, the persistence context will grow with each returned search result. And every search will check all entities in the persistence context if they are dirty. This leads to an exponential slowdown.
Some of the areas do this recursively:
keycloak/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java
Lines 197 to 211 in 26c87af
Other areas read data for the cache.
In those areas it is not necessary to perform dirty checking all the time, and it seems to be a waste of time to do it for each query.
Huge impacts have been seen when evaluating #11799 that aims to support a large number of realms in a single Keycloak instance.
Approach
When entering an area that is known to be reading:
When exiting the area:
Even if a store would modify data in that area, it would still be written to the database at the end of the transaction (or the next auto-flush), it would only not be reflected in queries to the database within that block.
How this would be implemented:
Benefits
Evaluating complex DB structures would no longer take an exponential amount of time. Those DB statement happening after such a block would still be impacted from a large persistence context.
Alternatives
Those alternatives would be evaluated for the new map store, but not the legacy JPA store. Exceptions might apply for #11799, but still would have to be decided.
Other
The "read-only" mode could be the default when the standard request processing is complete and the results of for example long lists are streamed via the API.
All reactions