On 01/11/2011 06:03 AM, Marius Dumitru Florea wrote:

Hi Caleb,

Sounds good. Great job! One comment below,

On 01/10/2011 03: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-t... 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!

Is this generic enough to allow us to easily test a TransactionRunnable that has explicitly specified the runnable it must be run in?

When you say "generics" I guess you are referring to:

public class DoSomethingTransactionRunnable extends TransactionRunnable<HibernateTransactionRunnable> { ... }

From what I understood HibernateTransactionRunnable must be a concrete class so that you can instantiate it. Then it's not easy/elegant to mock it.

Since runIn() takes a TransactionRunnable<? extends T> instead of T itself, there are 2 options: 1. Do as you say and then DoSomethingTransactionRunnable may be runIn() any TransactionRunnable<HibernateTransactionRunnable> so you need not mock HibernateTransactionRunnable, only define a TestTransactionRunnable which extends TransactionRunnable<HibernateTransactionRunnable> or as I prefer: 2. Create an empty interface called HibernateTransaction which exists for the sole purpose of being in the generic. Thus to define a StartableTransactionRunnable<HibernateTransaction> is to promise that you have indeed started and stopped the hibernate transaction in your runnable. I did spend the better part of a day trying to make sure that an evil StartableTransactionRunnable could not make promises that it couldn't keep but in the end I decided that this method was better. In regards to mocking, I have not found a need to mock anything since I can just instantiate an anonymous class extending TransactionRunnable, put my test logic in that, and run my tested runnable inside of it. Along a similar line, you might ask why TransactionRunnable itself is not an interface. I tried to do this and found that first, there were a number of private methods used for chaining which had to be made public and was less than pleasant although acceptable. Second, runIn() relies on access to a private field to attach the runnable to it's parent and I couldn't face the thought of a public unsafeAdd() method which said "please don't use this" in the javadoc comment. I could not think of any use case for another implementation of the base TransactionRunnable class and it made no sense to me to sacrifice the outward simplicity and stability of TransactionRunnable for something which I could not imagine any use for. Caleb

...

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?

+1

Thanks, Marius

...

Caleb

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

On 01/14/2011 04:21 AM, Denis Gervalle wrote:

Hi Caleb,

We have found your proposal really interesting, and we have stated to use it against a true database

That is awesome! I am so glad to hear this is being used in the field, it is too easy to write code which looks good but in real life turns out to be difficult or impossible to use.

(using JDBC) for one XWiki related project we have in development today. I have had not much time myself to look at the details, but here is comments/questions from my colleague Olivier. If we plan to use it later for the xwiki-store, I would really appreciate your comments.

Absolutely, and please please tell me what is good and bad about usability, IMO this is still fluid enough that we can make changes to fix issues.

In my understanding, for a database, I need to have a

...

StartableTransactionRunnable that roughly: * Open the connection in onPreRun

Yes and acquire locks if need be.

...

* Commit the transaction in onCommit * Rollback the transaction in onRollback * Close the connection in onComplete Yes and release acquired locks.

...

Then, I need to have classes extending TransactionRunnable and implementing the desired code in the onRun method.

Have I correctly understood how this library should be used ? If I'm correct, how do the TransactionRunnable instances retrieve the connection held in the StartableTransactionRunnable ? Currently I have a code similar to this:

DBStartableTransactionRunnable run = new DBStartableTransactionRunnable (dataSourceName); new InsertDataTransactionRunnable(run, data).runIn(run); run.start();

Yes and for safety you can define: public interface DBTransaction { /* Empty. */ } and public class DBStartableTransactionRunnable extends StartableTransactionRunnable<DBTransaction> and public class InsertDataTransactionRunnable extends TransactionRunnable<DBTransaction> thus protecting InsertDataTransactionRunnable from running outside of a transaction.

...

In which I need to pass the DBStartableTransactionRunnable instance in order to be able to access the exposed Connection object which is instanciated in the onPreRun method

Suppose we added this: protected T getTransaction() Then StartableTransactionRunnable can only be constructed by supplying an instance of T. So TransactionRunnable<DBTransaction>.getTransaction() would return an instance of the DBTransaction interface. Then you define DBTransaction interface with a getConnection() method to get the connection. Does that sound workable? Should we make T be required in the constructor on in the start() function? Do you have a preference?

...

My second interrogation concern transactions that return data. What's the best way to implement them ? Currently what I've come up with is something like:

