MWS: SQL Thread

[hoelzro]

Per @Jermolene's suggestion, let's start a discussion around the usage of SQL (in particular, SQLite) within multi wiki server.

Existing comments:

#7915 (comment)

@Jermolene I took a quick look at the SQLite parts of this today - I'm going to look deeper, but I wanted to mention a few things I noticed in my first pass:

• I see you're using foreign keys - strangely enough, SQLite doesn't enforce foreign keys by default 😞 You need to turn them on via the foreign_keys pragma.
• I'd say the schema looks pretty good, but a few thoughts:
• I personally put NOT NULL on all of my columns unless I explicitly want to allow a NULL there, but that's just me!
• Since some fields like created, modified, type, and text will be on almost every tiddler, you might want to consider having nullable columns for those in the tiddlers table - but having them in the fields table does definitely simplify the logic! Another approach could be to just store extra fields in a JSON blob and use the JSON operators/functions to work with those.
• Since the number of distinct field names is probably going to be fairly small, you might want to consider having a field_names table in the future - it might be overkill at this point, though.
• One thing to be aware of: SQLite has this weird feature known as type affinity, which (for example) allows you to insert a string into a column typed as INTEGER. If you're targeting a more recent version of SQLite (3.37.0 and higher) and you don't want this behavior, you can use strict tables.
• When working with TEXT columns, you'll want to be mindful of collations, which can cause some comparison operations and things like distinctivness checking/ordering/grouping to be case-insensitive.

#7915 (comment)

