I tested this patch with 1.2.x, but I'd like people to check if all is
cool before it gets committed to any branch/trunk. Essentially, we're
not assuming any more that the sqlite3_step() call is always going to be
successful - this call can return SQLITE_BUSY just the same as the one
inside the dbd_sqlite3_select() function.

The gain from this patch was rather obvious in my testing, where I
hammered Apache at concurrency level 10 with requests (using ab) and
with APU DBD doing UPDATE to a single SQLite3 database. With this patch,
I had 0 failed queries out of 1000 requests. Without it, I'd get
anywhere between 750 and 1000 (!) failed queries out of 1000 requests in
this high contention scenario.

-- 
Bojan
Index: apr_dbd_sqlite3.c
===================================================================
--- apr_dbd_sqlite3.c	(revision 394609)
+++ apr_dbd_sqlite3.c	(working copy)
@@ -253,12 +253,29 @@
     apr_dbd_mutex_lock();
 
     do {
+        int retry_count = 0;
+
         ret = sqlite3_prepare(sql->conn, query, length, &stmt, &tail);
         if (ret != SQLITE_OK) {
             sqlite3_finalize(stmt);
             break;
         }
-        ret = sqlite3_step(stmt);
+
+        while(retry_count++ <= MAX_RETRY_COUNT) {
+            ret = sqlite3_step(stmt);
+            if (dbd_sqlite3_is_success(ret))
+                break;
+
+            if (ret == SQLITE_BUSY){
+                apr_thread_mutex_unlock(sql->mutex);
+                apr_sleep(MAX_RETRY_SLEEP);
+                apr_thread_mutex_lock(sql->mutex);
+                continue;
+            } else {
+                break;
+            }
+        }
+
         *nrows = sqlite3_changes(sql->conn);
         sqlite3_finalize(stmt);
         length -= (tail - query);

Reply via email to