DBStartableTransactionRunnable run = new DBStartableTransactionRunnable (dataSourceName); GetDataTransactionRunnable t = new GetDataTransactionRunnable(run, dataID) t.runIn(run); run.start(); MyData d = t.getResult();

My thinking was either: hand it an object which will be the container for the data, or do not use TransactionRunnable for loading since nothing is being altered and there should be no danger of data corruption.

...

I have a third interrogation, but I'm still unsure about it's validity. It consist of data communication between transactions. Assume that running in the same StartableTransactionRunnable we have: * TR1 fetch data A1 * TR2 update another data A2 * TR3 perform some operation involving A1

Where TR means TransactionRunnable instance. How does TR3 has access to the data that is retrieved by TR1 ?

This is related to a problem which has bothered me for some time. A TR3 can only run safely if run inside of a TR1 but we can assume that TR1 and TR3 must both run inside of a DBStartableTransactionRunnable. How does TR3 signal that it needs both? Your example has given me what I think is the answer, I will write: public <R super P> class ProvidingTransactionRunnable<R, P> extends TransactionRunnable<P> R and P being "requires" and "provides". Any TransactionRunnable<P> will be able to be run inside of a ProvidingTransactionRunnable<R, P> and the ProvidingTransactionRunnable<R, P> will have a modified runIn() method allowing it to run inside of any TransactionRunnable<R>. Now you can safely require that TR3 runs inside of TR1 by saying: public interface MyInterfaceWithDataA1 extends DBTransaction { Data getA1(); } public class ImplementationWithDataA1 implements MyInterfaceWithDataA1 { ... } public class TR1 extends ProvidingTransactionRunnable<DBTransaction, MyInterfaceWithDataA1> { private Data dataA1; protected void onRun() { this.dataA1 = ..... } /** From the proposal above, "getTransaction" is too specific, can't think if a better term. */ @Override protected MyInterfaceWithDataA1 getTransaction() { // super.getTransaction() will return a DBTransaction which our implementation can wrap. return new ImplementationWithDataA1(super.getTransaction(), this.dataA1); } } public class TR2 extends TransactionRunnable<DBTransaction> { ... } public class TR3 extends TransactionRunnable<MyInterfaceWithDataA1> { protected void onRun() { // This call defers to the parent by default, TR2 can ruin the day by // overriding getTransaction() but I don't see how this can be helped. final Data A1 = this.getTransaction(); } } Then: DBStartableTransactionRunnable run = new DBStartableTransactionRunnable(dataSourceName); TransactionRunnable<MyInterfaceWithDataA1> tr1 = new TR1(); tr1.runIn(run); TransactionRunnable<DBTransaction> tr2 = new TR2(); TransactionRunnable<MyInterfaceWithDataA1> tr2WithNewCapabilities = tr2.runIn(tr1); new TR3().runIn(tr2WithNewCapabilities); // This would fail at compile time: new TR3().runIn(tr2); run.start(); This pattern will provide TR3 with it's needed data and it will make sure that TR3 cannot be run without the proper requirements being in place beforehand which was my original goal. Thank you for the feedback and for helping me solve this problem. Please tell me about anything else you don't understand or don't like. Caleb

WDYT ? Thanks,

Denis

On Tue, Jan 11, 2011 at 14:50, Caleb James DeLisle <[email protected]

...

wrote:

...

On 01/11/2011 06:03 AM, Marius Dumitru Florea wrote:

...

Hi Caleb,

Sounds good. Great job! One comment below,

On 01/10/2011 03: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-t...

...

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!

Is this generic enough to allow us to easily test a TransactionRunnable that has explicitly specified the runnable it must be run in?

When you say "generics" I guess you are referring to:

public class DoSomethingTransactionRunnable extends TransactionRunnable<HibernateTransactionRunnable> { ... }

From what I understood HibernateTransactionRunnable must be a concrete class so that you can instantiate it. Then it's not easy/elegant to mock it.

Since runIn() takes a TransactionRunnable<? extends T> instead of T itself, there are 2 options: 1. Do as you say and then DoSomethingTransactionRunnable may be runIn() any TransactionRunnable<HibernateTransactionRunnable> so you need not mock HibernateTransactionRunnable, only define a TestTransactionRunnable which extends TransactionRunnable<HibernateTransactionRunnable> or as I prefer:

2. Create an empty interface called HibernateTransaction which exists for the sole purpose of being in the generic. Thus to define a StartableTransactionRunnable<HibernateTransaction> is to promise that you have indeed started and stopped the hibernate transaction in your runnable.

I did spend the better part of a day trying to make sure that an evil StartableTransactionRunnable could not make promises that it couldn't keep but in the end I decided that this method was better.

In regards to mocking, I have not found a need to mock anything since I can just instantiate an anonymous class extending TransactionRunnable, put my test logic in that, and run my tested runnable inside of it.

Along a similar line, you might ask why TransactionRunnable itself is not an interface. I tried to do this and found that first, there were a number of private methods used for chaining which had to be made public and was less than pleasant although acceptable. Second, runIn() relies on access to a private field to attach the runnable to it's parent and I couldn't face the thought of a public unsafeAdd() method which said "please don't use this" in the javadoc comment. I could not think of any use case for another implementation of the base TransactionRunnable class and it made no sense to me to sacrifice the outward simplicity and stability of TransactionRunnable for something which I could not imagine any use for.

Caleb

...

...

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?

+1

Thanks, Marius

...

Caleb

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

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. 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-t... 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 [email protected] http://lists.xwiki.org/mailman/listinfo/devs

On 01/20/2011 02:42 AM, Denis Gervalle wrote:

On Wed, Jan 19, 2011 at 19:54, Caleb James DeLisle <[email protected]

...

wrote:

...

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.

What you can do is make DBTransaction (the context) extend Map and then put things in that map. I really don't like that path though since is means that TR3 uses things provided by TR1 and doesn't declare that need. It breaks atomicity by sharing data between transactions, TR3 can't just be plugged in somewhere else where TR1 is not used. It also breaks breaks compile time safety, plugging TR3 in somewhere else (or reordering them) will compile but fail to run. That pattern doesn't really afford you any of the benefits of TR other than the try, catch safety so, to me, it makes sense in that case for the code for TR1, TR2, and TR3 to all be inside of the same TransactionRunnable.

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.

I don't see this as making sense. My goal is to make sure when something goes wrong it fails as early as possible so there is the least amount to rollback. SaveDocumentRunnable | +--SaveObjectRunnable | | | +--SavePropertyRunnable EXCEPTION | +--SavePropertyRunnable <-- Doesn't run. | +--SaveObjectRunnable <-- doesn't run | etc. Doesn't run. I'm interested to know what is the rationale for wanting them to run one level then the next? IE: save document, then all objects then all properties.

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.

I still don't quite follow, can you give a real world example?

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 !

Thanks :) 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-t...

...

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 [email protected] http://lists.xwiki.org/mailman/listinfo/devs

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

On 01/20/2011 12:33 PM, Denis Gervalle wrote:

On Thu, Jan 20, 2011 at 17:15, Caleb James DeLisle <[email protected]

...

wrote:

...

On 01/20/2011 02:42 AM, Denis Gervalle wrote:

...

On Wed, Jan 19, 2011 at 19:54, Caleb James DeLisle < [email protected]

...

wrote:

...

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.

What you can do is make DBTransaction (the context) extend Map and then put things in that map. I really don't like that path though since is means that TR3 uses things provided by TR1 and doesn't declare that need. It breaks atomicity by sharing data between transactions, TR3 can't just be plugged in somewhere else where TR1 is not used. It also breaks breaks compile time safety, plugging TR3 in somewhere else (or reordering them) will compile but fail to run. That pattern doesn't really afford you any of the benefits of TR other than the try, catch safety so, to me, it makes sense in that case for the code for TR1, TR2, and TR3 to all be inside of the same TransactionRunnable.

...

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.

I don't see this as making sense. My goal is to make sure when something goes wrong it fails as early as possible so there is the least amount to rollback.

SaveDocumentRunnable | +--SaveObjectRunnable | | | +--SavePropertyRunnable EXCEPTION | +--SavePropertyRunnable <-- Doesn't run. | +--SaveObjectRunnable <-- doesn't run | etc. Doesn't run.

I'm interested to know what is the rationale for wanting them to run one level then the next? IE: save document, then all objects then all properties.

...

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.

I still don't quite follow, can you give a real world example?

Well, let me base my real world example on your sample above. Imagine that documents are linked to their objects using their primary key, PK which is generated by the underlying database when you records the document for the first time, and that we needs creating a new documents with Objects in a single transaction. Your SaveObjectRunnable will need that PK as a FK, how does it get it? Or said in another way, it needs a context that provide the PK of the document for which it is saving the objects.

If the object knows what document it belongs to couldn't you use: object.getDocument().getKey() ? I think allowing the document to know it's primary key is less ugly than passing everything around in a map. Alternatively you could use: UPDATE object SET object.foreignKey=(SELECT doc.id where doc.name = <object.getDoc().getName()>) or the like. It's harder on the DB but it will make each TR modular and atomic.

Well, lets insert a CreateDocumentRunnable before it, that may have some lower TR to complete, like saving document and retrieving the generated PK. How does this CreateDocumentRunnable is maybe a sibling of SaveObjectRunnable provide the PK to it ?

I don't understand why the SaveObjectRunnable would be a sibling and of the SaveDocumentRunnable. If the Object is part of the document then shouldn't it be a child?

How do you check that SaveObjectRunnable requirement to have that PK is fulfilled ? Have you an idea of the structure needed for this example ?

IMO siblings should never depend on one another, if the TR needs something from the last TR then it really needs to be a child of it. this also has the benefit that you can pass around a TR for saving a document and it will have all TRs for saving each object and each property inside of it. Thank you, I really appreciate your review. Caleb

Denis

...

...

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 !

Thanks :)

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-t...

