Always truncate the pager cache when truncating the database file. Also reorganize the code to check the change-counter after first obtaining a shared lock. (CVS 3814)

FossilOrigin-Name: 9dc4100eff71be579480ce7939c7da712d28f0ae

manifest

@@ -1,5 +1,5 @@
-C Use\sthe\sMEMDB\smacro\sinstead\sof\sOMIT_MEMORYDB\sin\spager_recycle().\s(CVS\s3813)
+C Always\struncate\sthe\spager\scache\swhen\struncating\sthe\sdatabase\sfile.\sAlso\sreorganize\sthe\scode\sto\scheck\sthe\schange-counter\safter\sfirst\sobtaining\sa\sshared\slock.\s(CVS\s3814)
-D 2007-04-05T14:29:43
+D 2007-04-05T17:15:53
 F Makefile.in 29fbf08ce0989973bfed0b5a052a6bdf3e60fd0a
 F Makefile.linux-gcc 2d8574d1ba75f129aba2019f0b959db380a90935
 F README 9c4e2d6706bdcc3efdd773ce752a8cdab4f90028
@@ -86,7 +86,7 @@ F src/os_unix.c 13c6f73a7b0c2c6c131c97ea26274db101b594cd
 F src/os_unix.h 5768d56d28240d3fe4537fac08cc85e4fb52279e
 F src/os_win.c c9a99524d6b2bdec636264cad1b67553925e3309
 F src/os_win.h 41a946bea10f61c158ce8645e7646b29d44f122b
-F src/pager.c 6c70842fe6621968be7d87143032640b7bc7f3f8
+F src/pager.c 642f804b20a71933fb83b13a1ce93cb4dd1390f7
 F src/pager.h e79a24cf200b8771366217f5bca414f5b7823f42
 F src/parse.y 207ab04273ae13aa4a729b96008d294d5f334ab3
 F src/pragma.c 3b992b5b2640d6ae25cef05aa6a42cd1d6c43234
@@ -196,7 +196,7 @@ F test/delete3.test 555e84a00a99230b7d049d477a324a631126a6ab
 F test/descidx1.test 2177c4ad55edcf56ad5f4c6490f307d7774e8a10
 F test/descidx2.test eb3a2882ec58aa6e1e8131d9bb54436e5b4a3ce2
 F test/descidx3.test 3a55b8d73bc3e9ad084e0da7fec781cf0d2a0356
-F test/diskfull.test f592f37d9ee9d32155e0d30e4e35ec4a28c9ee93
+F test/diskfull.test a91fa95a8729b71fdac4738a49755f70b48c61f3
 F test/distinctagg.test 2b89d1c5220d966a30ba4b40430338669301188b
 F test/enc.test 7a03417a1051fe8bc6c7641cf4c8c3f7e0066d52
 F test/enc2.test 45710bacfa9df29720bc84c067dfdf8c8ddfb797
@@ -280,7 +280,7 @@ F test/misc7.test 292c9ec0245d3602e9d36555efa0a1a8c9df9c54
 F test/misuse.test 30b3a458e5a70c31e74c291937b6c82204c59f33
 F test/notnull.test 44d600f916b770def8b095a9962dbe3be5a70d82
 F test/null.test 9503e1f63e959544c006d9f01709c5b5eab67d54
-F test/pager.test 3e12bef9c8512adb3f5db13d5745dc68fb4b92fc
+F test/pager.test 6c644725db2a79528f67a6f3472b9c9ddee17f05
 F test/pager2.test c025f91b75fe65e85febda64d9416428b8a5cab5
 F test/pager3.test 2323bf27fd5bd887b580247e5bce500ceee994b4
 F test/pagesize.test 05c74ea49f790734ec1e9ab765d9bf1cce79b8f2
@@ -450,7 +450,7 @@ F www/tclsqlite.tcl bb0d1357328a42b1993d78573e587c6dcbc964b9
 F www/vdbe.tcl 87a31ace769f20d3627a64fa1fade7fed47b90d0
 F www/version3.tcl 890248cf7b70e60c383b0e84d77d5132b3ead42b
 F www/whentouse.tcl 97e2b5cd296f7d8057e11f44427dea8a4c2db513
-P c20f7563c0ffa1df47df5464f1f1cc4703ffa9b4
+P 97c5159816e211d9c71aa68db7c5e01df535d6a4
-R 208747f649a24ee705a02097c6e213aa
+R 3fc0ad20eb98e5c7a10bce0fbefb439e
 U danielk1977
