SQLiteCpp icon indicating copy to clipboard operation
SQLiteCpp copied to clipboard

Provide an Exception method to check for SQLITE_BUSY

Open ncoghlan opened this issue 5 years ago • 4 comments

I'm updating an SQLiteCpp project to handle concurrent database writes from multiple processes, and am currently detecting that situation with code like:

// There's an outer retry loop to try the transaction again if we get blocked by another writer
try {
    ... // Code that writes to the DB
} catch (SQLite::Exception& e) {
    /* Suppress SQLITE_BUSY errors if write attempts remain */
    if (remainingWriteAttempts && e.getErrorCode() == SQLITE_BUSY) {
        --_remainingWriteAttempts;
        continue;
    }
    // Otherwise let the exception propagate...
throw;
}

To get access to the SQLITE_BUSY error code value, I've had to include the embedded sqlite3.h header directly, as I wasn't able to find anywhere that SQLiteCpp exports that value.

Rather than suggesting exporting the symbol directly, I'm thinking it may make more sense to expose this as a dedicated isDatabaseBusyError() query method:

try {
    ... // Code that writes to the DB
} catch (SQLite::Exception& e) {
    /* Suppress SQLITE_BUSY errors if write attempts remain */
    if (remainingWriteAttempts && e.isDatabaseBusyError()) {
        --_remainingWriteAttempts;
        continue;
    }
    // Otherwise let the exception propagate...
throw;
}

Aside from potentially exposing isTableLockedError() for SQLITE_LOCKED, it doesn't seem to me that there'd be a need to do this for every error category, but as far as I've been able to work out, handling concurrent writers requires the "check for SQLITE_BUSY and try again" logic, and I haven't been able to find an existing way to do it without bypassing the SQLiteCpp abstraction layer.

ncoghlan avatar Jun 17 '19 08:06 ncoghlan

+1

saadlu avatar Sep 18 '19 09:09 saadlu

question:

the constractor of Database does take a timeout value. Alternative to above can't we just use that value to match the remainingWriteAttempts * <whatever your default> is?

saadlu avatar Sep 19 '19 17:09 saadlu

Unfortunately not, as SQLite will sometimes kick processes out anyway in order to avoid deadlocks. When that happens, for our particular use case, we want to roll back the original transaction attempt and try again from the start.

This variant is actually our 3rd iteration on handling the write conflicts correctly, with a new thing learned each time:

  • the first version tried to rely solely on the internal database timeout, which ran into the above issue with deadlock avoidance (seen as nominal "timeouts" that were far shorter than the configured timeout period). The description in https://www.sqlite.org/c3ref/busy_handler.html is the one that I think covers this most clearly.
  • the second version didn't protect read operations, which turned out to be incomplete, as even a read operation in Write-Ahead Log mode may need to reconcile a previous failed write operation that was interrupted by an abrupt loss of power (plus it looks like dropping a TEMP table may trigger a write lock acquisition on the main DB, although I haven't fully confirmed that case yet)

Having done it this way once, my long term goal is now to split this particular database into two different files (since each affected process is actually writing to different tables), but the "if it's locked, try again" strategy covered in the SQLite docs is working pretty well in the meantime.

ncoghlan avatar Sep 20 '19 11:09 ncoghlan

thanks - good to know. For our case, I am going to increase the timeout and hope for the best. If the call fails due to lock, then be it. During a unit test it did happen and failed by test, but in production we will live with it

saadlu avatar Sep 23 '19 08:09 saadlu