cypherean commented on code in PR #17249:
URL: https://github.com/apache/pinot/pull/17249#discussion_r2564495836
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -438,10 +442,10 @@ protected void
replaceSegmentIfCrcMismatch(SegmentDataManager segmentDataManager
_logger.info("Segment: {} has CRC: {} same as before, not replacing it",
segmentName, localMetadata.getCrc());
return;
}
- if (!_instanceDataManagerConfig.shouldCheckCRCOnSegmentLoad()) {
+ if (!_instanceDataManagerConfig.shouldCheckCRCOnSegmentLoad() ||
_skipCrcCheckForThisTable) {
Review Comment:
The idea was to provide granular control for skipping CRC at a table level
instead of a server level config. IIUC skip crc at server level would cause the
same issue `!_instanceDataManagerConfig.shouldCheckCRCOnSegmentLoad()`.
We can remove these two configs from here(& `reloadSegment`) and only check
it in the `tryLoadExistingSegment` path. That way, we ensure that replace and
reload behaves as intended. What do you think?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
