feat: FanoutOnly topic option

[Wondertan]

Adds a FanoutOnly TopicOpt which forces Topic to remain in fanout mode, even after Subscribe invocations.

Prompt used to oneshot it with opus

When you PubSub.Join topic, a Topic is created but not actually subscribed over p2p for messages within the topic. The actual p2p message listening for the Topic starts only after at least one Topic.Subscribe invocation. This is to support fanout only mode, meaning node can publish on the Topic without listening for p2p messages. Now, we need to add an option to PubSub.Join that enforces the fanout only mode, even if Topic.Subscribe is called. As a side-effect then, only locally published messages will be served, using Topic.Publish.

[vyzo]

why is this useful or even a good idea?

[Wondertan]

@vyzo, pubsub API is quite ergonomic and we use it for interprocess message delivery besides over the network. You publish in one place and every service subscribed on the network gets the message, including local services of the publisher. Its clean and symmetric and doesn't require us to build our own internal pubsub.

However, we have a special type of publishers on a topic who don't need to listen for incoming messages from peers as they are the sole producers, but we still want their messages to be received by local services and the network and so when we Topic.Subscribe, it triggers such producers to receive messages from the peers which is what we want to avoid with this patch.

[vyzo]

yeah, but the problem is that the patch completely breaks subscribe.

[vyzo]

so you instantiate with fanout only and then you subscribe and it does nothing.

Maybe you should have subscribe rerurn an error when its fanoutonly.

I will leave it to @MarcoPolo to review nonetheless.

[Wondertan]

I am not sure I follow how it breaks the subscribe. The test proves the subscribe works in the expected way

[vyzo]

in process only, right?

[Wondertan]

Yes, the subscribe returns in-process messages only, while publish delivers to everyone: network and local subscribers. I wouldn't say it breaks the subscribe if that's the expected behavior governed by the option for power users.

[vyzo]

well, for your use case maybe not. It is still kind of weird to not receive network messages.

Anyway, no particularly strong opinions here, as i said i'll leave it to marco to review the LLM code, my eyes hurt when i read bot code.

[Wondertan]

FYI, there was a similar PR along ago #481 which was accepted and thus I thought there should not be any issues with this one.

As to bot code, it did generated the code, but I still carefully reviewed it. Also, it is unlikely anyone would notice if I didn't disclose it. The only thing that stands out here from a usual PR is slightly higher comment verbosity, other then that, its the most minimal and simple implementation for this feature human would do.

[MarcoPolo]

What if you add the option to control the mesh size per topic? Then for your usecase, you'd set the mesh size to 0. Something like the changes from #675 would make sure you use the fanout peers if your mesh is empty.

This solution is a bit more general and avoids special casing this use case. I also remember there being interest in changing the mesh size parameters per topic.

[Wondertan]

Interesting, so you say a side effect of setting mesh to 0 per topic would be the behavior where publisher doesn't receive anything from its peers? So a peer configured in this mode would never be grafted into meshes of its peers and thus they will never send msgs to it?

As to #675, do you think its sufficient? It feels like it solves an unrelated edge case from the description rather then doing enforcing fanout when mesh is empty. So is another patch needed besides the mesh size per topic opt?

And to clarify, does fanout map peers mean every peer who we are connected to with the topic? Meaning skipping the overlay mesh entirely, in floodsub style?

[vyzo]

it would still receive gossip.

[MarcoPolo]

it would still receive gossip.

Yes, that's a good point. Hlib probably doesn't want that gossip.

Alternative idea: Does this need to live in this library at all? Can @Wondertan make his own FanoutTopic type that has the same Publish signature and Subscribe signature but under the hood services subscriptions directly without involving gossipsub? Something like the following:

type FanoutOnlySub chan []byte

type FanoutOnlyTopic struct {
	innerTopic *Topic
	localSubs  []FanoutOnlySub
	// ...
}

func (t *FanoutOnlyTopic) Publish(ctx context.Context, data []byte, opts ...PubOpt) error {
	err := t.innerTopic.Publish(ctx, data, opts...)
	if err != nil {
		return err
	}
	for _, sub := range t.localSubs {
		select {
		case sub <- data:
		default:
			// Drop if chan has no capacity
		}
	}
	return nil
}

[Wondertan]

Does this need to live in this library at all?

Ultimately, this is up to you to decide. My general approach is to upstream features for others to reuse in a shared library, but maintainers may not see this as a right fit and that's fine.

We did consider doing our own pubsubing again when we stumbled on the problem, but I pushed for upstreaming, as we would need to do a subscription state with cancellations, decide locks vs chans, etc. Its all pretty standard mechanisms to implement, but it felt redundant given pubsub does have all that in place and upstreaming features is generally a good practice, especially since there is already pattern for new options in a non-breaking way.

[MarcoPolo]

My view is that upstreaming features is welcome when we notice this pattern being used by others. So far I don't know of anyone else needing this, but happy to be shown otherwise. If it's only for your use case, I'd suggest keeping it with your application as you can be a bit more efficient and tailored.

[sukunrt]

Apologies if this has been answered before.

Why don't you want all messages on the topic to be delivered for a Subscription?

However, we have a special type of publishers on a topic who don't need to listen for incoming messages from peers as they are the sole producers, but we still want their messages to be received by local services and the network and so when we Topic.Subscribe, it triggers such producers to receive messages from the peers which is what we want to avoid with this patch.

If the fanout only publisher is the sole producer, there aren't any other peers in the network publishing, right?

[Wondertan]

However, we have a special type of publishers on a topic who don't need to listen for incoming messages from peers as they are the sole producers

I intentionally used plural for publishers/producers to emphasize there are multiple of them on the network. They act as a relayers/bridges between networks and can be deployed permissionlessly. Due to unrelated recent changes local messages and peer messages started to cause a race condition motivating this patch.

[Wondertan]

My view is that upstreaming features is welcome when we notice this pattern being used by others.

This makes sense, though it makes me wonder why partial messages are part of pubsub in the first place, then. They seem to be a rather Ethereum-specific pubsub scaling strategy with a single user. I know they are behind protocol extensions, but this feature is behind an option as well. In both cases, they could live outside or inside the library.

[MarcoPolo]

My view is that upstreaming features is welcome when we notice this pattern being used by others.

This makes sense, though it makes me wonder why partial messages are part of pubsub in the first place, then. They seem to be a rather Ethereum-specific pubsub scaling strategy with a single user. I know they are behind protocol extensions, but this feature is behind an option as well. In both cases, they could live outside or inside the library.

This is a fair point. I'll add that partial messages itself has no ethereum specific parts to it (that's all done by the application), and does require a bit of integration within the library that isn't possible from outside the library. But the point still stands that it has a single user.

To add a bit more nuance, the inclusion of a feature is also welcome when it enables something that couldn't be done otherwise.

We did consider doing our own pubsubing again when we stumbled on the problem, but I pushed for upstreaming, as we would need to do a subscription state with cancellations, decide locks vs chans, etc. Its all pretty standard mechanisms to implement, but it felt redundant given pubsub does have all that in place

Right, this is a bit awkward to do within the application side as you end up with a lot of duplicated logic if you want to be consistent with what the library does internally, so I see your argument that this option is simpler than a wrapper.

Thanks for the discussion @Wondertan, I'll review this code today, and lean towards merging

[vyzo]

there is peecedence for something like this from IP Multicast.

[sukunrt]

@vyzo

there is peecedence for something like this from IP Multicast.

Can you share more details here?