gortiz commented on code in PR #12704: URL: https://github.com/apache/pinot/pull/12704#discussion_r1566905246
########## pinot-common/src/main/java/org/apache/pinot/common/datablock/MetadataBlock.java: ########## @@ -36,108 +39,129 @@ */ public class MetadataBlock extends BaseDataBlock { - private static final ObjectMapper JSON = new ObjectMapper(); - + private static final Logger LOGGER = LoggerFactory.getLogger(MetadataBlock.class); @VisibleForTesting - static final int VERSION = 1; + static final int VERSION = 2; Review Comment: I think we are not going to add support for 2nd version in the code we have in master, but the code in this PR actually reads 1st and 2nd version. We don't need to do the former because we are lucky and the binary format should be compatible (though we may need to verify that adding a test in master). The implementation in both branches mostly ignore metrics in the other, but even they ignore the content, they are able to read them. We are just ignoring the content because we think it is not worth to actually convert from one format to the other given the impact will be to do not have all stats during the upgrade, but we could do that if needed. I think it is fine to increase the version. -- 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