9

I have a C# project which writes data to a TSQL database. There are two update statements which run within a loop, eg.:

 for (int i = 0; i < customersProducts.Count; i++) {
     CustomerProducts c = customersProducts[i];

     // Update product dimensions
     for (int j = 0; j < c.Count; j++) {
         Product p = c[j];
         updateProductDimensions(p);
     }

     // ... some processing

     // Update product
     for (int j = 0; j < c.Count; j++) {
         Product p = c[j];
         updateProduct(p);
     }
 }

The updateProductDimensions() and updateProduct() both trigger SQL Update statements. There is some overlap in the columns that are updated:

string updateProductDimensions = "UPDATE products SET width = @width, height = @height, length = @length WHERE id = @id";
string updateProduct = "UPDATE products SET width = @width, height = @height, length = @length, customer_id = @customer_id, weight = @weight .... WHERE id = @id";

Example updateProductDimensions() method - updateProduct() is also similar:

public void updateProductDimensions(Product p) {
     SqlConnection connection = DBFactory.getConnection();
     string updateProductDimensions = "UPDATE products SET width = @width, height = @height, length = @length WHERE id = @id";

     try
     {
         SqlCommand sqlCmd = new sqlCmd(updateProductDimensions, connection);
         sqlCmd.Parameters.AddWithValue("@width", 20);
         sqlCmd.Parameters.AddWithValue("@height", 10);
         sqlCmd.Parameters.AddWithValue("@length", 30);
         sqlCmd.Parameters.AddWithValue("@id", p.id);
         sqlCmd.CommandType = CommandType.Text;

         sqlCmd.ExecuteNonQuery();

     }
     catch (Exception e)
     {
         // Handle exception
     }
     finally
     {
         connection.Close();
     }
}

I have run an SQL Server deadlock trace, and it shows that the updateProduct statement is failing (ie. the victim process) and the surviving process is the one running the updateProductDimensions statement.

A simplified version of the deadlock trace is as follows (with most recent process first):

 - updateProduct2: fail
 - updateProduct2: success
 - updateProduct1: success
 - updateProductDimensions4: success
 - updateProductDimensions3: success
 - updateProductDimensions2: success
 - updateProductDimensions1: success

Each line represents one product per for loop iteration being updated.

And the resource/owner list for updateProduct2:

  - owner: updateProductDimensions1 (mode = U, isolationLevel = read committed (2))
  - waiter: updateProduct2 (mode= U, requestType = wait, isolationLevel = read committed (2))

My question is, why is there a deadlock happening? Even though the two statements update the same row, it is the same table. The server communicates with multiple clients, where the clients can update only their own products - ie. a single product can only be updated by one particular client. In this way multiple DB updates are happening at the same time, but for different rows (products).

How can this be solved without removing the duplicate updated columns?


Create statement for the products table:

    CREATE TABLE Products (
        [id]               VARCHAR (255)    NOT NULL,
        [width]            INT              NOT NULL,
        [length]           INT              NOT NULL,
        [height]           INT              NOT NULL,
        [weight]           INT              NOT NULL,
        // more fields
        [customer_id]      INT                  CONSTRAINT [F_KEY_CUSTOMER] DEFAULT ((0)) NOT NULL,
        CONSTRAINT [P_KEY_PRODUCT] PRIMARY KEY CLUSTERED ([id] ASC),
        CONSTRAINT [F_KEY_CUSTOMER] FOREIGN KEY ([customer_id]) REFERENCES [dbo].[Customer] ([id])
    );

Query plans

Update product dimensions statement:

Update product dimensions statement

Update product statement:

Update product statement


Deadlock trace

    <TextData>
    <deadlock-list>
    <deadlock victim="victimProcess">
    <process-list>
    <process id="victimProcess" taskpriority="0" logused="0" waitresource="PAGE: 15:1:1259" waittime="4594" ownerId="21610772296" transactionname="UPDATE" lasttranstarted="2018-02-21T08:46:44.777" XDES="0x859b9c580" lockMode="U" schedulerid="20" kpid="34240" status="suspended" spid="64" sbid="3" ecid="3" priority="0" trancount="0" lastbatchstarted="2018-02-21T08:46:44.777" lastbatchcompleted="2018-02-21T08:46:44.777" clientapp=".Net SqlClient Data Provider" hostname="" hostpid="636" isolationlevel="read committed (2)" xactid="21610772296" currentdb="15" lockTimeout="4294967295" clientoption1="671088672" clientoption2="128056">
        <executionStack>
        <frame procname="adhoc" line="1" stmtstart="422" sqlhandle="0x02000000696bc4026d3a5eb5fc3835e32324ce9f3e4bdd28">
    UPDATE products SET width = @width, height = @height, length = @length, customer_id = @customer_id, weight = @weight WHERE id = @id     </frame>
        <frame procname="unknown" line="1" sqlhandle="0x000000000000000000000000000000000000000000000000">
    unknown     </frame>
        </executionStack>
        <inputbuf>
        </inputbuf>
    </process>
    <process id="survivorProcess4" taskpriority="0" logused="0" waitresource="PAGE: 15:1:2795" waittime="4593" ownerId="21610772296" transactionname="UPDATE" lasttranstarted="2018-02-21T08:46:44.777" XDES="0x45ebe3ca0" lockMode="U" schedulerid="18" kpid="254204" status="suspended" spid="64" sbid="3" ecid="6" priority="0" trancount="0" lastbatchstarted="2018-02-21T08:46:44.777" lastbatchcompleted="2018-02-21T08:46:44.777" clientapp=".Net SqlClient Data Provider" hostname="" hostpid="636" isolationlevel="read committed (2)" xactid="21610772296" currentdb="15" lockTimeout="4294967295" clientoption1="671088672" clientoption2="128056">
        <executionStack>
        <frame procname="adhoc" line="1" stmtstart="422" sqlhandle="0x02000000696bc4026d3a5eb5fc3835e32324ce9f3e4bdd28">
    UPDATE products SET width = @width, height = @height, length = @length, customer_id = @customer_id, weight = @weight WHERE id = @id     </frame>
        <frame procname="unknown" line="1" sqlhandle="0x000000000000000000000000000000000000000000000000">
    unknown     </frame>
        </executionStack>
        <inputbuf>
        </inputbuf>
    </process>
    <process id="survivorProcess3" taskpriority="0" logused="224" waitresource="PAGE: 15:1:2795" waittime="4527" ownerId="21610772095" transactionname="UPDATE" lasttranstarted="2018-02-21T08:46:44.680" XDES="0x859b9c300" lockMode="U" schedulerid="20" kpid="16324" status="suspended" spid="123" sbid="2" ecid="1" priority="0" trancount="0" lastbatchstarted="2018-02-21T08:46:44.680" lastbatchcompleted="2018-02-21T08:46:44.673" clientapp=".Net SqlClient Data Provider" hostname="" hostpid="636" isolationlevel="read committed (2)" xactid="21610772095" currentdb="15" lockTimeout="4294967295" clientoption1="671088672" clientoption2="128056">
        <executionStack>
        <frame procname="adhoc" line="1" stmtstart="102" sqlhandle="0x020000007e9c95155af7dd6044d8697705c48a1d5856dba4">
    UPDATE products SET width = @width, height = @height, length = @length WHERE id = @id     </frame>
        <frame procname="unknown" line="1" sqlhandle="0x000000000000000000000000000000000000000000000000">
    unknown     </frame>
        </executionStack>
        <inputbuf>
        </inputbuf>
    </process>
    <process id="survivorProcess2" taskpriority="0" logused="224" waitresource="PAGE: 15:1:1259" waittime="4529" ownerId="21610772095" transactionname="UPDATE" lasttranstarted="2018-02-21T08:46:44.680" XDES="0x270bf8b20" lockMode="U" schedulerid="13" kpid="406864" status="suspended" spid="123" sbid="2" ecid="4" priority="0" trancount="0" lastbatchstarted="2018-02-21T08:46:44.680" lastbatchcompleted="2018-02-21T08:46:44.673" clientapp=".Net SqlClient Data Provider" hostname="" hostpid="636" isolationlevel="read committed (2)" xactid="21610772095" currentdb="15" lockTimeout="4294967295" clientoption1="671088672" clientoption2="128056">
        <executionStack>
        <frame procname="adhoc" line="1" stmtstart="102" sqlhandle="0x020000007e9c95155af7dd6044d8697705c48a1d5856dba4">
    UPDATE products SET width = @width, height = @height, length = @length WHERE id = @id     </frame>
        <frame procname="unknown" line="1" sqlhandle="0x000000000000000000000000000000000000000000000000">
    unknown     </frame>
        </executionStack>
        <inputbuf>
        </inputbuf>
    </process>
    <process id="survivorProcess1" taskpriority="0" logused="10000" waittime="4315" schedulerid="17" kpid="30464" status="suspended" spid="123" sbid="2" ecid="0" priority="0" trancount="2" lastbatchstarted="2018-02-21T08:46:44.680" lastbatchcompleted="2018-02-21T08:46:44.673" clientapp=".Net SqlClient Data Provider" hostname="" hostpid="636" loginname="" isolationlevel="read committed (2)" xactid="21610772095" currentdb="15" lockTimeout="4294967295" clientoption1="671088672" clientoption2="128056">
        <executionStack>
        <frame procname="adhoc" line="1" stmtstart="102" sqlhandle="0x020000007e9c95155af7dd6044d8697705c48a1d5856dba4">
    UPDATE products SET width = @width, height = @height, length = @length WHERE id = @id     </frame>
        <frame procname="unknown" line="1" sqlhandle="0x000000000000000000000000000000000000000000000000">
    unknown     </frame>
        </executionStack>
        <inputbuf>
    (@width int,@height int,@length int,@id nvarchar(255))UPDATE products SET width = @width, height = @height, length = @length WHERE id = @id    </inputbuf>
    </process>
    </process-list>
    <resource-list>
    <pagelock fileid="1" pageid="1259" dbid="15" objectname="MyDB.dbo.Product" id="lock15a855b00" mode="U" associatedObjectId="72057594038845440">
        <owner-list>
        <owner id="survivorProcess1" mode="U" />
        </owner-list>
        <waiter-list>
        <waiter id="victimProcess" mode="U" requestType="wait" />
        </waiter-list>
    </pagelock>
    <pagelock fileid="1" pageid="2795" dbid="15" objectname="MyDB.dbo.Product" id="lockbb9f0f80" mode="U" associatedObjectId="72057594038845440">
        <owner-list>
        <owner id="survivorProcess1" mode="U" />
        </owner-list>
        <waiter-list>
        <waiter id="survivorProcess4" mode="U" requestType="wait" />
        </waiter-list>
    </pagelock>
    <pagelock fileid="1" pageid="2795" dbid="15" objectname="MyDB.dbo.Product" id="lockbb9f0f80" mode="U" associatedObjectId="72057594038845440">
        <owner-list />
        <waiter-list>
        <waiter id="survivorProcess3" mode="U" requestType="wait" />
        </waiter-list>
    </pagelock>
    <pagelock fileid="1" pageid="1259" dbid="15" objectname="MyDB.dbo.Product" id="lock15a855b00" mode="U" associatedObjectId="72057594038845440">
        <owner-list />
        <waiter-list>
        <waiter id="survivorProcess2" mode="U" requestType="wait" />
        </waiter-list>
    </pagelock>
    <exchangeEvent id="Pipe49e4ca380" WaitType="e_waitPipeGetRow" nodeId="2">
        <owner-list>
        <owner id="survivorProcess3" />
        <owner id="survivorProcess2" />
        </owner-list>
        <waiter-list>
        <waiter id="survivorProcess1" />
        </waiter-list>
    </exchangeEvent>
    </resource-list>
    </deadlock>
    </deadlock-list>
    </TextData>
user2181948
  • 1,646
  • 3
  • 33
  • 60
  • 1
    Is there only one instance of your application running? – HABO Feb 26 '18 at 02:02
  • @HABO Yes, there is only one instance. – user2181948 Feb 26 '18 at 02:06
  • You need to find out the resource they fighting for - table, row, index. I've used sql profiler - https://www.red-gate.com/simple-talk/sql/learn-sql-server/how-to-track-down-deadlocks-using-sql-server-2005-profiler/. – nikita Feb 26 '18 at 02:31
  • 3
    May I know why you're doing this? It seems like one update should be enough for me. – Erlangga Hasto Handoko Feb 26 '18 at 02:37
  • @mjwills I have modified the initial example code, added code for `updateProductDimensions` (this replaces `updateProductAttributes`), and added the isolation level (read committed (2)). – user2181948 Feb 26 '18 at 04:35
  • 3
    If deadlock happens then there _is_ some parallelism involved. – Evk Feb 26 '18 at 05:10
  • @Evk Yes, sorry for being unclear. The server is being called by multiple clients at a time. However each product can only be controlled by a single client at a time. – user2181948 Feb 26 '18 at 05:18
  • @user2181948, where is the logic that constrains each process to a single product at a time? A naive reading of your code suggests that it's operating on multiple products. Is it inconceivable that each process will, at some point, try to update the same product (or a set of products with the same keys)? – Steve Feb 26 '18 at 05:23
  • @Steve I have not included the logic, but essentially each client is assigned multiple `CustomersProducts` objects to update, and no single product can be in more than one `CustomersProducts` object. – user2181948 Feb 26 '18 at 05:30
  • Do you have a transaction around all the loops, or every loop iteration runs in it's own transaction? – Evk Feb 26 '18 at 05:39
  • @user2181948, do you have more detailed information on the locks that were being held when the deadlock occurred? That is, the scope and granularity of the lock. If row-level locks are being taken on specific Product Ids, and the separate clients do not ever touch the same Product Ids, it is hard to see how the contention arises! – Steve Feb 26 '18 at 05:45
  • @Evk Every loop iteration runs in its own transaction. – user2181948 Feb 26 '18 at 05:55
  • @mjwills Yes I have considered just retrying on deadlock, however really would like to understand why the issue is happening first as there may be a better solution (ie. prevention of the deadlock altogether) – user2181948 Feb 26 '18 at 05:55
  • 1
    @user2181948, the answer will almost certainly be found in the additional details of the conflicting locks being held. – Steve Feb 26 '18 at 05:58
  • @Steve Where would I be able to get this information? Currently the deadlock trace shows an update lock on both statements (U), read committed 2, `lasttranstarted` is the exact same timestamp on both statements down to the millisecond, same `associatedObjectId` being acted upon. – user2181948 Feb 26 '18 at 05:59
  • @user2181948, does the trace you have contain the information about *what* is being locked? For example, a key or a page? – Steve Feb 26 '18 at 06:04
  • `however really would like to understand why the issue is happening first` Because you are running two statements which are attempting to take the same lock (row lock, page lock, table lock etc). This is _likely_ due to two connections trying to update the same row at the same time (but not necessarily just that). – mjwills Feb 26 '18 at 06:05
  • @user2181948, if in your trace you have something like:`KEY: 12:12341234123412340 (1234567890a0)`, then you can run `SELECT * FROM products WHERE %%LOCKRES%% = '(1234567890a0)'` to get the row of the Product table which is responsible. This will almost certainly flush out that the same product *is* being updated by both clients. – Steve Feb 26 '18 at 06:12
  • @mjwills Interesting, I thought that since `updateProduct()` is called after `updateProductDimensions()`, and only one client instance is updating the product at a time, then there is no chance of these two operations happening at the same time? ie. one will finish, then the next will execute? – user2181948 Feb 26 '18 at 08:57
  • https://stackoverflow.com/questions/9784172/what-are-row-page-and-table-locks-and-when-they-are-acquired – mjwills Feb 26 '18 at 08:58
  • Thank you @mjwills, I'm seeing `waitresource="PAGE: XX:X:XXXX"` in the trace file, which I understand is showing a page lock occurring because `updateProductDimensions` is locking more than one row. However I still don't understand why this lock is released by the time `updateProduct` is called... – user2181948 Feb 26 '18 at 23:31
  • Thanks @mjwills - `@@LOCK_TIMEOUT` is not set (it is -1). – user2181948 Feb 27 '18 at 05:37
  • What is the best way forward in this case? My current thoughts are to add a table index, or to batch the UPDATE statements so they all execute under a single transaction (as opposed to one per for-loop iteration), or even to retry on deadlock - but which of these would be best? – user2181948 Feb 27 '18 at 21:03
  • 1
    It sounds like you have a business case for updating two different sets of columns on the same table. It would be nice to have one update per table. Since your where clause is targeting the clustered index, more indexes probably won't help. Taking a peek at the query plan for each query is generally a good move. As a fall back, adding SQL user locks can serialize requests based on the lock name you define, this serializes access which can reduce parallelism. – user3112728 Mar 05 '18 at 02:24
  • Wait, *why* is your ID column a `VARCHAR(255)`?!? Since ID is your clustered primary key, that is really undesirable. – RBarryYoung Mar 06 '18 at 19:49
  • Are there any SQL triggers on this table? Also, show us the query plan for both UPDATE statements. – RBarryYoung Mar 06 '18 at 19:50
  • @RBarryYoung Thanks for your help so far. Can you please explain why it is undesirable to have `VARCHAR(255)` for the ID column? I have added the query plans for both statements. There are no SQL triggers on the table. – user2181948 Mar 09 '18 at 01:20
  • I have also added the deadlock trace to the original question. – user2181948 Mar 09 '18 at 03:44
  • Is there a reason why you update the same columns on the same row twice for each product? From what I see the only difference between these two update statements is that the second one includes also the `customer_id` column. Why do you need the first update as well? – Zohar Peled Mar 12 '18 at 08:56
  • You can see from deadlock trace that your update statements acquire page locks, so no surprise statements to update different ids might deadlock (when those rows are in the same page). – Evk Mar 12 '18 at 13:13
  • Because a 255 byte key is really inefficient, and by making it the primary key it is by default the clustered index for the table which means that any other indexes must also use it as their reference key. – RBarryYoung Mar 12 '18 at 14:20
  • It seems that your updates introduce table scan (I see there are many `U` locks). Sql server reads rows in your table to check whether that row needs to be updated. During that process, sql server places `U` locks on the rows, in case a row needs to be updated, sql server converts `U` lock to `X` lock. It could be that your processes put locks in opposite order and run into deadlock – Khanh TO Mar 12 '18 at 15:03
  • Do you have the actual error message? From the deadlock trace, it appears to me that it is only the UpdateProducts statement deadlocking with itself. If so, then you should try adding the `OPTION(MAXDOP 1)` to it (or possibly both of them, since there may be more than one deadlocking case). This is generally a problem with having to restructure index and data pages during an update (which can be exacerbated by over-large clustered key). If so, then you might try rebuilding the table with more Fill. – RBarryYoung Mar 12 '18 at 15:04
  • Is that "some processing" thing in between involving the database as well? I wish I could change the codes to process the information first and then push it later to the database at once. Is it also possible to use a single db connection instance for the entire updating process? – Erlangga Hasto Handoko Mar 14 '18 at 09:21
  • Your query plan suggests, that you did not post the full sql statements. – sa.he Mar 17 '18 at 12:45

3 Answers3

4

The question doesn't contain enough of the scenario for me to be able to replicate the example, so I'm going to speculate.

SqlCommand is disposable; but is not in a using block, and is not being disposed, so I would suspect that the previous command is still interfering with the database when the subsequent command takes place.

Put both SqlCommands into "using" blocks; and while you're at it, remove the "finally{connection.Close();}", and also put the SqlConnection into a "using" block as well (the Dispose will do the Close).

Richardissimo
  • 5,596
  • 2
  • 18
  • 36
  • I've got a good feeling this is it, There should be a Compile Warning for anything that implements IDisposable that isn't wrapped in a `using` or calling Dispose explicitly. – Jeremy Thompson Mar 13 '18 at 00:23
  • It isn't part of the problem (yet) but I'm also going to throw in the obligatory link to [Can we stop using AddWithValue](https://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/) as a bonus thing to fix about this code. – Richardissimo Mar 16 '18 at 17:45
  • Disposing SqlCommand does not perform any DB related operation, but dispose resource in the client. You should definitely dispose (close) the connection, regarding the SqlCommand, this is what the ADO.NET guide says:"Although you can repeatedly use the same SqlCommand object to execute the same command multiple times, do not reuse the same SqlCommand object to execute different commands. " https://bytes.com/topic/c-sharp/answers/229225-should-i-dispose-sqlcommand-object#post938527 – M.Hassan Mar 23 '18 at 01:08
2

As would have determined by now, dead-locks are 'complex beasts' !

Theoretically, since you are updating a single table, there is no 2 tables involved to explain a 'classic dead-lock' scenario. So, you would expect to NOT get a dead-lock, BUT you are getting it ! Welcome to the real world :-)

Based on your deadlock trace xml, you seem to be getting dead-locks due to "Page Lock" i.e., SQL server is locking a page and your process is dead-locking on a Page (i.e., not just your record).

If you look at the resource-list section of your deadlock trace, you can see that your victim process is waiting for a page which is locked by another process.

One simple technique you can try is to use the ROWLOCK hint for your update statement and see if that helps in your scenario.

Related SO post: https://dba.stackexchange.com/questions/121610/how-to-force-sql-server-to-use-row-locking-for-specific-update-delete-statements

UPDATE Table1 WITH (ROWLOCK)
SET FirstName = 'first'
WHERE ID = 1

In the above example WITH (ROWLOCK) is your hint to SQL server to try to use row level locks

Also, some good reading about SQL server dead-locks is in this Simple Talk link

Subbu
  • 2,130
  • 1
  • 19
  • 28
1

Depending on circumstances, MSSQL server locks whole pages (multiple rows) rather than single rows. This is why a deadlock can occour even if every client accesses only it's own rows. Also I have experienced fake-deadlocks that where really timeouts of a very busy server.

1) Tell SQL Server to use row locking (instead of page locking). This might be performance-expensive.

 UPDATE products WITH (ROWLOCK) SET ...

2) Make sure to use the primary key in your where condition.

Unrelated to your deadlock problem:

3) You have nested loops where you fire single statements to MSSQL. Reduce the number of queries by building up one or two big statements. This should boost your runtime performance

4) Dispose your SqlConnection and SqlCommand.

sa.he
  • 1,391
  • 12
  • 25