siddharthteotia commented on pull request #6710: URL: https://github.com/apache/incubator-pinot/pull/6710#issuecomment-806261871
> With the addition of new data structure in this PR, there are essentially two places in DataTable where the key-value / name-value style structure is located. > > * First is the existing DataTable metadata which is also a series of key-value pairs where key is string and value is some statistic/metric. This is towards the beginning of the byte stream > * Second is the structure introduced in this PR which is written as a footer. > > Since we are anyway bumping up the version, how about we move the existing metadata of key-value pairs to the end of file to keep consistency in the format. So, all the metadata stuff (aka key-value pairs) + new positional stuff can be a file footer. > With this PR, we should resolve a couple of TODOs introduced in PR #6680 > > * Expose the serialization time through an API at the DataTable level and log it in [QueryScheduler](https://github.com/apache/incubator-pinot/pull/6710/files#diff-2bff83abd3f6e831acfe4b6d31a022f228710def4eea47db3929c6d90b3147ecR222). You need to serialize before the logging line. Currently it is after. > * Revisit [this](https://github.com/apache/incubator-pinot/pull/6710/files#diff-2bff83abd3f6e831acfe4b6d31a022f228710def4eea47db3929c6d90b3147ecR255). The execution cpu time is not yet serialized as part of metadata. May be we can just remove line 258. @mqliang , please make sure to address these TODOs -- 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. 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
