songgaoye commented on code in PR #3621:
URL: https://github.com/apache/brpc/pull/3621#discussion_r4349734102
##########
test/brpc_http_rpc_protocol_unittest.cpp:
##########
@@ -2319,6 +2320,43 @@ TEST_F(HttpTest, h2_ping_ack_respects_socket_write_cap) {
ASSERT_EQ(brpc::PARSE_ERROR_ABSOLUTELY_WRONG, pr.error());
}
+TEST_F(HttpTest, socket_overcrowded_state_tracks_pending_writes) {
+ int fds[2];
+ ASSERT_EQ(0, pipe(fds));
+ butil::fd_guard read_fd(fds[0]);
+ ASSERT_EQ(0, butil::make_non_blocking(read_fd));
+ brpc::SocketOptions options;
+ options.fd = fds[1];
+ brpc::SocketId id;
+ ASSERT_EQ(0, brpc::Socket::Create(options, &id));
+ brpc::SocketUniquePtr sock;
+ ASSERT_EQ(0, brpc::Socket::Address(id, &sock));
+
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+ brpc::FLAGS_socket_max_unwritten_bytes = 64;
+
+ butil::IOBuf data;
+ data.append(std::string(1024 * 1024, 'x'));
+ ASSERT_EQ(0, sock->Write(&data));
+ ASSERT_TRUE(sock->is_overcrowded());
+
+ brpc::FLAGS_socket_max_unwritten_bytes = 0;
+ ASSERT_FALSE(sock->is_overcrowded());
+ brpc::FLAGS_socket_max_unwritten_bytes = 64;
Review Comment:
I checked the call paths for this test. After Socket::Write() returns, the
background write path only updates _unwritten_bytes; it does not call
is_overcrowded() or read FLAGS_socket_max_unwritten_bytes. The reads in this
test are all initiated by the test thread, and this GoogleTest binary does not
run these tests in parallel. The temporary flag change is therefore not
concurrent with a read in this case, and intentionally verifies the
disabled-throttle behavior. No change needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
