Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Reduce consensus spam #1658

Merged
merged 11 commits into from
Feb 1, 2019
Merged

Reduce consensus spam #1658

merged 11 commits into from
Feb 1, 2019

Conversation

andresilva
Copy link
Contributor

@andresilva andresilva commented Feb 1, 2019

  • The predicate that was being used to clean up gossip messages was wrong, so all messages were being pruned.
  • The logic for keeping known messages relied on this predicate as well, so these messages would be immediately forgotten after getting pruned.
  • Right after pruning we'd get them all back from the network and import them all (20K+ messages).
  • Commit topics weren't being pruned at all.
  • I set the expiration-based pruning to be a bit more agressive (the higher layers already have logic for resending messages when necessary).
  • Garbage collection is now done on a topic, and we ignore any further messages for that topic.
  • Commit messages are no longer broadcast to all peers (this was probably a bad idea all along, sorry). This might make it so that non-authority nodes observe finality with a bit of delay (temporarily until Extend network protocol with justification announcements polkadot-sdk#571), but at least for change blocks they should request justifications through sync and therefore not get stuck waiting for finality.
  • There was also old code (from RHD) that assumed that topics were header hashes (and looked it up on the DB). We should probably create an issue for this since I'm not sure whether RHD is relying on this logic.

Fix #1646. Should help with #1645.

@andresilva andresilva added A0-please_review Pull request needs code review. M4-core labels Feb 1, 2019
@andresilva
Copy link
Contributor Author

Just need to write a test for the topic-based pruning.

@gavofyork
Copy link
Member

@rphmeier @arkpar pair of eyes would be good :)

@rphmeier rphmeier added A8-looksgood and removed A0-please_review Pull request needs code review. labels Feb 1, 2019
@rphmeier rphmeier merged commit bd85e08 into master Feb 1, 2019
andresilva added a commit that referenced this pull request Feb 1, 2019
* core: fix predicate for dropping grandpa round messages

* core: grandpa: drop commits topic on authority set change

* core: gossip: only drop known messages based on expiration time

* core: grandpa: don't broadcast commit messages

* core: gossip: don't assume topics are header hashes

* core: gossip: expire messages more agressively

* core: grandpa: fix test environment

* core: gossip: fix tests

* core: gossip: track dead topics (and ignore messages)

* core: gossip: test dead topic pruning
@andresilva andresilva deleted the andre/gossip-less branch March 18, 2019 23:12
MTDK1 pushed a commit to bdevux/substrate that referenced this pull request Apr 12, 2019
* core: fix predicate for dropping grandpa round messages

* core: grandpa: drop commits topic on authority set change

* core: gossip: only drop known messages based on expiration time

* core: grandpa: don't broadcast commit messages

* core: gossip: don't assume topics are header hashes

* core: gossip: expire messages more agressively

* core: grandpa: fix test environment

* core: gossip: fix tests

* core: gossip: track dead topics (and ignore messages)

* core: gossip: test dead topic pruning
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Topic-aware consensus-gossip pruning
5 participants