walterddr commented on code in PR #9711: URL: https://github.com/apache/pinot/pull/9711#discussion_r1014152556
########## pinot-common/src/main/java/org/apache/pinot/common/datablock/MetadataBlock.java: ########## @@ -18,28 +18,118 @@ */ package org.apache.pinot.common.datablock; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.annotations.VisibleForTesting; import java.io.IOException; import java.nio.ByteBuffer; -import org.apache.pinot.common.utils.DataSchema; /** - * Wrapper for row-wise data table. It stores data in row-major format. + * A block type to indicate some metadata about the current processing state. + * For the different types of metadata blocks see {@link MetadataBlockType}. */ public class MetadataBlock extends BaseDataBlock { - private static final int VERSION = 1; - public MetadataBlock() { - super(0, null, new String[0], new byte[]{0}, new byte[]{0}); + private static final ObjectMapper JSON = new ObjectMapper(); + + @VisibleForTesting + static final int VERSION = 1; + + public enum MetadataBlockType { + /** + * Indicates that this block is the final block to be sent + * (End Of Stream) as part of an operator chain computation. + */ + EOS, + + /** + * An {@code ERROR} metadata block indicates that there was + * some error during computation. To retrieve the error that + * occurred, use {@link MetadataBlock#getExceptions()} + */ + ERROR, + + /** + * A {@code NOOP} metadata block can be sent at any point to + * and should be ignored by downstream - it is often used to + * indicate that the operator chain either has nothing to process + * or has processed data but is not yet ready to emit a result + * block. + */ + NOOP; + + MetadataBlockType() { + } } - public MetadataBlock(DataSchema dataSchema) { - super(0, dataSchema, new String[0], new byte[]{0}, new byte[]{0}); + /** + * Used to serialize the contents of the metadata block conveniently and in + * a backwards compatible way. Use JSON because the performance of metadata block + * SerDe should not be a bottleneck. + */ + @JsonIgnoreProperties(ignoreUnknown = true) + @VisibleForTesting + static class Contents { + + private String _type; + + @JsonCreator + public Contents(@JsonProperty("type") String type) { + _type = type; + } + + @JsonCreator + public Contents() { + _type = null; + } + + public String getType() { + return _type; + } + + public void setType(String type) { + _type = type; + } + } + + private final Contents _contents; + + public MetadataBlock(MetadataBlockType type) { + super(0, null, new String[0], new byte[]{0}, toContents(new Contents(type.name()))); + _contents = new Contents(type.name()); + } + + private static byte[] toContents(Contents type) { + try { + return JSON.writeValueAsBytes(type); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } } public MetadataBlock(ByteBuffer byteBuffer) throws IOException { super(byteBuffer); + if (_variableSizeDataBytes != null) { + _contents = JSON.readValue(_variableSizeDataBytes, Contents.class); + } else { + _contents = new Contents(); + } + } + + public MetadataBlockType getType() { + String type = _contents.getType(); + + // if type is null, then we're reading a legacy block where we didn't encode any + // data. assume that it is an EOS block if there's no exceptions and an ERROR block + // otherwise + return type == null + ? (getExceptions().isEmpty() ? MetadataBlockType.EOS : MetadataBlockType.ERROR) + : MetadataBlockType.valueOf(type); Review Comment: we dont need to consider backward compatibility here. --> if the type is null, that means it is a legacy block. you will throw when deserializing anyway. -- 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