...

...

...

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 [email protected] http://lists.xwiki.org/mailman/listinfo/devs

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

On 01/20/2011 03:28 PM, Denis Gervalle wrote:

On Thu, Jan 20, 2011 at 18:50, Caleb James DeLisle <[email protected]

...

wrote:

...

On 01/20/2011 12:33 PM, Denis Gervalle wrote:

...

On Thu, Jan 20, 2011 at 17:15, Caleb James DeLisle < [email protected]

...

wrote:

...

On 01/20/2011 02:42 AM, Denis Gervalle wrote:

...

On Wed, Jan 19, 2011 at 19:54, Caleb James DeLisle < [email protected]

...

wrote:

...

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.

What you can do is make DBTransaction (the context) extend Map and then put things in that map. I really don't like that path though since is means that TR3 uses things provided by TR1 and doesn't declare that need. It breaks atomicity by sharing data between transactions, TR3 can't just be plugged in somewhere else where TR1 is not used. It also breaks breaks compile time safety, plugging TR3 in somewhere else (or reordering them) will compile but fail to run. That pattern doesn't really afford you any of the benefits of TR other than the try, catch safety so, to me, it makes sense in that case for the code for TR1, TR2, and TR3 to all be inside of the same TransactionRunnable.

...

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.

I don't see this as making sense. My goal is to make sure when something goes wrong it fails as early as possible so there is the least amount to rollback.

