DBTransactions:The great do everything or do nothing problem

wolfganguhr

Hello

Actually in the year end routines the problem arises that we have to do the complete year end routine (do everything) or in case of an error to roll back any activities (do nothing).

In this case I've a small problem.

First let us have a look onto the routine TransactionWebConnector.LoadAAnalysisAttributes. This routine does some reading and the last command is a DBAccess.GDBAccessObj.RollbackTransaction();

The rollback is a non complicated method to reset a data base connection. The other way is a DBAccess.GDBAccessObj.CommitTransaction();.

A something more correct kind of doing this would be:

try
{
  TDBTransaction transaction = DBAccess.GDBAccessObj.BeginTransaction();
  ledger = ALedgerAccess.LoadByPrimaryKey(ledgerNumber, transaction);
  DBAccess.GDBAccessObj.CommitTransaction();
  row = (ALedgerRow)ledger[0];
}
catch (Exception exception)
{
  DBAccess.GDBAccessObj.RollbackTransaction();
  throw exception;
}

If you only have to read some data you can either do:

[]Nothing[/:m]
[]CommitTransaction[/:m]
[]RollbackTransaction[/:m][/list:u]

Regardless of a little bit of unused resources everything is correct unless you do not write anything to the data base.

Furthermore, if you have to write a little bit to the data base this will not be a problem too. Whatever the last end of a read command is, it will be terminated after the next write, because in this case a commit is required.

But in periodic end operations like year and this will be difficult. The year end operation for example requires a set of reallocation accountings. Actually the routines

CloseGifts();
CloseAccounts();
CloseGLMaster();
CloseBudget();
UpdatePeriods();
SetNewYear();
Finish();

shall be filled with content and all of them writes something to the data base.

So the final construction is:

try
{
  TDBTransaction transaction = DBAccess.GDBAccessObj.BeginTransaction();
  CloseGifts(transaction);
  CloseAccounts(transaction);
  CloseGLMaster(transaction);
  CloseBudget(transaction);
  UpdatePeriods(transaction);
  SetNewYear(transaction);
  Finish(transaction);
  DBAccess.GDBAccessObj.CommitTransaction();
}
catch (DoNothingException exception)
{
  DBAccess.GDBAccessObj.RollbackTransaction();
  GenerateErrorMessage();
}

or better - we only have one transaction and can use them without a parameter list queue.

try
{
  TDBTransaction transaction = DBAccess.GDBAccessObj.BeginTransaction();
  CloseGifts();
  CloseAccounts();
  CloseGLMaster();
  CloseBudget();
  UpdatePeriods();
  SetNewYear();
  Finish();
  DBAccess.GDBAccessObj.CommitTransaction();
}
catch (DoNothingException exception)
{
  DBAccess.GDBAccessObj.RollbackTransaction();
  GenerateErrorMessage();
}

And here it is very difficult if for example a read operation in CloseBudget() always terminates with a commit or a rollback.

First idea is to split the software in a PreparationPart and in a MakeAllChangesPart in which no database readings shall be used any more. But actually there is another problem.

I've written a small Accouting tool TCommonAccountingTool which helps to organize a large number of accountings. It creates one batch data set and here a number of journals and to each journal a number of accounting transactions can be added.

Following points are important:

[]Nearly each of those operations require a "small write" (without commit!) because commonAccountingTool.AddBaseCurrencyJournal needs an intern new journal number and so on.[/:m]
[]Running a list of accountings is not the only job of a periodic end operation like year end. Other database writings shall be done before and after the accounting(s).[/:m][/list:u]

So we have to do the following:
- []We need a DBAccess.GDBAccessObj.ThrowDoNothingException-Option in the database module. If this option is set, any commit is ignored and a rollback is translated to a DoNothingException. The developer himself is responsible to catch this exception and run a rollback for the complete operation.[/:m]
  [](Optional – I’m not sure if it is really necessary if we have realized the first part. If we need it, we need it in the initialization part and if a table is not already been cached)We have to define and to use a DBAccess.GDBAccessObj.ReadOnlyTransaction() which may be closed by something like DBAccess.GDBAccessObj.CloseReadOnlyTransaction() or not. Important is: There is no need for a physical commit or rollback to the real database connection.[/:m][/list:u]
