Terry
Terry

Reputation: 103

Having TRANSACTION In All Queries

Do you think always having a transaction around the SQL statements in a stored procedure is a good practice? I'm just about to optimize this legacy application in my company, and one thing I found is that every stored procedure has BEGIN TRANSACTION. Even a procedure with a single select or update statement has one. I thought it would be nice to have BEGIN TRANSACTION if performing multiple actions, but not just one action. I may be wrong, which is why I need someone else to advise me. Thanks for your time, guys.

Upvotes: 6

Views: 2724

Answers (8)

Martin Smith
Martin Smith

Reputation: 453426

I don't know of any benefit of not just using auto commit transactions for these statements.

Possible disadvantages of using explicit transactions everywhere might be that it just adds clutter to the code and so makes it less easy to see when an explicit transaction is being used to ensure correctness over multiple statements.

Additionally it increases the risk that a transaction is left open holding locks unless care is taken (e.g. with SET XACT_ABORT ON).

Also there is a minor performance implication as shown in @8kb's answer. This illustrates it another way using the visual studio profiler.

Setup

(Testing against an empty table)

CREATE TABLE T (X INT)

Explicit

SET NOCOUNT ON

DECLARE @X INT

WHILE ( 1 = 1 )
  BEGIN
      BEGIN TRAN

      SELECT @X = X
      FROM   T

      COMMIT
  END

Explicit

Auto Commit

SET NOCOUNT ON

DECLARE @X INT

WHILE ( 1 = 1 )
  BEGIN
      SELECT @X = X
      FROM   T
  END 

Auto Commit

Both of them end up spending time in CMsqlXactImp::Begin and CMsqlXactImp::Commit but for the explicit transactions case it spends a significantly greater proportion of the execution time in these methods and hence less time doing useful work.

+--------------------------------+----------+----------+
|                                | Auto     | Explicit |
+--------------------------------+----------+----------+
| CXStmtQuery::ErsqExecuteQuery  | 35.16%   | 25.06%   |
| CXStmtQuery::XretSchemaChanged | 20.71%   | 14.89%   |
| CMsqlXactImp::Begin            | 5.06%    | 13%      |
| CMsqlXactImp::Commit           | 12.41%   | 24.03%   |
+--------------------------------+----------+----------+

Upvotes: 3

8kb
8kb

Reputation: 11406

You mentioned that you'll be optimizing this legacy app.

One of the first, and easiest, things you can do to improve performance is remove all the BEGIN TRAN and COMMIT TRAN for the stored procedures that only do SELECTs.

Here is a simple test to demonstrate:

/* Compare basic SELECT times with and without a transaction */

DECLARE @date DATETIME2
DECLARE @noTran INT
DECLARE @withTran INT

SET @noTran = 0
SET @withTran = 0

DECLARE @t TABLE (ColA INT)
INSERT @t VALUES (1)

DECLARE 
  @count INT,
  @value INT

SET @count = 1

WHILE @count < 1000000
BEGIN

  SET @date = GETDATE()
  SELECT @value = ColA FROM @t WHERE ColA = 1
  SET @noTran = @noTran + DATEDIFF(MICROSECOND, @date, GETDATE())

  SET @date = GETDATE()
  BEGIN TRAN
  SELECT @value = ColA FROM @t WHERE ColA = 1
  COMMIT TRAN
  SET @withTran = @withTran + DATEDIFF(MICROSECOND, @date, GETDATE())

  SET @count = @count + 1
END

SELECT 
  @noTran / 1000000. AS Seconds_NoTransaction, 
  @withTran / 1000000. AS Seconds_WithTransaction

/** Results **/

Seconds_NoTransaction                   Seconds_WithTransaction
--------------------------------------- ---------------------------------------
14.23600000                             18.08300000

You can see there is a definite overhead associated with the transactions.

Note: this is assuming your these stored procedures are not using any special isolation levels or locking hints (for something like handling pessimistic concurrency). In that case, obvously you would want to keep them.

So to answer the question, I would only leave in the transactions where you are actually attempting to preserve the integrity of the data modifications in case of an error in the code, SQL Server, or the hardware.

Upvotes: 2

gbn
gbn

Reputation: 432331

One plus point is you can add another INSERT (for example) and it's already safe.

Then again, you also have the problem of nested transactions if a stored procedure calls another one. An inner rollback will cause error 266.

If every call is simple CRUD with no nesting then it's pointless: but if you nest or have multiple writes pre TXN then it's good to have a consistent template.

Upvotes: 2

Registered User
Registered User

Reputation: 8395

BEGIN TRANSACTION / COMMIT syntax shouldn't be used in every stored procedure by default unless you are trying to cover the following scenarios:

  1. You include the WITH MARK option because you want to support restoring the database from a backup to a specific point in time.

  2. You intend to port the code from SQL Server to another database platform like Oracle. Oracle does not commit transactions by default.

Upvotes: 1

Sharique
Sharique

Reputation: 4219

When performing multiple insert/update/delete, it is better to have a transaction to insure atomicity on operation and it insure that all the tasks of operation are executed or none.

For single insert/update/delete statement, it depends upon what kind of operation (from business layer perspective) you are performing and how important it is. If you are performing some calculation before single insert/update/delete, then better use transaction, may be some data changed after you retrieve data for insert/update/delete.

Upvotes: 2

Naveed Butt
Naveed Butt

Reputation: 2901

I can only say that placing a transaction block like this to every stored procedure might be a novice's work.

A transaction should be placed only in a block that has more than one insert/update statements, other than that, there is no need to place a transaction block in the stored procedure.

Upvotes: 1

Panagiotis Kanavos
Panagiotis Kanavos

Reputation: 131512

It is entirely unnecessary as each SQL statement executes atomically, ie. as if it were already running in its own transaction. In fact, opening unnecessary transactions can lead to increased locking, even deadlocks. Forgetting to match COMMITs with BEGINs can leave a transaction open for as long as the connection to the database is open and interfere with other transactions in the same connection.

Such coding almost certainly means that whoever wrote the code was not very experienced in database programming and is a sure smell that there may be other problems as well.

Upvotes: 5

Donnie
Donnie

Reputation: 46923

The only possible reason I could see for this is if you have the possibility of needing to roll-back the transaction for a reason other than a SQL failure.

However, if the code is literally

begin transaction
statement
commit

Then I see absolutely no reason to use an explicit transaction, and it's probably being done because it's always been done that way.

Upvotes: 4

Related Questions