On Wed, Jan 19, 2011 at 19:54, Caleb James DeLisle <calebdelisle(a)lavabit.com
On 01/19/2011 12:13 PM, Vincent Massol wrote:
Hi Caleb,
I see you're excited, that's good! :)
Some general comments:
* This looks more like a design for a transaction module than for a
persistence
engine. I don't see anything related to persistence in your
proposal below. Your proposal could work on stuff others than storage,
right?
Yes, this proposal only covers the transaction sub-module of the
persistence engine. The so far
un-proposed modules include xwiki-store-serialization,
xwiki-store-filesystem and a legacy
attachment storage module: xwiki-store-filesystem-attachments.
Anything which requires transactions could use the TransactionRunnable
although I'm at a loss to
think of anything other than storage which would require transactions.
* I was expecting to see some
Store/Storage/Persistence interfaces with
proposed APIs and explanation on how they
could be implemented both with
Hibernate and JCR for example. And the relationship with the proposed new
Model defined.
I don't like to propose an interface until I have tried to implement it.
Also I do not like to
propose an implementation until I have tried to use it. At this point it's
far enough off that I
would rather wait than propose APIs blind.
My experience with attachment store has shown that what we want is a set of
functions which provide
TransactionRunnables to do various things:
aka:
TransactionRunnable<T> getDocumentSaveTransactionRunnable(XWikiDocument
toSave);
In a hibernate implementation it would return
TransactionRunnable<HibernateTransaction> and in a JCR
it would return TransactionRunnable<JCRTransaction>.
We cannot have APIs like this until TransactionRunnable is agreed upon
these will return instances
of it.
* I was also expecting a strategy defined to
migrate users from the
current implementation of the storage to the new one
IMO we should change the persistence engine and implement the same schema,
once the persistence
engine is rebuilt, then we can consider modifying the schema. The schema is
a specification, it may
not be perfect but it is something to comply with. It is important to me
that a we prove that a new
persistence engine is able comply with existing specifications before we
start designing new ones
around it.
I noticed some discussions between Denis and you on IRC about all this.
Does you
latest findings change the proposal below?
Everything proposed still holds true but I did add 2 new features.
1. There is a way for a TransactionRunnable<DBTransaction> to be passed an
instance of DBTransaction
using a new method called getContext().
2. There is a new class which serves what I believe is an edge use case.
Suppose you want to define a TransactionRunnable (we will call it
YourTransactionRunnable) which
must run inside of a DBTransaction but it must _also_ run after an instance
of MyTransactionRunnable.
You can make MyTransactionRunnable a
"ProvidingTransactionRunnable<DBTransaction, MyInterface>" and
then MyTransactionRunnable must run inside of a DBTransaction and we define
YourTransactionRunnable
as a TransactionRunnable<MyInterface>. This also allows
MyTransactionRunnable to share information
since YourTransactionRunnable.getContext() will provide an implementation
of MyInterface. Of course
this feature must be used with care as it provides the tools to write
horrible constructs but IMO it
is the type of feature which when you need it, there is no other way
around.
I just need to add that we have not been able to apply TR and PTR in
particular to a simple store with a JDBC connected database. From our
experience, the PTR cause more issue than it solve and we have to find a
better way to convey datas between transactions of a given transaction
chain. Data dependencies between transaction could not be easily solved at
compile time since there is many combinatory situation in real life.
My suggestion would be to provide access to previous transaction using a
hash of Interfaces exposed by previous transaction. Checking availability of
needed interfaces could be done by transactions à preRun time which could
avoid the need of uselessly running the whole chain, if there is a
dependency problem.
I also doubt that the way of multiple transactions are executed in a same
single one is correct or useful. Would really prefer to see all transactions
run at a given level before going down the chain. This would provide the way
to bundle transaction (and even transaction chain) together by running them
in a single transaction. This would also helps not mixing dependencies since
currently there is an implicit availability of earlier sibling chain of
transactions that does not fit the idea of a context in evolution checked at
compile time and could provide unchecked implicit dependencies.
To conclude, this is a very interesting proposal, that needs more
refinements before being used wildly in all situation requiring
transactional processing. Nice idea and good job Caleb !
Denis
Caleb
Thanks
-Vincent
On Jan 10, 2011, at 2:15 PM, Caleb James DeLisle wrote:
> Hi,
> I have been working hard on filesystem attachments and I found that
synchronizing manual filesystem
> transactions with automatic database
transactions was a difficult job
and I needed a new tool to do
> it. So I wrote what I am proposing to be the
next XWiki Persistence
Engine.
>
> I'll start off with the fun part of the proposal, I have been calling it
xwiki-store so far but I am
> so excited about the capabilities of this
engine that I don't think it
does it justice to name it
> "store" after the place on the
corner with milk and eggs. I am proposing
it be named "XWiki
> Persistence Engine", the directory will
be renamed xwiki-persistence,
the artifact name
> xwiki-core-persistence, and the package name
org.xwiki.persistence.
Persistence is an attribute of
> castles, mountains and redwood trees which I
think is fitting for a
conservatively designed storage
> engine.
>
> Now a little explanation of what I'm so excited about:
> The common and error prone way of saving things in the database is to
open a
transaction, enter a
> try clause, do something then commit. If we
catch an exception, then we
rollback.
> something like this:
>
> begin transaction;
> try {
> do something;
> do something else;
> commit;
> } catch (Any exception which may occur) {
> rollback;
> }
>
> There are 3 things which can go wrong. 1 we forget to begin the
transaction, 2
we forget to commit
> and 3 we do not rollback properly. What makes
things worse is often the
database will "assume we
> meant to..." and things will work ok
most of the time which makes things
much worse because bugs
> will hide very well.
>
> My answer to this problem is a class called TransactionRunnable. It
provides a
set of 5 empty
> methods to override: onPreRun(), onRun(),
onCommit(), onRollback(), and
onComplete(). the exact
> circumstances under which each are called is
documented in the javadoc
comments here:
>
http://svn.xwiki.org/svnroot/xwiki/contrib/sandbox/xwiki-store/xwiki-store-…
> I wrote TransactionRunnable twice, I wrote
it, used it for attachments,
then after having real
>> experience as a user, I wrote it again.
>
>> To repeat our original
example with TransactionRunnable you might say
this:
>
>> public class
DoSomethingTransactionRunnable extends TransactionRunnable
>> {
>> public void onRun()
>> {
>> do something;
>> do something else;
>> }
>> }
>
>> Now we can use another
TransactionRunnable which opens and closes the
transaction for us.
>
>> StartableTransactionRunnable
transaction = new
HibernateTransactionRunnable();
>> new DoSomethingTransactionRunnable().runIn(transaction);
>> transaction.start();
>
>> the runIn() function allows
us to run one TransactionRunnable inside of
another. Supposing we wanted
> to reuse "do something else" in
other places, we can make it a separate
TransactionRunnable and use
> the runIn() function to hook it to our
DoSomethingTransactionRunnable
ie:
>
>> public class
DoSomethingTransactionRunnable extends TransactionRunnable
>> {
>> public DoSomethingTransactionRunnable()
>> {
>> new DoSomethingElseTransactionRunnable().runIn(this);
>> }
>> ..
>
>> The only limitations on
running TransactionRunnables inside of one
another are they cannot run more
> than once and they cannot call themselves
(this would be an infinite
loop).
>
>> This pattern makes each job
which is done on storage easily isolated
and, as I have so far
> experienced, trivial to test. However, it
still leaves the possibility
that we might forget that
> DoSomethingTransactionRunnable must be run
inside of a hibernate
transaction. I have devised a
> solution for this too. Using generics, I
offered a means for the author
of a TransactionRunnable to
> communicate to the compiler what other
TransactionRunnable their
runnable must be run in and without
> explicit casting or defining of an
intermediary runnable, this
requirement cannot be violated or
>> else it wouldn't compile!
>
>> Finally we have the issue of
starting the runnable. Who's to say I won't
be tired one day and just
> write new
DoSomethingTransactionRunnable().start() without opening a
transaction first? If
> DoSomethingTransactionRunnable cannot be
safely run outside of a
transaction all it needs to do is
> not extend StartableTransactionRunnable and
it won't have any start
function.
>
>> I have taken a multitude of
very easy mistakes and given the author of a
TransactionRunnable the
> tools to make it very hard for the user to
make them. Also, since a
TransactionRunnable has no
> reason to be very long (it can just branch
off into another runnable)
this will make testing and
> code review easy in the place where it is
most important. This part of
the code is entirely generic
>> and has no dependence on hibernate or anything else.
>
>> I propose we move:
>> contrib/sandbox/xwiki-store/xwiki-store-transaction/
>> to:
>> platform/core/xwiki-persistence/xwiki-persistence-transaction
>
>> And I will propose moving
each additional piece in the coming days.
>
>> WDYT?
>
>> Caleb
>
> _______________________________________________
> devs mailing list
> devs(a)xwiki.org
>
http://lists.xwiki.org/mailman/listinfo/devs
>
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs