deepthi912 commented on code in PR #13789:
URL: https://github.com/apache/pinot/pull/13789#discussion_r1715982997
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -1013,6 +1013,47 @@ public boolean tryLoadExistingSegment(SegmentZKMetadata
zkMetadata, IndexLoading
}
}
+ @Override
+ public boolean checkReloadSegment(SegmentZKMetadata zkMetadata,
IndexLoadingConfig indexLoadingConfig) {
+ String segmentName = zkMetadata.getSegmentName();
+
+ // Creates the SegmentDirectory object to access the segment metadata.
+ // The metadata is null if the segment doesn't exist yet.
+ SegmentDirectory segmentDirectory =
+ tryInitSegmentDirectory(segmentName,
String.valueOf(zkMetadata.getCrc()), indexLoadingConfig);
+ SegmentMetadataImpl segmentMetadata = (segmentDirectory == null) ? null :
segmentDirectory.getSegmentMetadata();
+
+ // If the segment doesn't exist on server or its CRC has changed, then we
+ // need to fall back to download the segment from deep store to load it.
+ if (segmentMetadata == null || !hasSameCRC(zkMetadata, segmentMetadata)) {
+ if (segmentMetadata == null) {
+ _logger.info("Segment: {} does not exist", segmentName);
+ } else if (!hasSameCRC(zkMetadata, segmentMetadata)) {
+ _logger.info("Segment: {} has CRC changed from: {} to: {}",
segmentName, segmentMetadata.getCrc(),
+ zkMetadata.getCrc());
+ }
+ closeSegmentDirectoryQuietly(segmentDirectory);
+ return true;
+ }
+
+ try {
+ Schema schema = indexLoadingConfig.getSchema();
+ //if the reload of the segment is not needed then return false
+ if (!ImmutableSegmentLoader.needPreprocess(segmentDirectory,
indexLoadingConfig, schema)) {
+ _logger.info("Segment: {} is consistent with latest table config and
schema", segmentName);
+ return false;
+ } else {
+ //if re processing or reload is needed on a segment then return true
+ _logger.info("Segment: {} needs reprocess to reflect latest table
config and schema", segmentName);
+ return true;
+ }
+ } catch (Exception e) {
+ _logger.error("Failed to check if reload is needed for a segment {}",
segmentName, e);
+ closeSegmentDirectoryQuietly(segmentDirectory);
+ return false;
Review Comment:
@Jackie-Jiang I had a question for this case of exception, should this case
return that reload is needed or not? By default I returned false but the actual
situation is we didn't check for the reload due to some exception.
--
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
