Publisher and Sender interface overlap

[masih]

It seems publisher and sender overlap in terms of interface. in index-provider library announce methods in publisher interface are never called and instead senders are used. Do we need to revise this interface?

https://github.com/ipni/go-libipni/blob/9116d75b32601f01dc7d42ba0e5d9c47b3ca1a67/dagsync/interface.go#L20C1-L26

[gammazero]

There is no overlap. This is just a confusing use within the index-provider.

The index-provider's publishers use an HTTP sender that sends direct announcements to the URLs configured for the engine to send to. The PublishLatestHTTP function creates its own announcement sender, and does not use those of the publisher. It needs to do this in order to:

• send direct HTTP announcements to URLs other than those configured for the engine.
• send direct HTTP announcements even when the publisher does not have an HTTP sender.

PublishLatestHTTP is only used when an admin command is used to make the index-provider send direct HTTP announcements to specific URLs. The admin server calls it here.

This comment here is wrong:

// * engine takes pubHttpAnnounceAddrs option to allow configuring which addrs should be announced.
//   But those addrs are only used by the bespoke sender, _not_ the HTTP sender inside publishers.

In fact the pubHttpAnnounceAddrs are used by the "bespoke" sender, and by the http sender inside the publishers,
here. Maybe that is the "workaround" mentioned in the comment.

Conclusion

The code in index-provider appears to be correct, and I have created a PR to update the comments.

A new method to allow the HTTP sender to send to alternate destination URLs does not work, since that engine's publisher may not be configured with an HTTP sender, and direct announcements can only be sent using the HTTP sender (pubsub sender cannot send direct).

Details

In indexer-provider an HTTP sender is created here, and a pubsub sender is created here if configured.

Note: The HTTP sender is created with e.announceURLs configured for the engine. These are the URLs of indexers to send the announcements to. Might be better named destinationIndexerURLs.

If the publisher is an HTTP publisher, it is given this set of senders here
If the publisher is a data-transfer publisher, it is given the set of senders here

When the publisher's UpdateRoot method is called here or here, this updates the head ad CID and sends an announce using all the senders, here

When using an HTTP publisher, it is necessary to explicitly put the provider's URL, where the advertisements are available at, into the announcement message. This is done here with e.pubHttpAnnounceAddrs (better named adPublishedAtURLs).

[MichaelMure]

@gammazero I think the point got missed: AnnounceHead , AnnounceHeadWithAddrs and UpdateRootWithAddrs in the go-libipni's Publisher interface are never called by the engine, should they be removed?

I'd also add: if the engine is doing the announcing part, can that announcing be removed from the Publisher so that it only handle the sync part? That would greatly reduce the complexity of httpsync and dtsync, and align them in "single-responsibility" with httpsender and p2psender.

Those two things would greatly reduce the coupling between the engine and dagsync, which would gives the flexibility I need to implement a HTTP sync where the HTTP server is delegated (say, a S3 bucket).

[gammazero]

The Announce methods are not being used, and the UpdateRootWithAddrs is only being used because the Publisher is also doing the announcing (an not exposing its senders). The coupling between publisher and senders is not necessary, and was only done more as a convenience so that updates would automatically sending announcements.

I will separate the senders from the publisher so that sending announcements is completely independent of publishing (serving) the advertisements. Thank you for helping me understand what you are looking for.

[MichaelMure]

Awesome, thank you.

[gammazero]

@MichaelMure, @masih Here is a PR for index-provider that separates the publisher from the sender: ipni/index-provider#392

@masih Given the above, it now makes sense to separate the publishers from the senders in go-libipni. There is no strong reason to associate a set of announce senders with a publisher. The only association is that a sender is often used to after updating the publisher's head CID.