Jackie-Jiang commented on a change in pull request #7249:
URL: https://github.com/apache/pinot/pull/7249#discussion_r685422325
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -1776,15 +1776,20 @@ public void refreshSegment(String offlineTableName,
SegmentMetadata segmentMetad
sendSegmentRefreshMessage(offlineTableName, segmentName, true, true);
}
- public int reloadAllSegments(String tableNameWithType) {
- LOGGER.info("Sending reload message for table: {}", tableNameWithType);
+ public int reloadAllSegments(String tableNameWithType, boolean
forceDownload) {
+ LOGGER.info("Sending reload message for table: {} with forceDownload: {}",
tableNameWithType, forceDownload);
+
+ if (forceDownload) {
+ TableType tt =
TableNameBuilder.getTableTypeFromTableName(tableNameWithType);
+ Preconditions.checkArgument(tt == TableType.OFFLINE, "OFFLINE table is
required to force segment download");
Review comment:
Add the table name into the error message. Also add a TODO to support
the realtime tables
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/messages/SegmentReloadMessage.java
##########
@@ -49,4 +55,8 @@ public SegmentReloadMessage(Message message) {
Preconditions.checkArgument(msgSubType.equals(RELOAD_SEGMENT_MSG_SUB_TYPE),
"Invalid message sub type: " + msgSubType + " for
SegmentReloadMessage");
}
+
+ public boolean getForceDownload() {
Review comment:
```suggestion
public boolean isForceDownload() {
```
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -472,13 +474,15 @@ public SuccessResponse reloadSegmentDeprecated2(
@ApiOperation(value = "Reload all segments", notes = "Reload all segments")
public SuccessResponse reloadAllSegments(
@ApiParam(value = "Name of the table", required = true)
@PathParam("tableName") String tableName,
- @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String
tableTypeStr) {
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String
tableTypeStr,
+ @ApiParam(value = "Whether to force server to download segment")
@QueryParam("forceDownload") @DefaultValue("false") boolean forceDownload) {
Review comment:
When force download is enabled, we should check the table type and
replace `null` with `OFFLINE` to avoid reloading the realtime table
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -1799,16 +1804,22 @@ public int reloadAllSegments(String tableNameWithType) {
return numMessagesSent;
}
- public int reloadSegment(String tableNameWithType, String segmentName) {
- LOGGER.info("Sending reload message for segment: {} in table: {}",
segmentName, tableNameWithType);
+ public int reloadSegment(String tableNameWithType, String segmentName,
boolean forceDownload) {
+ LOGGER.info("Sending reload message for segment: {} in table: {} with
forceDownload: {}", segmentName,
+ tableNameWithType, forceDownload);
+
+ if (forceDownload) {
+ TableType tt =
TableNameBuilder.getTableTypeFromTableName(tableNameWithType);
+ Preconditions.checkArgument(tt == TableType.OFFLINE, "OFFLINE table is
required to force segment download");
Review comment:
Same here
##########
File path:
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
##########
@@ -347,6 +355,60 @@ public void testInvertedIndexTriggering()
throw new RuntimeException(e);
}
}, 600_000L, "Failed to generate inverted index");
+
+ final long tableSizeWithNewIndex = getTableSize(getTableName());
Review comment:
(code format) we don't usually put `final` for local variables
--
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]
