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

lightningd: refuse to upgrade db on non-released versions by default. #5550

Merged
merged 1 commit into from
Sep 15, 2022

Conversation

rustyrussell
Copy link
Contributor

This is a good sanity check that users understand that if they upgrade
to master mid-cycle they can't go back!

Suggested-by: @wtogami
Changelog-Added: Config: --database-upgrade=true required if a non-release version wants to (irrevocably!) upgrade the db.

@rustyrussell rustyrussell added this to the v22.10 milestone Aug 25, 2022
@cdecker cdecker self-requested a review August 25, 2022 16:58
@cdecker
Copy link
Member

cdecker commented Aug 25, 2022

Looks like we are getting an access to uninitialized memory in options.c: ld->db_upgrade_ok seems to be uninitialized.

@Sjors
Copy link
Contributor

Sjors commented Aug 25, 2022

What about release candidates? (I tend to start those using systemd so it's a slight inconvenience to relaunch with the appropriate argument)

@wtogami
Copy link
Contributor

wtogami commented Aug 26, 2022

What about release candidates? (I tend to start those using systemd so it's a slight inconvenience to relaunch with the appropriate argument)

You need only launch it differently once. You could also havedatabase-upgrade=true in your config file if you rather opt into the old behavior.

@rustyrussell
Copy link
Contributor Author

What about release candidates? (I tend to start those using systemd so it's a slight inconvenience to relaunch with the appropriate argument)

Yeah, tell systemd to stop, do lightningd --database-upgrade=true --offline and then kill it and tell systemd to restart.

But really, you can just put databse-upgrade=true in your config and be careful upgrading! I trust you @Sjors!

@rustyrussell
Copy link
Contributor Author

Rebased on master, fixed uninitialized var.

@cdecker cdecker force-pushed the db-ugprade-stop branch 2 times, most recently from 0ee3a42 to 3a612b8 Compare September 12, 2022 12:19
@rustyrussell rustyrussell force-pushed the db-ugprade-stop branch 2 times, most recently from 4c73cf1 to 3bbcf1d Compare September 13, 2022 00:47
This is a good sanity check that users understand that if they upgrade
to master mid-cycle they can't go back!

Suggested-by: @wtogami
Changelog-Added: Config: `--database-upgrade=true` required if a non-release version wants to (irrevocably!) upgrade the db.
Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
Copy link
Contributor Author

That was weird... First CI seemed to have the wrong version, then I seemed to push an empty tree. But I've rebased onto master now...

@cdecker
Copy link
Member

cdecker commented Sep 14, 2022

Rebased on top of master, waiting for CI to complete before merging.

ACK 262dbf9

@cdecker
Copy link
Member

cdecker commented Sep 14, 2022

Seems we have a false positive for memleak (guessing the bool flag?).

@rustyrussell
Copy link
Contributor Author

rustyrussell commented Sep 15, 2022

Seems we have a false positive for memleak (guessing the bool flag?).

No, it's a valgrind error. But I do initialize it in this commit (I didn't initialize it previously), and indeed, I checked it's correct locally...

Trivial rebase
Ack 1cbfa92

@rustyrussell rustyrussell merged commit 4ca6b36 into ElementsProject:master Sep 15, 2022
@Sjors
Copy link
Contributor

Sjors commented Dec 2, 2022

But really, you can just put databse-upgrade=true in your config and be careful upgrading! I trust you @Sjors!

Ok fine...

Scherm­afbeelding 2022-12-02 om 19 04 05

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.

4 participants