Add several EXPENSIVE_ASSERT code blocks to validate the wal-index hash table.

Fix the bugs that these code blocks fine.  Rename walClearHash() to
walCleanupHash() and simplify its interface.

FossilOrigin-Name: 7aade899e55f4565f02d301e1e83fb0bac2ea500

diff --git a/manifest b/manifest
index 1fcbcd7dba667ae5923f4973a27b50e3f59138be..1baf9ed9dae7350dda52849314f05a3d17d69f14 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,8 @@
-C Fix\sanother\sbug\sin\swalClearHash().
-D 2010-05-21T19:15:05
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA1
+
+C Add\sseveral\sEXPENSIVE_ASSERT\scode\sblocks\sto\svalidate\sthe\swal-index\shash\stable.\nFix\sthe\sbugs\sthat\sthese\scode\sblocks\sfine.\s\sRename\swalClearHash()\sto\s\nwalCleanupHash()\sand\ssimplify\sits\sinterface.
+D 2010-05-22T00:55:40
 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0
 F Makefile.in a5cad1f8f3e021356bfcc6c77dc16f6f1952bbc3
 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654
@@ -224,7 +227,7 @@ F src/vdbeblob.c 5327132a42a91e8b7acfb60b9d2c3b1c5c863e0e
 F src/vdbemem.c 2a82f455f6ca6f78b59fb312f96054c04ae0ead1
 F src/vdbetrace.c 864cef96919323482ebd9986f2132435115e9cc2
 F src/vtab.c a0f8a40274e4261696ef57aa806de2776ab72cda
-F src/wal.c ad8e423a197525e04c2fa3ef51cc97a1c38934da
+F src/wal.c 8371cedf1f9fb8b0d99f31598e276d7a03fb2b05
 F src/wal.h 434f76f51225bb614e43ccb6bd2341541ba6a06e
 F src/walker.c 3112bb3afe1d85dc52317cb1d752055e9a781f8f
 F src/where.c 75fee9e255b62f773fcadd1d1f25b6f63ac7a356
@@ -813,7 +816,14 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff
 F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224
 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e
 F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f
-P 7d01309da658d6b658c1b2e53bbdc5112fb0a4d9
-R 2e048d2ddbf28060ebdb21766a34092e
-U dan
-Z ab0da23d8b99c2d63fccda81279534cd
+P 40f80ffe70ca691dfa146f6d84956ed0784fc63d
+R 524fa1e4249768664dfa82575a2741a4
+U drh
+Z 028b5cd7fb9973ef295584c4a0b1ec8f
+-----BEGIN PGP SIGNATURE-----
+Version: GnuPG v1.4.6 (GNU/Linux)
+
+iD8DBQFL9yuPoxKgR168RlERAnVbAJ48JgsskKwH9nQ/ST9nadbql9iCqQCggJCz
+rLTdl7W6OQ9+Vg0rmcQfA+4=
+=Wnao
+-----END PGP SIGNATURE-----

diff --git a/manifest.uuid b/manifest.uuid
index a96e1a542b13cba318ca8ba914a4dc7d0b735664..90cdb13785ab6c571f52a30e325d70bb1cd8f15d 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-40f80ffe70ca691dfa146f6d84956ed0784fc63d
\ No newline at end of file
+7aade899e55f4565f02d301e1e83fb0bac2ea500
\ No newline at end of file

diff --git a/src/wal.c b/src/wal.c
index 17d1144c7401b785d5d2aec87992c57fa6a546e7..5e5419105351a5359b6cc0bac949d7ad778b585e 100644 (file)
--- a/src/wal.c
+++ b/src/wal.c
@@ -652,8 +652,36 @@ static int walIndexAppend(Wal *pWal, u32 iFrame, u32 iPage){
       assert( nCollide++ < idx );
     }
     aHash[iKey] = idx;
+
+#ifdef SQLITE_ENABLE_EXPENSIVE_ASSERT
+    /* Verify that the number of entries in the hash table exactly equals
+    ** the number of entries in the mapping region.
+    */
+    {
+      int i;           /* Loop counter */
+      int nEntry = 0;  /* Number of entries in the hash table */
+      for(i=0; i<HASHTABLE_NSLOT; i++){ if( aHash[i] ) nEntry++; }
+      assert( nEntry==idx );
+    }
+
+    /* Verify that the every entry in the mapping region is reachable
+    ** via the hash table.  This turns out to be a really, really expensive
+    ** thing to check, so only do this occasionally - not on every
+    ** iteration.
+    */
+    if( (idx&0x3ff)==0 ){
+      int i;           /* Loop counter */
+      for(i=1; i<=idx; i++){
+        for(iKey=walHash(aPgno[i+iZero]); aHash[iKey]; iKey=walNextHash(iKey)){
+          if( aHash[iKey]==i ) break;
+        }
+        assert( aHash[iKey]==i );
+      }
+    }
+#endif /* SQLITE_ENABLE_EXPENSIVE_ASSERT */
   }
 
+
   return rc;
 }
 
