klsince commented on a change in pull request #7319:
URL: https://github.com/apache/pinot/pull/7319#discussion_r708510718
##########
File path:
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java
##########
@@ -259,53 +255,50 @@ private void reloadSegment(String tableNameWithType,
SegmentMetadata segmentMeta
return;
}
- Preconditions.checkState(indexDir.isDirectory(), "Index directory: %s is
not a directory", indexDir);
- File parentFile = indexDir.getParentFile();
- File segmentBackupDir =
- new File(parentFile, indexDir.getName() +
CommonConstants.Segment.SEGMENT_BACKUP_DIR_SUFFIX);
+ SegmentZKMetadata zkMetadata =
+ ZKMetadataProvider.getSegmentZKMetadata(_propertyStore,
tableNameWithType, segmentName);
+ Preconditions.checkNotNull(zkMetadata);
// This method might modify the file on disk. Use segment lock to prevent
race condition
Lock segmentLock = SegmentLocks.getSegmentLock(tableNameWithType,
segmentName);
try {
segmentLock.lock();
- // First rename index directory to segment backup directory so that
original segment have all file descriptors
- // point to the segment backup directory to ensure original segment
serves queries properly
-
- // Rename index directory to segment backup directory (atomic)
- Preconditions.checkState(indexDir.renameTo(segmentBackupDir),
- "Failed to rename index directory: %s to segment backup directory:
%s", indexDir, segmentBackupDir);
+ // Reloads an existing segment, and the local segment metadata is
existing as asserted above.
+ tableDataManager
+ .reloadSegment(segmentName, new
IndexLoadingConfig(_instanceDataManagerConfig, tableConfig), zkMetadata,
+ segmentMetadata, schema, forceDownload);
+ LOGGER.info("Reloaded segment: {} of table: {}", segmentName,
tableNameWithType);
+ } finally {
+ segmentLock.unlock();
+ }
+ }
- // Copy from segment backup directory back to index directory
- FileUtils.copyDirectory(segmentBackupDir, indexDir);
+ @Override
+ public void addOrReplaceSegment(String tableNameWithType, String segmentName)
+ throws Exception {
+ LOGGER.info("Adding or replacing segment: {} for table: {}", segmentName,
tableNameWithType);
- // Load from index directory
- ImmutableSegment immutableSegment = ImmutableSegmentLoader
- .load(indexDir, new IndexLoadingConfig(_instanceDataManagerConfig,
tableConfig), schema);
+ // Get updated table config and segment metadata from Zookeeper.
+ TableConfig tableConfig =
ZKMetadataProvider.getTableConfig(_propertyStore, tableNameWithType);
Review comment:
I kinda prefer to get tableConfig here in HelixInstanceDataMgr, e.g.
`reloadAllSegments()` method gets tableConfig outside the for-loop, inside
which it calls `reloadSegment(...tablcConfig)` for each segment. If getting
tableConfig in those segment level method in tablemgr, it'd incur a zk call for
each segment.
--
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
