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

DB changes to shrink unneeded HTLCs, only pass minimal set to onchaind. #4850

Merged
merged 16 commits into from
Oct 15, 2021

Conversation

rustyrussell
Copy link
Contributor

@rustyrussell rustyrussell commented Oct 7, 2021

This became a bigger opus than intended, sorry! But everyone's dbs should shrink as a result.

Edit: rebased on #4863

@rustyrussell rustyrussell added this to the v0.10.2 milestone Oct 7, 2021
@rustyrussell rustyrussell requested a review from cdecker as a code owner October 7, 2021 23:29
@rustyrussell rustyrussell force-pushed the guilt/smaller-db branch 7 times, most recently from 378c9e8 to 9c67881 Compare October 13, 2021 01:08
sendonionmessage can fail when sending a reply, either because
the reply had a bad first peer, or because it went offline.  The
latter happens in CI, which is how I found this.

Also fixed typo "onio" -> "onion".

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
How did this pass CI?  I saw this break in my PR, but it's (long)
broken in master.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We are usually dealing with unsigned values, so use this.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
All enums in the db should be wrapped this way on reading/writing them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And initialize max to current height max when htlcs are already dead.
Turns out (thanks CI!) that MAX() of multiple columns is GREATEST() in
Postgres.  That's clearer (MAX is used elsewhere for single columns),
so translate on the sqlite3 side.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In particular, the onion and errors can be large, but now we'll never
need to retransmit them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is particularly useful after our recent field deletion:

before: 362,573,824 bytes
after: 124,190,720 bytes

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: db: removal of old HTLC information and vacuuming shrinks large lightningd.sqlite3 by a factor of 2-3.
This makes more sense than two args.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I started pulling this thread, and the entire codebase got unravelled.

Oh well, it's done now!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes init a two-stage, and causes some code hoisting.

And we can now send all the HTLCs in a single message, since we have
an 128MB limit and each HTLC is 37 bytes.

This breaks the onchaind stresstest, which uses canned internal messages.
It's time to finally delete that.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In particular, we could get depth notifications.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This will make closing long-lived channels more efficient, and it's
just nicer.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
[gw1] [ 98%] PASSED tests/test_wallet.py::test_hsmtool_dump_descriptors 
tests/test_wallet.py::test_fundchannel_listtransaction 
[gw0] [ 98%] PASSED tests/test_plugin.py::test_channel_opened_notification 
tests/test_wallet.py::test_hsmtool_generatehsm 
[gw0] [ 98%] PASSED tests/test_wallet.py::test_hsmtool_generatehsm 
tests/test_wallet.py::test_withdraw_nlocktime_fuzz 
[gw1] [ 98%] ERROR tests/test_wallet.py::test_fundchannel_listtransaction 
tests/test_wallet.py::test_fundchannel_listtransaction 
tests/test_wallet.py::test_withdraw_nlocktime_fuzz 
tests/test_wallet.py::test_fundchannel_listtransaction 
[gw0] [ 99%] ERROR tests/test_wallet.py::test_withdraw_nlocktime_fuzz 
tests/test_wallet.py::test_multiwithdraw_simple 
[gw1] [ 99%] ERROR tests/test_wallet.py::test_fundchannel_listtransaction 
tests/test_wallet.py::test_withdraw_nlocktime 
tests/test_wallet.py::test_multiwithdraw_simple 
tests/test_wallet.py::test_withdraw_nlocktime 
tests/test_wallet.py::test_multiwithdraw_simple 
tests/test_wallet.py::test_withdraw_nlocktime 
[gw0] [ 99%] ERROR tests/test_wallet.py::test_multiwithdraw_simple 
tests/test_wallet.py::test_repro_4258 
[gw1] [ 99%] ERROR tests/test_wallet.py::test_withdraw_nlocktime
...
2021-10-12 06:36:09.203 UTC [224552] STATEMENT:  SELECT version FROM version LIMIT 1
2021-10-12 06:36:09.566 UTC [224523] PANIC:  could not write to file "pg_wal/xlogtemp.224523": No space left on device
2021-10-12 06:36:09.566 UTC [224523] STATEMENT:  VACUUM FULL;
Error vacuuming db: BEGIN command failed: PANIC:  could not write to file "pg_wal/xlogtemp.224523": No space left on device
server closed the connection unexpectedly
	This probably means the server terminated abnormally
	before or while processing the request.
```
Copy link
Member

@cdecker cdecker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c9d1b7f

{
assert(!amount_msat_eq(amount, AMOUNT_MSAT(7206000)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did this random constant come from?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rustyrussell ^^ (I guess I have seen a similar number and it was related to some altcoin pre-mine :-D ROTFL

@cdecker cdecker merged commit c012a71 into ElementsProject:master Oct 15, 2021
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