We definitely need transactions with our coding pattern. We will get
inconsistent data states without them if the server goes down or a
request hits an exception after partially updating.
We are using transactions, or were as of about 1.2. I'm not sure why
you say we aren't using them. I fixed some bugs related to this in 1.0
or so. It does require that your db and db configuration support
transactions.
Only a few calls are necessary, so the number of calls is not an
indication of lack of use; the transaction demarcation calls should be
early in the entry points of requests into the system or in tasks
executed asynchronously in their own threads. These are supposed to
associate a transaction with the thread. Everything else gets the
current transaction on the thread. It's a standard pattern.
--a.
Allen Gilliland wrote:
I have been going through some of the Roller backend and looking at some
parts of our Hibernate implementation and I have gotten myself to
wondering if we really need the transaction methods begin() and commit()
which are part of the Roller and PersistenceStrategy interfaces.
I think I understand where the idea for these methods came from and they
make some sense, but after inspecting the code a bit it seems like we
definitely don't actually use them.
In the current code I only see this ...
Roller.begin() - 6 uses
Roller.begin(user) - 4 uses
Roller.commit() - 7 uses
Roller.rollback() - 0 uses
PersistenceStrategy.begin() - 4 uses, just from RollerImpl classes
PersistenceStrategy.commit() - 4 uses, mainly RollerImpl classes
I also noticed that in our Hibernate implementation we aren't actually
using Transactions anyways, so even if these methods were in full use we
wouldn't have been taking advantage of them.
So, the deal is that if we want to use these transaction methods then we
need to make sure we are using them *everywhere*, otherwise they are
doing us no good and only confusing things. Alternatively I would
propose that we drop these methods and let implementors of a persistence
strategy handle the details of transactions within their own
implementations.
My vote is to drop these methods. I can see the potential value in
having them (if they were working properly), but my opinion is that we
aren't really doing any persistence transactions complicated enough to
really warrant the additional complexity and overhead of having them.
We have to go one way or the other, so we either remove them or add them
in everywhere.
Who else has an opinion?
-- Allen
