search

filecoin-project / go-fil-markets

Shared Implementation of Storage and Retrieval Markets for Filecoin Node Implementations
Other
78 stars 59 forks source link

Protect libp2p connection for handling query protocol. #772

Open hannahhoward opened 1 year ago

hannahhoward commented 1 year ago

Goals

When making queries on the QueryAsk protocol, we get a number of stream resets while waiting for Query responses, even though the client is protecting its side of the connection.

We may need to protect it on the server side, somewhere in the code path of https://github.com/filecoin-project/go-fil-markets/blob/master/retrievalmarket/network/libp2p_impl.go#L100 or https://github.com/filecoin-project/go-fil-markets/blob/master/retrievalmarket/impl/provider.go#L328

This should be a relatively short lived request/response cycle. Is it still reasonable to believe Libp2p might cleanup the connections before the response is sent under some conditions? We are seeing a number of stream reset messages on both sides.

cc: @marten-seemann

marten-seemann commented 1 year ago

It's possible, but unfortunately, there are tons of reasons why streams are reset. You won't believe how many times this has already annoyed me, it's basically a black box at the moment! It could be the connection manager killing connection (as you correctly note, the probability of this is roughly proportional to the time between a request and a response). It could also be the resource manager killing streams because some limit is exceeded.

Protecting the connection doesn't sound like a crazy idea.

I've recently opened https://github.com/libp2p/specs/issues/479, a proposal to add error codes to stream resets and connection closes. Communicating just a few bits of information would go a long way in helping in situations like this.