gortiz commented on code in PR #13303:
URL: https://github.com/apache/pinot/pull/13303#discussion_r1738779658


##########
pinot-common/src/main/java/org/apache/pinot/common/datablock/DataBlock.java:
##########
@@ -36,21 +39,25 @@ public interface DataBlock {
 
   int getNumberOfRows();
 
+  int getNumberOfColumns();
+
   void addException(ProcessingException processingException);
 
   void addException(int errCode, String errMsg);
 
   Map<Integer, String> getExceptions();
 
-  byte[] toBytes()
+  /**
+   * This is a wrapper on top of {@link DataBlockUtils#serialize(DataBlock)} 
but implementations can cache
+   * the result so messages that are sent to more than one receiving mailbox 
don't need to be serialized as many times.
+   */
+  List<ByteBuffer> serialize()

Review Comment:
   This method is mainly called in `GrpcSendingMailbox` to get the list of byte 
buffers and wrap them into a GRPC ByteString.
   
   It is not useful to return a `DataBuffer` because there is no way to safely 
convert that into a ByteString. We would need to do something like:
   ```
   ByteString byteString;
   DataBuffer buffer = block.serialize();
   if (buffer instanceof CompoundDataBuffer) {
     List<ByteBuffer> bbs = new ArrayList<>()
     ((CompoundDataBuffer) buffer).appendTo(bbs);
     
     byteString = UnsafeByteOperations.unsafeWrap(bytes.get(0));
     for (int i = 1; i < bytes.size(); i++) {
       byteString = 
byteString.concat(UnsafeByteOperations.unsafeWrap(bytes.get(i)));
     }
   } else {
     byteString =  UnsafeByteOperations.unsafeWrap(buffer.toByteBuffer());
   }
   ```
   
   As you said, we could force to return a CompoundDataBuffer in the signature, 
but I though that was an arbitrary limit.
   
   The pure requirement is to return a list of ByteBuffer and that is why I 
decided to use that.



-- 
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: commits-unsubscr...@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to