DB Access Exception Handling Policy: Difference between revisions

From OpenPetra Wiki
Jump to navigation Jump to search
m (moved Inconsistent DB Access Exception Handling to DB Access Exception Handling: We now have consistent DB Access Exception Handling strategies documented on this page.)
Line 1: Line 1:
==Overview==
==Overview==
'''We do not yet have a consistent way of handling DB Access Exceptions in all situations. This is strongly desired.'''
'''The correct handling of Exceptions that occur because of database access is very important for the following reasons:'''
# DB Exception Handling '''must ensure''' that
## Any DB Transaction that was started in the same program scope (e.g. C# Method, or a particular 'chain' of C# Method calls) and that was used for the DB command that resulted in an Exception is ''rolled back'' (in case of write access to the DB) ''or committed'' (in case of read access to the DB). That way a DB transaction can never be left running ('dangling') where it shouldn't.
# DB Exceptions '''must not get 'swallowed''''. While suppressing an Exception (by 'swallowing' it) is never a good idea and is [[Error and Exception Handling Policy#'Swallowing'_Exceptions_-_*** DON'T_DO_IT!_*** | actively discouraged]], suppressing a DB Exception is especially bad as the caller of a Method that suppressed the Exception that occurred because of the DB access has no way of knowing of problems that really happened while accessing the DB and hence can not know whether the DB access succeeded, or not.


