Re: [PATCH] Sqlite DB Pool

Alex,

SQLite doesn't wait for the lock to free, it returns with a BUSY error  
inmediately. The query will return BUSY without waiting for the lock  
even when the process is writing to another table on the same DB.

 From my tests, I can confirm that this is what it happens.

I understand your point about "DB agnosticism", but as it is now  
sqlite is not a viable alternative for sqlbox at least. The same  
probably applies to DLR's, if more than a thread is accessing the DB  
at the same time.

As I've said before, the PHP client implementation set this value to  
60 seconds (which is a little high imho), DBD::SQlite sets it to 30  
seconds and most other client libraries I've found provides a mean to  
set this, because its default value is non-practical for most cases.

Are you fine with adding a fixed value instead of being user- 
selectable? This wouldn't change the user interface, only the  
internals when dealing with sqlite DB's.

Regards,

Alex

El 20/11/2008, a las 12:56 p.m., Alexander Malysh escribió:

> Hi Alex,
>
> thanks for explaining of the sqlite internals but it didn't answer  
> my question: why do we need in dbpool to care about some internals  
> of any DB?
>
> For kannel it's not a point when DB blocks and how oft and how  
> long... Kannel just waits for a DB to be unlocked and do it so long  
> as DB think is needed.
>
> Thanks,
> Alex
>
> Alejandro Guerrieri schrieb:
>> Alex,
>> Sqlite makes a lock on the _whole DB file_ each time it access it.  
>> That means that if an external application is doing an INSERT on  
>> send_sms, it will lock the WHOLE FILE, so sqlbox won't get a LOCK  
>> to DELETE on send_sms nor to INSERT on sent_sms.
>> If the DB is to sustain moderate load, the default won't cut it.  
>> I've experienced the error when trying to enqueue messages from  
>> Sqlite's command line utility while running sqlbox. I'm talking  
>> about sending a _single_ message on an idle sqlbox process.
>> Using the default "busy_timeout" is too low for most practical uses  
>> IMHO.
>> For example, the PHP driver sets it to 60 seconds.
>> If not allowed to change it on the DBPool, it should be set to some  
>> more realistic value at least, or maybe even set a default and  
>> allow the client to change it (I can modify the patch to accomodate  
>> either case of course).
>> Regards,
>> Alejandro Guerrieri
>> El 20/11/2008, a las 07:40 a.m., Alexander Malysh escribió:
>>> Hi Alex,
>>>
>>> why should it be necessary to define lock_timeout? I ask because  
>>> we don't allow it for any DBs from our dbpool why should we make  
>>> exception for sqlite?
>>>
>>> Thanks,
>>> Alex
>>>
>>> Alejandro Guerrieri schrieb:
>>>> Alex? Did you see this? I've already commited the updates on  
>>>> sqlbox and this would definitely improve sqlite2/3 usability.
>>>> Regards,
>>>> Alejandro
>>>> El 13/11/2008, a las 01:20 a.m., Alejandro Guerrieri escribió:
>>>>> Did anyone reviewed this again? I think it's now fixed.
>>>>>
>>>>> I'm about to commit a big update on sqlbox and it would be great  
>>>>> to be able to have sqlite2 and sqlite3 behaving properly.
>>>>>
>>>>> Regards,
>>>>>
>>>>> Alejandro Guerrieri
>>>>>
>>>>> El 08/11/2008, a las 12:17 p.m., Alejandro Guerrieri escribió:
>>>>>
>>>>>> Ups, > 0 would be better right?
>>>>>>
>>>>>> Changed the datatype to int because that's what the  
>>>>>> sqlite_busy_timeout() expects.
>>>>>>
>>>>>> [Too little coffee that night] ;)
>>>>>>
>>>>>> Fixed on the code plese re-download.
>>>>>>
>>>>>> One last note: the value must be properly initialized,  
>>>>>> otherwise the struct default could be anything. I've checked  
>>>>>> the Kannel code and since there's no dlr_sqlite.c nor any other  
>>>>>> use for this (yet), no further patching is necessary.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Alejandro Guerrieri
>>>>>>
>>>>>> El 07/11/2008, a las 01:57 p.m., Alexander Malysh escribió:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> hmm, are you sure ? ;)
>>>>>>>
>>>>>>> +    if (conf->lock_timeout != NULL) {
>>>>>>>
>>>>>>> but lock_timeout defined as long...
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Alex
>>>>>>>
>>>>>>> Alejandro Guerrieri schrieb:
>>>>>>>> Hi,
>>>>>>>> I've made this small patch that fixes a couple of things on  
>>>>>>>> the Sqlite implementation:
>>>>>>>> * Sqlite2's [sqlite-connection] group was missing from  
>>>>>>>> cfg.def, though dbpool_sqlite.c and all the hooks were in  
>>>>>>>> place. This patch adds the missing section.
>>>>>>>> * This patch adds a "lock-timeout" configuration option to  
>>>>>>>> both sqlite connection groups, where a value in milliseconds  
>>>>>>>> can be specified. That value controls the time sqlite waits  
>>>>>>>> before throwing a lock ("busy") error. This could be an issue  
>>>>>>>> because sqlite locks at the database level (yuk!), and this  
>>>>>>>> can surely be a problem under moderate load (I wouldn't  
>>>>>>>> recommend sqlite for heavy traffic at all). If not set the  
>>>>>>>> default behaviour is maintained, so nothing's broken ;)
>>>>>>>> Please check the post here:
>>>>>>>> http://www.blogalex.com/archives/50
>>>>>>>> Direct download:
>>>>>>>> http://www.blogalex.com/wp-content/uploads/2008/11/kannel-sqlite-pool-patches.diff 
>>>>>>>>  BTW, my long-term goal is to have on Sqlbox the same DB  
>>>>>>>> capabilities as Kannel have (Sqlbox only supports MySql and  
>>>>>>>> Postgres and _maybe_ MS SQL). I'm making good progress with  
>>>>>>>> sqlite so far, and this patches aim to complete the Kannel  
>>>>>>>> support to make it practical to use the DB pool with Sqlbox.
>>>>>>>> MS SQL by Ct-Lib is supposed to be working, though is not  
>>>>>>>> supported at Kannel afaik (I'll try to port the code to  
>>>>>>>> Kannel so it can be linked from gwlib as the rest does when I  
>>>>>>>> have some spare time).
>>>>>>>> Regards,
>>>>>>>> Alejandro Guerrieri
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>
>>>
>
>