Jackie-Jiang commented on a change in pull request #7289:
URL: https://github.com/apache/pinot/pull/7289#discussion_r687278829
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java
##########
@@ -204,8 +205,8 @@ private void checkCRC(HttpHeaders headers, String
offlineTableName, String segme
}
private void processNewSegment(SegmentMetadata segmentMetadata, URI
finalSegmentLocationURI,
- File currentSegmentLocation, String zkDownloadURI, String crypter,
String tableNameWithType, String segmentName,
- boolean moveSegmentToFinalLocation) {
+ File currentSegmentLocation, String zkDownloadURI, HttpHeaders headers,
String crypter, String tableNameWithType,
Review comment:
I think `headers` is nullable?
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java
##########
@@ -204,8 +205,8 @@ private void checkCRC(HttpHeaders headers, String
offlineTableName, String segme
}
private void processNewSegment(SegmentMetadata segmentMetadata, URI
finalSegmentLocationURI,
- File currentSegmentLocation, String zkDownloadURI, String crypter,
String tableNameWithType, String segmentName,
- boolean moveSegmentToFinalLocation) {
+ File currentSegmentLocation, String zkDownloadURI, HttpHeaders headers,
String crypter, String tableNameWithType,
+ String segmentName, boolean moveSegmentToFinalLocation) throws Exception
{
Review comment:
(nit) code format
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java
##########
@@ -214,14 +216,34 @@ private void processNewSegment(SegmentMetadata
segmentMetadata, URI finalSegment
.info("Moved segment {} from temp location {} to {}", segmentName,
currentSegmentLocation.getAbsolutePath(),
finalSegmentLocationURI.getPath());
} catch (Exception e) {
- LOGGER.error("Could not move segment {} from table {} to permanent
directory", segmentName, tableNameWithType, e);
+ LOGGER
+ .error("Could not move segment {} from table {} to permanent
directory", segmentName, tableNameWithType, e);
throw new RuntimeException(e);
}
} else {
LOGGER.info("Skipping segment move, keeping segment {} from table {} at
{}", segmentName, tableNameWithType,
zkDownloadURI);
}
_pinotHelixResourceManager.addNewSegment(tableNameWithType,
segmentMetadata, zkDownloadURI, crypter);
+
+ // Update zk metadata customer map for offline table
+ String segmentZKMetadataCustomMapModifierStr =
+
headers.getHeaderString(FileUploadDownloadClient.CustomHeaders.SEGMENT_ZK_METADATA_CUSTOM_MAP_MODIFIER);
+ if (TableType.OFFLINE ==
TableNameBuilder.getTableTypeFromTableName(tableNameWithType)
Review comment:
Is this check only for reading the correct type of the ZK metadata? If
so, let's wait for #7255 to be merged first, then this check can be avoided.
This metadata modifier should apply to any segment instead of only segments
from OFFLINE table
--
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]
