klsince commented on issue #15846: URL: https://github.com/apache/pinot/issues/15846#issuecomment-2949949500
Good point about the need for enforcing seqid in name. Although subclasses could extend shouldReplaceOnComparisonTie() to check those, but the initial logic in the base class is better to be fixed anyway. While understanding why creation.meta files could be different even for committed segments, I see that the creation.meta file is not included when computing segment crc value. Otherwise different crc values would trigger the slow servers to sync up their segment replicas with the committed segments. Including creation.meta in crc computation could be costly, as the servers would pretty much always see different crc values and have to download segment to sync up replicas. Looking into the idea proposed in [the PR](https://github.com/apache/pinot/pull/16006), instead of using an explicit Map, I think we may try to check and fix the creation.meta file during segment commit process (for the slower replicas). Like where crc is checked, we can double check creation time there too. Then the slower replicas can init the SegmentMetadata object to have the right creation time, before finish the commit process with segment it's built locally. (Feel free to tag me in the PR to continue the discussions there) -- 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
