Change Details

It has been observed that if both sides of a P2P connection have a significant amount of data to send, a stall can occur, where both try to drain their own send queue before trying to receive. The same issue seems to apply to the current Bitcoin Core codebase, though I don't know whether it's a frequent issue for us. The core issue is that whenever our optimistic send fails to fully send a message, we do subsequently not even select() for receiving; if it then turns out that sending is not possible either, no progress is made at all. To address this, the solution used in this PR is to still select() for both sending and receiving when an optimistic send fails, but skip receiving if sending succeeded, and (still) doesn't fully drain the send queue. This is a significant reduction in how aggressive the "receive pushback" mechanism is, because now it will only mildly push back while sending progress is made; if the other side stops receiving entirely, the pushback disappears. I don't think that's a serious problem though: - We still have a pushback mechanism at the application buffer level (when the application receive buffer overflows, receiving is paused until messages in the buffer get processed; waiting on our own net_processing thread, not on the remote party). - There are cases where the existing mechanism is too aggressive; e.g. when the send queue is non-empty, but tiny, and can be sent with a single send() call. In that case, I think we'd prefer to still receive within the same processing loop of the network thread. Co-authored-by: Anthony Towns <aj@erisian.com.au> This is a backport of [[https://github.com/bitcoin/bitcoin/pull/27981 | core#27981]] Depends on D17158

It has been observed that if both sides of a P2P connection have a significant amount of data to send, a stall can occur, where both try to drain their own send queue before trying to receive. The same issue seems to apply to the current Bitcoin Core codebase, though I don't know whether it's a frequent issue for us. The core issue is that whenever our optimistic send fails to fully send a message, we do subsequently not even select() for receiving; if it then turns out that sending is not possible either, no progress is made at all. To address this, the solution used in this PR is to still select() for both sending and receiving when an optimistic send fails, but skip receiving if sending succeeded, and (still) doesn't fully drain the send queue. This is a significant reduction in how aggressive the "receive pushback" mechanism is, because now it will only mildly push back while sending progress is made; if the other side stops receiving entirely, the pushback disappears. I don't think that's a serious problem though: - We still have a pushback mechanism at the application buffer level (when the application receive buffer overflows, receiving is paused until messages in the buffer get processed; waiting on our own net_processing thread, not on the remote party). - There are cases where the existing mechanism is too aggressive; e.g. when the send queue is non-empty, but tiny, and can be sent with a single send() call. In that case, I think we'd prefer to still receive within the same processing loop of the network thread. Co-authored-by: Anthony Towns <aj@erisian.com.au> core#28287: > rpc: add test-only sendmsgtopeer rpc > > This rpc can be used when we want a node to send a message, but > cannot use a python P2P object, for example for testing of low-level > net transport behavior. > test: add basic tests for sendmsgtopeer to rpc_net.py > test: add functional test for deadlock situation This is a backport of [[https://github.com/bitcoin/bitcoin/pull/27981 | core#27981]] and [[https://github.com/bitcoin/bitcoin/pull/28287 | core#28287]]

It has been observed that if both sides of a P2P connection have a significant amount of data to send, a stall can occur, where both try to drain their own send queue before trying to receive. The same issue seems to apply to the current Bitcoin Core codebase, though I don't know whether it's a frequent issue for us. The core issue is that whenever our optimistic send fails to fully send a message, we do subsequently not even select() for receiving; if it then turns out that sending is not possible either, no progress is made at all. To address this, the solution used in this PR is to still select() for both sending and receiving when an optimistic send fails, but skip receiving if sending succeeded, and (still) doesn't fully drain the send queue. This is a significant reduction in how aggressive the "receive pushback" mechanism is, because now it will only mildly push back while sending progress is made; if the other side stops receiving entirely, the pushback disappears. I don't think that's a serious problem though: - We still have a pushback mechanism at the application buffer level (when the application receive buffer overflows, receiving is paused until messages in the buffer get processed; waiting on our own net_processing thread, not on the remote party). - There are cases where the existing mechanism is too aggressive; e.g. when the send queue is non-empty, but tiny, and can be sent with a single send() call. In that case, I think we'd prefer to still receive within the same processing loop of the network thread. Co-authored-by: Anthony Towns <aj@erisian.com.au> This is a backport of [[https://github.com/bitcoin/bitcoin/pull/27981 | core#27981]]8287: Depends on D17158> rpc: add test-only sendmsgtopeer rpc > > This rpc can be used when we want a node to send a message, but > cannot use a python P2P object, for example for testing of low-level > net transport behavior. > test: add basic tests for sendmsgtopeer to rpc_net.py > test: add functional test for deadlock situation This is a backport of [[https://github.com/bitcoin/bitcoin/pull/27981 | core#27981]] and [[https://github.com/bitcoin/bitcoin/pull/28287 | core#28287]]