Re: [PR] feat: support limit WriteBatch size,client outbuf limit and maxmemory clients [kvrocks]

["AntiTopQuark (via GitHub)" <gi...@apache.org>]

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.
   
   ![image](https://github.com/user-attachments/assets/13da9b02-3072-4096-9b75-9f5caa980a69)
   ![image](https://github.com/user-attachments/assets/966f4357-20df-4dcf-ad83-e241326b0e81)
   
   
   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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org