Re: [ovirt-devel] Caching of data from the database done properly

[Roman Mohr]

On Mon, Jul 4, 2016 at 11:58 PM, Roman Mohr  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/bll/src/main/java/org/ovirt/engine/core/bll/quota/QuotaManager.java
> [2] 
> https://github.com/oVirt/ovirt-engine/blob/master/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macpool/MacPoolPerCluster.java
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

[ovirt-devel] Caching of data from the database done properly

[Roman Mohr]

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.

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/bll/src/main/java/org/ovirt/engine/core/bll/quota/QuotaManager.java
[2] 
https://github.com/oVirt/ovirt-engine/blob/master/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/macpool/MacPoolPerCluster.java
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

[ovirt-devel] drop-in directories for vdsm configuration

[Irit Goihman]

Hi, A drop-in directories feature has been added to vdsm-4.18.4.

This allows the user to put partial configurations into a drop-in file and
create new parameters or override default ones.

The semantics of the directories is as follows:

-  /etc/vdsm/vdsm.conf: For default configuration. We install this file if
missing, and never touch this
file during upgrade.
- /etc/vdsm/vdsm.conf.d/
- / run/vdsm/vdsm.conf.d/
- /usr/lib/vdsm/vdsm.conf.d/

Files with a .conf suffix can be placed into any of the vdsm.conf.d drop-in
directories.
The default configuration file is read before all other files and has the
lowest priority.

The priority of configuration files under vdsm.conf.d is determined by
lexicographic order of file names, so the lexicographic latest file name
takes precedence.

It is highly recommended to prefix all file names with a two-digit number
in order to keep the ordering simple.

-- 
Irit Goihman
Software Engineer
Red Hat Israel Ltd.
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Integration tests future (and very nice alternative for the DAO fixture file)

[Roman Mohr]

Hi,

an update on the ovirt-engine application wide tests.
When [6] is merged, everything is ready to use.

Please have a look on the post here:


http://rmohr.github.io/continuous%20integration/2016/07/04/application-scoped-tests-for-ovirt-with-arquillian

for a How-To and why it is implemented the way it is.

Reply here or ping me anytime if you have questions or want some help.

Happy testing :)
Roman

On Wed, Apr 13, 2016 at 3:22 PM, Roman Mohr  wrote:
> Hi all,
>
> In [1] you can find some patches which are meant to improve the test writing
> experience in ovirt-engine.
>
> They provide the following things:
>
>   A) Domain Object builders which can be used for creating and/or persisting
> domain objects [2]
>   B) DAO testing without writing fixtures because of the builders
>   C) Integration testing for commands in conjunction with a real database
> Arquillian, injectable commands and the builders [3]
>
> # How to run what?
>
> A) In normal unit tests just create a new instance of a builder and use it.
> This should help us to get rid of all the small createDefaultVm(),
> createHostWithX() helper methods in our tests.
>
> B) In dao tests just inject them and go ahead. The advantage of not using
> the fixture file is that we can now set up clean scenarios for every test in
> a setup method. See example 2 below on how easy it is to set up a new
> cluster.
>
> C) Arquillian integration tests need to be marked with
> "@Category(IntegrationTest.class)" and can inherit from
> TransactionalTestBase. The @Category annotation makes sure that the
> integration tests are only run when
>
> mvn clean verify -DskipITs=false
>
> is invoked. Note that these tests are then executed in the integration test
> phase of maven. For them we use the maven-failsafe-plugin[5] which will also
> make sure that the testing database is up to date. See [4] for more details.
>
> # Examples
>
> 1) Add a running VM to a host, persist everything to the database and load
> all VMs which are running on the host:
>
> VDS host = vdsBuilder.cluster(persistedCluster).persist();
> vmBuilder.host(host).up().persist();
> List vms = vmDao.getAllRunningForVds(host.getId());
>
> 2) Add 10 hosts with 1 GB of RAM to a cluster, persist the hosts to the
> database in a DAO test:
>
> public class MyHostDaoTest extends BaseDaoTestCase {
>
> @Inject
> private VdsBuilder vdsBuilder;
>
> @Test
> public void createHosts() {
> VdsBuilder builder =
> vdsBuilder.cluster(persistedCluster).physicalMemory(1000);
> for (int x =0; x < 10; x++){
> builder.id(Guid.newGuid()).persist();
> }
> }
> }
>
> 3) Full integration test with arquillian and the database
>
> @Category(IntegrationTest.class)
> public class VmDaoIntegrationTest extends TransactionalTestBase {
>
> @Inject
> VmDao vmDao;
>
> private final Guid VM1_GUID =
> Guid.createGuidFromString("0fe4bc81-5999-4ab6-80f8-7a4a2d4bfacd");
>
> @Deployment
> public static JavaArchive deploy(){
> return createDeployment();
> }
>
> @Test
> public void shouldFailOnExistingEntity() {
>
> vmBuilder.id(VM1_GUID).cluster(clusterBuilder.reset().persist()).persist();
> // This uses assertThat from assertj:
> assertThat(vmDao.get(VM1_GUID)).isNotNull();
> }
> }
>
> 4) Using the builders in a normal unit test without a database:
>
> VM vm = new VmBuilder().id(Guid.newGuid()).up().build();
>
>
> # How to add your own Domain objects?
>
> There are just a few simple rules:
>
> 1) Your builder should extend org.ovirt.engine.core.builder.AbstractBuilder
>
> 2) Make sure that you only access DAOs injected into the builder during
> #prePersist() and #persist(). This allows to use the #build() method also
> without injections
>
> 3) #prePersist() should set all fields which are necessary to suffice
> database constraints. The fields should only be set if they are not already
> set before by the builder. When following this rule we can always persist
> new objects to the database by simply calling myBuilder.reset().persist().
>
> 4) Mark your builder with @Repository to make them useable for our Spring
> DAO tests and our Arquillian integration tests.
>
> So have a look at the patches at [1] and let me know what you think about
> them.
>
> Best Regards and happy testing,
>
> Roman
>
> [1] https://gerrit.ovirt.org/#/q/topic:integration
> [2] https://gerrit.ovirt.org/#/c/47008/17
> [3] https://gerrit.ovirt.org/#/c/47007/10
> [4] https://gerrit.ovirt.org/#/c/47008/17
> [5] https://maven.apache.org/surefire/maven-failsafe-plugin/

[6] https://gerrit.ovirt.org/#/c/59912/4
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] [lago-devel] ovirt tests failing on missing libxml2-python

[Yaniv Kaul]

On Sun, Jul 3, 2016 at 3:31 PM, Barak Korren  wrote:

> >> Maybe we can take a middle ground, pre-fetch, but also enable external
> >> repos in CI (perhaps with some way to log and find out what was not
> >> pre-fetched).
> >
> > This is what the code is supposed to do, I suspect. reposync syncs
> between
> > what you already have and what you fetch, no?
> > Y.
> >
> I was referring to the deployment/test code.
> AFAIK right now the external repos are disabled before the test starts
>

Indeed, to save time not fetching their metadata, and ensure we have the
correct deps. I guess in some cases we can enable them - need to see how
much time it wastes.
Y.


>
>
> --
> Barak Korren
> bkor...@redhat.com
> RHEV-CI Team
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel