0

I have multiple servers running the same application (distributed container). That application modifies a SQL Server table, so what it does, it checks one table for a flag (e.g. status = open), blocks some of those (there is a top(10) for performance concerns) and handles those open rows. When finished it sets the flag accordingly (highly simplified)

The issue I'm running in right now is, that randomly some server crash with

Transaction was deadlocked on lock resources with another process and has been chosen as the deadlock victim

I assume this is a SQL Server error. So my question: When starting with

using (var connection = new SqlConnection(connectionString))

And dapper, like

connection.Query("my Query");

Are all queries treated like in one SQL Transaction up until the using ends? Does this here BeginTransaction solve the problem or is everything being treated as a nested transaction?

It is really hard to test, my guess is, that this issue arises every 10th execution or so. The whole procedure requires to handle HTTP requests to another server that might last longer or shorter, so the whole procedure can take up to some hundred milliseconds or several seconds (I have no influence on that).

Update

So the app (doesn't help to show the real code) does something like

connection.Query(query1);
doOtherStuff
connection.Query(query2);
doOtherStuff
connection.Query(query3);
doOtherStuff

This other stuff is async (sometimes), however, each query can be executed isolated and does not require to be in a closed transaction.

rst
  • 2,510
  • 4
  • 21
  • 47
  • Deadlocks are not that simple unfortunately... you need to capture and inspect the deadlock graph to see where and why its happening. Long story short though, you can never fully prevent deadlocks, so you should have some form of retry in place for when they occur (as well as trying to avoid them). – Dale K Jun 02 '21 at 05:54
  • @DaleK So (at least I believe) I made all queries executed in the application in a way, that the can be treated as single isolated transactions. I don't need any two queries to be run in the same transaction. Does this change anything? – rst Jun 02 '21 at 06:06
  • Not really, I recommend reading up about deadlocks, its not a simple topic. Suffice to say that two processes acquire resources, that the other process needs, but in reverse order. i.e. process 1 takes a lock on resource 1, and then tries to take a lock on resource 2. Process 2 takes a lock on resource 2, then tries to take a lock on resource 1. No one can win, so SQL Server kills one process. Transactions affect many things relating to locks especially the duration, but transactions don't affect the order the locks are taken in. Hence you need to inspect the deadlock graph. – Dale K Jun 02 '21 at 06:10
  • All queries are either simple `SELECT` or simple `UPDATE` s and one `INSERT` into a completly different table, I don't see any deadlocks anywhere except, that all 100 Update statements or so occur in the same app and might block the table? – rst Jun 02 '21 at 06:18
  • 1
    As I said the **only** way to know what is going on is look at the deadlock graph... save yourself the guessing :) – Dale K Jun 02 '21 at 06:23
  • 1
    @rst all operations take locks that leave as long as the connection is open or, if a transaction is used, as long as the transaction is active. This isn't about SqlConnection or Dapper, that's how databases work. That's why you need to use close connections as soon as possible. Neither Dapper or SqlCommand actually start a transaction. Dapper won't close an already open connection. If your code keeps a connection open for too long, it will cause blocking or deadlocks – Panagiotis Kanavos Jun 02 '21 at 07:06
  • @rst besides, you haven't posted any real code so we can only guess. It's clear the connection stays open for too long. If you use a database table as a lock or queue and keep the connection open to block others from modifying that "lock" or "queue", you'll encounter deadlocks unless you're very careful. Queue readers are taking Shared locks but what happens when they try to update the "flag" on a row read by another reader? Blocked. And when two readers try to update a flag locked by each other, deadlock – Panagiotis Kanavos Jun 02 '21 at 07:14
  • 1
    Check [Using tables as Queues](https://rusanu.com/2010/03/26/using-tables-as-queues/), especially the phrase `Implementing a queue backed by a table is notoriously difficult, error prone and susceptible to deadlocks.` I learned that the The easy solution is the destructive read shown in that article. `DELETE .. OUTPUT` will return the deleted data, essentially acting as a `Dequeue` in a real queue – Panagiotis Kanavos Jun 02 '21 at 07:19
  • [This SO question](https://stackoverflow.com/questions/2177880/using-a-database-table-as-a-queue) asks how to use a table as a queue. Notice that no answer is complete. The accepted answer takes *exclusive long lived locks* in the `SELECT` to prevent deadlocks, with `UPDLOCK,HOLDLOCK`. One way to prevent deadlocks is to take the most restrictive locks needed from the start. Even then, unless all the involved fields are indexed, the server may lock more rows than necessary. If the server needs to scan multiple rows because eg `SomeFlag` isn't indexed, they'll be locked as well. – Panagiotis Kanavos Jun 02 '21 at 07:22
  • If the table is too large and the flag field not very selective, the server may escalate row locks to page locks, essentially "hiding" some rows from other readers even if they aren't returned. Tricky. That's why SQL Server added Message Broker as a full-featured queuing and pub/sub system. Unfortunately, it's a bit... overengineered. That's why the `DELETE OUTPUT ...` is the easiest solution to create a queue - instead of using flags, create a queue table that only holds the data that needs to be dequeued – Panagiotis Kanavos Jun 02 '21 at 07:26
  • It's impossible to diagnose a deadlock without more details, because deadlocks are highly situational. For a start, we need to see the deadlock graph, the queries that are deadlocking, the table *and index* definitions, and share the query plan via https://brentozar.com/pastetheplan. In this case the relevant client C# code would be useful also. – Charlieface Jun 02 '21 at 08:24
  • @PanagiotisKanavos [Here is a good example](https://stackoverflow.com/a/67611638/14868997) of a queue with the `OUTPUT` setup, in that case it was doing an `UPDATE` but rowlock hint is sufficient to prevent deadlocks or missing rows – Charlieface Jun 02 '21 at 08:29
  • @Charlieface I can see the bug - without READPAST other readers will be blocked. `SERIALIZABLE` is equivalent to `HOLDLOCK`. ROWLOCK can be ignored. The whole point of `OUTPUT` is that it's used with `DELETE` to do an actual dequeue without maintaining locks – Panagiotis Kanavos Jun 02 '21 at 08:38
  • @rst please post the actual SQL code at least. This has nothing to do with Dapper or SqlConnection. If you do try to use a table as a queue, you have to get the SQL code, locking hints and indexes exactly right to avoid deadlocks. If you use a SELECT without UPDLOCK, you can get deadlocks, because other readers won't be blocked from reading and locking the same rows needed by another's UPDATE – Panagiotis Kanavos Jun 02 '21 at 08:40
  • `UPDATE OUTPUT WITH (HOLDLOCK,UPDLOCK,READPAST,ROWLOCK)` *may* work as long as the fields are indexed, and the PK is returned to allow the reader to update just that row. It's still easier to use a separate `OpenQueue` table and dequeue rows with `DELETE OUTPUT` – Panagiotis Kanavos Jun 02 '21 at 08:48
  • @PanagiotisKanavos You wrote a lot and many good things. How about putting them combined in an answer and mark it as solved? – rst Jun 02 '21 at 08:50
  • 1
    @rst you still haven't verified that this is what you want, or what SQL technique you used for the "queue". I'm "guessing" from past painful experience. – Panagiotis Kanavos Jun 02 '21 at 08:52
  • The thing is, I'm not even sure it is a SQL Problem anymore. If it really is a deadlock, then all manipulations should be rolled back, but they aren't, some row's just stay half backed, so I'm stuck somewhere not knowing what really is happening. Nevertheless, you wrote many good things that could hopefully help others that might land on this question. – rst Jun 02 '21 at 18:53

1 Answers1

-3

This is really tricky, I have built EntityWorker.Core *and this issue has been handled there. I suppose that Dapper does not handle those issue.

To first understand the problem you could read this

Now after you had a look and understood why this happened, you could have a workaround this.

Here is how I would solve this, look at the code below and let me know what you think and understood.

private static object internalChangeLocker = new object();
// here you should do db changes, this should be a general method only called by external calls.
public static int Save(object someObject) {
  lock(internalChangeLocker) {
    return internalSave(someObject);
  }
}

// this method should only be called from within Save or internalSave
private static int internalSave(object someObject, Transaction tran = null) {
  var tr = tran;
  if (tran == null)
    // then create and begin a new transaction here

    // here do the db operation here
    // you can also call internalSave(newobject, tr); agen and pass on the already created transaction.


    if (tran == null) {
      /// here you should then commit and end the transaction
      // you may ask why the check, this is importend becouse you may calls 
      // internalSave(newobject, tr); within the internalSave and have to use the same old transaction
    }
}
Alen.Toma
  • 4,684
  • 2
  • 14
  • 31
  • A client lock doesn't solve any kind of database concurrency issue. It does hint at other problems in the data access code though, eg reusing connections. Opening/closing transactions doesn't solve deadlocks either. – Panagiotis Kanavos Jun 02 '21 at 06:37
  • Yes it dose, it make sure that only one operation is Done at a time despite the many operation calls that is Done at the same time. – Alen.Toma Jun 02 '21 at 06:40
  • Which means your code has threading issues. Databases are used by multiple clients and connections, not just one. Deadlocks typically occur between different *clients*, not the same application. All but the smallest web sites use *multiple* web servers, which means multiple concurrent connections. And even on a single server, each web request is served by a different thread, different controller instance, and different connection. – Panagiotis Kanavos Jun 02 '21 at 06:44
  • You want to limit deadlocks? Limit the life of *connections*. Both connections and transactions keep locks active as long as they're open. If you only read data from a long-lived connection, the Shared locks remain active and block updates. With transactions, the life of locks is limited to the transaction that started them. It's still better and *cheaper* to limit the life of a connection though – Panagiotis Kanavos Jun 02 '21 at 06:47
  • BTW that article contains many inaccuracies and just bad advice, while missing the things that really solve deadlocks. Applocks???? READ UNCOMMITED??? And no mention of snapshot isolation???? – Panagiotis Kanavos Jun 02 '21 at 06:49
  • Dont you think that i know that. But when using transaction and multithreading operation. Change to the database has to be Done one at a time. Now you you use connection and simple select operation, you dont need to think about this. Read the answer i wrote friend it semester that you do not understand the current issue. – Alen.Toma Jun 02 '21 at 06:50
  • Or I do, having worked on high volume banking and air ticket sites, and actually having to clean up the mess caused by such tricks. `use connection and simple select operation, you dont need to think about this.` Only if you work on a single-user desktop application - as in only one user in the entire company at a time. What you just said is how deadlocks occur. With multiple clients, or multiple web servers, those open connections will keep Shared locks forever. If one of the clients does tries to update on of the locked rows, it gets blocked. – Panagiotis Kanavos Jun 02 '21 at 06:56
  • And if the clients block different rows, then try to modify rows locked by each other, deadlock. That's why *all* courses, tutorials and examples use short-lived connections in `using` blocks. That's why connection pooling is used to keep the number of actual transactions to a minimum. Why optimistic concurrency is used instead of transactions, increasing throughput by several *orders* of magnitude. None of this is a new idea, it's common practice in all transactional systems – Panagiotis Kanavos Jun 02 '21 at 06:59
  • OH, and still i know that but even with this you will still have problems when using multithraded transaction. Im not talking about connection but transaction. Connection do not lock the db . So do have a better solution then mine. Please do answer him then why talk to me. – Alen.Toma Jun 02 '21 at 07:17