From d0720eee5e19d9d6889b275bdb652e5e0160899c Mon Sep 17 00:00:00 2001 From: dan Date: Thu, 26 Sep 2024 18:02:17 +0000 Subject: [PATCH 1/2] When possible, avoid taking wal file read-lock 0 in sqlite3_snapshot_get(). FossilOrigin-Name: 34b6ac3d76dbc6819778ec2a0f81cbcdcc0cd1a6303381d97f1c479e4ecdd132 --- manifest | 22 ++++++++++++---------- manifest.uuid | 2 +- src/main.c | 4 ++++ src/wal.c | 18 ++++++++++++++++-- test/snapshot3.test | 39 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 13 deletions(-) diff --git a/manifest b/manifest index e74bcacf3c..83d6936a7d 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C In\sthe\sCLI,\sfor\scolumnar\soutput\sformats,\stry\sto\saccount\sfor\sthe\spresence\sof\nzero-width\sand\sdouble-width\scharacters\sin\sthe\soutput\sand\sadjust\scolumn\swidths\naccordingly. -D 2024-09-25T09:39:11.501 +C When\spossible,\savoid\staking\swal\sfile\sread-lock\s0\sin\ssqlite3_snapshot_get(). +D 2024-09-26T18:02:17.495 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -730,7 +730,7 @@ F src/insert.c f8d1a0f8ee258411009c6b7f2d93170e351bd19f5ad89d57e1180644297cbe70 F src/json.c 68a98c020c22127f2d65f08855f7fc7460ff352a6ce0b543d8931dde83319c22 F src/legacy.c d7874bc885906868cd51e6c2156698f2754f02d9eee1bae2d687323c3ca8e5aa F src/loadext.c 7432c944ff197046d67a1207790a1b13eec4548c85a9457eb0896bb3641dfb36 -F src/main.c 4db6e3bde55ba0b24ccc83600c2b6ea11429f61ce7b3a2e7e3b42e1b45366c3e +F src/main.c 3bb768eeeee1cceaed391327362b8be8b5ce61a12fe24d5b555ce9c6b4a883de F src/malloc.c 410e570b30c26cc36e3372577df50f7a96ee3eed5b2b161c6b6b48773c650c5e F src/mem0.c 6a55ebe57c46ca1a7d98da93aaa07f99f1059645 F src/mem1.c 3bb59158c38e05f6270e761a9f435bf19827a264c13d1631c58b84bdc96d73b2 @@ -849,7 +849,7 @@ F src/vdbetrace.c fe0bc29ebd4e02c8bc5c1945f1d2e6be5927ec12c06d89b03ef2a4def34bf8 F src/vdbevtab.c fc46b9cbd759dc013f0b3724549cc0d71379183c667df3a5988f7e2f1bd485f3 F src/vtab.c 316cd48e9320660db3047cd306cd056e4361180cebb4d0f10a39244e10c11422 F src/vxworks.h d2988f4e5a61a4dfe82c6524dd3d6e4f2ce3cdb9 -F src/wal.c ef68130ba330ee18c1cb22da36a881c82e3a3b109badbdc6a9b9acaf788a6688 +F src/wal.c a0d42bfdef935e1389737152394d08e59e7c48697f40a9fc2e0552cb19dc731f F src/wal.h ba252daaa94f889f4b2c17c027e823d9be47ce39da1d3799886bbd51f0490452 F src/walker.c d5006d6b005e4ea7302ad390957a8d41ed83faa177e412f89bc5600a7462a014 F src/where.c 461d41017d900d4248a268df96d2d30506c4dcc2257f4167c4f46072003ce2cf @@ -1647,7 +1647,7 @@ F test/skipscan5.test 0672103fd2c8f96bd114133f356192b35ece45c794fe3677e1d9e5e310 F test/skipscan6.test bddbb35dd335e2d21b7791a61e3b2e1f3255dc307ce80aa6fe19cc298e6feb13 F test/snapshot.test a504f2e7009f512ef66c719f0ea1c55a556bdaf1e1312c80a04d46fc1a3e9632 F test/snapshot2.test 8d6ff5dd9cc503f6e12d408a30409c3f9c653507b24408d9cd7195931c89bc54 -F test/snapshot3.test 8744313270c55f6e18574283553d3c5c5fe4c5970585663613a0e75c151e599b +F test/snapshot3.test 41350216abc6c7da37113ad462259c070786e5ad70bdc8709daaed148b1b3a2c F test/snapshot4.test d4e9347ef2fcabc491fc893506c7bbaf334da3be111d6eb4f3a97cc623b78322 F test/snapshot_fault.test 129234ceb9b26a0e1000e8563a16e790f5c1412354e70749cbd78c3d5d07d60a F test/snapshot_up.test 77dc7853bfb2b4fa249f76e1714cfa1e596826165d9ef22c06ac3a0b7b778d9a @@ -2213,9 +2213,11 @@ F vsixtest/vsixtest.tcl 6195aba1f12a5e10efc2b8c0009532167be5e301abe5b31385638080 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 42bb941584a1ac922ee6b0b6ecadce71c9259555563cf49913a6f820f3f9b887 b217e3004b58af0e777726bdd652b999ad41815261299ef4ce8f8d2f6b0afe8d -R 5fcf6775e686fdd76943c66cce860cb0 -T +closed b217e3004b58af0e777726bdd652b999ad41815261299ef4ce8f8d2f6b0afe8d -U drh -Z af457fc67e3efab459ef7260d9a5da46 +P 9592b9ba3ad7a842cdd4c4010da278485a6fdec7e811bda01ebe640162a8c3b6 +R 8553e4183ac33940fe95918021cfe535 +T *branch * snapshot_get-locking +T *sym-snapshot_get-locking * +T -sym-trunk * +U dan +Z 2b5e37766517d26ed8f4d4495c7d929b # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 5d05a8c7ec..986575a33e 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -9592b9ba3ad7a842cdd4c4010da278485a6fdec7e811bda01ebe640162a8c3b6 +34b6ac3d76dbc6819778ec2a0f81cbcdcc0cd1a6303381d97f1c479e4ecdd132 diff --git a/src/main.c b/src/main.c index 6ab09c5560..fceb7957b9 100644 --- a/src/main.c +++ b/src/main.c @@ -4963,7 +4963,11 @@ int sqlite3_snapshot_get( if( iDb==0 || iDb>1 ){ Btree *pBt = db->aDb[iDb].pBt; if( SQLITE_TXN_WRITE!=sqlite3BtreeTxnState(pBt) ){ + Pager *pPager = sqlite3BtreePager(pBt); + i64 dummy = 0; + sqlite3PagerSnapshotOpen(pPager, (sqlite3_snapshot*)&dummy); rc = sqlite3BtreeBeginTrans(pBt, 0, 0); + sqlite3PagerSnapshotOpen(pPager, 0); if( rc==SQLITE_OK ){ rc = sqlite3PagerSnapshotGet(sqlite3BtreePager(pBt), ppSnapshot); } diff --git a/src/wal.c b/src/wal.c index 89106038b8..f4aa663fd3 100644 --- a/src/wal.c +++ b/src/wal.c @@ -541,6 +541,7 @@ struct Wal { #endif #ifdef SQLITE_ENABLE_SNAPSHOT WalIndexHdr *pSnapshot; /* Start transaction here if not NULL */ + int bGetSnapshot; /* Transaction opened for sqlite3_get_snapshot() */ #endif #ifdef SQLITE_ENABLE_SETLK_TIMEOUT sqlite3 *db; @@ -3097,7 +3098,7 @@ static int walTryBeginRead(Wal *pWal, int *pChanged, int useWal, int *pCnt){ SEH_INJECT_FAULT; if( !useWal && AtomicLoad(&pInfo->nBackfill)==pWal->hdr.mxFrame #ifdef SQLITE_ENABLE_SNAPSHOT - && (pWal->pSnapshot==0 || pWal->hdr.mxFrame==0) + && ((pWal->bGetSnapshot==0 && pWal->pSnapshot==0) || pWal->hdr.mxFrame==0) #endif ){ /* The WAL has been completely backfilled (or it is empty). @@ -4497,7 +4498,20 @@ void sqlite3WalSnapshotOpen( Wal *pWal, sqlite3_snapshot *pSnapshot ){ - pWal->pSnapshot = (WalIndexHdr*)pSnapshot; + if( pSnapshot && ((WalIndexHdr*)pSnapshot)->iVersion==0 ){ + /* iVersion==0 means that this is a call to sqlite3_snapshot_get(). In + ** this case set the bGetSnapshot flag so that if the call to + ** sqlite3_snapshot_get() is about to read transaction on this wal + ** file, it does not take read-lock 0 if the wal file has been completely + ** checkpointed. Taking read-lock 0 would work, but then it would be + ** possible for a subsequent writer to destroy the snapshot even while + ** this connection is holding its read-transaction open. This is contrary + ** to user expectations, so we avoid it by not taking read-lock 0. */ + pWal->bGetSnapshot = 1; + }else{ + pWal->pSnapshot = (WalIndexHdr*)pSnapshot; + pWal->bGetSnapshot = 0; + } } /* diff --git a/test/snapshot3.test b/test/snapshot3.test index 4dca202b1c..470d463a66 100644 --- a/test/snapshot3.test +++ b/test/snapshot3.test @@ -96,4 +96,43 @@ do_test 1.8 { list [catch { sqlite3_snapshot_open_blob db3 main $snap } msg] $msg } {1 SQLITE_ERROR_SNAPSHOT} +#------------------------------------------------------------------------- +reset_db +do_execsql_test 2.0 { + PRAGMA journal_mode = wal; + CREATE TABLE t1(a, b); + INSERT INTO t1 VALUES(1, 2); + INSERT INTO t1 VALUES(3, 4); +} {wal} + +sqlite3 db2 test.db +sqlite3 db3 test.db +do_execsql_test -db db2 2.0.1 { + SELECT * FROM t1 +} {1 2 3 4} +do_execsql_test -db db3 2.0.2 { + SELECT * FROM t1 +} {1 2 3 4} + +do_execsql_test -db db2 2.2 { + PRAGMA wal_checkpoint; +} {0 4 4} + +do_test 2.1 { + db eval { BEGIN } + set snap [sqlite3_snapshot_get db main] + set {} {} +} {} + +do_execsql_test -db db2 2.3 { + INSERT INTO t1 VALUES(5, 6); +} {} + +do_test 2.2 { + execsql { BEGIN } db3 + sqlite3_snapshot_open db3 main $snap +} {} + +sqlite3_snapshot_free $snap + finish_test From f9d1141a3b34e36cf26be87dbd199b036985b2d6 Mon Sep 17 00:00:00 2001 From: dan Date: Wed, 2 Oct 2024 11:11:00 +0000 Subject: [PATCH 2/2] Update docs for sqlite3_snapshot_get(). FossilOrigin-Name: 78c3892ab777a39406da8a9df84d0634397514e25512b0363a13bff3b8bc8925 --- manifest | 15 ++++++--------- manifest.uuid | 2 +- src/sqlite.h.in | 8 ++++++++ 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/manifest b/manifest index 83d6936a7d..7342f3b4a1 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C When\spossible,\savoid\staking\swal\sfile\sread-lock\s0\sin\ssqlite3_snapshot_get(). -D 2024-09-26T18:02:17.495 +C Update\sdocs\sfor\ssqlite3_snapshot_get(). +D 2024-10-02T11:11:00.069 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -769,7 +769,7 @@ F src/resolve.c 9750a281f7ba073b4e6da2be1a6c4071f5d841a7746c5fb3f70d6d793b6675ea F src/rowset.c 8432130e6c344b3401a8874c3cb49fefe6873fec593294de077afea2dce5ec97 F src/select.c 4b14337a2742f0c0beeba490e9a05507e9b4b12184b9cd12773501d08d48e3fe F src/shell.c.in 9b68a945f3aafc78eac1a256a4a588a9310dbc61a0cd60378c5b7a78f789af50 -F src/sqlite.h.in 77f55bd1978a04a14db211732f0a609077cf60ba4ccf9baf39988f508945419c +F src/sqlite.h.in 496f927cf2a687f313c6ac41c03e46f45c8f91c84d8f3ff6607aa9f2794e2ec3 F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8 F src/sqlite3ext.h 3f046c04ea3595d6bfda99b781926b17e672fd6d27da2ba6d8d8fc39981dcb54 F src/sqliteInt.h 5978cbb11becc3ce6471015d770d95f694ece06336c496f691df1b02460e9cd5 @@ -2213,11 +2213,8 @@ F vsixtest/vsixtest.tcl 6195aba1f12a5e10efc2b8c0009532167be5e301abe5b31385638080 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 9592b9ba3ad7a842cdd4c4010da278485a6fdec7e811bda01ebe640162a8c3b6 -R 8553e4183ac33940fe95918021cfe535 -T *branch * snapshot_get-locking -T *sym-snapshot_get-locking * -T -sym-trunk * +P 34b6ac3d76dbc6819778ec2a0f81cbcdcc0cd1a6303381d97f1c479e4ecdd132 +R a30055f90af028e040102b1e9655fa8f U dan -Z 2b5e37766517d26ed8f4d4495c7d929b +Z 068c609e961149f2eac59f5d7b1ce4b0 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 986575a33e..00ceed77c8 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -34b6ac3d76dbc6819778ec2a0f81cbcdcc0cd1a6303381d97f1c479e4ecdd132 +78c3892ab777a39406da8a9df84d0634397514e25512b0363a13bff3b8bc8925 diff --git a/src/sqlite.h.in b/src/sqlite.h.in index 5546793c94..013be20371 100644 --- a/src/sqlite.h.in +++ b/src/sqlite.h.in @@ -10539,6 +10539,14 @@ typedef struct sqlite3_snapshot { ** If there is not already a read-transaction open on schema S when ** this function is called, one is opened automatically. ** +** If a read-transaction is opened by this function, then it is guaranteed +** that the returned snapshot object may not be invalidated by a database +** writer or checkpointer until after the read-transaction is closed. This +** is not guaranteed if a read-transaction is already open when this +** function is called. In that case, any subsequent write or checkpoint +** operation on the database may invalidate the returned snapshot handle, +** even while the read-transaction remains open. +** ** The following must be true for this function to succeed. If any of ** the following statements are false when sqlite3_snapshot_get() is ** called, SQLITE_ERROR is returned. The final value of *P is undefined