agavra commented on code in PR #9711: URL: https://github.com/apache/pinot/pull/9711#discussion_r1014154668
########## 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: I was very careful to make sure that it won't throw when deserializing (see the tests). is there any reason why we don't need to consider backwards compatibility? if I don't then I can be less hacky in the serialization format! -- 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