Re: [sqlite] Locking in 3.0.5
At 6:06 PM -0400 9/1/04, D. Richard Hipp wrote: I fear that your patch has been overcome by events. A subtle bug has been uncovered in another area of locking which is going to require reworking large sections of the commit/rollback logic. It is very doubtful that your patch will survive this rework. I am assuming, then, that the next release of SQLite will also officially be beta status, even though the web site says the current one is probably the last. Staying beta is really the only option when making such a large change, as I see it. That is, unless 'beta' simply means that the API is not frozen yet. -- Darren Duncan
[sqlite] PATCH Re: [sqlite] Locking in 3.0.5
On Wed, 1 Sep 2004, D. Richard Hipp wrote:
>Christian Smith wrote:
>> Created ticket #884, with patch against latest cvs:
>> http://www.sqlite.org/cvstrac/tktview?tn=884
>>
>
>I fear that your patch has been overcome by events.
>A subtle bug has been uncovered in another area
>of locking which is going to require reworking large
>sections of the commit/rollback logic. It is very
>doubtful that your patch will survive this rework.
No problem. As I said, it was mainly a RFC, for such things as the SQL
syntax. The implementation was broken anyway, just enough to get the
desired semantics in a single session. I was hoping it *wouldn't* go into
the release code as is:)
My patch was mainly at the parser/vdbe level. I've not got in to the
details of the btree code or where I could plug such read only transaction
functionality in, so I hacked it in around the vdbe. Hence my comments
earlier about it being broken.
Could more robust read only transaction support be done as part of this
work? In the meantime, I've attached the final patch I'll do for now, for
people to play with, as well as updated the now closed ticket 884.
>
>
>
Christian
--
/"\
\ /ASCII RIBBON CAMPAIGN - AGAINST HTML MAIL
X - AGAINST MS ATTACHMENTS
/ \Index: src/build.c
===
RCS file: /sqlite/sqlite/src/build.c,v
retrieving revision 1.250
diff -c -p -r1.250 build.c
*** src/build.c 31 Aug 2004 13:45:11 - 1.250
--- src/build.c 1 Sep 2004 22:49:59 -
*** void sqlite3FinishCoding(Parse *pParse){
*** 74,81
int iDb;
sqlite3VdbeChangeP2(v, pParse->cookieGoto-1, sqlite3VdbeCurrentAddr(v));
for(iDb=0, mask=1; iDbnDb; mask<<=1, iDb++){
if( (mask & pParse->cookieMask)==0 ) continue;
! sqlite3VdbeAddOp(v, OP_Transaction, iDb, (mask & pParse->writeMask)!=0);
sqlite3VdbeAddOp(v, OP_VerifyCookie, iDb, pParse->cookieValue[iDb]);
}
sqlite3VdbeAddOp(v, OP_Goto, 0, pParse->cookieGoto);
--- 74,83
int iDb;
sqlite3VdbeChangeP2(v, pParse->cookieGoto-1, sqlite3VdbeCurrentAddr(v));
for(iDb=0, mask=1; iDbnDb; mask<<=1, iDb++){
+ int wr;
if( (mask & pParse->cookieMask)==0 ) continue;
! wr = ( (mask & pParse->writeMask)!=0 && 0==db->roTrans );
! sqlite3VdbeAddOp(v, OP_Transaction, iDb, wr);
sqlite3VdbeAddOp(v, OP_VerifyCookie, iDb, pParse->cookieValue[iDb]);
}
sqlite3VdbeAddOp(v, OP_Goto, 0, pParse->cookieGoto);
*** void sqlite3BeginTransaction(Parse *pPar
*** 2418,2423
--- 2420,2441
}
/*
+ ** Begin a read only transaction
+ */
+ void sqlite3BeginReadOnlyTransaction(Parse *pParse){
+ sqlite *db;
+ Vdbe *v;
+
+ if( pParse==0 || (db=pParse->db)==0 || db->aDb[0].pBt==0 ) return;
+ if( pParse->nErr || sqlite3_malloc_failed ) return;
+ if( sqlite3AuthCheck(pParse, SQLITE_TRANSACTION, "BEGIN", 0, 0) ) return;
+
+ v = sqlite3GetVdbe(pParse);
+ if( !v ) return;
+ sqlite3VdbeAddOp(v, OP_AutoCommit, -1, 0);
+ }
+
+ /*
** Commit a transaction
*/
void sqlite3CommitTransaction(Parse *pParse){
Index: src/parse.y
===
RCS file: /sqlite/sqlite/src/parse.y,v
retrieving revision 1.135
diff -c -p -r1.135 parse.y
*** src/parse.y 25 Aug 2004 04:07:02 - 1.135
--- src/parse.y 1 Sep 2004 22:49:59 -
*** explain ::= . { sqlite3BeginPa
*** 82,87
--- 82,88
//
cmd ::= BEGIN trans_opt. {sqlite3BeginTransaction(pParse);}
+ cmd ::= BEGIN trans_opt FOR READ ONLY. {sqlite3BeginReadOnlyTransaction(pParse);}
trans_opt ::= .
trans_opt ::= TRANSACTION.
trans_opt ::= TRANSACTION nm.
Index: src/sqliteInt.h
===
RCS file: /sqlite/sqlite/src/sqliteInt.h,v
retrieving revision 1.318
diff -c -p -r1.318 sqliteInt.h
*** src/sqliteInt.h 1 Sep 2004 03:06:35 - 1.318
--- src/sqliteInt.h 1 Sep 2004 22:49:59 -
*** struct sqlite {
*** 389,394
--- 389,395
int errCode; /* Most recent error code (SQLITE_*) */
u8 enc; /* Text encoding for this database. */
u8 autoCommit;/* The auto-commit flag. */
+ u8 roTrans; /* Read-only transaction in progress */
void(*xCollNeeded)(void*,sqlite3*,int eTextRep,const char*);
void(*xCollNeeded16)(void*,sqlite3*,int eTextRep,const void*);
void *pCollNeededArg;
*** void sqlite3Randomness(int, void*);
*** 1283,1288
--- 1284,1290
void sqlite3RollbackAll(sqlite*);
void sqlite3CodeVerifySchema(Parse*, int);
void sqlite3BeginTransaction(Parse*);
+ void sqlite3BeginReadOnlyTransaction(Parse*);
void sqlite3CommitTransaction(Parse*);
void sqlite3RollbackTransaction(Parse*);
int
Re: [sqlite] Locking in 3.0.5
Created ticket #884, with patch against latest cvs: http://www.sqlite.org/cvstrac/tktview?tn=884 Minimal testing done, tested only within sqlite3 shell on Linux. A bit of a hack. Mainly a RFC. Quick transcript: [EMAIL PROTECTED] sqlite]$ ./sqlite3 db SQLite version 3.0.5 Enter ".help" for instructions sqlite> begin for read only; sqlite> create table t (a,b); SQL error: access permission denied sqlite> commit; sqlite> begin ; sqlite> create table t (a,b); sqlite> insert into t values (1,2); sqlite> commit; sqlite> .dump BEGIN TRANSACTION; CREATE TABLE t (a,b); INSERT INTO "t" VALUES(1, 2); COMMIT; sqlite> Already found first bug:( "DELETE FROM ;" doesn't use OpenWrite, which I updated to enforce the read only nature of a transaction. I'll need to update OP_Clear and OP_Destroy as well. This could get messy quickly, hence the implementation is considered a hack:) Copyright disclaimed, etc. Christian On Wed, 1 Sep 2004, Christian Smith wrote: >OK, how about this suggestion. > >Add a new "BEGIN [TRANSACTION] FOR READONLY" statement, which begins the >transaction with a read lock only and doesn't allow the transaction to >even try to promote to a write lock. > >Then, the default behaviour for (default) non-readonly transactions is the >same as 2.8.x, ie. only one writer. The second thread beginning a new >writeable transaction blocks immediately. The benefits of concurrent >readers will not be lost if using auto-transactions or the new read-only >transactions. > >No rollback. Consistent busy behaviour. Job's a good'un. > >Christian > >On Wed, 1 Sep 2004, Miguel Angel Latorre wrote: > >>I agree. >>Applications shouldn't worry about locking and timeouts. That would be the >>job for the db core engine, returning as few (none ideally) busy status code >>as possible. >> >>Also, it is not always possible to decide which thread is better to rollback >>since no info is provided about the status of what it is performing (commit, >>update, etc). >> >>----- Original Message - >>From: "Rob Groves" <[EMAIL PROTECTED]> >>To: <[EMAIL PROTECTED]> >>Sent: Wednesday, September 01, 2004 12:34 AM >>Subject: RE: [sqlite] Locking in 3.0.5 >> >> >>> >>So, Rob, are you go to tell us if you think the change >>> >>is an improvement or not? >>> >>> It seems that with either of the new schemes, when using >>> sqlite3_busy_timeout() one thread is going to timeout sooner >>> or later. That being the case I prefer the new version on >>> efficiency grounds. >>> >>> Being a lazy programmer, I like the behaviour of 2.8.15 >>> where both threads can get to complete their update, timeouts >>> allowing. This is behaviour that I am also used to with >>> MS SQL Server. >>> >>> I agree with you that many programmers (myself included) >>> don't want to have to worry about this stuff too much >>> when using SQLite. >>> >>> Rob. >>> >> > > -- /"\ \ /ASCII RIBBON CAMPAIGN - AGAINST HTML MAIL X - AGAINST MS ATTACHMENTS / \
Re: [sqlite] Locking in 3.0.5
On Wed, 1 Sep 2004, Matt Wilson wrote: >On Wed, Sep 01, 2004 at 02:46:39PM +0100, Christian Smith wrote: >> >> Add a new "BEGIN [TRANSACTION] FOR READONLY" statement, which begins the >> transaction with a read lock only and doesn't allow the transaction to >> even try to promote to a write lock. > >Why do you need a transaction at all if you're not going to commit? > >In my code, readers never use BEGIN, only writers do. A transaction gives you a snapshot in time of the database. You may need to do more than one query, and require a consistent snapshot for the duration of the multiple queries. > >Cheers, > >Matt > -- /"\ \ /ASCII RIBBON CAMPAIGN - AGAINST HTML MAIL X - AGAINST MS ATTACHMENTS / \
Re: [sqlite] Locking in 3.0.5
OK, how about this suggestion. Add a new "BEGIN [TRANSACTION] FOR READONLY" statement, which begins the transaction with a read lock only and doesn't allow the transaction to even try to promote to a write lock. Then, the default behaviour for (default) non-readonly transactions is the same as 2.8.x, ie. only one writer. The second thread beginning a new writeable transaction blocks immediately. The benefits of concurrent readers will not be lost if using auto-transactions or the new read-only transactions. No rollback. Consistent busy behaviour. Job's a good'un. Christian On Wed, 1 Sep 2004, Miguel Angel Latorre wrote: >I agree. >Applications shouldn't worry about locking and timeouts. That would be the >job for the db core engine, returning as few (none ideally) busy status code >as possible. > >Also, it is not always possible to decide which thread is better to rollback >since no info is provided about the status of what it is performing (commit, >update, etc). > >- Original Message - >From: "Rob Groves" <[EMAIL PROTECTED]> >To: <[EMAIL PROTECTED]> >Sent: Wednesday, September 01, 2004 12:34 AM >Subject: RE: [sqlite] Locking in 3.0.5 > > >> >>So, Rob, are you go to tell us if you think the change >> >>is an improvement or not? >> >> It seems that with either of the new schemes, when using >> sqlite3_busy_timeout() one thread is going to timeout sooner >> or later. That being the case I prefer the new version on >> efficiency grounds. >> >> Being a lazy programmer, I like the behaviour of 2.8.15 >> where both threads can get to complete their update, timeouts >> allowing. This is behaviour that I am also used to with >> MS SQL Server. >> >> I agree with you that many programmers (myself included) >> don't want to have to worry about this stuff too much >> when using SQLite. >> >> Rob. >> > -- /"\ \ /ASCII RIBBON CAMPAIGN - AGAINST HTML MAIL X - AGAINST MS ATTACHMENTS / \
Re: [sqlite] Locking in 3.0.5
I agree. Applications shouldn't worry about locking and timeouts. That would be the job for the db core engine, returning as few (none ideally) busy status code as possible. Also, it is not always possible to decide which thread is better to rollback since no info is provided about the status of what it is performing (commit, update, etc). - Original Message - From: "Rob Groves" <[EMAIL PROTECTED]> To: <[EMAIL PROTECTED]> Sent: Wednesday, September 01, 2004 12:34 AM Subject: RE: [sqlite] Locking in 3.0.5 > >>So, Rob, are you go to tell us if you think the change > >>is an improvement or not? > > It seems that with either of the new schemes, when using > sqlite3_busy_timeout() one thread is going to timeout sooner > or later. That being the case I prefer the new version on > efficiency grounds. > > Being a lazy programmer, I like the behaviour of 2.8.15 > where both threads can get to complete their update, timeouts > allowing. This is behaviour that I am also used to with > MS SQL Server. > > I agree with you that many programmers (myself included) > don't want to have to worry about this stuff too much > when using SQLite. > > Rob. >
RE: [sqlite] Locking in 3.0.5
>>So, Rob, are you go to tell us if you think the change >>is an improvement or not? It seems that with either of the new schemes, when using sqlite3_busy_timeout() one thread is going to timeout sooner or later. That being the case I prefer the new version on efficiency grounds. Being a lazy programmer, I like the behaviour of 2.8.15 where both threads can get to complete their update, timeouts allowing. This is behaviour that I am also used to with MS SQL Server. I agree with you that many programmers (myself included) don't want to have to worry about this stuff too much when using SQLite. Rob.
Re: [sqlite] Locking in 3.0.5
Rob Groves wrote: I have just read the archive mailing list from 16/08/2004, and it looks like this behaviour is on purpose (checkin 1879). So, Rob, are you go to tell us if you think the change is an improvement or not? -- D. Richard Hipp -- [EMAIL PROTECTED] -- 704.948.4565
RE: [sqlite] Locking in 3.0.5
I have just read the archive mailing list from 16/08/2004, and it looks like this behaviour is on purpose (checkin 1879). My mistake, Rob. -Original Message- From: Rob Groves [mailto:[EMAIL PROTECTED] Sent: 31 August 2004 22:17 To: [EMAIL PROTECTED] Subject: [sqlite] Locking in 3.0.5 Hi, I have observed different behaviour between 3.0.3 and 3.0.5. I didn't download 3.0.4 so can't comment on that. I am using two threads and setting a busy timeout on each with sqlite3_busy_timeout(). In 3.0.3 two threads trying to update the same row(s) would both retry until the one with the shortest busy timeout expired, and SQLITE_BUSY is returned. In 3.0.5, the 2nd thread trying to obtain the lock returns SQLITE_BUSY immediately. Any ideas? Rob.
