feat: Add gossip and eviction to the mempool #200
Conversation
FrederikBolding
force-pushed
the
fb/mempool
branch
from
9945334 to
84c2000
Compare
FrederikBolding
changed the title
src/core/message.rs
Outdated
|
|
||
| pub fn mempool_key(&self) -> MempoolKey { | ||
| if let Some(data) = &self.data { | ||
| return MempoolKey::new(data.timestamp as u64, self.hex_hash()); |
There was a problem hiding this comment.
I think this function should live inside the mempool. I would add a trait in the mempool for this, and implement the trait for the structs in there. Rest of the code doesn't have to care about this.
There was a problem hiding this comment.
Moved things around a bit in 4842eaf - lmk what you think!
| @@ -149,6 +170,43 @@ impl Mempool { | |||
| return true; | |||
| } | |||
|
|
|||
| async fn insert(&mut self, message: MempoolMessage) { | |||
There was a problem hiding this comment.
Let's emit some stats here, number of inserts tagged by success/failure and message type, and a guage for the current size of the mempool by shard.
There was a problem hiding this comment.
Let me know if you want additional stats or tweaks to the formatting
| Some(messages) => { | ||
| messages.insert(MempoolKey { inserted_at: Instant::now()}, message.clone()); | ||
| for system_message in transaction.system_messages { | ||
| mempool.remove(&system_message.mempool_key()); |
There was a problem hiding this comment.
We should record the same stats as above when removing from the mempool as well
FrederikBolding
force-pushed
the
fb/mempool
branch
from
0893a18 to
4842eaf
Compare
src/mempool/mempool_test.rs
Outdated
| const HOST_FOR_TEST: &str = "127.0.0.1"; | ||
| const PORT_FOR_TEST: u32 = 9388; | ||
|
|
||
| fn setup_gossip_mempool( |
There was a problem hiding this comment.
Why are there 2 different setup functions? Do the existing tests break with the gossip set up?
There was a problem hiding this comment.
I don't think so, I was just assuming that we didn't want to complicate the other tests further by adding additional complexity.
I can unify the setup function if preferred!
There was a problem hiding this comment.
I think it makes sense to set up the full mempool with gossip for all tests.
There was a problem hiding this comment.
If you find that the tests are getting out of hand with this approach, let's reconsider though.
This PR adds a basic implementation of the gossipped mempool described in #180.
It uses the existing mempool implementation but changes the key to use a key derived from the timestamp and identity of a message. Additions to the mempool are broadcast using a new gossip message. Messages are also evicted from the mempool when shard chunks are emitted that contain messages contained in the mempool.
Some minor changes to the channels and overall plumbing was required to setup the mempool communication.