jt2594838 commented on a change in pull request #1561:
URL: https://github.com/apache/incubator-iotdb/pull/1561#discussion_r460605165



##########
File path: 
cluster/src/main/java/org/apache/iotdb/cluster/log/catchup/LogCatchUpTask.java
##########
@@ -178,11 +181,21 @@ void doLogCatchUpInBatch() throws TException, 
InterruptedException {
     }
 
     List<ByteBuffer> logList = new ArrayList<>();
-    for (int i = 0; i < logs.size() && !abort; i += LOG_NUM_IN_BATCH) {
+    for (int i = 0; i < logs.size() && !abort;) {
       logList.clear();
-      for (int j = i; j < i + LOG_NUM_IN_BATCH && j < logs.size(); j++) {
-        logList.add(logs.get(j).serialize());
+      long totalLogSize = 0;
+      int newStart = i;
+      for (int curNum = 0; curNum < LOG_NUM_IN_BATCH && i < logs.size(); i++, 
curNum++) {
+        ByteBuffer logData = logs.get(i).serialize();
+        totalLogSize += logData.array().length;
+        // we should send logs who's size is smaller than the max frame size 
of thrift
+        // left 200 byte for other fields of AppendEntriesRequest
+        if (totalLogSize > 
IoTDBDescriptor.getInstance().getConfig().getThriftMaxFrameSize() - 
LEFT_SIZE_IN_REQUEST) {
+          break;
+        }

Review comment:
       Report an error or warning here if the logList is empty when breaking, 
which means the configured size is too small to hold even one log and this may 
end up in a dead loop.
   
   But more importantly, does it matter if a request's size exceeds FrameSize? 
Will not thrift break a request into several frames? If so, what is the point 
to enforce request size, and if not, why does thrift call it a FramedTransport?




----------------------------------------------------------------
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.

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


Reply via email to