Open Stack

I’ve done a lot of work on this issue and from my perspective, the code is mostly ready to go, however we’re in an extended phase of getting folks to sign off as well as that I’m waiting for some last-minute fixup from Robert Collins.   Patch: [1]  Blueprint, which is to be moved to Kilo: [2]

A “nested” transaction can actually mean one of two things, either a real SAVEPOINT, or a logical transaction block.  However, either kind of block will be removed if a ROLLBACK is emitted, which also rolls back the actual transactional state all the way to the end.    The patch above makes this work as the result of two fixes.  One is that I replaced the system by which we do transactions with the pysqlite driver so that SAVEPOINT actually works [3].  The other is that I created a comprehensive fixture in [1] that will maintain a transaction + savepoint block at all times, working smoothly with tests that call commit or rollback any number of times.

From an isolation perspective, we create on-demand databases per process, so that each serial test process uses a distinct database per backend.   The entire process is managed by testresources which will organize the order of tests to most effectively leave a single schema in place with minimal teardown/setup.

I’m hoping that my patches can go right in at the top of Kilo and we can begin rolling it out in projects that are participating in oslo.db, with the hopes that consuming projects will be able to remove a lot of boilerplate setup/teardown code. 


1: https://review.openstack.org/#/c/120870/  
2: https://review.openstack.org/#/c/117335/ 
3: https://review.openstack.org/#/c/113152/


On Sep 18, 2014, at 5:59 AM, Salvatore Orlando <sorlando at nicira.com> wrote:

> Nested commits in sqlalchemy should be seen as a single transaction on the backend, shouldn't they?
> I don't know anything about this specific problem, but the fact that unit tests use sqlite might be a reason, since it's not really a full DBMS...
> 
> I think that wrapping tests in transaction also will require some changes in the architecture of the tests themselves, as many tests call the API router or the plugin which then gets a db session and open a new transaction. Furthermore, it will change the test behaviour possibly hiding errors; some operations indeed perform several distinct transactions, which in this case will be seen a single transaction.
> 
> What Kevin is doing here I think was the original way we used to do that in Neutron (Folsom). Then at some point we realised that due to plugin schema differences we were laving tables behind and switched to drop_all and rebuilding the schema using autogeneration at each test.
> 
> I think it should be ok to merge this patch. I will hold off the +A to give other core reviewers a chance to look at it.
> 
> Salvatore
> 
> 
> On 18 September 2014 11:44, Maru Newby <marun at redhat.com> wrote:
> For legacy reasons, the Neutron test suite creates and destroys a db for each test.  There is a patch proposed to create the tables once and then ensure the tables are wiped at the end of each test [1], providing a performance improvement of ~10%.  I was wondering if this is the best way to provide isolation, since I’ve heard that isolation via per-test transactions should also work.  The patch author reported problems with this approach - apparently nested commits were not being rolled back.  Is there some trick to isolating with transactions that wouldn’t be immediately obvious?
> 
> Thanks,
> 
> 
> Maru
> 
> 1: https://review.openstack.org/#/c/122028/
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140918/cb851d8d/attachment.html>