-
Notifications
You must be signed in to change notification settings - Fork 189
Improve gossipsub #913
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
base: main
Are you sure you want to change the base?
Improve gossipsub #913
Conversation
@sukhman-sukh : This is wonderful. Great work. Reviewing this in detail. Thank you so much for the contribution. CCing @sumanjeet0012 , @lla-dane and @Winter-Soren , who are also working on different Gossipsub branches. Asking them to review this important PR with me. @acul71 (Luca) had some github admin issues. They are now resolved and you can definitely tag Luca again :) |
@sukhman-sukh: I had one question about the splitting logic, if an outgoing RPC is larger than |
Hey @lla-dane, according to me, every RPC is standalone. We were first piggybacking multiple messages into a single RPC which might exceed it sometime, so now it is taking that big RPC and breaking it into a standalone list of RPCs. |
Yeah I understand that. But suppose we break a big chunk RPC into individual RPCs, so isn't there a chamce that the content of those 2 smaller RPCs would be dependent, and since our ultimate goal was to somehow deliver the complete content of the big RPC on the receiver end, so the content of the 2 smaller RPCs need to agregated or something on the receiver end. |
@sukhman-sukh : Hi Sukhman. Appreciate your efforts. Wish if you could resolve the CI/CD issues. Wish if you could also respond to @lla-dane on his question/comment, whenever you get a chance. CCing @Winter-Soren and @sumanjeet0012 for their thoughts and feedback too. |
Have a look here at go-libp2p impl (https://github.com/libp2p/go-libp2p-pubsub/blob/ed53c170f9a784c45f81d88dd6051218fbd5ece7/pubsub.go#L268). |
Oh I see, I was thinking it was splitting a single RPC message into parts. That's good then, now I understood. So would it possible for a single RPC message to exceed max size ? |
Yes, it is possible. In that case:
will be executed and that RPC will be dropped for now. |
@seetadev, please re run the ci |
@sukhman-sukh : We had some CI/CD performance issues over the last 7 hours. Re-ran the CI/CD pipeline. We should now get test results soon. |
What was wrong?
Current version of gossipsub did not have message queuing, priority message queuing, and also, message splitting based on maxMessageSize as discussed for interoperability with go-libp2p.
Issue # #891
How was it fixed?
Cute Animal Picture