```
try
{
  DBAccess.GDBAccessObj.ThrowDoNothingException = true;
  CloseGifts();
  CloseAccounts();
  CloseGLMaster();
  CloseBudget();
  UpdatePeriods();
  SetNewYear();
  Finish();
  DBAccess.GDBAccessObj.ThrowDoNothingException = false;
  DBAccess.GDBAccessObj.CommitTransaction();
}
catch (DoNothingException exception)
{
  DBAccess.GDBAccessObj.ThrowDoNothingException = false;
  DBAccess.GDBAccessObj.RollbackTransaction();
  GenerateErrorMessage();
}
```
  Best regards
  Wolfgang

pokorra

We have currently already several concepts in OpenPetra, eg. GetNewOrExistingTransaction or passing a transaction to each method.
The architecture team need to come to a decision how often we should create a transaction.

This is my opinion:
For each longer task, we only create one transaction, and pass this as a parameter to all methods.
So for example for the year end, we only create one DB transaction, that is used for reading and writing.
If something goes wrong we throw an exception, and in the top method that is calling all the year end methods, there will be a finally with a RollBack for the transaction.
If everything is fine the last thing the top method does, is to commit the transaction.

This way we still have the most choices how to deal with transactions.
It is also less complicated than to check all the time whether there is an open transaction available, with GetNewOrExistingTransaction.

Timotheus

wolfganguhr

Ok and I've forgotten to say it. I'll have to change the following routines too:

[]to crete a batch, [/:m]
[]to crete a yournal and [/:m]
[]to crete financial transactions[/:m]
[]to save the data and [/:m]
[]to post them.[/:m][/list:u]

thiasg

Hi all,

pokorra wrote
...
This is my opinion:
For each longer task, we only create one transaction, and pass this as a parameter to all methods.
So for example for the year end, we only create one DB transaction, that is used for reading and writing.
...

If would vote for a session object instead of a DB transaction. In my opinion, this parameter should be the first mandatory parameter of a method. This gives the freedom to easily add additional informations, if we find out later, that something additional would be helpful.

The whole topic session (DB) transactions is on the agenda of the Architecture team. Christian wanted to prototype, if it is possible to make it transparent, so that a developer would only need to declare, if the transaction is longer then one server call (see also sourceforge.net/apps/mediawiki/ ... g;_Caching).
Christian: Has you done something already?

Thias

wolfganguhr

Ok Thias now you know where I'ld have implemented a singleton. You can either create such a singleton or you have a session object als a mandatory object in each class.

Implementing a session object is a little bit of work so let us discuss this a little bit more. If we do this we might have the follwing "type tree"

[]Ict.Petra.Server.MCommon.TSessionObject[/:m]
[]Ict.Petra.Server.MFinance.TSessionObject : Ict.Petra.Server.MCommon.TSessionObject[/:m][/list:u]

In this tree we can handle a little bit more than only the problem of a data base transaction.

wolfganguhr

Some words about the size of the job ...

DBAccess.GDBAccessObj.BeginTransaction will be called 43 times in finance and 159 times in the complete project.

[]upgrade the parameter list of the routine which is called[/:m]
[]add the parameter any time the routine is called[/:m]
[]change the routine for the data base request[/:m][/list:u]

christiankatict

Hi Wolfgang,

If you only have to read some data you can either do:

Nothing
CommitTransaction
RollbackTransaction

This is not correct! 'Nothing' should never be done - if you open a DB Transaction in a section of code, this section of code is responsible for closing it as well. If this is not done, you leave an open DB Transaction 'dangling', which results in other server-side code tripping over it if that piece of code wants to open a DB Transaction and that piece of code is not designed to pick up an existing DB Transaction. This can easily happen: the user just needs to open another screen in parallel which needs data loaded from the DB. By doing so a DB Transaction will be started on that occasion, which will result in an Exception if you left a DB Transaction open from a previous DB read operation.

