fix(request-response): Avoid hanging at capacity and on dial IO errors

[oblique]

Description

This is an alternative fix of #5417 that does not introduce a breaking change.

The idea is to reschedule the request but also avoid a potential infinite rescheduling by applying a timeout on it.

This fixes also a potential infinite rescheduling issue on dial IO errors:

rust-libp2p/protocols/request-response/src/handler.rs

Lines 238 to 244 in 94fef37

	StreamUpgradeError::Io(e) => {
	tracing::debug!(
	"outbound stream for request {} failed: {e}, retrying",
	message.request_id
	);
	self.requested_outbound.push_back(message);
	}

Notes & open questions

Due to the nature of rescheduling and timeout, it is very hard to create a unit test. I couldn't find a reliable way of doing it.

Change checklist

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• A changelog entry has been made in the appropriate crates

[thomaseizinger]

I don't think rescheduling like that is a good idea. Causing more load on an overloaded system is not good. Any form of retry should have an (exponential) backoff.

So far, we've avoided retries and left them to the user. Only they know whether a message is safe to retry upon an IO or other error.

HTTP can do automated retries for the user but only because it is extremely rich in semantics. We don't have that, we just ship bytes around.

[oblique]

Do you prefer the #5417 then? But without the new variant?

The retry on dial failure was there from before. Should I remove it or keep the fix for that?

[zvolin]

Maybe we could skip retrying here and just keep the hard timeout + io error on max streams opened

[thomaseizinger]

Do you prefer the #5417 then? But without the new variant?

I think so yes. Unless we have a requirement to specifically match on that error, I would say using the Io error variant is preferable.

The retry on dial failure was there from before. Should I remove it or keep the fix for that?

Hmm, I didn't remember that. Don't really have an opinion here. I'll defer to @jxs.

[oblique]

Closing in favor of #5417 and #5429