filecoin-project / venus

Filecoin Full Node Implementation in Go
https://venus.filecoin.io
Other
2.06k stars 459 forks source link

Avoid instantiating a message pool on nodes that are not mining #2145

Closed anorth closed 3 years ago

anorth commented 5 years ago

Description

The message pool is an in-memory pool of un-mined messages. Messages are added to the pool when received from other nodes and when generated locally. Messages are drawn from the pool when mining a block; this is it's reason for existence. If not mining, there's no point having a message pool, and the way we use it causes confusion. More background in #1936 and Message Tracking Design

Avoid instantiating a message pool on a node that is not mining. Instantiate one dynamically when mining starts.

A prerequisite is separating an outbound message queue from the message pool: design in #2132

Acceptance criteria

Risks + pitfalls

Where to begin

Message pool initialisation and the mining start command.

anorth commented 5 years ago

Moving to backlog for now.

anorth commented 5 years ago

Deferring subscription to the message pubsub is blocked by #2959 so that the message_propagate_test can be updated to have the nodes start mining (with a miner actor configured in the genesis).

anorth commented 5 years ago

See also discussion in #3052, in which one proposal is merging the message pool and queue. In that case, this issue would be mainly avoid avoiding subscribing to the message pubsub.

anorth commented 5 years ago

Note that I'm less sure this is a good idea now. Block messages/receipts have been separated from headers and so now a validating (but non-mining) node is expected to source most of the messages referenced by a pubsub message from the local message pool in order to form and validate a complete block.

Without a pool, a node will fetch all the messages from a peer (although with Graphsync it can do this fairly efficiently). It's not clear whether participating in pubsub or fetching message lists on demand is better.

acruikshank commented 5 years ago

If I'm understanding correctly, we may want validating nodes to maintain a pubsub message pool, because, at the time when a new block header arrives, the node should probably already received the block's messages through pubsub into its message pool and won't have to query it's peers. In this case, pubsub may be more efficient than Graphsync.

Given that we will need to retrieve historical messages from peers while syncing the chain, using the message pool only for mining and graphsync exclusively for block message population would make the network dynamics easier to model. I still like this idea for that reason.

On the other hand, removing message pubsub nodes from the network means less gossip and potentially lower message transmission rates (assuming libp2p doesn't already force you to relay topics you are not subscribed to). Maybe maintaining a message pool is a small burden we require of all go-filecoin users to maintain network health.

All that said, my vote is to keep this low priority, but to keep discussing it.