To prevent the problem mentioned in the previous paragraph, every DB Transaction (ncluding DB Transactions 'just for reading data'!) needs to be ended within a clearly defined scope - either with CommitTransaction or RollbackTransaction. That scope needs to be the procedure in which data is read or written, or in case of complex scenarios as described by you, the scope will be a 'control' procedure which calls many other sub-procedures which need to run within the same DB Transaction. If this rule is not followed, it can quickly get highly intransparent: what piece of code is committing or rolling back a DB Transaction that has been started somewhere completely else? What happens if an Exception is thrown somewhere in the call stack??? That rule is therefore not a technical necessity, but a design rule.

For cases like those:

And here it is very difficult if for example a read operation in CloseBudget() always terminates with a commit or a rollback.

... the CloseBudget() simply needs to either (1) return bool true/false to the calling procedure to delegate the committing /roll back to the calling 'control' procedure, or (2) needs to follow the pattern in which it checks for a possible running transaction (using GetNewOrExistingTransaction) and commits/rolls back the transaction only in case it started it itself. (2) should only be done if the Method in question is a 'library-kind-of-Method' which can be called from various pieces of server code and where the calling procedures might have started a DB Transaction in which the called Method should enlist itself, or in which the calling procedures might not have done that.

To summarise, I agree with what pokorra said about how things should be done - CURRENTLY.

As thiasg mentioned, the Architecture Team will look into the issue of more automatic DB Transaction handling and in particular how we can keep a DB Transaction open between server calls, which is not possible with the current model - unless UIConnector objects are used, which are stateful and could therefore keep a DB Transaction open.
Should we end up with the concept of a 'Session object', this could be a public static object which is simply created once within a Client AppDomain when a user logs in. Then we don't need to pass it into every Method call. Just a quick thought, it still needs more fleshing out.
Unfortunately, I did not find the time yet to look into those issues in detail.

wolfganguhr

Hi Christian

This is not correct! 'Nothing' should never be done

Christian this is done (I've found the code) but not by me and furthermore it is not allways wrong.

If this is not done, you leave an open DB Transaction 'dangling', which results in other server-side code tripping over it if that piece of code wants to open a DB Transaction and that piece of code is not designed to pick up an existing DB Transaction. This can easily happen

Yes that can be happen but not if you add a commit somewhere at the very end.

And if we commit every small transaction inside the object we actually use, we cannot use it inside a large transaction.

the CloseBudget() simply needs to either (1) return bool true/false to the calling procedure

Please Christian: Not "CloseBudget();" is the "common operation", the sequence "CloseGifts(); CloseAccounts(); CloseGLMaster(); CloseBudget(); UpdatePeriods(); SetNewYear(); Finish();" is. We either do all of this parts in one time or nothing. So I can get the same effect if I throw a DoNothingException if a problem occurs.

Throwing an exeption is one way, boolean value an other. And the session varialbe is a third.

As thiasg mentioned, the Architecture Team will look into the issue of more automatic DB Transaction handling and in particular how we can keep a DB Transaction open between server calls, which is not possible with the current model

Operations in more than one server calls are not my problem here. This I've discussed in the singleton thread.

Let us see to an example code of ABudgetAccess-Table:

            try
            {
                TDBTransaction transaction = DBAccess.GDBAccessObj.BeginTransaction();
                aBudgetTable = ABudgetAccess.LoadViaALedger(tHandleLedgerInfo.LedgerNumber, transaction);
                DBAccess.GDBAccessObj.CommitTransaction();
            }
            catch (Exception exception)
            {
                DBAccess.GDBAccessObj.RollbackTransaction();
                throw exception;
            }

But if this routine calls a "CommitTransaction" inside the CloseBudget() then the actual state of any "global transaction" is committed and this must not happen. Of course you can do "nothing" as I said above. The question is: How do we manage it in a unique way.

And furthermore we have actually about 159 such sequences in the code. Ho can we change the module easily not to run a commit or a rollback because we want to do it later.

Should we end up with the concept of a 'Session object',

That is the idea of thiasg.

this could be a public static object which is simply created once within a Client AppDomain when a user logs in. Then we don't need to pass it into every Method call. Just a quick thought, it still needs more fleshing out. Unfortunately, I did not find the time yet to look into those issues in detail.

But DBAccess.GDBAccessObj is nothing else but this. And I only would have include some additional functionality. Let me repeat:

DBAccess.GDBAccessObj.ThrowDoNothingException = true;

That means:

A routine like "ABudgetAccess.LoadViaALedger(...);" throws a "DoNothingException" if an error occurs.
No Commit will be done any more and of course no rollback until the final commit is done:

DBAccess.GDBAccessObj.ThrowDoNothingException = false;
  DBAccess.GDBAccessObj.CommitTransaction();

Unfortunately, I did not find the time yet to look into those issues in detail.

The problem is: Solving this problem is actually an important part of my work. But I think we will find something else.

Best regards
Wolfgang

christiankatict

Hi Wolfgang,

wolfganguhr wrote
This is not correct! 'Nothing' should never be done

Christian this is done (I've found the code) but not by me and furthermore it is not allways wrong.

1) The fact that it is done somewhere else in code does not mean that it is correct;

2) It is not 'wrong' in a technical sense as it 'works', but it is always wrong in a design sense. Let me explain that: If some section of code simply runs a DB command without a reference to a DB Transaction that was started in a controlling super-structure (I call it a 'control procedure'), then that piece of code must never commit or roll back that global DB Transaction as it could not know what else would be committed or rolled back. And the poor programmer who looks at that piece of code cannot tell either in case it does run a Commit or a Rollback - that is why it is 'wrong' in a design sense! It makes for impossible debugging as the user can do many things in parallel and - by doing that - start and committ/rollback DB transactions 'here, there and everywhere' in server code - this makes for 1001 possibilities in which some data outside of the current code scope can be committed to the DB that is not wanted, or some data that was changed outside of the current code scope never ending up in the DB because a Rollback was issued in this piece of code, or 'somewhere else' indeed. There is no control, and that must not happen.

Methods wihich you found that do not start their own DB Transaction and issue a Commit or Rollback inside this Method need to be changed to prevent such loss of control!!!

wolfganguhr wrote
If this is not done, you leave an open DB Transaction 'dangling', which results in other server-side code tripping over it if that piece of code wants to open a DB Transaction and that piece of code is not designed to pick up an existing DB Transaction. This can easily happen

Yes that can be happen but not if you add a commit somewhere at the very end.

Precisely - you are talking about a controlling super-structure(I call it a 'control procedure').

wolfganguhr wrote
And if we commit every small transaction inside the object we actually use, we cannot use it inside a large transaction.

Correct - that is where an controlling super-structure comes into play, whose purpose is the execution of several steps in the correct order and the management of starting of a transaction and the eventual Commit or Rollback of it.

wolfganguhr wrote
the CloseBudget() simply needs to either (1) return bool true/false to the calling procedure

Please Christian: Not "CloseBudget();" is the "common operation", the sequence "CloseGifts(); CloseAccounts(); CloseGLMaster(); CloseBudget(); UpdatePeriods(); SetNewYear(); Finish();" is. We either do all of this parts in one time or nothing. So I can get the same effect if I throw a DoNothingException if a problem occurs.

There is an existing example of a controlling super-structure(a 'control procedure')in the server code that addresses the problem of execution flow and Exception handling as you need it for that sequence:

\csharp\ICT\Petra\Server\lib\MPartner\ui\Partner.PartnerEdit.cs, Method 'SubmitChanges'

This Method starts a DB Transaction, executes two steps in sequence('SubmitChangesOther' and 'SubmitChangesAddresses') and - depending on whether they ran fine or not (in one case determined by a bool and in the other case determined by a TSubmitChangesResult) - a DB Commit or a DB Rollback is issued. In case an Exception happens somewhere in those call paths, a DB Rollback is issued, the Exception gets logged and it gets re-thrown (it ends up on the client side in this case, which is able to handle certain specific Exceptions, and shows the 'Unhandled Exception Dialog' in all other cases).
The Method 'SubmitChanges' only 'chains' two steps, but an arbitrary number of steps can be chained using this pattern. It is up to the control procedure whether to proceed or not to proceed with (a) further step(s) once one of the steps fails - that depends on the requirements; usually it will not execute any further steps in such a case (as is done in this example). Note that the call to 'SubmitChangesOther' results in a huge and varying number of DB operations, depending on many conditions of the business logic contained within that Method. This Method has its own control logic, whose result is a simple true/false return value, which tells the control procedure 'SubmitChanges' whether 'anything' went wrong.