@@ -1467,37 +1495,49 @@ int sqlite3WalWriteLock(Wal *pWal, int op){
 }
 
 /*
-** Remove entries from zero or more hash-table indexes in the wal-index 
-** file.
-**
-** This function is called when rolling back a transaction or savepoint
-** transaction in WAL mode. Argument iNewMx is the value that 
-** Wal.hdr.mxFrame will be set to following the rollback. Argument iOldMx
-** is the value that it had before the rollback. This function removes 
-** entries that refer to frames with frame numbers greater than iNewMx 
-** from the hash table that contains the entry associated with iNewMx.
-** It is not necessary to remove any entries from any subsequent hash
-** tables, as they will be zeroed by walIndexAppend() before they are
-** next used.
+** Remove entries from the hash table that point to WAL slots greater
+** than pWal->hdr.mxFrame.
+**
+** This function is called whenever pWal->hdr.mxFrame is decreased due
+** to a rollback or savepoint.
+**
+** At most only the very last hash table needs to be updated.  Any
+** later hash tables will be automatically cleared when pWal->hdr.mxFrame
+** advances to the point where those hash tables are actually needed.
 */
-static void walClearHash(Wal *pWal, u32 iOldMx, u32 iNewMx){
-  if( iOldMx>iNewMx ){
-    volatile HASHTABLE_DATATYPE *aHash;     /* Pointer to hash table to clear */
-    volatile u32 *unused1;                  /* Only to satisfy walHashFind() */
-    u32 iZero;                              /* frame == (aHash[x]+iZero) */
-    int iLimit;                             /* Zero values greater than this */
-
-    walHashFind(pWal, iNewMx+1, &aHash, &unused1, &iZero);
-    iLimit = iNewMx - iZero;
-    if( iLimit>0 ){
-      int i;                      /* Used to iterate through aHash[] */
-      for(i=0; i<HASHTABLE_NSLOT; i++){
-        if( aHash[i]>iLimit ){
-          aHash[i] = 0;
-        }
+static void walCleanupHash(Wal *pWal){
+  volatile HASHTABLE_DATATYPE *aHash;  /* Pointer to hash table to clear */
+  volatile u32 *aPgno;                 /* Unused return from walHashFind() */
+  u32 iZero;                           /* frame == (aHash[x]+iZero) */
+  int iLimit;                          /* Zero values greater than this */
+
+  assert( pWal->lockState==SQLITE_SHM_WRITE );
+  walHashFind(pWal, pWal->hdr.mxFrame+1, &aHash, &aPgno, &iZero);
+  iLimit = pWal->hdr.mxFrame - iZero;
+  if( iLimit>0 ){
+    int i;                      /* Used to iterate through aHash[] */
+    for(i=0; i<HASHTABLE_NSLOT; i++){
+      if( aHash[i]>iLimit ){
+        aHash[i] = 0;
       }
     }
   }
+
+#ifdef SQLITE_ENABLE_EXPENSIVE_ASSERT
+  /* Verify that the every entry in the mapping region is still reachable
+  ** via the hash table even after the cleanup.
+  */
+  {
+    int i;           /* Loop counter */
+    int iKey;        /* Hash key */
+    for(i=1; i<=iLimit; i++){
+      for(iKey=walHash(aPgno[i+iZero]); aHash[iKey]; iKey=walNextHash(iKey)){
+        if( aHash[iKey]==i ) break;
+      }
+      assert( aHash[iKey]==i );
+    }
+  }
+#endif /* SQLITE_ENABLE_EXPENSIVE_ASSERT */
 }
 
 /*
@@ -1521,12 +1561,12 @@ int sqlite3WalUndo(Wal *pWal, int (*xUndo)(void *, Pgno), void *pUndoCtx){
   
     assert( pWal->pWiData==0 );
     rc = walIndexReadHdr(pWal, &unused);
-    for(iFrame=pWal->hdr.mxFrame+1; rc==SQLITE_OK && iFrame<=iMax; iFrame++){
-      assert( pWal->lockState==SQLITE_SHM_WRITE );
-      rc = xUndo(pUndoCtx, pWal->pWiData[walIndexEntry(iFrame)]);
-    }
     if( rc==SQLITE_OK ){
-      walClearHash(pWal, iMax, pWal->hdr.mxFrame);
+      walCleanupHash(pWal);
+      for(iFrame=pWal->hdr.mxFrame+1; rc==SQLITE_OK && iFrame<=iMax; iFrame++){
+        assert( pWal->lockState==SQLITE_SHM_WRITE );
+        rc = xUndo(pUndoCtx, pWal->pWiData[walIndexEntry(iFrame)]);
+      }
     }
     walIndexUnmap(pWal);
   }
@@ -1548,12 +1588,15 @@ int sqlite3WalSavepointUndo(Wal *pWal, u32 iFrame){
   int rc = SQLITE_OK;
   assert( pWal->lockState==SQLITE_SHM_WRITE );
 
-  rc = walIndexMap(pWal, walMappingSize(pWal->hdr.mxFrame));
-  if( rc==SQLITE_OK ){
-    walClearHash(pWal, pWal->hdr.mxFrame, iFrame);
-    walIndexUnmap(pWal);
+  assert( iFrame<=pWal->hdr.mxFrame );
+  if( iFrame<pWal->hdr.mxFrame ){
+    rc = walIndexMap(pWal, walMappingSize(pWal->hdr.mxFrame));
+    pWal->hdr.mxFrame = iFrame;
+    if( rc==SQLITE_OK ){
+      walCleanupHash(pWal);
+      walIndexUnmap(pWal);
+    }
   }
-  pWal->hdr.mxFrame = iFrame;
   return rc;
 }