Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove db accesses in listpeerchannels #7679

Conversation

rustyrussell
Copy link
Contributor

This is an alternative to #7611

Like other fields, mirror them into the db.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This avoids a db lookup on every iteration of listpeerchannels, which
can be slow on large nodes (Postgres, I assume).

We can now simply add the fields we want to channel load, and remove
wallet_channel_stats_load entirely.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Indirection via a string and an enum is just adding confusion here.

And move the `struct channel_stats` into channel.h.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And instead of loading them in listpeerchannels, use them.  This means
listpeerchannels no longer touches the db.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSONRPC: `listpeerchannels` (and thus, pay) sped up on very large nodes.
…e pointers.

This name is clearer than the old one.

And since the struct contains a string, it's more natural for the
struct to be the tal parent of the string so it's a real object.  This means
we need an array of pointers, so each struct can be its own tal object.

wallet_state_change_get is hoisted higher in the code and made static.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell added this to the v24.08.1 milestone Sep 20, 2024
@ShahanaFarooqui ShahanaFarooqui modified the milestones: v24.08.1, v24.08.2 Sep 20, 2024
@JssDWt
Copy link
Contributor

JssDWt commented Sep 20, 2024

I think this looks good. ACK

@rustyrussell rustyrussell merged commit aba67a7 into ElementsProject:master Sep 20, 2024
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants