Partial Messages: Generic PeerState and Iterator style publish#674
Partial Messages: Generic PeerState and Iterator style publish#674MarcoPolo wants to merge 3 commits intosimplify-partial-messagesfrom
Conversation
MarcoPolo
force-pushed
the
partialmsgs-generic-iter
branch
from
f299bad to
2dc2470
Compare
MarcoPolo
force-pushed
the
partialmsgs-generic-iter
branch
from
2dc2470 to
13a9c3a
Compare
| @@ -191,6 +203,45 @@ func WithPartialMessagesExtension(pm *partialmessages.PartialMessagesExtension) | |||
| } | |||
| } | |||
There was a problem hiding this comment.
Should this be per topic?
Here we require all topics with partial messages to use PeerState as the state type.
There was a problem hiding this comment.
Possibly, but I think that's worth doing separately. For now we have the same PeerState across all topics.
extensions.go
Outdated
| return | ||
| } | ||
|
|
||
| resp <- pme.PublishPartial(topic, message, opts) |
There was a problem hiding this comment.
I don't like the control flow.
app => pubsub loop => app => sendRPC
But I see why you had to do this given the current gossipsub single goroutine system. IIUC, you can't give the app an iterator for (peer, peerstate) without copying all the state.
There was a problem hiding this comment.
It's correct that I would have to copy the state if I wanted to give the list of peers and peerstate to the application.
But I don't think this pattern is that uncommon. Specifically the pattern of calling a function with a function parameter that you expect to be used by the function you are calling. slices.SortFunc comes to mind, but I'm sure there are many other examples.
The latest commit hopefully makes this clearer by making the PublishActions a function type that is passed into to PublishPartial.
We only need a single function from the application. Have the application provide this function in PublishPartial instead of a interface type.
MarcoPolo
force-pushed
the
partialmsgs-generic-iter
branch
from
13a9c3a to
f1f43c3
Compare
2 participants
Worth reviewing commit by commit and with a focus on partialmsgs.go