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.

Sign up for GitHub

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

Jump to bottom

Db code cleanups #6128

Merged
merged 4 commits into from
Apr 6, 2023
Merged

Db code cleanups #6128

merged 4 commits into from
Apr 6, 2023

Conversation

rustyrussell
Copy link
Contributor

No code changes, just some cleanups.

@rustyrussell rustyrussell added this to the v23.05 milestone Mar 27, 2023
vincenzopalazzo
vincenzopalazzo approved these changes Mar 29, 2023
View reviewed changes
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK 2be271d

@rustyrussell
Copy link
Contributor Author

This broke bookkeeper under postgres it seems? @niftynei ?

@niftynei
Copy link
Collaborator

niftynei commented Apr 3, 2023

Does #6141 fix this?

@niftynei
Copy link
Collaborator

niftynei commented Apr 3, 2023

Also thanks for this, we definitely needed to be reporting errors...

@rustyrussell
Copy link
Contributor Author

Does #6141 fix this?

I've pulled it in, let's find out!

@vincenzopalazzo
Copy link
Collaborator

Looks like the io return the wrong reference? this is happening also with lnprototest sometimese

--------------------------- Captured stderr teardown ---------------------------
lightningd: lightningd/connect_control.c:734: connectd_activate: Assertion `ret == ld->connectd' failed.
lightningd: FATAL SIGNAL 6 (version 798d3a7-modded)
0x55dc786700ac send_backtrace
	common/daemon.c:33
0x55dc78670158 crashdump
	common/daemon.c:46
0x7f6ff207451f ???
	???:0
0x7f6ff20c8a7c ???
	???:0
0x7f6ff2074475 ???
	???:0
0x7f6ff205a7f2 ???
	???:0
0x7f6ff205a71a ???
	???:0
0x7f6ff206be95 ???
	???:0
0x55dc785f1afe connectd_activate
	lightningd/connect_control.c:734
0x55dc78604df3 main
	lightningd/lightningd.c:1152
0x7f6ff205bd8f ???
	???:0
0x7f6ff205be3f ???
	???:0
0x55dc785d3d34 ???
	???:0
0xffffffffffffffff ???
	???:0
2023-04-03 07:35:24.132 UTC [100292] LOG:  received fast shutdown request

@rustyrussell
Copy link
Contributor Author

Looks like the io return the wrong reference? this is happening also with lnprototest sometimese

--------------------------- Captured stderr teardown ---------------------------
lightningd: lightningd/connect_control.c:734: connectd_activate: Assertion `ret == ld->connectd' failed.
lightningd: FATAL SIGNAL 6 (version 798d3a7-modded)
0x55dc786700ac send_backtrace
	common/daemon.c:33
0x55dc78670158 crashdump
	common/daemon.c:46
0x7f6ff207451f ???
	???:0
0x7f6ff20c8a7c ???
	???:0
0x7f6ff2074475 ???
	???:0
0x7f6ff205a7f2 ???
	???:0
0x7f6ff205a71a ???
	???:0
0x7f6ff206be95 ???
	???:0
0x55dc785f1afe connectd_activate
	lightningd/connect_control.c:734
0x55dc78604df3 main
	lightningd/lightningd.c:1152
0x7f6ff205bd8f ???
	???:0
0x7f6ff205be3f ???
	???:0
0x55dc785d3d34 ???
	???:0
0xffffffffffffffff ???
	???:0
2023-04-03 07:35:24.132 UTC [100292] LOG:  received fast shutdown request

Yeah, this happens if you shut down CLN while it's still waiting for connectd to start up. It's actually a real bug, but it's ok since we're shutting down anyway. We just don't do it gracefully!!

niftynei and others added 4 commits April 5, 2023 12:29
@niftynei @rustyrussell
postgres: add missing 'update_count' to stmt
19f0ad7 
Reported-By: @rustyrussell
Changelog-Fixed: Plugins: `bookkeeper` onchain fees calculation was incorrect with PostgresQL.
@rustyrussell
db: catch SQL errors unless we're expecting them.
e654298 
I couldn't figure out why my new SQL query was returning 0 rows,
and it was because we were ignoring errors.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
db: make db_exec_prepared_v2 return void.
c07fc30 
It calls db_fatal() if it fails anyway, so don't expect anyone to check.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
db: db_set_intvar/db_get_var should take a const char *.
2f49be0 
Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
Copy link
Contributor Author

rustyrussell commented Apr 6, 2023 •
edited
Loading

Since @niftynei patch fixes it:

Ack 2f49be0

@rustyrussell rustyrussell merged commit eee3965 into ElementsProject:master Apr 6, 2023
@rustyrussell rustyrussell mentioned this pull request Apr 6, 2023
Closed
@evansmj evansmj mentioned this pull request Dec 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vincenzopalazzo vincenzopalazzo vincenzopalazzo approved these changes

Assignees
No one assigned
Labels
cleanup
Projects
None yet
Milestone
v23.05
Development

Successfully merging this pull request may close these issues.
3 participants
@rustyrussell @niftynei @vincenzopalazzo