From 6993f52d944decd9fbf1bee4393e99e7532e7a43 Mon Sep 17 00:00:00 2001 From: stephan Date: Tue, 2 Dec 2025 16:29:28 +0000 Subject: [PATCH] Consolidate kvvfs's page size workaround into a central flag and add some logging to facilitate testing and debugging. FossilOrigin-Name: 8d683130d2f2aaf6300a8c858802eb671e831ed520621af59c9e830e87a9d753 --- ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js | 94 ++++++++++++++------------ ext/wasm/tester1.c-pp.js | 39 +++++------ manifest | 14 ++-- manifest.uuid | 2 +- 4 files changed, 74 insertions(+), 75 deletions(-) diff --git a/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js b/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js index 499c1448d6..caada9c31b 100644 --- a/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js +++ b/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js @@ -119,14 +119,6 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ Most of the VFS-internal state. */ const cache = Object.assign(Object.create(null),{ - /** - Bug: kvvfs is currently fixed at a page size of 8kb because - changing it is leaving corrupted dbs for reasons as yet - unknown. This value is used in certain validation to ensure - that if that limitation is lifted, it will break potentially - affected code. - */ - fixedPageSize: 8192, /** Regex matching journal file names. */ rxJournalSuffix: /-journal$/, /** Frequently-used C-string. */ @@ -649,15 +641,23 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ some cases. */ const kvvfs = sqlite3.kvvfs = Object.create(null); + const kvvfsInternal = Object.assign(Object.create(null),{ + pFileHandles, + cache, + storageForZClass, + KVVfsStorage, + /** + BUG: changing to a page size other than the default, + then vacuuming, corrupts the db. As a workaround, + until this is resolved, we forcibly disable + (pragma page_size=...) changes. + */ + disablePageSizeChange: true + }); if( sqlite3.__isUnderTest ){ /* For inspection via the dev tools console. */ - sqlite3.kvvfs.test = Object.assign(Object.create(null),{ - pFileHandles, - cache, - storageForZClass, - KVVfsStorage - }); - sqlite3.kvvfs.log = Object.assign(Object.create(null),{ + kvvfs.internal = kvvfsInternal; + kvvfs.log = Object.assign(Object.create(null),{ xOpen: false, xClose: false, xWrite: false, @@ -952,7 +952,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ if( --s.refc<=0 && s.deleteAtRefc0 ){ deleteStorage(s); } - originalIoMethods(h.file).xClose(pFile); + originalMethods.ioDb/*same for journals*/.xClose(pFile); h.file.dispose(); s.listeners && notifyListeners('close', s, s.files.length); }else{ @@ -970,35 +970,36 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ try{ const h = pFileHandles.get(pFile); util.assert(h, "Missing KVVfsFile handle"); - kvvfs?.log?.xFileControl && debug("xFileControl",h,opId); - if( 0 && opId===capi.SQLITE_FCNTL_PRAGMA ){ + kvvfs?.log?.xFileControl && debug("xFileControl",h,'op =',opId); + if( opId===capi.SQLITE_FCNTL_PRAGMA + && kvvfs.internal.disablePageSizeChange ){ /* pArg== length-3 (char**) */ + //const argv = wasm.cArgvToJs(3, pArg); // the easy way const zName = wasm.peekPtr(wasm.ptr.add(pArg, wasm.ptr.size)); - //const argv = wasm.cArgvToJs(3, pArg); if( "page_size"===wasm.cstrToJs(zName) ){ - //debug("xFileControl pragma",wasm.cstrToJs(zName)); + kvvfs?.log?.xFileControl + && debug("xFileControl pragma",wasm.cstrToJs(zName)); const zVal = wasm.peekPtr(wasm.ptr.add(pArg, 2*wasm.ptr.size)); if( zVal ){ /* Without this, pragma page_size=N; followed by a vacuum breaks the db. With this, it continues working but does not actually change the page size. */ - warn("xFileControl pragma", h, - "NOT setting page size to", wasm.cstrToJs(zVal)); + kvvfs?.log?.xFileControl + && warn("xFileControl pragma", h, + "NOT setting page size to", wasm.cstrToJs(zVal)); h.file.$szPage = -1; - if( h.file.$aJournal ){ - warn("This file has a journal of", h.file.$nJrnl, "bytes"); - } - return 0/*corrupts, but not until much later? capi.SQLITE_NOTFOUND*/; - }else if( 0 && h.file.$szPage>0 ){ - warn("xFileControl", h, "getting page size",h.file.$szPage); - wasm.pokePtr(pArg, wasm.allocCString(""+h.file.$szPage)); - // memory now owned by sqlite. + return 0/*corrupts: capi.SQLITE_NOTFOUND*/; + }else if( h.file.$szPage>0 ){ + kvvfs?.log?.xFileControl && + warn("xFileControl", h, "getting page size",h.file.$szPage); + wasm.pokePtr(pArg, wasm.allocCString(""+h.file.$szPage) + /* memory now owned by the library */); return 0;//capi.SQLITE_NOTFOUND; } } } - const rc = originalIoMethods(h.file).xFileControl(pFile, opId, pArg); + const rc = originalMethods.ioDb.xFileControl(pFile, opId, pArg); if( 0==rc && capi.SQLITE_FCNTL_SYNC===opId ){ h.store.listeners && notifyListeners('sync', h.store, false); } @@ -1025,13 +1026,17 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ }, //#if not nope + // We override xRead/xWrite only for logging/debugging. They + // should otherwise be disabled (it's faster that way). xRead: function(pFile,pTgt,n,iOff64){ cache.popError(); try{ - const h = pFileHandles.get(pFile); - util.assert(h, "Missing KVVfsFile handle"); - kvvfs?.log?.xRead && debug("xRead", n, iOff64, h); - return originalIoMethods(h.file).xRead(pFile, pTgt, n, iOff64); + if( kvvfs?.log?.xRead ){ + const h = pFileHandles.get(pFile); + util.assert(h, "Missing KVVfsFile handle"); + debug("xRead", n, iOff64, h); + } + return originalMethods.ioDb.xRead(pFile, pTgt, n, iOff64); }catch(e){ error("xRead",e); return cache.setError(e); @@ -1040,12 +1045,14 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ xWrite: function(pFile,pSrc,n,iOff64){ cache.popError(); try{ - const h = pFileHandles.get(pFile); - util.assert(h, "Missing KVVfsFile handle"); - kvvfs?.log?.xWrite && debug("xWrite", n, iOff64, h); - return originalIoMethods(h.file).xWrite(pFile, pSrc, n, iOff64); + if( kvvfs?.log?.xWrite ){ + const h = pFileHandles.get(pFile); + util.assert(h, "Missing KVVfsFile handle"); + debug("xWrite", n, iOff64, h); + } + return originalMethods.ioDb.xWrite(pFile, pSrc, n, iOff64); }catch(e){ - error("xRead",e); + error("xWrite",e); return cache.setError(e); } }, @@ -1364,9 +1371,6 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ const nDec = kvvfsDecode( z, zDec, cache.buffer.n ); - if( cache.fixedPageSize !== nDec ){ - util.toss3(capi.SQLITE_ERROR,"Unexpected decoded page size:",nDec); - } //debug("Decoded",nDec,"page bytes"); pages[kk] = heap.slice(Number(zDec), wasm.ptr.addn(zDec, nDec)); }else{ @@ -1420,7 +1424,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ toss3(capi.SQLITE_MISUSE, "Malformed export object."); }else if( !exp.size || (exp.size !== (exp.size | 0)) - || (exp.size % cache.fixedPageSize) + //|| (exp.size % cache.fixedPageSize) || exp.size>=0x7fffffff ){ toss3(capi.SQLITE_RANGE, "Invalid db size: "+exp.size); } @@ -1461,7 +1465,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ //debug("pages",exp.pages); exp.pages.forEach((u,ndx)=>{ const n = u.length; - if( cache.fixedPageSize !== n ){ + if( 0 && cache.fixedPageSize !== n ){ util.toss3(capi.SQLITE_RANGE,"Unexpected page size:", n); } zEnc ??= cache.memBuffer(1); diff --git a/ext/wasm/tester1.c-pp.js b/ext/wasm/tester1.c-pp.js index 028088518e..e0eb6673ee 100644 --- a/ext/wasm/tester1.c-pp.js +++ b/ext/wasm/tester1.c-pp.js @@ -2943,7 +2943,7 @@ globalThis.sqlite3InitModule = sqlite3InitModule; const JDb = sqlite3.oo1.JsStorageDb; const pVfs = capi.sqlite3_vfs_find('kvvfs'); T.assert(looksLikePtr(pVfs)); - let x = sqlite3.kvvfs.test.storageForZClass('session'); + let x = sqlite3.kvvfs.internal.storageForZClass('session'); T.assert( 0 === x.files.length ) .assert( globalThis.sessionStorage===x.storage ) .assert( 'kvvfs-session-' === x.keyPrefix ); @@ -3182,7 +3182,10 @@ globalThis.sqlite3InitModule = sqlite3InitModule; if( 1 ){ error("Begin vacuum/page size test..."); - const pageSize = 1024 * 8; + const defaultPageSize = 1024 * 8 /* build-time default */; + const pageSize = 0 + ? defaultPageSize + : 1024 * 16 /* any valid value other than the default */; if( 0 ){ debug("Export before vacuum", exportDb(expOpt)); debug("page size before vacuum", @@ -3190,36 +3193,28 @@ globalThis.sqlite3InitModule = sqlite3InitModule; "select page_size from pragma_page_size()" )); } + kvvfs.log.xFileControl = true; db.exec([ "BEGIN;", "insert into kvvfs(a) values(randomblob(16000/*>pg size*/));", "COMMIT;", "delete from kvvfs where octet_length(a)>100;", - //"pragma page_size="+pageSize+";", + "pragma page_size="+pageSize+";", "vacuum;", "select 1;" ]); - expectRows; - if( 0 ){ - debug("page size after", - db.selectArray( - "select page_size from pragma_page_size()" - )); - } + const expectPageSize = kvvfs.internal.disablePageSizeChange + ? defaultPageSize + : pageSize; + const gotPageSize = db.selectValue( + "select page_size from pragma_page_size()" + ); + T.assert(+gotPageSize === expectPageSize, + "Expecting page size",expectPageSize, + "got",gotPageSize); T.assert(expectRows === duo.selectValue(sqlCount), "Unexpected record count."); - T.assert( 8192 == db.selectValue( - /* kvvfs actively resists attempts to change the page size - because _not_ doing so causes a vacuum to corrupt - the db. */ - "select page_size from pragma_page_size()" - ) ); - if( 0 ){ - // It's stuck at 8k - T.assert(pageSize == duo.selectValue("pragma page_size"), - "Unexpected page size."); - } - // It fails at this point for non-8k sizes + kvvfs.log.xFileControl = false; T.assert(expectRows === duo.selectValue(sqlCount), "Unexpected record count."); exp = exportDb(expOpt); diff --git a/manifest b/manifest index af64acc0c0..5b26201fd3 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Add\smore\slogging\sto\skvvfs\sto\stry\sto\strace\sdown\swhy\sit\scannot\srecover\sfrom\sa\spage\ssize\schange. -D 2025-12-02T15:47:32.634 +C Consolidate\skvvfs's\spage\ssize\sworkaround\sinto\sa\scentral\sflag\sand\sadd\ssome\slogging\sto\sfacilitate\stesting\sand\sdebugging. +D 2025-12-02T16:29:28.643 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea @@ -600,7 +600,7 @@ F ext/wasm/api/sqlite3-api-worker1.c-pp.js 1041dd645e8e821c082b628cd8d9acf70c667 F ext/wasm/api/sqlite3-license-version-header.js 0c807a421f0187e778dc1078f10d2994b915123c1223fe752b60afdcd1263f89 F ext/wasm/api/sqlite3-opfs-async-proxy.js 9654b565b346dc609b75d15337f20acfa7af7d9d558da1afeb9b6d8eaa404966 F ext/wasm/api/sqlite3-vfs-helper.c-pp.js 3f828cc66758acb40e9c5b4dcfd87fd478a14c8fb7f0630264e6c7fa0e57515d -F ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js 3146bcb9830ed41027909305d54f41505724606409dc443edabb2f6f5ce65103 +F ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js 08c8b8546c89f2e5bfa52ccc659db9d4dc01955cf4407407cb1e123c47b66e30 F ext/wasm/api/sqlite3-vfs-opfs-sahpool.c-pp.js a2eea6442556867b589e04107796c6e1d04a472219529eeb45b7cd221d7d048b F ext/wasm/api/sqlite3-vfs-opfs.c-pp.js 88ce2078267a2d1af57525a32d896295f4a8db7664de0e17e82dc9ff006ed8d3 F ext/wasm/api/sqlite3-vtab-helper.c-pp.js 366596d8ff73d4cefb938bbe95bc839d503c3fab6c8335ce4bf52f0d8a7dee81 @@ -647,7 +647,7 @@ F ext/wasm/test-opfs-vfs.html 1f2d672f3f3fce810dfd48a8d56914aba22e45c6834e262555 F ext/wasm/test-opfs-vfs.js 1618670e466f424aa289859fe0ec8ded223e42e9e69b5c851f809baaaca1a00c F ext/wasm/tester1-worker.c-pp.html 0e432ec2c0d99cd470484337066e8d27e7aee4641d97115338f7d962bf7b081a F ext/wasm/tester1.c-pp.html 52d88fe2c6f21a046030a36410b4839b632f4424028197a45a3d5669ea724ddb -F ext/wasm/tester1.c-pp.js 57d465d0313178524e1749fbd7f651383b210a245d3815eb9cd4614daa58095e +F ext/wasm/tester1.c-pp.js e34021c29b7bd0cdcba9cb5fa0ee6efc9509f46b0c97120698ddbc388245c086 F ext/wasm/tests/opfs/concurrency/index.html 657578a6e9ce1e9b8be951549ed93a6a471f4520a99e5b545928668f4285fb5e F ext/wasm/tests/opfs/concurrency/test.js d08889a5bb6e61937d0b8cbb78c9efbefbf65ad09f510589c779b7cc6a803a88 F ext/wasm/tests/opfs/concurrency/worker.js 0a8c1a3e6ebb38aabbee24f122693f1fb29d599948915c76906681bb7da1d3d2 @@ -2180,8 +2180,8 @@ F tool/version-info.c 33d0390ef484b3b1cb685d59362be891ea162123cea181cb8e6d2cf6dd F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee87c1b31a7 F tool/warnings.sh d924598cf2f55a4ecbc2aeb055c10bd5f48114793e7ba25f9585435da29e7e98 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f -P 09e9255828ed6a7ccbe18f701f4a88b859cfdbd1ddca7ac5dac09744542c25fa -R 26de08b1230e759818946db5817cbd59 +P 0bf0b8a98a2cc5128aa0e510ef2fe411a6859ce807d6159175f5eaf3bc14183b +R bdf2c95d466c8e43e29937d2fd3fb9ba U stephan -Z a2609685c3f2eaa0fbe4894a2be65512 +Z b42a739c26e9a29ed2bf89c88b59d1d6 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 1521b54239..70ed40ac4a 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -0bf0b8a98a2cc5128aa0e510ef2fe411a6859ce807d6159175f5eaf3bc14183b +8d683130d2f2aaf6300a8c858802eb671e831ed520621af59c9e830e87a9d753 -- 2.47.3