'''TODO''': Improve section on DB Access Exceptions in the [[Error and Exception Handling Policy#DB_Access | Error and Exception Handling Policy]].


=== The Danger of 'Dangling DB Transactions' ===
DB Transactions that are left running where they shouldn't (sometimes referred to as 'dangling DB Transactions') will likely cause problems further down the execution path of the program code, or with the next program code path that is executed by the user (esp. after an Exception happened), as other program code...
* might attempt to start a new DB Transaction, which will fail as the 'dangling DB Transaction' is still running and only one DB Transaction can be running at a given time;
* might be written in such a way that it can re-use an existing Transaction, but ...
** that 'piggy-backing of an existing DB Transaction' might fail if the Isolation Level that is required by that program code is higher (=more restrictive) than the Isolation Level of the running DB Transaction. Also, that program code could ask for a specific, exact Isolation Level and the the Isolation Level of the running DB Transaction is different. In both cases an Exception is raised.
** in case the re-using of the DB Transaction doesn't fail when the DB Transaction is requested by that program code then the Isolation Level of the running DB Transaction could be higher than the Isolation Level of the DB Transaction of that program code, which will lead at least to unnecessary DB Locks taken out and can potentially lead to concurrency problems that would otherwise not happen. Both of those issues are very hard to pin-point once they happen as it is often not obvious what program code has ''initially'' taken out a DB Transaction that then causes the mentioned problems later on (esp. once the application is used by users)!


===Exception Handling===
 
===DB Exception Handling===
* '''Scenario 1''': just reading data
* '''Scenario 1''': just reading data
** '''Way 1''': Commit before first Catch Clause + Commit in Catch Clause(s), no Finally Clause
** '''Way 1''': Commit before first Catch Clause + Commit in Catch Clause(s), no Finally Clause
*** ''Advantage'':  
*** ''Advantage'':  
**** No Finally Clause needed, therfore less deep nesting of code.  
**** No Finally Clause needed, therefore less deep nesting of code.  
*** ''Disadvantage'':
*** ''Disadvantage'':
**** Commit can be accidently forgotten about by developers, leaving an open, 'dangling' DB Transaction - this is also true for the case where a 'return' statement is executed in the 'try' code block of the try-catch Clause!!!
**** Commit can be accidentally forgotten about by developers, leaving an open, 'dangling' DB Transaction - this is also true for the case where a 'return' statement is executed in the 'try' code block of the try-catch Clause!!!
** '''Way 2''': Commit in Finally Clause which encloses try-catch Clause
** '''Way 2''': Commit in Finally Clause which encloses try-catch Clause
*** ''Advantage'': Commit can never be forgotten about - this is also true for the case where a 'return' statement is executed in the 'try' code block of the try-catch-finally Clause!!!
*** ''Advantage'': Commit can never be forgotten about - this is also true for the case where a 'return' statement is executed in the 'try' code block of the try-catch-finally Clause!!!
*** ''Disadvantage'': Finally Clause is needed, therfore deeper nesting of code.
*** ''Disadvantage'': Finally Clause is needed, therefore deeper nesting of code.


'''Recommendation''' (ChristianK): We should discourage 'Way 1' and ensure that 'Way 2' is implemented everywhere.
'''Recommendation''' (ChristianK): We should discourage 'Way 1' and ensure that 'Way 2' is implemented everywhere.
Line 22: Line 31:
** '''Way 1''': Commit before first Catch Clause + Rollback in Catch Clause(s), no Finally Clause
** '''Way 1''': Commit before first Catch Clause + Rollback in Catch Clause(s), no Finally Clause
*** ''Advantages'':  
*** ''Advantages'':  
**** No Finally Clause needed, therfore less deep nesting of code.  
**** No Finally Clause needed, therefore less deep nesting of code.  
**** No bool Variable needed (see below).
**** No bool Variable needed (see below).
*** ''Disadvantages'':  
*** ''Disadvantages'':  
**** Developers must not forget to do a Rollback in ''ANY'' Catch Branch!  
**** Developers must not forget to do a Rollback in ''ANY'' Catch Branch!  
**** Commit can be accidently forgotten about by developers, leaving an open, 'dangling' DB Transaction.
**** Commit can be accidentally forgotten about by developers, leaving an open, 'dangling' DB Transaction.
** '''Way 2''': Commit or Rollback in Finally Clause which encloses try-catch Clause, depending on value of bool Variable ('SubmissionOK'). DB Transaction is not committed before first Catch Clause and not Committed / Rolled Back in any Catch Clauses. (Example: <code>C:\openpetraorg\trunk\csharp\ICT\Petra\Server\lib\MSysMan\UserDefaults.cs, line 964</code>)
** '''Way 2''': Commit or Rollback in Finally Clause which encloses try-catch Clause, depending on value of bool Variable ('SubmissionOK'). DB Transaction is not committed before first Catch Clause and not Committed / Rolled Back in any Catch Clauses. (Example: <code>C:\openpetraorg\trunk\csharp\ICT\Petra\Server\lib\MSysMan\UserDefaults.cs, line 964</code>)
*** ''Advantage'': Commit or Rollback can never be forgotten about - this is also true for the case where a 'return' statement is executed in the 'try' code block of the try-catch-finally Clause!!!
*** ''Advantage'': Commit or Rollback can never be forgotten about - this is also true for the case where a 'return' statement is executed in the 'try' code block of the try-catch-finally Clause!!!
*** ''Disadvantages'':  
*** ''Disadvantages'':  
**** Finally Clause is needed, therfore deeper nesting of code.
**** Finally Clause is needed, therefore deeper nesting of code.
**** bool Variable needed.
**** bool Variable needed.


Line 36: Line 45:




===DB Transaction Handling===
===DB Transaction Handling When DB Transactions Span Several Methods ===
'''TODO'''
 
* handling of long running DB Transaction over many Method Calls ('Control Method' / 'Super Structure')
* handling of long running DB Transaction over many Method Calls ('Control Method' / 'Super Structure')
** Discussion: http://sourceforge.net/apps/phpbb/openpetraorg/viewtopic.php?p=364#p364
** Discussion: http://sourceforge.net/apps/phpbb/openpetraorg/viewtopic.php?p=364#p364
*** Starting of a local DB Transaction or use of a Session Object?
*** Starting of a local DB Transaction or use of a Session Object?
*** [[Inconsistent DB Access Exception Handling#Exception Handling | Exception handling]]
*** [[DB Access Exception Handling#Exception Handling | Exception handling]]

Revision as of 10:20, 24 April 2014

Overview

The correct handling of Exceptions that occur because of database access is very important for the following reasons:

  1. DB Exception Handling must ensure that
    1. Any DB Transaction that was started in the same program scope (e.g. C# Method, or a particular 'chain' of C# Method calls) and that was used for the DB command that resulted in an Exception is rolled back (in case of write access to the DB) or committed (in case of read access to the DB). That way a DB transaction can never be left running ('dangling') where it shouldn't.
  2. DB Exceptions must not get 'swallowed'. While suppressing an Exception (by 'swallowing' it) is never a good idea and is actively discouraged, suppressing a DB Exception is especially bad as the caller of a Method that suppressed the Exception that occurred because of the DB access has no way of knowing of problems that really happened while accessing the DB and hence can not know whether the DB access succeeded, or not.


The Danger of 'Dangling DB Transactions'

DB Transactions that are left running where they shouldn't (sometimes referred to as 'dangling DB Transactions') will likely cause problems further down the execution path of the program code, or with the next program code path that is executed by the user (esp. after an Exception happened), as other program code...

  • might attempt to start a new DB Transaction, which will fail as the 'dangling DB Transaction' is still running and only one DB Transaction can be running at a given time;
  • might be written in such a way that it can re-use an existing Transaction, but ...
    • that 'piggy-backing of an existing DB Transaction' might fail if the Isolation Level that is required by that program code is higher (=more restrictive) than the Isolation Level of the running DB Transaction. Also, that program code could ask for a specific, exact Isolation Level and the the Isolation Level of the running DB Transaction is different. In both cases an Exception is raised.
    • in case the re-using of the DB Transaction doesn't fail when the DB Transaction is requested by that program code then the Isolation Level of the running DB Transaction could be higher than the Isolation Level of the DB Transaction of that program code, which will lead at least to unnecessary DB Locks taken out and can potentially lead to concurrency problems that would otherwise not happen. Both of those issues are very hard to pin-point once they happen as it is often not obvious what program code has initially taken out a DB Transaction that then causes the mentioned problems later on (esp. once the application is used by users)!


DB Exception Handling

  • Scenario 1: just reading data
    • Way 1: Commit before first Catch Clause + Commit in Catch Clause(s), no Finally Clause
      • Advantage:
        • No Finally Clause needed, therefore less deep nesting of code.
      • Disadvantage:
        • Commit can be accidentally forgotten about by developers, leaving an open, 'dangling' DB Transaction - this is also true for the case where a 'return' statement is executed in the 'try' code block of the try-catch Clause!!!
    • Way 2: Commit in Finally Clause which encloses try-catch Clause
      • Advantage: Commit can never be forgotten about - this is also true for the case where a 'return' statement is executed in the 'try' code block of the try-catch-finally Clause!!!
      • Disadvantage: Finally Clause is needed, therefore deeper nesting of code.

Recommendation (ChristianK): We should discourage 'Way 1' and ensure that 'Way 2' is implemented everywhere.


  • Scenario 2: Writing Data / mixed reading/writing of data
    • Way 1: Commit before first Catch Clause + Rollback in Catch Clause(s), no Finally Clause
      • Advantages:
        • No Finally Clause needed, therefore less deep nesting of code.
        • No bool Variable needed (see below).
      • Disadvantages:
        • Developers must not forget to do a Rollback in ANY Catch Branch!
        • Commit can be accidentally forgotten about by developers, leaving an open, 'dangling' DB Transaction.
    • Way 2: Commit or Rollback in Finally Clause which encloses try-catch Clause, depending on value of bool Variable ('SubmissionOK'). DB Transaction is not committed before first Catch Clause and not Committed / Rolled Back in any Catch Clauses. (Example: C:\openpetraorg\trunk\csharp\ICT\Petra\Server\lib\MSysMan\UserDefaults.cs, line 964)
      • Advantage: Commit or Rollback can never be forgotten about - this is also true for the case where a 'return' statement is executed in the 'try' code block of the try-catch-finally Clause!!!
      • Disadvantages:
        • Finally Clause is needed, therefore deeper nesting of code.
        • bool Variable needed.

Recommendation (ChristianK): We should discourage 'Way 1' and ensure that 'Way 2' is implemented everywhere.


DB Transaction Handling When DB Transactions Span Several Methods

TODO