-Z 977a9e0a2f802e57fc1f9452318b947c
+Z 9f9e23b7682ec7bf9ad2d8fa486a39d9

manifest.uuid

@@ -1 +1 @@
-97c5159816e211d9c71aa68db7c5e01df535d6a4
+9dc4100eff71be579480ce7939c7da712d28f0ae

src/pager.c

@@ -18,7 +18,7 @@
 ** file simultaneously, or one process from reading the database while
 ** another is writing.
 **
-** @(#) $Id: pager.c,v 1.322 2007/04/05 14:29:43 danielk1977 Exp $
+** @(#) $Id: pager.c,v 1.323 2007/04/05 17:15:53 danielk1977 Exp $
 */
 #ifndef SQLITE_OMIT_DISKIO
 #include "sqliteInt.h"
@@ -889,7 +889,7 @@ static void pagerUnlockAndRollback(Pager *p){
 
 
 /*
-** Unlock the database and clear the in-memory cache.  This routine
+** Clear the in-memory cache.  This routine
 ** sets the state of the pager back to what it was when it was first
 ** opened.  Any outstanding pages are invalidated and subsequent attempts
 ** to access those pages will likely result in a coredump.
@@ -1265,13 +1265,22 @@ static int pager_reload_cache(Pager *pPager){
 }
 #endif
 
+static void pager_truncate_cache(Pager *pPager);
+
 /*
 ** Truncate the main file of the given pager to the number of pages
-** indicated.
+** indicated. Also truncate the cached representation of the file.
 */
 static int pager_truncate(Pager *pPager, int nPage){
-  assert( pPager->state>=PAGER_EXCLUSIVE );
+  int rc = SQLITE_OK;
-  return sqlite3OsTruncate(pPager->fd, pPager->pageSize*(i64)nPage);
+  if( pPager->state>=PAGER_EXCLUSIVE ){
+    rc = sqlite3OsTruncate(pPager->fd, pPager->pageSize*(i64)nPage);
+  }
+  if( rc==SQLITE_OK ){
+    pPager->dbSize = nPage;
+    pager_truncate_cache(pPager);
+  }
+  return rc;
 }
 
 /*
@@ -1398,14 +1407,11 @@ static int pager_playback(Pager *pPager, int isHot){
     /* If this is the first header read from the journal, truncate the
     ** database file back to it's original size.
     */
-    if( pPager->state>=PAGER_EXCLUSIVE && 
+    if( pPager->journalOff==JOURNAL_HDR_SZ(pPager) ){
-        pPager->journalOff==JOURNAL_HDR_SZ(pPager) ){
-      assert( pPager->origDbSize==0 || pPager->origDbSize==mxPg );
       rc = pager_truncate(pPager, mxPg);
       if( rc!=SQLITE_OK ){
         goto end_playback;
       }
-      pPager->dbSize = mxPg;
     }
 
     /* Copy original pages out of the journal and back into the database file.
@@ -1492,11 +1498,8 @@ static int pager_stmt_playback(Pager *pPager){
   
   /* Truncate the database back to its original size.
   */
-  if( pPager->state>=PAGER_EXCLUSIVE ){
+  rc = pager_truncate(pPager, pPager->stmtSize);
-    rc = pager_truncate(pPager, pPager->stmtSize);
-  }
   assert( pPager->state>=PAGER_SHARED );
-  pPager->dbSize = pPager->stmtSize;
 
   /* Figure out how many records are in the statement journal.
   */
@@ -2020,13 +2023,19 @@ static void unlinkPage(PgHdr *pPg){
   unlinkHashChain(pPager, pPg);
 }
 
-#ifndef SQLITE_OMIT_MEMORYDB
 /*
-** This routine is used to truncate an in-memory database.  Delete
+** This routine is used to truncate the cache when a database
-** all pages whose pgno is larger than pPager->dbSize and is unreferenced.
+** is truncated.  Drop from the cache all pages whose pgno is
+** larger than pPager->dbSize and is unreferenced.
+**
 ** Referenced pages larger than pPager->dbSize are zeroed.
+**
+** Actually, at the point this routine is called, it would be
+** an error to have a referenced page.  But rather than delete
+** that page and guarantee a subsequent segfault, it seems better
+** to zero it and hope that we error out sanely.
 */
-static void memoryTruncate(Pager *pPager){
+static void pager_truncate_cache(Pager *pPager){
   PgHdr *pPg;
   PgHdr **ppPg;
   int dbSize = pPager->dbSize;
@@ -2047,9 +2056,6 @@ static void memoryTruncate(Pager *pPager){
     }
   }
 }
-#else
-#define memoryTruncate(p)
-#endif
 
 /*
 ** Try to obtain a lock on a file.  Invoke the busy callback if the lock
@@ -2100,7 +2106,7 @@ int sqlite3PagerTruncate(Pager *pPager, Pgno nPage){
   }
   if( MEMDB ){
     pPager->dbSize = nPage;
-    memoryTruncate(pPager);
+    pager_truncate_cache(pPager);
     return SQLITE_OK;
   }
   rc = syncJournal(pPager);
@@ -2115,9 +2121,6 @@ int sqlite3PagerTruncate(Pager *pPager, Pgno nPage){
   }
 
   rc = pager_truncate(pPager, nPage);
-  if( rc==SQLITE_OK ){
-    pPager->dbSize = nPage;
-  }
   return rc;
 }
 
@@ -2689,6 +2692,24 @@ int sqlite3PagerReleaseMemory(int nReq){
 }
 #endif /* SQLITE_ENABLE_MEMORY_MANAGEMENT */
 
+/*
+** Read the content of page pPg out of the database file.
+*/
+static int readDbPage(Pager *pPager, PgHdr *pPg, Pgno pgno){
+  int rc;
+  assert( MEMDB==0 );
+  rc = sqlite3OsSeek(pPager->fd, (pgno-1)*(i64)pPager->pageSize);
+  if( rc==SQLITE_OK ){
+    rc = sqlite3OsRead(pPager->fd, PGHDR_TO_DATA(pPg),
+                          pPager->pageSize);
+  }
+  IOTRACE(("PGIN %p %d\n", pPager, pgno))
+  PAGERTRACE3("FETCH %d page %d\n", PAGERID(pPager), pPg->pgno);
+  CODEC1(pPager, PGHDR_TO_DATA(pPg), pPg->pgno, 3);
+  return rc;
+}
+
+
 /*
 ** This function is called to obtain the shared lock required before
 ** data may be read from the pager cache. If the shared lock has already
@@ -2792,42 +2813,29 @@ static int pagerSharedLock(Pager *pPager){
         ** change-counter on page 1 of the file, the current cache contents
         ** must be discarded.
         */
+        u8 zC[4];
+        u32 iChangeCounter = 0;
+        sqlite3PagerPagecount(pPager);
 
-        PgHdr *pPage1 = pager_lookup(pPager, 1);
+        if( pPager->errCode ){
-        if( pPage1 ){
+          return pPager->errCode;
-          unlinkPage(pPage1);
-
-          /* Make sure the former page 1 is right at the start of the
-          ** free-list. This triggers a special case in pagerAllocatePage()
-          ** to re-use this page even if the total number of pages in
-          ** the cache is less than Pager.mxPage.
-          */
-          assert( pPager->pFirst==pPager->pFirstSynced );
-          pPage1->pNextFree = pPager->pFirst;
-          if( pPager->pFirst ){
-            pPager->pFirst->pPrevFree = pPage1;
-          }else{
-            assert( !pPager->pLast );
-            pPager->pLast = pPage1;
-          }
-          pPager->pFirst = pPage1;
-          pPager->pFirstSynced = pPage1;
         }
 
-        assert( !pager_lookup(pPager, 1) );
+        if( pPager->dbSize>0 ){
-        rc = sqlite3PagerAcquire(pPager, 1, &pPage1, 0);
+          /* Read the 4-byte change counter directly from the file. */
-        if( rc==SQLITE_OK ){
+          rc = sqlite3OsSeek(pPager->fd, 24);
-	  /* The change-counter is stored at offset 24. See also
+          if( rc!=SQLITE_OK ){
-          ** pager_incr_changecounter().
+            return rc;
-          */
-          u32 iChangeCount = retrieve32bits(pPage1, 24);
-          pPager->nRef++;
-          sqlite3PagerUnref(pPage1);
-          pPager->nRef--;
-          if( iChangeCount!=pPager->iChangeCount ){
-            pager_reset(pPager);
           }
-          pPager->iChangeCount = iChangeCount;
+          rc = sqlite3OsRead(pPager->fd, zC, 4);
+          if( rc!=SQLITE_OK ){
+            return rc;
+          }
+          iChangeCounter = (zC[0]<<24) + (zC[1]<<16) + (zC[2]<<8) + zC[3];
+        }
+
+        if( iChangeCounter!=pPager->iChangeCount ){
+          pager_reset(pPager);
         }
       }
     }
@@ -2841,17 +2849,48 @@ static int pagerSharedLock(Pager *pPager){
 }
 
 /*
-** Allocate or recycle space for a single page.
+** Allocate a PgHdr object.   Either create a new one or reuse
+** an existing one that is not otherwise in use.
+**
+** A new PgHdr structure is created if any of the following are
+** true:
+**
+**     (1)  We have not exceeded our maximum allocated cache size
+**          as set by the "PRAGMA cache_size" command.
+**
+**     (2)  There are no unused PgHdr objects available at this time.
+**
+**     (3)  This is an in-memory database.
+**
+**     (4)  There are no PgHdr objects that do not require a journal
+**          file sync and a sync of the journal file is currently
+**          prohibited.
+**
+** Otherwise, reuse an existing PgHdr.  In other words, reuse an
+** existing PgHdr if all of the following are true:
+**
+**     (1)  We have reached or exceeded the maximum cache size
+**          allowed by "PRAGMA cache_size".
+**
+**     (2)  There is a PgHdr available with PgHdr->nRef==0
+**
+**     (3)  We are not in an in-memory database
+**
+**     (4)  Either there is an available PgHdr that does not need
+**          to be synced to disk or else disk syncing is currently
+**          allowed.
 */
 static int pagerAllocatePage(Pager *pPager, PgHdr **ppPg){
   int rc = SQLITE_OK;
   PgHdr *pPg;
 
-  if( MEMDB || (!(pPager->pFirstSynced && pPager->pFirstSynced->pgno==0) && (
+  /* Create a new PgHdr if any of the four conditions defined 
-      pPager->nPage<pPager->mxPage || pPager->pFirst==0 || 
+  ** above is met: */
-      (pPager->pFirstSynced==0 && pPager->doNotSync)
+  if( pPager->nPage<pPager->mxPage
-  )) ){
+   || pPager->pFirst==0 
-    /* Create a new page */
+   || MEMDB
+   || (pPager->pFirstSynced==0 && pPager->doNotSync)
+  ){
     if( pPager->nPage>=pPager->nHash ){
       pager_resize_hash_table(pPager,
          pPager->nHash<256 ? 256 : pPager->nHash*2);
@@ -3001,15 +3040,7 @@ int sqlite3PagerAcquire(Pager *pPager, Pgno pgno, DbPage **ppPage, int clrFlag){
     if( nMax<(int)pgno || MEMDB || (clrFlag && !pPager->alwaysRollback) ){
       memset(PGHDR_TO_DATA(pPg), 0, pPager->pageSize);
     }else{
-      assert( MEMDB==0 );
+      rc = readDbPage(pPager, pPg, pgno);
-      rc = sqlite3OsSeek(pPager->fd, (pgno-1)*(i64)pPager->pageSize);
-      if( rc==SQLITE_OK ){
-        rc = sqlite3OsRead(pPager->fd, PGHDR_TO_DATA(pPg),
-                              pPager->pageSize);
-      }
-      IOTRACE(("PGIN %p %d\n", pPager, pgno))
-      PAGERTRACE3("FETCH %d page %d\n", PAGERID(pPager), pPg->pgno);
-      CODEC1(pPager, PGHDR_TO_DATA(pPg), pPg->pgno, 3);
       if( rc!=SQLITE_OK && rc!=SQLITE_IOERR_SHORT_READ ){
         pPg->pgno = 0;
         sqlite3PagerUnref(pPg);
@@ -3035,20 +3066,6 @@ int sqlite3PagerAcquire(Pager *pPager, Pgno pgno, DbPage **ppPage, int clrFlag){
   }else{
     /* The requested page is in the page cache. */
     assert(pPager->nRef>0 || pgno==1);
-    if( pgno>sqlite3PagerPagecount(pPager) ){
-      /* This can happen after a truncation in exclusive mode. The pager
-      ** cache contains pages that are located after the end of the 
-      ** database file. Zero such pages before returning. Not doing this 
-      ** was causing the problem reported in ticket #2285.
-      */
-      if( pPager->errCode ){
-        /* This case catches an IO error in sqlite3PagerPagecount(). If
-        ** the error occured, PagerPagecount() returned 0.
-        */
-        return pPager->errCode;
-      }
-      memset(PGHDR_TO_DATA(pPg), 0, pPager->pageSize);
-    }
     TEST_INCR(pPager->nHit);
     page_ref(pPg);
   }
@@ -3891,7 +3908,7 @@ int sqlite3PagerRollback(Pager *pPager){
     pPager->pDirty = 0;
     pPager->pStmt = 0;
     pPager->dbSize = pPager->origDbSize;
-    memoryTruncate(pPager);
+    pager_truncate_cache(pPager);
     pPager->stmtInUse = 0;
     pPager->state = PAGER_SHARED;
     return SQLITE_OK;
@@ -3918,6 +3935,7 @@ int sqlite3PagerRollback(Pager *pPager){
   }else{
     rc = pager_playback(pPager, 0);
   }
+  /* pager_reset(pPager); */
   pPager->dbSize = -1;
 
   /* If an error occurs during a ROLLBACK, we can no longer trust the pager
@@ -4067,7 +4085,7 @@ int sqlite3PagerStmtRollback(Pager *pPager){
         }
       }
       pPager->dbSize = pPager->stmtSize;
-      memoryTruncate(pPager);
+      pager_truncate_cache(pPager);
       rc = SQLITE_OK;
     }else{
       rc = pager_stmt_playback(pPager);

test/diskfull.test

@@ -12,7 +12,7 @@
 # focus of this file is testing for correct handling of disk full
 # errors.
 # 
-# $Id: diskfull.test,v 1.5 2007/03/31 10:00:48 danielk1977 Exp $
+# $Id: diskfull.test,v 1.6 2007/04/05 17:15:53 danielk1977 Exp $
 
 set testdir [file dirname $argv0]
 source $testdir/tester.tcl
@@ -56,7 +56,7 @@ integrity_check diskfull-1.6
 proc do_diskfull_test {prefix sql} {
   set ::go 1
   set ::sql $sql
-  set ::i 52
+  set ::i 1
   while {$::go} {
     incr ::i
     do_test ${prefix}.$::i.1 {

test/pager.test

@@ -11,7 +11,7 @@
 # This file implements regression tests for SQLite library.  The
 # focus of this script is page cache subsystem.
 #
-# $Id: pager.test,v 1.27 2007/04/02 05:07:48 danielk1977 Exp $
+# $Id: pager.test,v 1.28 2007/04/05 17:15:53 danielk1977 Exp $
 
 
 set testdir [file dirname $argv0]
@@ -119,7 +119,7 @@ do_test pager-2.12 {
 } {1}
 do_test pager-2.13 {
   pager_stats $::p1
-} {ref 1 page 1 max 10 size 0 state 1 err 0 hit 1 miss 2 ovfl 0}
+} {ref 1 page 1 max 10 size 0 state 1 err 0 hit 1 miss 1 ovfl 0}
 do_test pager-2.14 {
   set v [catch {
     page_write $::g1 "Page-One"
@@ -128,7 +128,7 @@ do_test pager-2.14 {
 } {0 {}}
 do_test pager-2.15 {
   pager_stats $::p1
-} {ref 1 page 1 max 10 size 1 state 2 err 0 hit 1 miss 2 ovfl 0}
+} {ref 1 page 1 max 10 size 1 state 2 err 0 hit 1 miss 1 ovfl 0}
 do_test pager-2.16 {
   page_read $::g1
 } {Page-One}
@@ -140,19 +140,19 @@ do_test pager-2.17 {
 } {0 {}}
 do_test pager-2.20 {
   pager_stats $::p1
-} {ref 1 page 1 max 10 size -1 state 1 err 0 hit 2 miss 2 ovfl 0}
+} {ref 1 page 1 max 10 size -1 state 1 err 0 hit 2 miss 1 ovfl 0}
 do_test pager-2.19 {
   pager_pagecount $::p1
 } {1}
 do_test pager-2.21 {
   pager_stats $::p1
-} {ref 1 page 1 max 10 size 1 state 1 err 0 hit 2 miss 2 ovfl 0}
+} {ref 1 page 1 max 10 size 1 state 1 err 0 hit 2 miss 1 ovfl 0}
 do_test pager-2.22 {
   page_unref $::g1
 } {}
 do_test pager-2.23 {
   pager_stats $::p1
-} {ref 0 page 1 max 10 size -1 state 0 err 0 hit 2 miss 2 ovfl 0}
+} {ref 0 page 1 max 10 size -1 state 0 err 0 hit 2 miss 1 ovfl 0}
 do_test pager-2.24 {
   set v [catch {
     page_get $::p1 1