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