Any other opinions on this? Shall I commit?
Regards,
--
Alejandro Guerrieri
[EMAIL PROTECTED]
El 20/11/2008, a las 02:51 p.m., Alexander Malysh escribió:
Alejandro Guerrieri schrieb:
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.
ahh now it's clear to me and I'm +1 to commit your patch.
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
