rust-libp2p icon indicating copy to clipboard operation
rust-libp2p copied to clipboard
verified publisher icon libp2p

feat(ping): making the ignoring of the first error configurable

Open stormshield-frb opened this issue 2 years ago • 5 comments

Description

Fix #5004

Notes & open questions

Instead of using a silent_first_error: bool, we could also use a ignored_errors_nb: usize. However, since there was previously a self.config.max_failures and it was removed, I did not re-do it. Of course, if it is preferred I can do it.

Change checklist

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

stormshield-frb avatar Dec 11 '23 13:12 stormshield-frb

@mxinden What do you think? Do you have any historical knowledge on this?

No historical knowledge beyond what is posted above. I am fine with either direction. Preference for no additional config flag if possible.

mxinden avatar Dec 28 '23 15:12 mxinden

@mxinden What do you think? Do you have any historical knowledge on this?

No historical knowledge beyond what is posted above. I am fine with either direction. Preference for no additional config flag if possible.

Okay thank you! Re-reading the spec, I think it is safe to move away from a long-lived stream and instead only ever send a single ping and close the stream afterwards. @stormshield-frb Would you mind re-writing this PR to do that? I think that should also solve #5004.

thomaseizinger avatar Dec 29 '23 01:12 thomaseizinger

About moving away from long-lived stream, will it not be a costly to open and close streams with TCP or Yamux for example ? I understand that it is almost free with QUIC but unfortunately it is not the case with every transports. What do you think about it ?

stormshield-frb avatar Jan 03 '24 08:01 stormshield-frb

About moving away from long-lived stream, will it not be a costly to open and close streams with TCP or Yamux for example ? I understand that it is almost free with QUIC but unfortunately it is not the case with every transports. What do you think about it ?

Yamux streams are similarly cheap to QUIC streams! :)

thomaseizinger avatar Jan 13 '24 23:01 thomaseizinger

This pull request has merge conflicts. Could you please resolve them @stormshield-frb? 🙏

mergify[bot] avatar Apr 15 '24 09:04 mergify[bot]