From: stephan Date: Sun, 23 Nov 2025 22:13:42 +0000 (+0000) Subject: More work on kvvfs v2. Db imports work, in the sense that the storage is properly... X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=853335af499dfbff0a2f27fa1618577d2049959b;p=thirdparty%2Fsqlite.git More work on kvvfs v2. Db imports work, in the sense that the storage is properly replaced, but the native side is not yet able to recover from that (and how to make it able to do so is not clear). FossilOrigin-Name: 4857c9d2fe428c19319244bf0589eaf93c124f020a633d6b7d40d35aaf969d24 --- diff --git a/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js b/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js index bf6d6b4e65..e0334a6ec3 100644 --- a/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js +++ b/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js @@ -144,7 +144,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ if( !hop(this.#map, k) ){ this.#keys = null; } - this.#map[k]=v; + this.#map[k] = v; } removeItem(k){ @@ -171,18 +171,18 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ that concurrent active xOpen()s on a given name, and within a given thread, use the same storage object. */ - cache.jzClassToStorage = Object.assign(Object.create(null),{ + cache.storagePool = Object.assign(Object.create(null),{ /* Start off with mappings for well-known names. */ localThread: { refc: 3/*never reaches 0*/, - s: new KVVfsStorage, + storage: new KVVfsStorage, files: [/*KVVfsFile instances currently using this storage*/] } }); if( globalThis.localStorage instanceof globalThis.Storage ){ - cache.jzClassToStorage.local = { + cache.storagePool.local = { refc: 3/*never reaches 0*/, - s: globalThis.localStorage, + storage: globalThis.localStorage, /* This is the storage prefix used for kvvfs keys. It is "kvvfs-STORAGENAME-" for local/session storage and an empty string for transient storage. local/session storage must @@ -196,31 +196,41 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ }; } if( globalThis.sessionStorage instanceof globalThis.Storage ){ - cache.jzClassToStorage.session = { + cache.storagePool.session = { refc: 3/*never reaches 0*/, - s: globalThis.sessionStorage, + storage: globalThis.sessionStorage, keyPrefix: "kvvfs-session-", files: [] } } - for(const k of Object.keys(cache.jzClassToStorage)){ + for(const k of Object.keys(cache.storagePool)){ /* Journals in kvvfs are are stored as individual records within their Storage-ish object, named "KEYPREFIXjrnl" (see above re. KEYPREFIX). We always map the db and its journal to the same Storage object. */ - const orig = cache.jzClassToStorage[k]; + const orig = cache.storagePool[k]; orig.jzClass = k; - cache.jzClassToStorage[k+'-journal'] = orig; + cache.storagePool[k+'-journal'] = orig; } + /** + Returns the storage object mapped to the given string zClass + (C-string pointer or JS string). + */ + const storageForZClass = (zClass)=> + 'string'===typeof zClass + ? cache.storagePool[zClass] + : cache.storagePool[wasm.cstrToJs(zClass)]; + /** True if v is one of the special persistant Storage objects. */ const kvvfsIsPersistentName = (v)=>'local'===v || 'session'===v; + /** - Keys in kvvfs have a prefix of "kvvfs-NAME", where NAME is the db - name. This key is redundant in JS but it's how kvvfs works (it - saves each key to a separate file, so needs a - distinct-per-db-name namespace). We retain this prefix in 'local' - and 'session' storage for backwards compatibility but elide them - from "v2" transient storage, where they're superfluous. + Keys in kvvfs have a prefix of "kvvfs-NAME-", where NAME is the + db name. This key is redundant in JS but it's how kvvfs works (it + saves each key to a separate file, so needs a distinct namespace + per data source name). We retain this prefix in 'local' and + 'session' storage for backwards compatibility but elide them from + "v2" storage, where they're superfluous. */ const kvvfsKeyPrefix = (v)=>kvvfsIsPersistentName(v) ? 'kvvfs-'+v+'-' : ''; @@ -234,26 +244,31 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ .prefix = the key prefix for this storage. Typically ("kvvfs-"+which) for local/sessionStorage and "" for transient storage. (The former is historical, retained for backwards - compatibility.) If which is falsy then the prefix is "kvvfs-" for - backwards compatibility (it will match keys for both local- and - sessionStorage, but not transient storage). + compatibility.) If `which` is falsy then the prefix is "kvvfs-" + for backwards compatibility (it will match keys for both local- + and sessionStorage, but not transient storage or non-kvvfs keys + in the persistent storages). .stores = [ array of Storage-like objects ]. Will only have >1 element if which is falsy, in which case it contains (if called from the main thread) localStorage and sessionStorage. It will be empty if no mapping is found or those objects are not available in the current environment (e.g. a worker thread). + + .cts = the underlying storagePool entry. This will only be set + of which is not empty. */ const kvvfsWhich = function callee(which){ const rc = Object.assign(Object.create(null),{ stores: [] }); if( which ){ - const s = cache.jzClassToStorage[which]; + const s = storageForZClass(which); if( s ){ //debug("kvvfsWhich",s.jzClass,rc.prefix, s.s); rc.prefix = s.keyPrefix ?? ''; - rc.stores.push(s.s); + rc.stores.push(s.storage); + rc.cts = s; }else{ rc.prefix = undefined; } @@ -291,6 +306,25 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ }; //#endif nope + /** + Expects an object from the storagePool map. The $szPage and + $szDb members of each store.files entry is set to -1 in an attempt + to trigger those values to reload. + */ + const alertFilesToReload = (store)=>{ + try{ + for( const f of store.files ){ + // FIXME: we need to use one of the C APIs for this, maybe an + // fcntl. + f.$szPage = -1; + f.$szDb = -1n + } + }catch(e){ + error("alertFilesToReload()",store,e); + throw e; + } + }; + /** Clears all storage used by the kvvfs DB backend, deleting any DB(s) stored there. @@ -325,6 +359,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ toRm.forEach((kk)=>s.removeItem(kk)); rc += toRm.length; }); + if( store.cts ) alertFilesToReload(store.cts); return rc; }; @@ -379,24 +414,16 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ TODO: prealloc a buffer on demand for this. We know its max size from kvvfsMethods.$nKeySize. */ const pstack = wasm.pstack; - /** - Returns the storage object mapped to the given string zClass - (C-string pointer or JS string). - */ - const storageForZClass = (zClass)=>{ - return 'string'===typeof zClass - ? cache.jzClassToStorage[zClass] - : cache.jzClassToStorage[wasm.cstrToJs(zClass)]; - }; /** sqlite3_file pointers => objects, each of which has: - .s = Storage object + .file = KVVfsFile instance - .f = KVVfsFile instance + .jzClass = JS-string form of f.$zClass - .n = JS-string form of f.$zClass + .storage = Storage object. It is shared between a db and its + journal. */ const pFileHandles = new Map(); @@ -471,8 +498,9 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ if( !store ) return -1; const zXKey = keyForStorage(store, zClass, zKey); if(!zXKey) return -3/*OOM*/; + const jXKey = wasm.cstrToJs(zXKey); //debug("xRcrdRead zXKey", jzClass, wasm.cstrToJs(zXKey), store ); - const jV = store.s.getItem(wasm.cstrToJs(zXKey)); + const jV = store.storage.getItem(jXKey); if(null===jV) return -1; const nV = jV.length /* We are relying 100% on v being ** ASCII so that jV.length is equal @@ -485,7 +513,12 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ wasm.poke(zBuf, 0); return nV; } - util.assert( nBuf>nV, "xRcrdRead Input buffer is too small" ); + if( nBuf+1v!==h.f); + const s = storageForZClass(h.jzClass); + s.files = s.files.filter((v)=>v!==h.file); if( 0===--s.refc ){ - const other = h.f.$isJournal - ? h.n.replace(cache.rxJournalSuffix,'') - : h.n+'-journal'; - debug("cleaning up storage handles [", h.n, other,"]",s); - delete cache.jzClassToStorage[h.n]; - delete cache.jzClassToStorage[other]; + const other = h.file.$isJournal + ? h.jzClass.replace(cache.rxJournalSuffix,'') + : h.jzClass+'-journal'; + debug("cleaning up storage handles [", h.jzClass, other,"]",s); + delete cache.storagePool[h.jzClass]; + delete cache.storagePool[other]; if( !sqlite3.__isUnderTest ){ - delete s.s; + delete s.storage; delete s.refc; } } - originalIoMethods(h.f).xClose(pFile); - h.f.dispose(); + originalIoMethods(h.file).xClose(pFile); + h.file.dispose(); }else{ /* Can happen if xOpen fails */ } @@ -926,12 +952,13 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ */ jdb.prototype.exportToObject = function(includeJournal=true){ this.affirmOpen(); - const store = cache.jzClassToStorage[this.affirmOpen().filename]; + const store = storageForZClass(this.affirmOpen().filename); if( !store ){ util.toss3(capi.SQLITE_ERROR,"kvvfs db '", this.filename,"' has no storage object."); } - const s = store.s; + debug("store=",store); + const s = store.storage; const rc = Object.assign(Object.create(null),{ name: this.filename, timestamp: Date.now(), @@ -974,6 +1001,10 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ }; /** + Does not yet work: it imports the db but the handle cannot + read from the modified-underneath-it storage yet. We have to + figure out how to get the file to re-read the db size. + The counterpart of exportToObject(). Its argument must be the result of exportToObject(). @@ -986,51 +1017,68 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ - This db is closed. + - A transaction is active. + + - If any statements are open. + + - Malformed input object. + - Other handles to the same storage object are opened. Performing this page-by-page import would invoke undefined behavior on them. - - A transaction is active. - Those are the error case it can easily cover. The room for undefined behavior in wiping a db's storage out from under it is a whole other potential minefield. If it throws after starting the input then it clears the storage before returning, to avoid leaving the db in an - undefined state. It has no inherent error conditions during the - input phase beyond out-of-memory but it may throw for any of - the above-listed conditions before reaching that step, in which - case the db is not modified. + undefined state. It may throw for any of the above-listed + conditions before reaching that step, in which case the db is + not modified. */ jdb.prototype.importFromObject = function(exp){ this.affirmOpen(); if( !exp?.timestamp || !exp.name || undefined===exp.size + || exp.size<0 || exp.size>=0x7fffffff || !Array.isArray(exp.pages) ){ util.toss3(capi.SQLITE_MISUSE, "Malformed export object."); - } - if( s.files.length>1 ){ - util.toss3(capi.SQLITE_IOERR_ACCESS, - "Cannot import a db when multiple handles to it", - "are opened."); + }else if( capi.sqlite3_next_stmt(this.pointer, null) ){ + util.toss3(capi.SQLITE_MISUSE, + "Cannot import when statements are active."); }else if( capi.sqlite3_txn_state(this.pointer, null)>0 ){ util.toss3(capi.SQLITE_MISUSE, "Cannot import the db while a transaction is active."); } - warn("importFromObject() is incomplete"); + //warn("importFromObject() is incomplete"); const store = kvvfsWhich(this.filename); - util.assert(store?.s, "Somehow missing a storage object for", this.filename); + if( !store?.cts ){ + util.toss3(capi.SQLITE_ERROR, + "Somehow missing a storage object for", this.filename); + }else if( store.cts.files.length>1 ){ + util.toss3(capi.SQLITE_IOERR_ACCESS, + "Cannot import a db when multiple handles to it", + "are opened."); + } + //debug("Importing store",store.cts.files.length, store); + //debug("object to import:",exp); const keyPrefix = kvvfsKeyPrefix(this.filename); this.clearStorage(); try{ - s.files.forEach((f)=>f.$szPage = $f.$szDb = -1) /* Force the native KVVfsFile instances to re-read the db and page size. */; + const s = store.cts.storage; s.setItem(keyPrefix+'sz', exp.size); if( exp.journal ) s.setItem(keyPrefix+'jrnl', exp.journal); - exp.pages.forEach((v,ndx)=>s.setItem(keyPrefix+(ndx+1))); + exp.pages.forEach((v,ndx)=>s.setItem(keyPrefix+(ndx+1), v)); + //debug("imported:",this.exportToObject()); + //this.exec("pragma page_size"); + for( const f of store.cts.files ){ + f.$szDb = exp.size; + f.$szPage = -1; + } }catch(e){ this.clearStorage(); throw e; diff --git a/ext/wasm/tester1.c-pp.js b/ext/wasm/tester1.c-pp.js index 0a007573b6..4393a35c2f 100644 --- a/ext/wasm/tester1.c-pp.js +++ b/ext/wasm/tester1.c-pp.js @@ -2994,18 +2994,19 @@ globalThis.sqlite3InitModule = sqlite3InitModule; 'create table kvvfs(a);', 'insert into kvvfs(a) values(1),(2),(3)' ]; + const sqlCount = 'select count(*) from kvvfs'; try { db = new JDb(filename); db.clearStorage(/*must not throw*/); db.exec(sqlSetup); - T.assert(3 === db.selectValue('select count(*) from kvvfs')); + T.assert(3 === db.selectValue(sqlCount)); - const duo = new JDb(filename); + duo = new JDb(filename); duo.exec('insert into kvvfs(a) values(4),(5),(6)'); - T.assert(6 === db.selectValue('select count(*) from kvvfs')); + T.assert(6 === db.selectValue(sqlCount)); console.debug("duo.exportToObject()",duo.exportToObject(false)); db.close(); - T.assert(6 === duo.selectValue('select count(*) from kvvfs')); + T.assert(6 === duo.selectValue(sqlCount)); duo.close(); T.mustThrowMatching(function(){ let ddb = new JDb(filename); @@ -3013,6 +3014,19 @@ globalThis.sqlite3InitModule = sqlite3InitModule; finally{ddb.close()} }, /.*no such table: kvvfs.*/); + if( 1 ){ + // The db does not yet work after an import. + duo = new JDb(filename); + duo.exec(sqlSetup); + const exp = duo.exportToObject(); + duo.exec('insert into kvvfs(a) values(4),(5),(6)'); + T.assert(6 === duo.selectValue(sqlCount)); + duo.importFromObject(exp); + console.debug("Before/after exports:",exp, duo.exportToObject()); + // FIXME: db access does not work beyond that after an import. + //T.assert(3 === duo.selectValue(sqlCount)); + } + /* TODO: more advanced concurrent use tests, e.g. looping over a query in one connection while writing from @@ -3026,7 +3040,7 @@ globalThis.sqlite3InitModule = sqlite3InitModule; duo?.close?.(); } } - }/*transient kvvfs*/) + }/*concurrent transient kvvfs*/) //#if enable-see .t({ name: 'kvvfs SEE encryption in sessionStorage', diff --git a/manifest b/manifest index b25708a529..579039b81b 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Add\sthe\slong-missing\ssqlite3_next_stmt()\sbinding\sto\sJS/WASM. -D 2025-11-23T22:09:16.783 +C More\swork\son\skvvfs\sv2.\sDb\simports\swork,\sin\sthe\ssense\sthat\sthe\sstorage\sis\sproperly\sreplaced,\sbut\sthe\snative\sside\sis\snot\syet\sable\sto\srecover\sfrom\sthat\s(and\show\sto\smake\sit\sable\sto\sdo\sso\sis\snot\sclear). +D 2025-11-23T22:13:42.828 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 1aaea0e80bbc992a365088b2f0c7ddb8fe9ec61d539fd75bc457db879e7a7eaa +F ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js cd88fa458519bb48f5113430402fe250fcf87bde361f76d1782945c85671207a F ext/wasm/api/sqlite3-vfs-opfs-sahpool.c-pp.js 26cb41d5a62f46a106b6371eb00fef02de3cdbfaa51338ba087a45f53028e0d0 F ext/wasm/api/sqlite3-vfs-opfs.c-pp.js 88ce2078267a2d1af57525a32d896295f4a8db7664de0e17e82dc9ff006ed8d3 F ext/wasm/api/sqlite3-vtab-helper.c-pp.js 9097074724172e31e56ce20ccd7482259cf72a76124213cbc9469d757676da86 @@ -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 43e48fd869532d8a335cb040e7987f8c4ba1d117686b492c581c0a1252c6a7ac +F ext/wasm/tester1.c-pp.js 420abc664cb6e5077f4822e66a200b95ccdbfd06610a8cbfe1b0cc3da7619562 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 @@ -2178,8 +2178,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 49db59aa9c74e49d878adc8671b0d32db8f1f898bde29d046ce0e368d8987868 -R 1b44ce6847f92a3ecf9fc6d286fc599a +P 53a99ff4ed5ef5f8620bf324a4f7a1d0812f5c80311228eb820039430ca04bd5 +R 9b53545349401af43f90d2908592ae9f U stephan -Z 7cec59ddbe382449db8322defecec3d4 +Z f328a4b235c18864031309f36fc54dd3 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 6606609ab3..dcb805657d 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -53a99ff4ed5ef5f8620bf324a4f7a1d0812f5c80311228eb820039430ca04bd5 +4857c9d2fe428c19319244bf0589eaf93c124f020a633d6b7d40d35aaf969d24