Jackie-Jiang commented on code in PR #17380:
URL: https://github.com/apache/pinot/pull/17380#discussion_r2688724670
##########
pinot-common/src/main/java/org/apache/pinot/common/metadata/segment/SegmentZKMetadata.java:
##########
@@ -182,6 +182,22 @@ public void setDataCrc(long dataCrc) {
setNonNegativeValue(Segment.DATA_CRC, dataCrc);
}
+ public boolean isUseDataCrcForReplace() {
Review Comment:
(minor) Keep name consistent, same for other places
```suggestion
public boolean isUseDataCrc() {
```
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -1782,6 +1782,7 @@ public static class Offline {
public static final String TOTAL_DOCS = "segment.total.docs";
public static final String CRC = "segment.crc";
public static final String DATA_CRC = "segment.data.crc";
+ public static final String USE_DATA_CRC_FOR_REPLACE =
"segment.use.data.crc";
Review Comment:
(minor) Let's keep the names consistent
```suggestion
public static final String USE_DATA_CRC = "segment.use.data.crc";
```
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -1659,8 +1659,16 @@ private SegmentDirectory initSegmentDirectory(String
segmentName, String segment
return segmentDirectoryLoader.load(indexDir.toURI(), loaderContext);
}
+ // CRC check can be performed on both segment CRC and data CRC (if
available) based on the ZK property value of
+ // useDataCRCForReplace.
private static boolean hasSameCRC(SegmentZKMetadata zkMetadata,
SegmentMetadata localMetadata) {
- return zkMetadata.getCrc() == Long.parseLong(localMetadata.getCrc());
+ if (zkMetadata.getCrc() == Long.parseLong(localMetadata.getCrc())) {
+ return true;
+ }
+ return zkMetadata.isUseDataCrcForReplace()
+ && zkMetadata.getDataCrc() >= 0
Review Comment:
(minor) Reformat
##########
pinot-common/src/main/java/org/apache/pinot/common/metadata/segment/SegmentZKMetadataUtils.java:
##########
@@ -111,6 +115,9 @@ private static void updateSegmentZKMetadata(String
tableNameWithType, SegmentZKM
} else {
// For uploaded segment
+ //clear the use data crc flag for uploaded segments
Review Comment:
(minor) Reformat
--
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]
