AntiTopQuark commented on code in PR #2498:
URL: https://github.com/apache/kvrocks/pull/2498#discussion_r1768895709
##########
src/server/worker.cc:
##########
@@ -446,6 +452,13 @@ Status Worker::Reply(int fd, const std::string &reply) {
auto iter = conns_.find(fd);
if (iter != conns_.end()) {
iter->second->SetLastInteraction();
+ if (iter->second->CheckClientReachOBufLimits(reply.size() +
evbuffer_get_length(iter->second->Output()))) {
Review Comment:
Hi, I noticed that the operator += and append are used quite frequently. I
need to convert all `redis::xxx` to `GET_OR_RET(conn->xxx())` to check whether
the output buffer size meets the expectations each time. Even if I change it to
check the client output buffer every 10 times, I suspect this might have a
significant impact on performance.
My idea is to create multiple member functions for conn to replace the
original redis::xxxx() functions, so that the memory size check can be
performed each time an output is made. If the check fails, an error code will
be thrown.
```c++
StatusOr<std::string> Connection::SimpleString(const std::string &msg) {
auto res = redis::SimpleString(msg);
if (CheckClientReachOutputBufferLimits(res)) {
return {Status::ReachClientOutputBufferLimit, "client reached output
buffer limits"};
}
return res;
}
- *output += redis::SimpleString("Capacity");
- *output += redis::SimpleString("Size");
+ *output += GET_OR_RET((conn->SimpleString("Capacity"));
+ *output += GET_OR_RET((conn->SimpleString("Size"));
```
--
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]
