Pedro, The @DB annotation adds a guard. It doesn't actually start or end a transaction. It will check that the transaction should be rolled back if one was started and not closed and that's about it. So the only time transactions are really started is when you see txn.start() in the code. So the flow of 1, 2, 3 that you wrote, has almost nothing to do with the actual transactions in play.
And this further illustrates how confusing this framework is and why I would like to abolish it. The @DB annotation should go away. The example you gave is code that runs in the background as part of an async job. What I would propose is that we do not rely on transaction as much as possible. Transactions in background code should be as small as possible. There is no point in trying to manage failures with transactions when the external resources we are interacting with are not transactional. So backend code should be written mostly as 1) set to transitioning state 2) make external thing so 3) set to done/finished/non-transitioning-state At any point there is no long running DB transaction. DB transactions are only used for atomic commits if your change spans a couple rows and needs to be atomic. If there is failure at any point because of the states we can recover. The only time I'd like DB transactions to be used is on the synchronous portion of APIs. The sync portion of APIs typically just writes data to the DB and nothing else, it is not supposed to interact with external resources. For sync APIs, one big transaction typically handles 90% of the failure cases. But I don't think this is fully possible right now for sync APIs. Darren Darren On Wed, Oct 9, 2013 at 10:58 AM, Pedro Roque Marques <pedro.r.marq...@gmail.com> wrote: > Darren, > I generally agree with you... just trying to point out what could be pitfalls > on the way to evolve the system. > > On Oct 9, 2013, at 10:29 AM, Darren Shepherd wrote: >> >> I wish we were doing transaction per API, but I don't think that was >> ever a consideration. I do think the sync portion of API commands >> should be wrapped in a single transaction. I really think the >> original intention of the Transaction framework was to assist in >> cleaning up resources that people always forget to close. I think >> that is mostly it. > > My understanding is that for instance when a VM is created you have a call > flow that looks a bit like: > > 1. UserVmManagerImpl.createVirtualMachine (@DB, persist) > 2. VirtualMachineManagerImpl.allocate (@DB, persist) > 3. NetworkOrchestrator.allocate (@DB, persist) > > My understanding is that an check in NetworkOrchestrator (e.g. nic parameters > not being kosher) is supposed to rollback the transaction and remove the VM > in the database... > > There are some errors for which this mechanism works OK today.... I believe > it would be desirable to have a proposal of how to deal with such an example > and then attempt to implement it consistently. Even if it requires the > programmer to understand that it needs to explicitly rollback the VM if the > underlying layers throw an exception. > > Pedro.
