[GitHub] [iotdb] Cpaulyz commented on a change in pull request #4279: [IOTDB-1609] Print the actual size of plan when it exceeds the wal_buffer_size

[GitBox <gi...@apache.org>]

Cpaulyz commented on a change in pull request #4279:
URL: https://github.com/apache/iotdb/pull/4279#discussion_r739770846



##########
File path: server/src/main/java/org/apache/iotdb/db/writelog/node/ExclusiveWriteLogNode.java
##########
@@ -115,8 +113,15 @@ public void write(PhysicalPlan plan) throws IOException {
     } catch (BufferOverflowException e) {
       // if the size of a single plan bigger than logBufferWorking
       // we need to clear the buffer to drop something wrong that has written.
+      DataOutputStream dos = new DataOutputStream(new ByteArrayOutputStream());
+      plan.serialize(dos);
+      int neededSize = dos.size();
+      dos.close();

Review comment:
       > It seems that there is no need to really do the serialization to get the actual size, maybe you can add a new method like `getSerializedSize()` in `PhysicalPlan`, each actual plan class to override that method to calculate its own size. In this way, you can avoid allocate extra memory in this `BufferOverflowException` situation.
   
   Yes, I totally agree with what you said. In fact, I wanted to do the same thing at first. 
   
   However, I noticed that there are quite a lot of classes extends `PhysicalPlan`. It needs to modify code massively. Considering that `getSerializedSize()` is only used for this exception and exception may be occurred rarely, I allocate extra memory to get the actual size. 
   
   Do you think it's necessary to do what you said? If so, I may need more time to implement `getSerializedSize()` in each actual plan class.




-- 
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: reviews-unsubscribe@iotdb.apache.org

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