Hi,

As I've learnt more about how to use SQLite (mainly from looking at this 
mailing list), I've realised I've coded somethings in a <cough> less than an 
optimal manner <cough>.

One of the things I've realised is that SQLite has a timing system built into 
accessing the database, so that if things are busy it will back off for a while 
and then retry for a certain amount of milliseconds (microseconds?) and then 
fail with SQLITE_IOERR_BLOCKED or SQLITE_BUSY.

Of course I realised this after I'd written my own system to do exactly the 
same thing. 

I'm not a big fan of reinventing the wheel and, to be honest, suspect (know?) 
that the writers of SQLite understand their system far, far better than I can, 
so want to do-the-right-thing and use sqlite3_busy_timeout as its supposed to 
be used rather than me mimicking it badly.

I've read through the on-line documentation and can see more or less how it 
works, but I want to check the pattern usage of this to see if it follows best 
practise, or failing that, somebody to comment on if this looks appropriate. 
Just to be clear I'm not asking anybody to write my code for me, and I have 
looked through the manual and as many examples as I can find of this function 
to see how its used. 

Here's how I used to do things using the code I used to close a database. This 
is a simple code example, the code I have for executing SQL is far longer but I 
don't feel it offers anything more to the discussion.

int DB_CloseDatabase(DB_p db)
{
    int rc;
    int do_loop = 1;
    int timer_interval = SQL_TIMER_RETRY_INTERVAL;
    int no_attempts = SQL_MAX_NO_ATTEMPTS;
    
    while (do_loop && no_attempts > 0)
    {
        rc = sqlite3_close(db);
        
        switch (rc)
        {
            case SQLITE_OK:
                do_loop = 0;
                break;
            case SQLITE_BUSY:
                if (debug) printf("1. Sleeping for %d\n" , timer_interval);
                
                USleep(timer_interval);
                timer_interval += timer_interval;
                no_attempts--;
                break;
            default:
            {                
                char error_string[EXCEPTION_MAX_STRING];                
                Exception_t exception;
                
                SetException(exception , rc , 
DB_ConstructErrorString(error_string , EXCEPTION_MAX_STRING , "Close 
Database%s:'" , (char*) sqlite3_errmsg(db) , "Close Database1));
                Throw(exception);
                break;
            }
        }
    }
    
    if (no_attempts <= 0)
    {
        char error_string[EXCEPTION_MAX_STRING];
        Exception_t exception;
        
        SetException(exception , rc , DB_ConstructErrorString(error_string , 
EXCEPTION_MAX_STRING , "Close Database%s:'" , (char*) sqlite3_errmsg(db) , 
"Close Database2"));
        Throw(exception);
    }
    
    return 0;
}

My intention with this is to try and close the database, if it fails the first 
time due to SQLITE_BUSY being returned, then I would sleep for a certain amount 
of time determined by timer_interval (in uSecs), I would then double the 
time_interval so that the next time it would back off for even longer, try 
again for SQL_MAX_NO_ATTEMPTS  and if that all failed, I would eventually drop 
into an Exception handling routine. This seems to work OK and I have managed to 
simulate a locked database, I can see the time_interval being incremented.

The logic in this is based on Ethernet packet handling (or how I remember it 
used to) when packets collided (without the randomisation of the back off).

Here's my rewrite of the simple function above:

int DB_CloseDatabase(DB_p db)
{
    int rc;
    int do_loop = 1;
    int no_attempts = SQL_MAX_NO_ATTEMPTS;
    
    sqlite3_busy_timeout(db , SQL_TIMER_RETRY_INTERVAL);
    
    while (do_loop && no_attempts > 0)
    {
        rc = sqlite3_close(db);
        
        switch (rc)
        {
            case SQLITE_OK:
                do_loop = 0;
                break;
            case SQLITE_IOERR_BLOCKED:
            case SQLITE_BUSY:
                no_attempts--;
                break;
            default:
            {
                char error_string[EXCEPTION_MAX_STRING];
                Exception_t exception;
                
                // Reset the database timeout to 0
                sqlite3_busy_timeout(db , 0);
                SetException(exception , rc , 
DB_ConstructErrorString(error_string , EXCEPTION_MAX_STRING , "Close 
Database%s:'" , (char*) sqlite3_errmsg(db) , "Close Database"));
                Throw(exception);
                break;
            }
        }
    }
    
    // Reset the database timeout to 0
    sqlite3_busy_timeout(db , 0);
    
    if (no_attempts <= 0)
    {
        char error_string[EXCEPTION_MAX_STRING];
        Exception_t exception;
        
        SetException(exception , rc , DB_ConstructErrorString(error_string , 
EXCEPTION_MAX_STRING , "Close Database%s:'" , (char*) sqlite3_errmsg(db) , 
"Close Database2"));
        Throw(exception);
    }

    return 0;
}

I have added in SQLITE_IOERR_BLOCKED  to the switch above as this was mentioned 
in the on-line documentation. The basic logic of the new DB_Close function is 
the same as above, it tries to close the database, the SQLite function fails, 
as it now has a sqlite3_busy_timeoutset it should keep retrying for 
SQL_TIMER_RETRY_INTERVAL.  If that fails it will try SQL_MAX_NO_ATTEMPTS to 
really, really make sure and then will throw an exception. 

I have a feeling in the pit of my stomach that this is over complicated and 
doesn't really follow the way SQLite should work, This is based on nothing more 
than looking through what code I can see and a 'funny feeling' . Should I keep 
retrying to close the database this way or should I increase the 
SQL_TIMER_RETRY_INTERVAL so that its significantly longer, this means it's 
doing much the same as my multiple loops. Once it's completed I simply check 
the result code and throw an exception or we have success..

Here's the 3rd pattern:

int DB_CloseDatabase(DB_p db)
{
    // Make the timeout longer, as long as the maximum timeout before
    sqlite3_busy_timeout(db , SQL_TIMER_RETRY_INTERVAL * SQL_MAX_NO_ATTEMPTS);
    
    int rc = sqlite3_close(db);
    
    switch (rc)
    {
        case SQLITE_OK:
            // Left in for completeness
            break;
        case SQLITE_IOERR_BLOCKED:
        case SQLITE_BUSY:
        default:
        {
            char error_string[EXCEPTION_MAX_STRING];
            Exception_t exception;
            
            // Reset the database timeout to 0
            sqlite3_busy_timeout(db , 0);
            SetException(exception , rc , DB_ConstructErrorString(error_string 
, EXCEPTION_MAX_STRING , "Close Database%s:'" , (char*) sqlite3_errmsg(db) , 
"Close Database"));
            Throw(exception);
            break;
        }
    }
    
    // Reset the database timeout to 0
    sqlite3_busy_timeout(db , 0);
    
    return 0;
}

I'd welcome any comments and views on this. As I said before I'm not looking 
for you to write my code, just to comment on the pattern it uses. I'm pretty 
thick skinned so can take bad news, but be gentle kind reader:)

Thanks for reading this far,

Rob
_______________________________________________
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Reply via email to