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

Revert zombie markers (minimal) #6069

Conversation

rustyrussell
Copy link
Contributor

Alternative to #6066

Then we can spend this release reworking the zombie logic, and checking that we're not eliminating any useful channels!

@endothermicdev
Copy link
Collaborator

Wow, very minimal. Just load as though the zombie flag doesn't exist, prune as normal and move on.

I think this is a fair approach. We'll probably want to run a couple nodes in parallel to validate our pruning behavior. As you suggested, new channels should probably have a different high water mark than existing channels, but that's a problem we have even now. This tests fine with my existing store (was under PR #6066).

ACK 295b4ea

@@ -2229,6 +2229,7 @@ def test_gossip_private_updates(node_factory, bitcoind):
wait_for(lambda: l1.daemon.is_in_log(r'gossip_store_compact_offline: 5 deleted, 3 copied'))


@pytest.mark.skipIf(True, "Zombie research had unexpected side effects")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@pytest.mark.skipIf(True, "Zombie research had unexpected side effects")
@pytest.mark.skip("Zombie research had unexpected side effects")

Signed-off-by: Rusty Russell <[email protected]>
This reverts us to the v22.11 behaviour, pending a revisit for the
next release.

Changelog-Changed: gossipd: revert zombification change, keep all gossip for now.
Signed-off-by: Rusty Russell <[email protected]>
@endothermicdev
Copy link
Collaborator

ACK 36e8ff2

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.

2 participants