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

Make floodPublish under CLI flag control #8559

Closed
tbenr opened this issue Sep 3, 2024 · 9 comments · Fixed by #8610
Closed

Make floodPublish under CLI flag control #8559

tbenr opened this issue Sep 3, 2024 · 9 comments · Fixed by #8610
Assignees

Comments

@tbenr
Copy link
Contributor

tbenr commented Sep 3, 2024

After some discussions with LH guys, seems like turning off floodPublish may help especially in case of low upload bandwidth.

The reason is that it seems to improve dissemination latency if the full message is received earlier by one of the peer of the mesh (rather than start sending the message in parallel to all peers of the mesh). This could help homestakers on low bandwidth, when publishing block and blobs (locally built)

the parameter could be named --p2p-flood-publish-enabled, default true

context:
.floodPublish(true) in addGossipParamsMiscValues

@tbenr
Copy link
Contributor Author

tbenr commented Sep 5, 2024

@Nashatyrev do you have any opinion about this?

@Nashatyrev
Copy link
Contributor

Yes, I totally support adding this flag.
Ideally we should come up with some automatic optimal strategy detection, but it doesn't seem easy with existing protocol capabilities

@rolfyone
Copy link
Contributor

rolfyone commented Sep 12, 2024

@KatyaRyazantseva - basically all of the p2p flags live together in NetworkConfig, and that would get plumbed through so that we can pass it in at addGossipParamsMiscValues in LibP2pParamsFactory...

Generally boolean flags follow a common pattern in picocli, called *-enabled by default, having a similar to --p2p-enabled in NetworkConfig, except i'd probably follow the defaulting that all of the other cli args are using in the P2pOptions class...

Then we just need to find the path to pass it into that factory...

@Nashatyrev
Copy link
Contributor

I would also add that this option when disabled would improve validator privacy. floodpublish when enabled greatly simplifies validator deanon.

@tbenr
Copy link
Contributor Author

tbenr commented Sep 12, 2024

Thinking about the risks:

@Nashatyrev
is it true that if you start sending the message to a peer that suddenly becomes significantly slow in receiving data, you may end up having a time-to-complete-sent-first-message to be very high?

A more clever way would be to start sending the next peer at some point by allowing overlapping. This can become complicated :)

@Nashatyrev
Copy link
Contributor

@tbenr Not really sure what you mean exactly
When publishing:

  • if floodPublish = disabled you are sending the full message to your mesh peers only (i.e. to nearly 8 peers when D = 8). Some of the others would receive just IHAVE. I.e. the same behaviour as if you just propagating the message received from another peer
  • if floodPublish = enabled you are sending the full message to all peer subscribed to the topic. E.g. if you have 150 peers and you are publishing a block then you will be sending full message to 150 peers

Sending is performed in parallel, so having a slow receiver would not affect other receivers.

If you e.g. have 100Mb/s bandwidth and sending 1Mb message then if calculate roughly in the first case the first message would be delivered to any other peer in 800ms, in the second case - in 15 seconds

A more clever way would be to start sending the next peer at some point by allowing overlapping.

You are probably refer to what I call 'staggered send'. It indeed might be more effective, but you would need estimation on your outbound bandwidth and all peers' inbound bandwidth at the moment of sending. So yeah, it looks pretty complicated

@tbenr
Copy link
Contributor Author

tbenr commented Sep 14, 2024

Oh I definitely confused the two. You are right, thanks for clarifying!

@KatyaRyazantseva
Copy link
Contributor

What could be the description of the flag for users?

@rolfyone
Copy link
Contributor

rolfyone commented Sep 22, 2024

Hi @KatyaRyazantseva - sorry this fell through the cracks, remember theres always teku-contribs if ppl are not answering...

We could use a placeholder to unblock, or just take a stab, ill do the latter...
Use flood publishing when broadcasting to the network. This may slow down block publishing on low bandwidth internet connections.

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 a pull request may close this issue.

4 participants