SaveDocumentRunnable | +--SaveObjectRunnable | | | +--SavePropertyRunnable EXCEPTION | +--SavePropertyRunnable <-- Doesn't run. | +--SaveObjectRunnable <-- doesn't run | etc. Doesn't run.

I'm interested to know what is the rationale for wanting them to run one level then the next? IE: save document, then all objects then all properties.

...

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.

I still don't quite follow, can you give a real world example?

Well, let me base my real world example on your sample above. Imagine that documents are linked to their objects using their primary key, PK which is generated by the underlying database when you records the document for the first time, and that we needs creating a new documents with Objects in a single transaction. Your SaveObjectRunnable will need that PK as a FK, how does it get it? Or said in another way, it needs a context that provide the PK of the document for which it is saving the objects.

If the object knows what document it belongs to couldn't you use: object.getDocument().getKey() ? I think allowing the document to know it's primary key is less ugly than passing everything around in a map.

For sure, this is a contrive example which I try to use to exposed what I means. It is not perfect, just too simple in fact.

It will become a real example if XWiki's Hibernate based storage is ported over.

...

Alternatively you could use: UPDATE object SET object.foreignKey=(SELECT doc.id where doc.name = <object.getDoc().getName()>) or the like. It's harder on the DB but it will make each TR modular and atomic.

But there is no assurance it will works since you have never say that the document should be created to save its objects.

Indeed there wouldn't. I suppose the best solution is object.getDocumen().getKey().

...

...

Well, lets insert a CreateDocumentRunnable before it, that may have some lower TR to complete, like saving document and retrieving the generated PK. How does this CreateDocumentRunnable is maybe a sibling of SaveObjectRunnable provide the PK to it ?

I don't understand why the SaveObjectRunnable would be a sibling and of the SaveDocumentRunnable. If the Object is part of the document then shouldn't it be a child?

