Avoiding Database Deadlocks

In a high-volume scenario, repeated access to the database could cause a database deadlock, where a table becomes unavailable. This could in turn cause the system to become stuck and for critical functions to fail.

The actual occurrence of database deadlocks will depend somewhat on the database itself. However, there are good general practices that you could follow to reduce your risk. To minimize the risk of deadlocks, Reducing SQL Server Deadlocks recommended the following:

Keep transactions as short as possible. One way to help accomplish this 
is to reduce the number of round trips between your application and SQL 
Server by using stored procedures or keeping transactions with a single 
batch.  Another way of reducing the time a transaction takes to complete 
is to make sure you are not performing the same reads over and over 
again. If your application does need to read the same data more than 
once, cache it by storing it in a variable or an array, and then 
re-reading it from there, not from SQL Server.

In opentaps, we recommend that you follow these best practices when writing more complex business logic:

• Do not rely on the automatic transaction management of the ofbiz service engine. If you have business logic which requires repeated access to the database, both to retrieve and store data, turn off transactions at the service level by using the

  use-transaction="false"

flag in the service XML definition. Manage the transaction inside of your Java code.

• As a general rule, if you find yourself needing to set a longer transaction timeout for your service, such as by using the

  transaction-timeout="600"

parameter in the service XML definition, then your service should be rewritten to avoid potential deadlocks.

• Put all of your database access code together, especially writes to the database, and put them inside a transaction block.
• Your code should be structured as much as possible to follow this three-part pattern: get data, process data, store data. A transaction should only be opened in the part of the code where you're actually storing data.

For example, this hypothetical service has a greater risk of database deadlocks:

  <!-- services XML definition -->
  <service name="myService" engine="java" path="org.opentaps.MyServices" invoke="myService"/>

  // inside of myService
  List orderHeaders = delegator.findByAnd("OrderHeader", UtilMisc.toMap("statusId", "ORDER_APPROVED"));
  for (GenericValue orderHeader: orderHeaders) {
     List orderItems = orderHeader.getRelatedByAnd("OrderItem", UtilMisc.toMap("statusId", "ITEM_APPROVED"));
   
     for (GenericValue orderItem: orderItems) {  
         delegator.create(myEntity, myMapOfValues);
         // or
         Map tmpResult = dispatcher.runSync("someOrderRelatedService", paramterMapValues);
     }
  }

Essentially, this code is going through the database again and again to read and write data. What if you had 1,000 approved orders with an average of 10 items per order? You would be doing 1,000 SELECT queries and nesting inside of each SELECT 10 INSERT queries. This is very risky and will probably lead to a deadlock, especially of several threads start trying to run the same service and do all those selects and inserts around the same time.

A better way to do it would be like this (remember this is just an example and is not meant to run in real life):

  <!-- services XML definition -->
  <service name="myService" engine="java" path="org.opentaps.MyServices" invoke="myService" use-transaction="false"/>

  // inside of myService

  //  group all your select queries together
  List orderHeaders = delegator.findByAnd("OrderHeader", UtilMisc.toMap("statusId", "ORDER_APPROVED"));
  List orderIds = EntityUtil.getFieldFromEntityList(orderHeaders, "orderId", true);  // get List of distinct orderIds
  List orderItems = delegator.findByAnd("OrderItem", UtilMisc.toList(
               new EntityCondition("orderId", EntityOperator.IN, orderIds),
               new EntityCondition("statusId", EntityOperator.EQUALS, "ITEM_APPROVED")));

  // make a list of values to store
  List valuesToCreate = new LinkedList();  
  for (GenericValue orderItem: orderItems) {  
        // do something
        GenericValue newValue = delegator.makeValue(myEntity, myMapOfValues);
        valuesToCreate.add(newValue);
     }
  }

  // one transaction to store all your values
  TransactionUtil.begin();
  delegator.storeAll(valuesToCreate);
  TransactionUtil.commit();