The concepts of "suspend" and "resume" are implemented by a ThreadLocal
stack. A "suspend" pushes the current transaction on the stack, and a
"resume" pops a transaction off the stack.
If you read the Jira issue, I point out another problem with this clunky
implementation - calling "commit" doesn't really commit the transaction.
That is why we end up with invalid data in the entity cache - because
developers are fooled into thinking the "commit" calls in Delegator code
actually commit the data, but they don't. The transaction is committed
by the first bit of code that began the transaction - either a request
event or a service invocation.
This is an arcane problem and it is difficult to describe, but I will
try to diagram it:
Request Event
Service Dispatcher
Begin Transaction (actual begin)
Begin Service
Some service logic
Delegator calls "commit" - nothing happens
Delegator puts uncommitted values in cache
More service logic
Delegator calls "commit" - nothing happens
Delegator puts uncommitted values in cache
End Service
Commit Transaction (actual commit)
Return service results to event handler
If something goes wrong in the service and the transaction is rolled
back, the uncommitted values in the cache are still there!
You really have to spend time in Entity Engine code to fully appreciate
how awful the transaction implementation really is.
My approach keeps a Transaction reference in the Delegator. Instead of
calling the fake "commit", the Delegator notifies the Transaction about
changed values. The Transaction saves the changed values locally. After
the transaction is committed, the Transaction instance copies the saved
values to the cache.
If you look at my previous code fragment, there will be no more
"suspend" or "resume" - if you want a new transaction, you just get
another instance and use it.
Adrian Crum
Sandglass Software
www.sandglass-software.com
On 8/26/2014 9:02 PM, Scott Gray wrote:
Okay so I guess I don't really understand what you're suggesting, or how it
really differs much from what we have now. It's also not clear what your
suggested API changes have to do with the ThreadLocal usages?
Thanks
Scott
On 26/08/2014, at 3:22 pm, Adrian Crum <[email protected]>
wrote:
Just use the Delegator factory.
Adrian Crum
Sandglass Software
www.sandglass-software.com
On 8/26/2014 2:43 PM, Scott Gray wrote:
Hi Adrian,
I'll probably have plenty of questions, but the first that comes to mind is:
how would you use a delegator outside of a transaction with this approach?
Thanks
Scott
On 25/08/2014, at 10:51 am, Adrian Crum <[email protected]>
wrote:
One persistent problem with the current Entity Engine implementation is the use
of ThreadLocal variables in the Delegator and Transactions. Their use makes it
difficult (and sometimes impossible) to fix Entity Engine bugs. They also make
it impossible to multi-thread a Delegator instance.
Here is what I have had percolating in my head the last few months:
Transaction tx = TransactionFactory.newTransaction();
Delegator delegator = tx.getDelegator("default");
// Do stuff with delegator
Transaction nestedTx = TransactionFactory.newTransaction();
Delegator nestedDelegator = nestedTx.getDelegator("default");
// Do stuff with nestedDelegator
nestedTx.commit();
tx.commit();
A Delegator instance always references the transaction it is running in.
The advantage to this approach is we gain the ability to hand off Delegator
instances to other threads. Other threads can even commit/rollback a
transaction:
Transaction tx = delegator.getTransaction();
tx.commit();
After a commit, the Delegator instance is discarded. Any attempt to use it
after a commit throws an exception (the same is true with the Transaction
instance).
Another problem is Delegator localization - which also uses ThreadLocal
variables. We can localize Delegator instances like this:
Transaction tx = TransactionFactory.newTransaction();
Delegator delegator = tx.getDelegator("default", locale);
Finally, the current implementation has a caching problem:
https://issues.apache.org/jira/browse/OFBIZ-5534
With the new design, the Delegator instance, Transaction instance, and entity
cache are tightly coupled - so that problem is easy to solve.
What do you think?
--
Adrian Crum
Sandglass Software
www.sandglass-software.com
