Re: [sqlite] PATCH ATTACHED: Re: sqlite assert failure in 2.8.15

2004-11-10 Thread Derrell . Lipman
"D. Richard Hipp" <[EMAIL PROTECTED]> writes:

> [EMAIL PROTECTED] wrote:
>> This presented itself with the following scenario:
>>  - sqlite_open() some unimportant database file
>>  - set PRAGMA SYNCHRONOUS=OFF
>>  - do some things that cause pPg->needSync to be true for some set of pages.
>>this will also have set pPager->needSync to be true.
>>  - ATTACH some more important database file
>>  - set PRAGMA SYNCHRONOUS=ON because now data integrity is important
>> When sqlitepager_commit() is later called, it will fail the assert because
>> sqlitepager_set_cachesize() which re-enabled synchronous mode, reset the 
>> pager
>> structure's needSync (pPager->needSync = 0) but did not actually flush the
>> pages to the journal (i.e. pPager->pAll->needSync is still 1).
>>
>
> Thanks for tracking this down.
>
> I have not analyzed this situation closely.  But I wonder if a
> simpler (and safer) solution to this problem would be to disallow
> the changing of PRAGMA SYNCHRONOUS in the middle of a transaction?
> Am I correct in understanding that the problem only occurs if
> you do something like this:
>
>  PRAGMA synchronous=off;
>  BEGIN;
>  -- do some things
>  PRAGMA synchronous=on;
>  -- possibly do other things
>  COMMIT;
>
> Changing the synchronous setting in the middle of a transaction
> seems dubious and risky to me.  Your patch may well fix the immediate
> problem but I worry that there may be other undetected problems that
> remain unresolved.  Disallowing a synchronous change in the middle
> of a transaction seems to be both safer and easier.  Or am I not
> understanding the problem correctly?

There were no explicit BEGIN/COMMIT statements happening, so I do not believe
the alternate solution would solve the problem in my case.  I did not
determine how/where the pPager->pAll chain of needSync flags got set, but the
queries that were executing between the two PRAGMA statements were querying
the sqlite_master table to determine if various tables existed, and CREATE
TABLE statements to create those that didn't.  (Maybe CREATE TABLE statements
aren't being commited at the appropriate time?)

My initial explanation was slightly incorrect.  Turning synchronous mode back
on occurred *before* the ATTACH statement, so the pPager->pAll chain of
needSync flags were already turned on before that occurred.

It does seem reasonable, I think, that if you decide you want synchronous
mode, and issue "PRAGMA SYNCHRONOUS=ON", that any pages currently in cache
should be flushed.  You indicate that this solution may not be entirely safe.
I'm interested what potential problems it could entail?  (Or is it just that
changing synchronous mode within an explicit transaction is unsafe?  That I
would certainly agree with!)

Cheers,

Derrell


Re: [sqlite] PATCH ATTACHED: Re: sqlite assert failure in 2.8.15

2004-11-09 Thread D. Richard Hipp
[EMAIL PROTECTED] wrote:
This presented itself with the following scenario:
 - sqlite_open() some unimportant database file
 - set PRAGMA SYNCHRONOUS=OFF
 - do some things that cause pPg->needSync to be true for some set of pages.
   this will also have set pPager->needSync to be true.
 - ATTACH some more important database file
 - set PRAGMA SYNCHRONOUS=ON because now data integrity is important
When sqlitepager_commit() is later called, it will fail the assert because
sqlitepager_set_cachesize() which re-enabled synchronous mode, reset the pager
structure's needSync (pPager->needSync = 0) but did not actually flush the
pages to the journal (i.e. pPager->pAll->needSync is still 1).
Thanks for tracking this down.
I have not analyzed this situation closely.  But I wonder if a
simpler (and safer) solution to this problem would be to disallow
the changing of PRAGMA SYNCHRONOUS in the middle of a transaction?
Am I correct in understanding that the problem only occurs if
you do something like this:
 PRAGMA synchronous=off;
 BEGIN;
 -- do some things
 PRAGMA synchronous=on;
 -- possibly do other things
 COMMIT;
Changing the synchronous setting in the middle of a transaction
seems dubious and risky to me.  Your patch may well fix the immediate
problem but I worry that there may be other undetected problems that
remain unresolved.  Disallowing a synchronous change in the middle
of a transaction seems to be both safer and easier.  Or am I not
understanding the problem correctly?
--
D. Richard Hipp -- [EMAIL PROTECTED] -- 704.948.4565