Skip to content

Commit d433599

Browse files
committed
sv2: gate cancel_recv on pending data
Track the total bytes forwarded to the transport and rely on the final MSG_MORE prediction when deciding whether to suppress receive work. This keeps the regular send path from throttling reads when the queue drains after a write. Assisted-by: GitHub Copilot Assisted-by: OpenAI GPT-5-Codex
1 parent 50ea909 commit d433599

File tree

1 file changed

+13
-12
lines changed

1 file changed

+13
-12
lines changed

src/sv2/connman.cpp

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,9 @@ void Sv2Connman::EventReadyToSend(NodeId node_id, bool& cancel_recv)
126126
LOCK(client->cs_send);
127127
auto it = client->m_send_messages.begin();
128128
std::optional<bool> expected_more;
129-
130129
size_t total_sent = 0;
131130

132-
while(true) {
131+
while (true) {
133132
if (it != client->m_send_messages.end()) {
134133
// If possible, move one message from the send queue to the transport.
135134
// This fails when there is an existing message still being sent,
@@ -142,8 +141,8 @@ void Sv2Connman::EventReadyToSend(NodeId node_id, bool& cancel_recv)
142141
}
143142
}
144143

145-
const auto& [data, more, _m_message_type] = client->m_transport->GetBytesToSend(/*have_next_message=*/it != client->m_send_messages.end());
146-
144+
const auto& [data, more, _m_message_type] =
145+
client->m_transport->GetBytesToSend(/*have_next_message=*/it != client->m_send_messages.end());
147146

148147
// We rely on the 'more' value returned by GetBytesToSend to correctly predict whether more
149148
// bytes are still to be sent, to correctly set the MSG_MORE flag. As a sanity check,
@@ -155,22 +154,24 @@ void Sv2Connman::EventReadyToSend(NodeId node_id, bool& cancel_recv)
155154
std::string errmsg;
156155

157156
if (!data.empty()) {
158-
LogPrintLevel(BCLog::SV2, BCLog::Level::Trace, "Send %d bytes to client id=%zu\n",
159-
data.size() - total_sent, node_id);
157+
LogPrintLevel(BCLog::SV2, BCLog::Level::Trace,
158+
"Send %d bytes to client id=%zu\n", data.size(), client->m_id);
160159

161-
sent = SendBytes(node_id, data, more, errmsg);
160+
sent = SendBytes(client->m_id, data, more, errmsg);
162161
}
163162

164163
if (sent > 0) {
165164
client->m_transport->MarkBytesSent(sent);
165+
total_sent += sent;
166166
if (static_cast<size_t>(sent) != data.size()) {
167167
// could not send full message; stop sending more
168168
break;
169169
}
170170
} else {
171171
if (sent < 0) {
172-
LogDebug(BCLog::NET, "socket send error for peer=%d: %s\n", node_id, errmsg);
173-
CloseConnection(node_id);
172+
LogDebug(BCLog::NET, "socket send error for peer=%d: %s\n",
173+
client->m_id, errmsg);
174+
CloseConnection(client->m_id);
174175
}
175176
break;
176177
}
@@ -179,16 +180,16 @@ void Sv2Connman::EventReadyToSend(NodeId node_id, bool& cancel_recv)
179180
// Clear messages that have been handed to transport from the queue
180181
client->m_send_messages.erase(client->m_send_messages.begin(), it);
181182

183+
const bool more_pending = expected_more.value_or(false);
184+
182185
// If both receiving and (non-optimistic) sending were possible, we first attempt
183186
// sending. If that succeeds, but does not fully drain the send queue, do not
184187
// attempt to receive. This avoids needlessly queueing data if the remote peer
185188
// is slow at receiving data, by means of TCP flow control. We only do this when
186189
// sending actually succeeded to make sure progress is always made; otherwise a
187190
// deadlock would be possible when both sides have data to send, but neither is
188191
// receiving.
189-
//
190-
// TODO: decide if this is useful for Sv2
191-
cancel_recv = total_sent > 0; // && more;
192+
cancel_recv = total_sent > 0 && more_pending;
192193
}
193194

194195
void Sv2Connman::EventGotData(Id id, std::span<const uint8_t> data)

0 commit comments

Comments
 (0)