SQLite backing store

[dbr]

Very WIP - still to implement the methods regarding the working set, operations, and base version

For first pass I'm just using SQLite table as dumb key-value store:

• there's a tasks column with uuid and data
• data is a serde_json dump of the task data

Reason for using serde_json is mostly that it was already there, and JSON is reasonably simple/fast. Most probably not as fast as Msgpack which was used with LMDB, but quite possibly adequate?

There's no reason this couldn't be stored in a more SQL'y manner, with:

• tasks column with just UUID column
• task data table with task_id, key and value

However this feels like it might be a little excessive, and may end up being slower since the task-data table would end up being much larger

Other misc thoughts:

• The UUID is using "rusqlite" built-in handling, and stores the UUID a BLOB. It might make the database easier to work with manually if the UUID were stored as a string (via Uuid::to_string), but not sure this is really a concern.
• I'm using rusqlite as I've used it before, and it exposes a Transaction type which made the implementation simpler. The sqlite project does not. Both have been around for >5 years and still 0.x 😢

[djmitche]

This is looking good! People shouldn't be accessing the DB "manually", so I don't think there's any need to make that easier. JSON is fine, but storing the data with msgpack would be A-OK too :)

(The reason being, the relationship between the tasks table and the operations table is rather precise, and Bad Things will happen if those get out of sync)

rusqlite seems like a good map to the needs of this app, particularly with its Transaction type.

[dbr]

I've implemented the last of the methods and this seems to be functioning

Still to do:

1. Need some better (i.e non-unwrap) error handling for a few of the deserialization calls (think I just need to use rusqlite's serde feature properly)
2. Quick check over changes to find any obvious errors and see if I can neaten anything up

I'm also pretty tempted to store the UUID's as strings - for debugging purposes, from the sqlite3 interface it's pretty awkward with the current stored-as-blob approach:

sqlite> select * from tasks;
�n�KI����)o�|{"modified":"1619619386","description":"yep","status":"P"}
}{�:�gBt�S+���Ɯ|{"description":"(no description)","modified":"1619619389","status":"P"}

Then on to the sync-server storage

[djmitche]

Yeah, that is pretty gross. I don't think there's any real downside to storing strings. Performance isn't a huge issue here.

[dbr]

@djmitche The trait Storage requires Send + Sync, but.. rusqlite's Transaction type is neither

SQLite can be fully threadsafe but rusqlite isn't - discussed here, and also here, neither has too satisfying an outcome.

Any thoughts on the best way to implement this?

[djmitche]

Ah, this has been a problem with Python as well. Perhaps the storage should run in a dedicated thread, and the operations be performed via mpsc channels? It might be easiest to just shuttle query text over the channel to the sqlite thread, with responses returned to the caller on dedicated channels. The callers would then send the query text and await a message on the response channel.

[dbr]

Yep sounds like a good plan!

[dbr]

Made separate tickets for the async-storage (#277) and SQLite connection pooling (#278)

[dbr]

I think this is good to merge? 🤔

[djmitche]

Apologies for taking so long to review this!

This looks good -- feel free to take or reject the suggestions below.

[djmitche]

Sheep It!

[dbr]

Moo-erged in main branch, added changelog entry, shall merge when CI is appeased

[djmitche]

once this merges I think we're done with 0.4.0?

[dbr]

I was just creating a new issue (#283) about making 0.4.0 release! Yep I think this is the last issue left in that milestone 🥳