No, SaveObjectRunnable will not be sibling of the SaveDocumentRunnable, it will be a child of it, for sure, but it could be a sibling of let say CreateDocumentRunnable which have itself some children for doing its own job.

...

...

How do you check that SaveObjectRunnable requirement to have that PK is fulfilled ? Have you an idea of the structure needed for this example ?

IMO siblings should never depend on one another, if the TR needs something from the last TR then it really needs to be a child of it. this also has the benefit that you can pass around a TR for saving a document and it will have all TRs for saving each object and each property inside of it.

To say it an other way, saving a document, could be divided in several pieces, and there is no assurance that you could always convert that in parent child relationships, especially when a third transaction depends on the result of 2 previous one that are not dependent from each other but are bundled together in a third one. That would be a perfect world if you could really expect that all dependencies are one to many, and never many to one.

A / \ B C \ / D

A is composed of B and C, and D depends on A.

How about this: getDocumentSaveRunnable() returns: A | +-B +-C and then if D requires A it can call: D.runIn(A) and now you will have A | +-B +-C +-D But this pattern will fail if A adds more subTR's in it's preRun function. Caleb

Thank you, I really appreciate your review.

...

Thanks ;), I really hope it could make it better !

Denis

...

Caleb

...

Denis

...

...

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 !

Thanks :)

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-t...

...

...

...

>> 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 > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs >

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

On Thu, Jan 20, 2011 at 09:23, Vincent Massol <[email protected]> wrote:

Hi Caleb,

On Jan 19, 2011, at 7:54 PM, Caleb James DeLisle wrote:

...

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.

Ok I was misled by the title of your mail "Introduce a new persistence engine". I guess it could/should have been "Introduce a generic transaction API independent of the underlying storage implementation", right?

So if we focus purely on the transaction part here are some questions:

* Why don't we use existing standards such as JTA/JTS? See http://en.wikipedia.org/wiki/Java_Transaction_API * If we were to define our own API, would we be able to implement it using JTA, ie is it a higher level transaction API than JTA? * Imagine we decide to use JCR as the storage implementation, how would this transaction API integrate with it knowing that JCR integrates with JTA? (http://www.day.com/specs/jcr/2.0/21_Transactions.html)

...

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 don't quite agree with this since it assumes this schema is universal which it definitely is not. It's only a schema that works with RDBMS. It wouldn't work with an ODBMS or a file system implementation.

The proposal of Caleb is not RDBMS related at all. He first use it for the filesystem persistence of attachement.

Also if there's one thing we shouldn't care about it's the schema. It's supposed to be opaque for the user and the user has to use the storage API to access it (and never go directly to the DB). In other words we should be clear that the schema is not part of the API since that would prevent any modification of it. I don't think we need this barrier.

Conclusion:

We're doing something really difficult here, which is defining a transaction API without defining the storage implementation we want to use and thus without defining how this transaction API would integrate with it. Right now the most common (if not the only one!) known and standard transaction API in the java world is JTA and most if not all known storage implementation support it. Thus if we really want a transaction API separate of the storage implementation I'd be in favor of looking at JTA and see whether it would fit our needs.

WDYT?

Proposal of Caleb is over anything, including JTA, it is absolutely not a concurrent of JTA. What he proposed is to helps managing transactional processing properly without having to write boring try/finally code and ensuring we will commit or rollback and we do not mix incompatible "sub"-transactions. I would like to see it extended to support transmission of datas between these "sub"-transaction. In regards to a DBMS, there will be a single transactions and what is proposed by Caleb helps allowing it to be customarily subdivide in smaller steps without having to take care of the overall process of committing/rollbacking and collecting errors. You concentrate on the work that should be done, and the rest is taken care by your TransactionRunnables. This is very interesting in my opinion, but this is not yet mature enough to manage all situations. Denis

Thanks -Vincent

...

...

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.

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-t...

...

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 [email protected] http://lists.xwiki.org/mailman/listinfo/devs

-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO

On 01/20/2011 04:24 AM, Vincent Massol wrote:

On Jan 20, 2011, at 10:12 AM, Denis Gervalle wrote:

...

On Thu, Jan 20, 2011 at 09:23, Vincent Massol <[email protected]> wrote:

...

Hi Caleb,

On Jan 19, 2011, at 7:54 PM, Caleb James DeLisle wrote:

...

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.

Ok I was misled by the title of your mail "Introduce a new persistence engine". I guess it could/should have been "Introduce a generic transaction API independent of the underlying storage implementation", right?

