12:14 a.m.
On Mon, Jul 4, 2016 at 11:58 PM, Roman Mohr <rmohr(a)redhat.com> wrote:
Hi Everyone,
I wanted to discuss a practice which seems to be pretty common in the
engine which I find very limiting, dangerous and for some things it
can even be a blocker.
There are several places in the engine where we are using maps as
cache in singletons to avoid reloading data from the database. Two
prominent ones are the QuotaManager[1] and the MacPoolPerCluster[2].
While it looks tempting to just use a map as cache, add some locks
around it and create an injectable singleton, this has some drawbacks:
1) We have an autoritative source for our data and it offers
transactions to take care of inconsistencies or parallel updates.
Doing all that in a service again duplicates this.
2) Caching on the service layer is definitely not a good idea. It can
introduce unwanted side effects when someone invokes the DAOs
directly.
3) The point is more about the question if a cache is really needed:
Do I just want that cache because I find it convenient to do a
#getMacPoolForCluster(Guid clusterId) in a loop instead of just
loading it once before the loop, or do my usage requirements really
force me to use a cache?
If you really need a cache, consider the following:
1) Do the caching on the DAO layer. This guarantees the best
consistency across the data.
2) Yes this means either locking in the DAOs or a transactional cache.
But before you complain, think about what in [1] and [2] is done. We
do exactly that there, so the complexity is already introduced anyway.
3) Since we are working with transactions, a custom cache should NEVER
cache writes (really just talking about our use case here). This makes
checks for existing IDs before adding an entity or similar checks
unnecessary, don't duplicate constraint checks like in [2].
4) There should always be a way to disable the cache (even if it is
just for testing).
5) If I can't convince you to move the cache to the DAO layer, still
add a way to disable the cache.
I forgot to mention one thing: There are of course cases where
something is loaded on startup. Mostly things which can have multiple
sources.
For instance for the application configuration itself it is pretty
common, or like in the scheduler the scheduling policies where some
are Java only,
some are coming from other sources. It is still good
But for normal business entities accessing parts of it through
services and parts of it through services is not the best thing to do
(if constructiong the whole business entity out of multiple daos is
complex, Repositories can help, but the cache should still be in the
dao layer).
I hope you get what I mean.
For as long as there is no general caching solution with something
like ehcache or infinispan, in my eyes such small things matter a lot
to keep a project maintainable.
That are some of the best practices I have seen around caching
database data. It would be great if we could agree on something like
that. Maybe there is already an agreement on something and I am just
not aware of it.
Looking forward to hear your feedback.
Roman
[1]
https://github.com/oVirt/ovirt-engine/blob/master/backend/manager/modules...
[2]
https://github.com/oVirt/ovirt-engine/blob/master/backend/manager/modules...