sql partial refresh support#8914
sql partial refresh support#8914rustyrussell wants to merge 9 commits intoElementsProject:masterfrom
Conversation
rustyrussell
force-pushed
the
guilt/sql-partial-refresh
branch
2 times, most recently
from
e3859eb to
6008f6c
Compare
Andezion
left a comment
Andezion
left a comment
There was a problem hiding this comment.
The refresh_needs bitmask, the watch_init/watch_for/wait_done chain, and the priority ordering in paginated_refresh are all interesting solutions!! Stealing these ideas for future reference!)) LGTM!
| f.result(TIMEOUT) | ||
|
|
||
|
|
||
| def test_sql_during_change(node_factory): |
There was a problem hiding this comment.
Cool test! Especially nice - the negative assertions after the create step confirm that only the minimal refresh path was triggered, not a heavier one. That kind of "we did the cheap thing" check is easy to skip but good to have!!
| return send_outreq(req); | ||
| } | ||
|
|
||
| static struct command_result *updated_list_done(struct command *cmd, |
There was a problem hiding this comment.
Small question: in updated_list_done, unused is passed as last_created_index because we dont want updates to advance that counter - but is there a risk that entries returned by the updated index query actually contain new created_index values that we had then miss?
| } | ||
|
|
||
| /* Mutual recursion */ | ||
| static void watch_for(struct sql *sql, |
There was a problem hiding this comment.
The watch_init + watch_for + wait_done trio is clean. Using nextvalue=0 in watch_init as a "give me current state" trick, then immediately chaining into the async watch loop is a nice pattern!!
1. Refresh time would say "dont refresh if it's only this old" but better is to simply make refresh more efficient, which we've done and continue to do. 2. time_msec could be used in two places, but floating point suffices. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is trivial now, as the invoice table gets reloaded every time, but is an important check as we improve the implementation. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's a "have we initialized the table" flag, and we're going to use it for more than setting up the indices, so rename it. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We can actually just make struct table_desc non-const where required, and hand its var directly. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We now always watch for deleted/created/updated, and keep multiple flags and indices (though we don't use them yet!). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're going to need this once we start using `wait updated` to track these. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The current variable name is confusing, since it's an INSERT! This becomes more noticible in the next patch, where we add a delete statement. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ndex values. This will let us (efficiently) delete a single entry, so we can wean tables with created_index off the default "delete all and reload" behavior. Change insert_stmt to a const, too (and use a temporary when we're building it). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For these, when things change, we simply delete amd recreate the changed entries. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Changed: Plugins: `sql` plugin tables "htlcs", "forwards", "invoices", "sendpays" and "networkevents" are now updated more efficiently.
rustyrussell
force-pushed
the
guilt/sql-partial-refresh
branch
from
6008f6c to
8eca1c5
Compare
3 participants
Replaces #8841
This properly adds wait support to all supported tables (ie. all list commands which support indexing) in the sql plugin. It's a little tricky, so we do this in easy stages.
The final result is far more optimal for queries to those tables.