xiangfu0 commented on code in PR #19231:
URL: https://github.com/apache/pinot/pull/19231#discussion_r3798298026
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java:
##########
@@ -735,6 +733,66 @@ private void cleanupTempFiles(List<File> tempFiles) {
}
}
+ @VisibleForTesting
+ static String resolveDestinationTableName(@Nullable String requestTableName,
@Nullable String headerTableName,
+ @Nullable String metadataTableName, TableType tableType, HttpHeaders
headers,
+ boolean requireMatchingMetadataTable) {
+ String normalizedRequestTable = normalizeTableName(requestTableName,
tableType, headers, "request tableName");
+ String normalizedHeaderTable = normalizeTableName(headerTableName,
tableType, headers,
+ CommonConstants.Controller.TABLE_NAME_HTTP_HEADER + " header");
+ String destinationTable = normalizedRequestTable != null ?
normalizedRequestTable
+ : normalizedHeaderTable;
+ String normalizedMetadataTable = null;
+ if (requireMatchingMetadataTable || destinationTable == null) {
+ normalizedMetadataTable =
+ normalizeTableName(metadataTableName, tableType, headers, "segment
metadata table name");
+ if (destinationTable == null) {
+ destinationTable = normalizedMetadataTable;
+ }
+ }
+ if (destinationTable == null) {
+ throw new ControllerApplicationException(LOGGER,
+ "Table name is required in the request, " +
CommonConstants.Controller.TABLE_NAME_HTTP_HEADER
+ + " header, or segment metadata",
+ Response.Status.BAD_REQUEST);
+ }
+
+ validateMatchingTableName(destinationTable, normalizedHeaderTable,
+ CommonConstants.Controller.TABLE_NAME_HTTP_HEADER + " header");
+ if (requireMatchingMetadataTable) {
+ validateMatchingTableName(destinationTable, normalizedMetadataTable,
"segment metadata table name");
+ }
+ return destinationTable;
+ }
+
+ @Nullable
+ private static String normalizeTableName(@Nullable String tableName,
TableType tableType, HttpHeaders headers,
+ String source) {
+ if (StringUtils.isBlank(tableName)) {
+ return null;
+ }
Review Comment:
Updated table-name normalization so only null is treated as absent; any
supplied blank request, header, or strict metadata value now returns HTTP 400.
Added resolver and batch endpoint regression coverage.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java:
##########
@@ -574,9 +561,20 @@ private SuccessResponse uploadReingestedSegment(String
tableName, FormDataMultiP
private SuccessResponse uploadSegments(String tableName, TableType
tableType, FormDataMultiPart multiPart,
boolean enableParallelPushProtection, boolean allowRefresh, HttpHeaders
headers, Request request) {
long segmentsUploadStartTimeMs = System.currentTimeMillis();
- String rawTableName = TableNameBuilder.extractRawTableName(tableName);
- String tableNameWithType = tableType == TableType.OFFLINE ?
TableNameBuilder.OFFLINE.tableNameWithType(rawTableName)
- : TableNameBuilder.REALTIME.tableNameWithType(rawTableName);
+ String rawTableName = normalizeTableName(tableName, tableType, headers,
"request tableName");
+ if (rawTableName == null) {
+ throw new ControllerApplicationException(LOGGER, "tableName is required
for batch segment upload",
+ Response.Status.BAD_REQUEST);
+ }
+ String tableNameInHeader = normalizeTableName(
+ extractHttpHeader(headers,
CommonConstants.Controller.TABLE_NAME_HTTP_HEADER), tableType, headers,
+ CommonConstants.Controller.TABLE_NAME_HTTP_HEADER + " header");
+ validateMatchingTableName(rawTableName, tableNameInHeader,
+ CommonConstants.Controller.TABLE_NAME_HTTP_HEADER + " header");
Review Comment:
Batch uploads now normalize and compare each extracted segment metadata
table with the canonical request destination before segment validation,
deep-store, or ZooKeeper work. Added a request-A/metadata-B regression without
a table header and verified that no destination segment state is created.
--
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]
