db-write hook #2421

rustyrussell · 2019-03-01T04:12:35Z

It's a bit limited (see final commits for caveats) but it works.

rustyrussell · 2019-03-01T06:29:46Z

Hmm, needs sqlite3 v3.14 or above. Not sure what that would break if we include it, might have to make hook optional? Yech...

cdecker · 2019-03-04T14:14:17Z

I was expecting to see an implementation of sqlite3_expanded_sql took me while to figure out that it is actually a native sqlite3 function. Maybe we can shim this? This hook seems far too useful not to exist. Notice though that it makes switching DBs even harder since plugins now may depend on the sqlite3 dialect being used.

niftynei

really like the db_select_step improvements.

may be worth noting that in theory, your 'backup' copy may be one command set ahead of the actual on disk database if it crashes/fails during write... tho i guess that's actually the point eh. :)

i should note that i didn't review the io_loop_exclusive additions very carefully because it looked like a straight ccan update.

niftynei · 2019-03-08T21:16:44Z

wallet/db.h

+/**
+ * db_select_step -- iterate through db results.
+ *
+ * Returns false and frees stmt if we've we've reached end, otherwise


Suggested change

* Returns false and frees stmt if we've we've reached end, otherwise

* Returns false and frees stmt if we've reached end, otherwise

niftynei · 2019-03-08T21:41:25Z

doc/PLUGINS.md

+
+1. a plugin registering for this hook should not perform anything that may cause
+   a db operation in response (pretty much, anything but logging).
+2. a plugin registering for this hook should not register for other hooks or


should we canonize this by returning an error if this is broken?

niftynei · 2019-03-08T21:47:31Z

ccan/ccan/io/io.h

+ *
+ * This allows you to temporarily service only one (or several) fds.
+ * For example, you might want to flush out one io_conn and not
+ * receive any new connections or read any otherninput.


Suggested change

* receive any new connections or read any otherninput.

* receive any new connections or read any other input.

niftynei · 2019-03-08T21:47:56Z

ccan/ccan/io/io.h

+ * For example, you might want to flush out one io_conn and not
+ * receive any new connections or read any otherninput.
+ *
+ * Returns true of there any exclusive io_conn remain, otherwise false.


Suggested change

* Returns true of there any exclusive io_conn remain, otherwise false.

* Returns true if any exclusive io_conn remain, otherwise false.

Replace with an explicit test for existence. This simplifies our db interface. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

I was tempted to create a new db_select_stmt wrapper type, but that means a lot of boilerplate around binding, which expects to work with db_prepare *and* db_select_prepare. This lets us clearly differentiate between db queries (which don't need to go to a plugin) and db changes (which do). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell · 2019-03-15T03:18:14Z

I was expecting to see an implementation of sqlite3_expanded_sql took me while to figure out that it is actually a native sqlite3 function. Maybe we can shim this? This hook seems far too useful not to exist. Notice though that it makes switching DBs even harder since plugins now may depend on the sqlite3 dialect being used.

I think we can try to use the (now obsolete) sqlite3 tracing feature to simulate this in older sqlite3. Let me try.

rustyrussell · 2019-03-15T03:28:43Z

really like the db_select_step improvements.

may be worth noting that in theory, your 'backup' copy may be one command set ahead of the actual on disk database if it crashes/fails during write... tho i guess that's actually the point eh. :)

Yes, but in this case both dbs are "valid". You could naively copy the whole db in the plugin every startup, but better would be a separate "db_init" hook, which would use https://www.sqlite.org/c3ref/c_fcntl_begin_atomic_write.html#sqlitefcntldataversion, and give plugin the version startup time.

If the plugin also has an sqlite3 db, that's a trivial check, and if not, it can fall back to copy.

rustyrussell · 2019-03-15T04:06:19Z

OK, put in backwards compat shim, rebased.

For the moment we just dump them to stderr, but later this will go to a hook. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

…sly. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

I started by trying to change the current infrastructure, but this is really the only completely sync hook which makes sense; it needs to avoid doing the db_transation, as well as waiting, and using a callback is just overkill. So with some regret, I open coded it. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

In theory we could allow a db_write-using plugin to have other hooks/commands by embargoing their other responses until the exclusive period is over. That would be nice for a 'dbmirrorinfo' command, for example. The other option would be to *always* go exclusive on a db_write-using plugin, so responses can never get intermingled. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

We didn't include stdlib, so these tests "failed" due to implicit declaration of exit(). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

…-08-08) Our Travis, for example, doesn't have 3.14. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

@niftynei

Suggested-by: @niftynei Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell · 2019-04-08T01:56:12Z

Ack 1d776eb

Let's get it in so we can make progress :)

Coinomatron · 2019-04-11T11:18:24Z

Updated to the latest commits, it's not clear to me how this backup function works. Can someone give a brief explanation?

cdecker · 2019-04-11T13:00:44Z

Updated to the latest commits, it's not clear to me how this backup function works. Can someone give a brief explanation?

It's not a backup mechanism on its own, it is the infrastructure necessary to write backup plugins on top.

Coinomatron · 2019-04-11T13:03:10Z

Ah, thanks.

cdecker · 2019-04-11T14:43:37Z

No problem, we'll get a move on and get some example implementations soon :-)

Coinomatron · 2019-12-10T09:58:47Z

Any updates on this?

ZmnSCPxj · 2019-12-17T01:39:53Z

My last information is that @jb55 was working on some backup system that utilizes this.

jb55 · 2019-12-17T06:06:44Z

oh yeah I should start that

jb55 · 2019-12-17T06:38:39Z

hmm now that you reminded me, I didn't go that route as I was going to see how the postgresql backend played out. if you could use a postgresql backend that replicates offsite, that perhaps could solve the problem as well? cc @cdecker

although that's not very user friendly, so perhaps a plugin is still in order...

that makes me wonder how this interacts with the multi-db abstraction now... 🤔 I haven't looked at the code in either case yet

rustyrussell requested a review from cdecker March 1, 2019 04:12

rustyrussell requested a review from ZmnSCPxj as a code owner March 1, 2019 04:12

rustyrussell force-pushed the guilt/db-hook branch from 75abb9d to 750cf4d Compare March 1, 2019 04:29

rustyrussell added this to the 0.7.1 milestone Mar 1, 2019

rustyrussell added db plugin labels Mar 1, 2019

niftynei reviewed Mar 8, 2019

View reviewed changes

rustyrussell added 2 commits March 15, 2019 13:19

db: avoid the only failing db_exec.

ab8bece

Replace with an explicit test for existence. This simplifies our db interface. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell force-pushed the guilt/db-hook branch 4 times, most recently from 4b5124c to 4395d9e Compare March 17, 2019 03:00

rustyrussell added 10 commits March 18, 2019 13:10

db: keep track of changes.

951452a

For the moment we just dump them to stderr, but later this will go to a hook. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

ccan: update to get ccan/io exclusive helpers.

68bb446

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

plugin: plugin_exclusive_loop helper to service one plugin synchronou…

ca37636

…sly. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

pytest: test the db hook plugin.

f98e4b1

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

configure: fix detection of builtin libs with -Werror.

da70214

We didn't include stdlib, so these tests "failed" due to implicit declaration of exit(). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

configure: check for sqlite3_expanded_sql (SQLite3 version 3.14, 2016…

6debf3c

…-08-08) Our Travis, for example, doesn't have 3.14. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

db: fall back to deprecated sqlite3_trace() for db hook on old sqlite3.

fb4f182

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

db: typo fixed from review.

1d776eb

Suggested-by: @niftynei Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell force-pushed the guilt/db-hook branch from 4395d9e to 1d776eb Compare March 18, 2019 02:43

rustyrussell merged commit f4d44d8 into ElementsProject:master Apr 8, 2019

trueptolemy mentioned this pull request May 25, 2019

Backup and other feature requests #2400

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

db-write hook #2421

db-write hook #2421

rustyrussell commented Mar 1, 2019 •

edited

rustyrussell commented Mar 1, 2019

cdecker commented Mar 4, 2019

niftynei left a comment •

edited

niftynei Mar 8, 2019

rustyrussell Mar 15, 2019

niftynei Mar 8, 2019

niftynei Mar 8, 2019

niftynei Mar 8, 2019

rustyrussell commented Mar 15, 2019

rustyrussell commented Mar 15, 2019

rustyrussell commented Mar 15, 2019

rustyrussell commented Apr 8, 2019

Coinomatron commented Apr 11, 2019

cdecker commented Apr 11, 2019

Coinomatron commented Apr 11, 2019

cdecker commented Apr 11, 2019

Coinomatron commented Dec 10, 2019

ZmnSCPxj commented Dec 17, 2019 •

edited

jb55 commented Dec 17, 2019

jb55 commented Dec 17, 2019

	* Returns false and frees stmt if we've we've reached end, otherwise
	* Returns false and frees stmt if we've reached end, otherwise

	* receive any new connections or read any otherninput.
	* receive any new connections or read any other input.

	* Returns true of there any exclusive io_conn remain, otherwise false.
	* Returns true if any exclusive io_conn remain, otherwise false.

db-write hook #2421

db-write hook #2421

Conversation

rustyrussell commented Mar 1, 2019 • edited

rustyrussell commented Mar 1, 2019

cdecker commented Mar 4, 2019

niftynei left a comment • edited

Choose a reason for hiding this comment

niftynei Mar 8, 2019

Choose a reason for hiding this comment

rustyrussell Mar 15, 2019

Choose a reason for hiding this comment

niftynei Mar 8, 2019

Choose a reason for hiding this comment

niftynei Mar 8, 2019

Choose a reason for hiding this comment

niftynei Mar 8, 2019

Choose a reason for hiding this comment

rustyrussell commented Mar 15, 2019

rustyrussell commented Mar 15, 2019

rustyrussell commented Mar 15, 2019

rustyrussell commented Apr 8, 2019

Coinomatron commented Apr 11, 2019

cdecker commented Apr 11, 2019

Coinomatron commented Apr 11, 2019

cdecker commented Apr 11, 2019

Coinomatron commented Dec 10, 2019

ZmnSCPxj commented Dec 17, 2019 • edited

jb55 commented Dec 17, 2019

jb55 commented Dec 17, 2019

rustyrussell commented Mar 1, 2019 •

edited

niftynei left a comment •

edited

ZmnSCPxj commented Dec 17, 2019 •

edited