/ Check-in [32ccf3ae]
Login

Many hyperlinks are disabled.
Use anonymous login to enable hyperlinks.

Overview
Comment:Fix a race condition in the sorter.
Downloads: Tarball | ZIP archive | SQL archive
Timelines: family | ancestors | descendants | both | threads
Files: files | file ages | folders
SHA1: 32ccf3ae18531682dfd039fa8df6ad9a907ac455
User & Date: dan 2014-05-03 19:33:00
Context
2014-05-03
20:43
Add an extra fault-injection test to sortfault.test. Remove an unreachable branch from vdbesort.c. check-in: a33a366b user: dan tags: threads
19:33
Fix a race condition in the sorter. check-in: 32ccf3ae user: dan tags: threads
14:28
Fix a problem in the sorter causing it to return spurious SQLITE_NOMEM errors when configured to use memsys3 or memsys5. check-in: 3a66c4e1 user: dan tags: threads
Changes
Hide Diffs Side-by-Side Diffs Ignore Whitespace Patch

Changes to src/vdbesort.c.

   646    646   /*
   647    647   ** Advance iterator pIter to the next key in its PMA. Return SQLITE_OK if
   648    648   ** no error occurs, or an SQLite error code if one does.
   649    649   */
   650    650   static int vdbePmaReaderNext(PmaReader *pIter){
   651    651     int rc = SQLITE_OK;             /* Return Code */
   652    652     u64 nRec = 0;                   /* Size of record in bytes */
          653  +
   653    654   
   654    655     if( pIter->iReadOff>=pIter->iEof ){
   655    656       IncrMerger *pIncr = pIter->pIncr;
   656    657       int bEof = 1;
   657    658       if( pIncr ){
   658    659         rc = vdbeIncrSwap(pIncr);
   659    660         if( rc==SQLITE_OK && pIncr->bEof==0 ){
................................................................................
  1840   1841   static int vdbeIncrInitMerger(
  1841   1842     SortSubtask *pTask, 
  1842   1843     MergeEngine *pMerger, 
  1843   1844     int eMode                       /* One of the INCRINIT_XXX constants */
  1844   1845   ){
  1845   1846     int rc = SQLITE_OK;             /* Return code */
  1846   1847     int i;                          /* For iterating through PmaReader objects */
         1848  +  int nTree = pMerger->nTree;
  1847   1849   
  1848         -  for(i=0; rc==SQLITE_OK && i<pMerger->nTree; i++){
         1850  +  for(i=0; rc==SQLITE_OK && i<nTree; i++){
  1849   1851       if( eMode==INCRINIT_ROOT ){
  1850         -      rc = vdbePmaReaderNext(&pMerger->aIter[i]);
         1852  +      /* Iterators should be normally initialized in order, as if they are
         1853  +      ** reading from the same temp file this makes for more linear file IO.
         1854  +      ** However, in the INCRINIT_ROOT case, if iterator aIter[nTask-1] is
         1855  +      ** in use it will block the vdbePmaReaderNext() call while it uses
         1856  +      ** the main thread to fill its buffer. So calling PmaReaderNext()
         1857  +      ** on this iterator before any of the multi-threaded iterators takes
         1858  +      ** better advantage of multi-processor hardware. */
         1859  +      rc = vdbePmaReaderNext(&pMerger->aIter[nTree-i-1]);
  1851   1860       }else{
  1852   1861         rc = vdbePmaReaderIncrInit(&pMerger->aIter[i], INCRINIT_NORMAL);
  1853   1862       }
  1854   1863     }
  1855   1864   
  1856   1865     for(i=pMerger->nTree-1; rc==SQLITE_OK && i>0; i--){
  1857   1866       rc = vdbeSorterDoCompare(pTask, pMerger, i);
................................................................................
  2192   2201                 assert( pIncr->pTask!=pLast );
  2193   2202               }
  2194   2203             }
  2195   2204             if( pSorter->nTask>1 ){
  2196   2205               for(iTask=0; rc==SQLITE_OK && iTask<pSorter->nTask; iTask++){
  2197   2206                 PmaReader *p = &pMain->aIter[iTask];
  2198   2207                 assert( p->pIncr==0 || p->pIncr->pTask==&pSorter->aTask[iTask] );
  2199         -              if( p->pIncr ){ rc = vdbePmaReaderBgIncrInit(p); }
         2208  +              if( p->pIncr ){ 
         2209  +                if( iTask==pSorter->nTask-1 ){
         2210  +                  rc = vdbePmaReaderIncrInit(p, INCRINIT_TASK);
         2211  +                }else{
         2212  +                  rc = vdbePmaReaderBgIncrInit(p);
         2213  +                }
         2214  +              }
  2200   2215               }
  2201   2216             }
  2202   2217           }
  2203   2218           pMain = 0;
  2204   2219         }
  2205   2220         if( rc==SQLITE_OK ){
  2206   2221           int eMode = (pSorter->nTask>1 ? INCRINIT_ROOT : INCRINIT_NORMAL);

Changes to test/sort.test.

   537    537             1          0       3     file      true    false
   538    538             2          0       3     file      true     true
   539    539             3          0       0     file      true    false
   540    540             4    1000000       3     file      true    false
   541    541             5          0       0   memory     false     true
   542    542   } {
   543    543     db close
   544         -
   545    544     sqlite3_shutdown
   546    545     sqlite3_config_worker_threads $nWorker
   547    546     if {$coremutex} {
   548    547       sqlite3_config multithread
   549    548     } else {
   550    549       sqlite3_config singlethread
   551    550     }
   552    551     sqlite3_initialize
   553         -
   554    552     sorter_test_fakeheap $fakeheap
   555    553   
   556    554     reset_db
   557    555     sqlite3_test_control SQLITE_TESTCTRL_SORTER_MMAP db $mmap_limit
   558    556     execsql "PRAGMA temp_store = $tmpstore"
   559    557     
   560    558     set ten [string repeat X 10300]
................................................................................
   593    591     } [list 2 $one 4 $ten]
   594    592   
   595    593     sorter_test_fakeheap 0
   596    594   }
   597    595   
   598    596   db close
   599    597   sqlite3_shutdown
   600         -#sqlite3_config_worker_threads $sqlite_options(worker_threads)
   601    598   sqlite3_config_worker_threads 0
   602    599   set t(0) singlethread
   603    600   set t(1) multithread
   604    601   set t(2) serialized
   605    602   sqlite3_config $t($sqlite_options(threadsafe))
   606    603   sqlite3_initialize
   607    604   
   608    605   finish_test
   609    606   

Changes to test/sortfault.test.

    19     19   set testprefix sortfault
    20     20   
    21     21   
    22     22   do_execsql_test 1.0 {
    23     23     PRAGMA cache_size = 5;
    24     24   }
    25     25   
    26         -foreach {tn mmap_limit} {
    27         -  1 0
    28         -  2 100000
           26  +foreach {tn mmap_limit nWorker tmpstore threadsmode fakeheap} {
           27  +          1          0       0     file multithread    false
           28  +          2     100000       0     file multithread    false
    29     29   } {
           30  +  catch { db close }
           31  +  sqlite3_shutdown
           32  +  sqlite3_config_worker_threads $nWorker
           33  +  sqlite3_config $threadsmode
           34  +  sqlite3_initialize
           35  +  sorter_test_fakeheap $fakeheap
           36  +
           37  +  set str [string repeat a 1000]
           38  +
    30     39     do_faultsim_test 1.$tn -prep {
    31     40       sqlite3 db test.db
    32     41       sqlite3_test_control SQLITE_TESTCTRL_SORTER_MMAP db $::mmap_limit
           42  +    execsql { PRAGMA cache_size = 5 }
    33     43     } -body {
    34     44       execsql { 
    35     45         WITH r(x,y) AS (
    36         -          SELECT 1, randomblob(1000)
           46  +          SELECT 1, $::str
    37     47             UNION ALL
    38         -          SELECT x+1, randomblob(1000) FROM r
    39         -          LIMIT 500
    40         -          )
    41         -        SELECT count(x), length(y) FROM r GROUP BY (x%5)
    42         -    } 
           48  +          SELECT x+1, $::str FROM r
           49  +          LIMIT 200
           50  +      )
           51  +      SELECT count(x), length(y) FROM r GROUP BY (x%5)
           52  +    }
    43     53     } -test {
    44         -    faultsim_test_result {0 {100 1000 100 1000 100 1000 100 1000 100 1000}}
           54  +    faultsim_test_result {0 {40 1000 40 1000 40 1000 40 1000 40 1000}}
    45     55     }
    46     56   }
    47     57   
           58  +catch { db close }
           59  +sqlite3_shutdown
           60  +sqlite3_config_worker_threads 0
           61  +set t(0) singlethread
           62  +set t(1) multithread
           63  +set t(2) serialized
           64  +sqlite3_config $t($sqlite_options(threadsafe))
           65  +sqlite3_initialize
    48     66   finish_test
    49     67