Peer-And-Content-Routing example: Remove or better document 100ms delay

[mcclure]

This example
https://github.com/libp2p/js-libp2p/blob/master/examples/peer-and-content-routing
Involves running two scripts simultaneously; they then connect to each other. Each one does this before calling findPeers

// The DHT routing tables need a moment to populate
await delay(100)

This is fine for an example/test but is not good enough for production code. There's no indication how to modify this 100ms delay when moving the example code into production code.

Expected behavior: There should be some other type of event— for example, js-libp2p-kad-dht announcing it has finished negotiating dht with the remote server— that can be awaited on, and the example should use it.

[alxroyer]

Also interested in this info.

I've tracked my experimentations on that topic through a dedicated repo: https://github.com/Alexis-ROYER/js-libp2p-content-routing-delay

According to my observations:

• No need to await for "propagation" in the given examples, in as much as there is a single intermediate node.
• Still, time must be awaited after the connection is established with the remote:
  1. before a content provider provides its content,
  2. and before a querier asks for this content.
• The required time to await got worse with [email protected] compared with [email protected],
  coming close to the 100 ms timings proposed in the examples.

Has anyone pointed out a formal event that should be awaited from the DHT before providing to / querying from it?

[inverted-capital]

In a system designed to operate with intermittent network connectivity, giving an error because you have no peers currently seems odd ?

Would a PR be accepted on this topic, if it added an abortable smart retry whenever there were no peers ?

For example, if no peers are found, we cache the request, and await the internal event (which is not exposed, but can be seen from enabling DEBUG='libp2p:*') for a new peer appearing, and then keep on trying whenever a new peer appears until we get acceptable success ?

This would close this ticket, because there would no longer be any delay required before using the DHT.

If acceptable, we would seek to do the same for pubsub - transient periods of no peers is expected behavior of this library - it says so on the tin

[inverted-capital]

Or, is the design intent of libp2p that retry functionality should be left up to the user, in which case we would aim to provide a module that could wrap the dht implementation by offering local persistence with retry ? @vasco-santos what are your thoughts ?

Our opinion is that if a repo was supplied to libp2p, the user intends for long term persistence, and that this implicitly means long term retry and refresh of dht operations, which would be started and stopped via libp2p.start() and libp2p.stop()

[inverted-capital]

We've had a chat about this, and would like to present a summary here.

Our belief is that libp2p presents a virtualized model of the conventional network as an ideal network node. The turmoil of the real world is gracefully abstracted away by libp2p and the developer can concentrate on the application logic, trusting that libp2p will eventually do what the developer asked. The tradeoff for this idealization of the network is time. In fact, 3 of the front page claims for libp2p imply eventuality of completion: Native Roaming, Work Offline, and Good For High Latency Scenarios.

To achieve this, libp2p has some memory, namely the peerstore, that can help accelerate the performance of its assurances to the developer. But two areas where it fails, due to no memory, is with the dht and pubsub modules.

Our expectation of an ideal dht is that it should:

1. Return an async iterable on put() that resolved with lifecycle events, like dialing, querying, put1, put2, put3, removed
2. Continue its lifecycle effort on node reload, if the storage indicates an incomplete task
3. Maintain its put() values across the network periodically
4. Retry forever until its key is overwritten, or until enough nodes have been reached

Our expectation of an ideal pubsub is that it should:

1. Return an async iterable on publish() that resolved with lifecycle events
2. Continue its lifecycle effort on node reload, if the storage indicates an incomplete task
3. On new peer subscriptions detected, we should republish whatever we published, since a new node is served infinitely better with an old publication rather than nothing. They might have missed the broadcast by a mere 1ms, so they should be 'caught up' by default.

@achingbrain may we please have some guidance on:

1. The errors in our presumptions here
2. How would you like this implemented in libp2p, then we can have a shot at it

Our suggestion would be to implement a Startable interface class for both dht and pubsub, and intercept all calls to the backing dht or pubsub modules, performing long retry and other tasks, along with reload tasks. The storage we would use would be datastore keys with prefix /dht/put and /pubsub/publish/topic

[inverted-capital]

This PR from @achingbrain helps show what is being waited for: https://github.com/libp2p/js-libp2p/pull/1549/files

[inverted-capital]

Sir this delay is still in the code for the example:

js-libp2p/examples/peer-and-content-routing/1.js

Line 53 in b57bca4

await delay(1000)

May we pretty please have some event that fires when the tables have propagated instead of polling for population ?

[maschad]

I think there is a fundamental misunderstanding with how Kademlia works. When looking for a peer, the local node contacts the k closest nodes to the remote peer’s ID asking them for closer nodes. The local node repeats the process until it finds the peer or determines that it is not in the network. This means that if you are unable to find the content you are looking for, the node needs to await more peers being discovered as opposed to continuously querying the DHT. In this particular example we could await the connection to be established for the remote, so that PR could be opened as a less hacky fix.

Now once a substantial amount of nodes are connected to the node, there is a PR that could introduce progress events to assess each query on the DHT but that still would not solve this issue.