Thanks @hoelzro much appreciated.

   I see you're using foreign keys - strangely enough, SQLite doesn't enforce foreign keys by default 😞 You need to turn them on via the [foreign_keys pragma](https://sqlite.org/pragma.html#pragma_foreign_keys).

Interesting. If they're not doing anything at the moment it does rather suggest that we don't need the functionality of foreign keys, and they should be removed.

   I personally put NOT NULL on all of my columns unless I explicitly want to allow a NULL there, but that's just me!

Good point, fixed in c26acfd

   Since some fields like created, modified, type, and text will be on almost every tiddler, you might want to consider having nullable columns for those in the tiddlers table - but having them in the fields table does definitely simplify the logic! Another approach could be to just store extra fields in a JSON blob and use the JSON operators/functions to work with those.

The earlier schema that I worked on over the summer in #7329 worked along exactly those lines. At some point I realised that the only way that we're going to get decent performance is by compiling filters into SQL queries, which I think would be much easier if we have a consistent storage format that treats all fields the same way.

   Since the number of distinct field names is probably going to be fairly small, you might want to consider having a field_names table in the future - it might be overkill at this point, though.

Yes, that would make sense, and if we're going to do it I'd be inclined to try to get it in now. Would you like to try your hand at a PR? It would be close to the edge of my SQL query skills.

   One thing to be aware of: SQLite has this weird feature known as [type affinity](https://www.sqlite.org/datatype3.html#type_affinity), which (for example) allows you to insert a string into a column typed as INTEGER. If you're targeting a more recent version of SQLite (3.37.0 and higher) and you don't want this behavior, you can use [strict tables](https://www.sqlite.org/stricttables.html).

I have read about that, and quite like the approach coming from a JS background. I notice that page doesn't mention a performance impact. I guess if there isn't an appreciable impact we might as well enable them to get the benefit of the error checking.

   When working with TEXT columns, you'll want to be mindful of [collations](https://sqlite.org/datatype3.html#collation), which can cause some comparison operations and things like distinctivness checking/ordering/grouping to be case-insensitive.

Yes indeed, back in #7329 I had to go to the trouble of creating a custom collation function to get the tests to pass.

I think we're going to end up needing a bunch of custom functions for SQLite, which I'm hoping we would write in C and compile with SQLite.

#7915 (comment)

One thing to be aware of: SQLite has this weird feature known as type affinity, which (for example) allows you to insert a string into a column typed as INTEGER. If you're targeting a more recent version of SQLite (3.37.0 and higher) and you don't want this behavior, you can use strict tables.

I have read about that, and quite like the approach coming from a JS background. I notice that page doesn't mention a performance impact. I guess if there isn't an appreciable impact we might as well enable them to get the benefit of the error checking.

After asking some question at talk I also did listen so some interviews of Richard Hipp the main author fo SQLite and he said that it's a feature and was intended to be that way. ... I think it was this interview, but I cannot find the timestamp atm.

So I think we should not prematurely try to "optimize" things, if we do not need to. -- Just my thoughts

#7915 (comment)

Yes, Dr. Hipp originally wrote SQLite for Tcl, and he wanted the type system to share Tcl's flexibility. I figured I'd mention it since it differs from most other SQL implementations, and in my opinion it's easier to start strict and relax things later than the other way around.

#7915 (comment)

Hi @hoelzro @pmario I think I've somewhat hardened my position to thinking that we should opt into strict tables now, keeping our options open for opting out later if there is a performance benefit. The benefit would be presumably that we'd get earlier and more useful error messages.

#7915 (comment)

Interesting. If they're not doing anything at the moment it does rather suggest that we don't need the functionality of foreign keys, and they should be removed.

Indeed - maybe they could be leveraged for some of the deletion logic by way of CASCADE, but the logic to clean up is already there, and it's nice and explicit in the code what's getting cleaned up when.

Good point, fixed in c26acfd

👍

You might want to add NOT NULL to the primary keys, too (unless AUTOINCREMENT implies that already - I'm not sure!), as well as to fields.tiddler_id.

Yes, that would make sense, and if we're going to do it I'd be inclined to try to get it in now. Would you like to try your hand at a PR? It would be close to the edge of my SQL query skills.

I could! I think @pmario has got a point with avoiding "premature optimization", though, and storing the field names directly is already working, so maybe what I'll do is trying to spin up a copy of my personal wiki using the SQLite database, do a little manual table manipulation, and then see what the storage impact would be.

#7915 (comment)

Indeed - maybe they could be leveraged for some of the deletion logic by way of CASCADE, but the logic to clean up is already there, and it's nice and explicit in the code what's getting cleaned up when.

My instinct is also to make the logic as explicit as possible, and avoid features that overlay invisible magic. But I'm still ignorant enough about the tradeoffs to be confident that we shouldn't be using those features in order to give SQLite the maximum opportunity to apply its own optimisations.

You might want to add NOT NULL to the primary keys, too (unless AUTOINCREMENT implies that already - I'm not sure!), as well as to fields.tiddler_id.

Ah, I wasn't sure about that either, but rather that row_id columns can't be NULL.

I could! I think @pmario has got a point with avoiding "premature optimization", though, and storing the field names directly is already working, so maybe what I'll do is trying to spin up a copy of my personal wiki using the SQLite database, do a little manual table manipulation, and then see what the storage impact would be.

That would be great, thank you.

[pmario]

@hoelzro please quote the comments here, so we can read them or there should be only 1 link

[hoelzro]

Ok - I was thinking of doing that earlier, but I only had a few minutes before I had to get my daughter to school and I wanted to get something up here 😞 I'll take care of it later!

[hoelzro]

I took some time to try MWS against my own wiki - I made some modifications to load my wiki's tiddlers instead of the docs/dev-docs and got it to populate a SQLite database. I ran these statements to create some tables to put field names into their own table:

create table field_names (id integer not null primary key, field_name text not null unique);
create table normalized_fields (tiddler_id integer, field_id integer not null, field_value text not null, unique(tiddler_id, field_id));
insert into field_names (field_name) select distinct field_name from fields;
insert into normalized_fields select f.tiddler_id, fn.id, f.field_value from fields as f inner join field_names as fn on fn.field_name = f.field_name;

Then I used the dbstat virtual table to get the sizes of the original fields table and the new tables:

select name, sum(pgsize) from dbstat where name in ('fields', 'field_names', 'normalized_fields') group by name

name	sum(pgsize)
field_names	4096
fields	16822272
normalized_fields	16441344

So the benefit, size-wise, at least, of normalizing the field names is pretty minimal - at least for my own wiki!

[Jermolene]

Interesting. My inclination is that we should still normalise the fields table for consistency with the way that we handle bags and recipes.

[hoelzro]

Another thing I noticed while working on the normalization idea - we might want MWS to leverage transactions more. Not only do they help from a consistency standpoint, but they can really speed up writes against the filesystem if those writes are batched. For example, when trying to load the docs/dev-docs tiddlers into SQLite, it takes 6-7 minutes to do so - but when I modify saveTiddlersFromPath to use a transaction, they load in 1-2 seconds!

[Jermolene]

Ooooh! That is pretty thrilling. Do you think it would be worth putting together a PR for that change? It will make our lives easier during testing.

[hoelzro]

Sure, I'll get on it! Another win in the realm of testing would be to conduct tests against an in-memory database; I can try to do that as well.

[hoelzro]

Done! #7991

[saqimtiaz]

In the sync thread Jeremy mentioned:

We need to discuss it over in the schema thread, but I am keen to update the schema to retain old versions of tiddlers. At that point, the database can be treated as a write only append store, which makes syncing significantly simpler.

Any thoughts around whether we would track history across renaming of tiddlers? It seems tricky considering how TiddlyWiki currently treats renames as the deletion of the old tiddler and the creation of a tiddler with the new title.

[Jermolene]

It would be very useful if we could meaningfully track history across renames but I also struggle to come up with a workable scheme that doesn't introduce a new GUID-type identifier for tiddlers.

I think that renames might be just the most common of a number of multi-step refactorings that wiki authors might perform. For example, it would be helpful to be able to see excising one tiddler from another, merging two tiddlers, splitting a tiddler etc.

So another approach might be to offer ways of annotating the history of a wiki, in the manner of git commit logs. The user could group arbitrary lower level operations and assign a description.

Also I should mention that the write only append store idea would also require that we extend the schema with explicit delete markers.

[hoelzro]

@Jermolene I saw your recent work using the node-sqlite3-wasm module, and the timings you posted there which showed a 100x slowdown using node-sqlite3-wasm over better-sqlite3. Do those numbers correspond to the mws-initial-load line printed when running npm test, or is there another way to gather these timings?

[pmario]

Using Windows 11 I got.

• better ~703ms (average 10 samples +- 10ms)
• wasm - ~1055ms (average 10 samples +- 4ms)

[Jermolene]

Thanks @pmario that's very helpful. It does look like @hoelzro's theory holds, and we need to investigate further if we can.

I would be very happy if node-sqlite3-wasm holds up. My preferred engine doesn't appear to exist yet: a Wasm build of SQLite3 that can run both in the browser and on Node.js. In the meantime, we're gaining experience of three different engines with this PR and #7521.

[hoelzro]

@Jermolene I think some performance tests would be wise as well; I was thinking that something using Playwright that measures performance closer to how users perceive it would be a good approach. IIRC you had started a repo playing around with this idea - is that right? I also think I remember @EvidentlyCube doing something in this space?

[hoelzro]

@Jermolene BTW, just to make sure I understand the broader context - the drive behind SQLite in the browser is to replace the this.tiddlers in-memory store with a SQLite-backed one, so that the code running TiddlyWiki's server components and the code running in the browser make use of the same logic, right? And the reasoning behind using SQLite for the server parts is to improve TiddlyWiki's management of larger amounts of data?

[Jermolene]

Hi @hoelzro I agree entirely about Playwright tests for browser performance, but in the case of MWS there still might be purely Node.js-based functionality that would be worth testing. See #7820 for the current Playwright-based CI.

@Jermolene BTW, just to make sure I understand the broader context - the drive behind SQLite in the browser is to replace the this.tiddlers in-memory store with a SQLite-backed one, so that the code running TiddlyWiki's server components and the code running in the browser make use of the same logic, right? And the reasoning behind using SQLite for the server parts is to improve TiddlyWiki's management of larger amounts of data?

Yes, that's right. The prime motivation for SQLite on the server is so that we are no longer limited by server memory. SQLite in the browser is for performance and flexibility. And as ever, we want as much unification between the client and server as possible.

[hoelzro]

@Jermolene I started on some store-level tests here - I don't know how much time I'll have for them over the next week or so, but I wanted to at least get something started.

[Jermolene]

Thanks @hoelzro that's great.

[hoelzro]

@Jermolene While working on some tests for SqlTiddlerStore.getBagTiddler this morning, I noticed that bag and revision are included in the result. I know those are kind of special fields - should their existence be enshrined as part of the public interface SqlTiddlerStore.getBagTiddler exposes, or should callers of SqlTiddlerStore.getBagTiddler not rely on their presence within the return value?

[Jermolene]

Hi @hoelzro the inclusion of the bag and revision fields is for backwards compatibility with the existing TiddlyWeb syncadaptor. I plan to rework the synchronisation completely (using SSE), at which point we won't need to use those fields.

[hoelzro]

Ok, thanks for the clarification! I'll make sure the tests don't expect those fields, then.

[hoelzro]

I just read this post from the folks over at Notion: How we sped up Notion in the browser with WASM SQLite

I found it interesting, considering the MWS work!

[Jermolene]

Thanks @hoelzro that does look interesting and I'll forward to reading, and reminds me that I have this recent Twitter thread in my queue that is also relevant:

https://x.com/meln1k/status/1813314113705062774?s=58

Full text of tweet

how I squeeze 60K RPS out of SQLite on a $5 VPS:

There are tons of tutorials on how to setup SQLite, and sometimes they contradict each other. Here is what worked for me.

1. Configuring PRAGMAs. We need to send the following PRAGMA commands right after opening the connection:

PRAGMA journal_mode = WAL;

• enables write-ahead log so that your reads do not block writes and vice-versa.

PRAGMA busy_timeout = 5000;

• sqlite will wait 5 seconds to obtain a lock before returning SQLITE_BUSY errors, which will significantly reduce them.

PRAGMA synchronous = NORMAL;

• sqlite will sync less frequently and be more performant, still safe to use because of the enabled WAL mode.

PRAGMA cache_size = -20000;

• negative number means kilobytes, in this case 20MB of memory for cache.

PRAGMA foreign_keys = true;

• because of historical reasons foreign keys are disabled by default, we should manually enable them.

PRAGMA temp_store = memory;

• moves temporary tables from disk into RAM, speeds up performance a lot.

Do NOT use cache=shared! Some tutorials recommend configuring it, but this is how you get nasty SQLITE_BUSY errors. It is disabled by default, so you don't have to do anything extra.

2. Use immediate transactions
   If you know that transaction can possibly do a write, always use BEGIN IMMEDIATE or you can a get SQLITE_BUSY error. Check your framework, you should be able to set this at the connection level.

3. Open two connection pools
   Another trick is to open 2 connection pools, one for reads only and another for reads/writes. Set the connection limit of write pool to 1, and the connection limit of the read pool to some reasonably high number, e.g. number of your CPU cores.

4. Bonus: how I configure sqlite with Go
   Here is the code in go I use to configure the sqlite connections:

func SQLiteDbString(file string, readonly bool) string {

connectionParams := make(url.Values)
connectionParams.Add("_journal_mode", "WAL")
connectionParams.Add("_busy_timeout", "5000")
connectionParams.Add("_synchronous", "NORMAL")
connectionParams.Add("_cache_size", "-20000")
connectionParams.Add("_foreign_keys", "true")
if readonly {
connectionParams.Add("mode", "ro")
} else {
connectionParams.Add("_txlock", "IMMEDIATE")
connectionParams.Add("mode", "rwc")
}

return "file:" + file + "?" + connectionParams.Encode()
}

func OpenSqliteDatabase(file string, readonly bool) (*sql.DB, error) {

dbString := SQLiteDbString(file, readonly)
db, err := sql .Open("sqlite3", dbString)

pragmasToSet := []string{
"temp_store=memory",
}

for _, pragma := range pragmasToSet {
_, err = db.Exec("PRAGMA " + pragma + ";")
if err != nil {
return nil, err
}
}

if readonly {
db.SetMaxOpenConns(max(4, runtime.NumCPU()))
} else {
db.SetMaxOpenConns(1)
}

return db, nil
}