wolfganguhr wrote
Throwing an exeption is one way, boolean value an other. And the session varialbe is a third.

That's right, but they have different purposes and scopes:
(A) A boolean value is always local to a Method (or to a Method call tree in case of a 'control procedure') and therefore completely safefrom causing any DB-Transaction-related side effects outside of such a scope (if the DB-Transaction handling is implemented diligently, of course!);
(B) Although an Exception always happens locally, the way you suggest with the proposed introduction of a 'ThrowDoNothingException' control structure by using the global GDBAccess object can potentially affect other, completely different code by causing DB-Transaction-related side effects in that 'other code' that cannot be predicted at runtime (more on that below)!;
(C) A Session Object/variable has a global scope for the one user session. It could potentially hold more than one open DB Transaction to a given RDBMS. This will be needed if we reach the point in which we will need 'nested' DB Transactions, as this is not supported by ADO.NET on a single DB Transaction. Also, it could hold other arbitrary information as well, as ThiasG pointed out. For those reasons (and probably a few more reasons once we start thinking about such an Object properly), a Session Object could be a useful thing to have and is distinctively different from our exsting global GDBAccess Object.

Regarding your suggestion of the proposed introduction of a 'ThrowDoNothingException' control structure, using the global GDBAccess object:
There are three problems with it:
1) Exceptions should not be used for the regular logical control flow of a program;
2) A programmer who does not know about the special (and quite hidden) implications of setting 'ThrowDoNothingException = true' and 'ThrowDoNothingException = false' cannot deduce the control flow from looking at the code (but can do so easily when looking at a Method like 'SubmitChanges');
3) Setting 'ThrowDoNothingException = true' in one specific piece of code on the server side enables this special behaviour globally for that user session. If that piece of code runs longer (as would be the case e.g. with Month End/Year End) and the user does 'something else that requires any DB Transaction' in parallel in that user session, that special behaviour is switched on from within the longer running code section and affects any other server-side code that requires a DB Transaction in that user session. Such parallel execution is perfectly possible if that piece of 'any other code' uses the 'GetNewOrExistingTransaction' technique to re-use an existing, running DB Transaction - and such parallel execution paths are not restricted to happen only when longer running code sections are executed, but there the likelihood of them happening are much higher. Such a dramatic side effect is not acceptable as it could lead to extremely-difficult-to-spot buggy behaviour and loss of data/unwanted saving of data in places a programmer doesn't expect and cannot even anticipate.

wolfganguhr wrote
Let us see to an example code of ABudgetAccess-Table ...

wolfganguhr wrote
But if this routine calls a "CommitTransaction" inside the CloseBudget() then the actual state of any "global transaction" is committed and this must not happen. Of course you can do "nothing" as I said above. The question is: How do we manage it in a unique way.

wolfganguhr wrote
And furthermore we have actually about 159 such sequences in the code. Ho can we change the module easily not to run a commit or a rollback because we want to do it later.

The solution to all of that is to pass a running transaction from the control procedure into such Methods and to return true/false to the control procedure, and have the control procedure deal with the DB Commit/DB Rollback (as is done in the Partner.PartnerEdit.cs Class).

wolfganguhr wrote
Unfortunately, I did not find the time yet to look into those issues in detail.

The problem is: Solving this problem is actually an important part of my work. But I think we will find something else.

I know that you need that for your work! I hope that this answer shows the solution to your current problem, and that we will soon find a good way ahead with the overarching architecture of database access, exception handling, and other related issues.
It is very unfortunate that ThiasG was not around for four weeks and that I could not talk with him about the issues for that reason, and that I am busy with the student project as well. Please also keep in mind that ThiasG works only part time for OpenPetra and therefore is not available on any given day. ThiasG and I will have a phone call (the first one after six weeks) on next Tuesday to talk about architectural matters, including those mentioned.

Kind regards,

ChristianK