"Introduce a generic transaction API independent of the underlying storage implementation" so boring compared to "The XWiki Persistence Engine" ;) Indeed you are correct but this proposal is for the foundation of the engine.

...

...

So if we focus purely on the transaction part here are some questions:

* Why don't we use existing standards such as JTA/JTS? See http://en.wikipedia.org/wiki/Java_Transaction_API

The original answer was that I defined my requirements and wrote rather than looking for something. Now looking at the JTA information, I still can't tell if it meets the requirements which TransactionRunnable was designed to meet.

1. Can multiple jobs be chained together inside of a transaction? Can each job be made atomic? Can they be tested without mocking? 2. Are try/catch completely handled? I notice a lot of transact.begin() and transact.commit() in the code. 3. Is is possible to run a job outside of the proper type of transaction? IE: Can a "save document in JCR" job accidentally be run in a Hibernate transaction? 4. Can JTA run on any storage platform? Hibernate, JCR, raw JDBC, Filesystem, Memcached, etc.

...

...

* If we were to define our own API, would we be able to implement it using JTA, ie is it a higher level transaction API than JTA? TransactionRunnable is designed to run on top of absolutely anything.

...

...

* Imagine we decide to use JCR as the storage implementation, how would this transaction API integrate with it knowing that JCR integrates with JTA? (http://www.day.com/specs/jcr/2.0/21_Transactions.html) Yes, anything. Also I notice in the example code given at that page, they are using the same (anti) pattern. begin, do stuff, commit. That code can't be incorporated into a larger transaction or else it would be rolled back when begin() is called. This is the kind of code which begs to be made into a TransactionRunnable.

...

...

...

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 don't quite agree with this since it assumes this schema is universal which it definitely is not. It's only a schema that works with RDBMS. It wouldn't work with an ODBMS or a file system implementation.

The proposal of Caleb is not RDBMS related at all. He first use it for the filesystem persistence of attachement.

I was responding to Caleb's point about keeping the schema...

From my perspective we need to rewrite the engine so it's not a mess, get the new engine working well and with all the kinks worked out, then when each job is in a TransactionRunnable, we can easily swap them out for new ones one at a time. To redesign the schema now means we need to interact with the old XWikiHibernateStore code in order to do migration, that's not something I'd like to do.

...

...

Also if there's one thing we shouldn't care about it's the schema. It's supposed to be opaque for the user and the user has to use the storage API to access it (and never go directly to the DB). In other words we should be clear that the schema is not part of the API since that would prevent any modification of it. I don't think we need this barrier.

"be clear that the schema is not part of the API" yes which is why I want to change one at a time. If we design a new API and a new schema at the same time, we risk making an API with limitations and not noticing it because we make a schema which hides those limitations.

...

...

Conclusion:

We're doing something really difficult here, which is defining a transaction API without defining the storage implementation we want to use and thus without defining how this transaction API would integrate with it. Right now the most common (if not the only one!) known and standard transaction API in the java world is JTA and most if not all known storage implementation support it. Thus if we really want a transaction API separate of the storage implementation I'd be in favor of looking at JTA and see whether it would fit our needs.

...

WDYT?

Proposal of Caleb is over anything, including JTA, it is absolutely not a concurrent of JTA. What he proposed is to helps managing transactional processing properly without having to write boring try/finally code and ensuring we will commit or rollback and we do not mix incompatible "sub"-transactions.

BTW there are other ways of doing this. In JEE servers it's done with annotations (no "boring" try/finally as you say :)). It's the container-managed part of JTA.

Boring but more importantly unsafe.

Normally transactions should be demarcated at the level of the services so that several calls to the DB can be in the same Tx for example. At least that's what I used to do in my J2EE days...

...

I would like to see it extended to support transmission of datas between these "sub"-transaction. In regards to a DBMS, there will be a single transactions and what is proposed by Caleb helps allowing it to be customarily subdivide in smaller steps without having to take care of the overall process of committing/rollbacking and collecting errors. You concentrate on the work that should be done, and the rest is taken care by your TransactionRunnables. This is very interesting in my opinion, but this is not yet mature enough to manage all situations.

Question: Since this looks generic, isn't there frameworks out there that do this? If not, why?]

There is... Now ;) Caleb

Thanks -Vincent

...

Denis

...

Thanks -Vincent

...

...

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.

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-t...

...

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 [email protected] http://lists.xwiki.org/mailman/listinfo/devs