vrajat commented on code in PR #15862:
URL: https://github.com/apache/pinot/pull/15862#discussion_r2107035759
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/logicaltable/LogicalTableWithTwoOfflineOneRealtimeTableIntegrationTest.java:
##########
@@ -62,15 +64,17 @@ public void testUpdateLogicalTableTimeBoundary()
private void updateTimeBoundaryTableInLogicalTable(LogicalTableConfig
logicalTableConfig)
throws IOException {
- List<String> includedTables =
- (List<String>)
logicalTableConfig.getTimeBoundaryConfig().getParameters().get("includedTables");
+ TimeBoundaryStrategy timeBoundaryStrategy =
TimeBoundaryStrategyService.getInstance()
+
.getTimeBoundaryStrategy(logicalTableConfig.getTimeBoundaryConfig().getBoundaryStrategy());
+ List<String> includedTables =
timeBoundaryStrategy.getTimeBoundaryTableNames(logicalTableConfig);
String timeBoundaryTableName =
TableNameBuilder.extractRawTableName(includedTables.get(0));
String newTimeBoundaryTableName = timeBoundaryTableName.equals("o_1") ?
"o_2" : "o_1";
newTimeBoundaryTableName =
TableNameBuilder.OFFLINE.tableNameWithType(newTimeBoundaryTableName);
Map<String, Object> parameters = Map.of("includedTables",
List.of(newTimeBoundaryTableName));
logicalTableConfig.getTimeBoundaryConfig().setParameters(parameters);
+ logicalTableConfig.setQueryConfig(null);
Review Comment:
Curious - why was this required ? Did a previous test not clean up ?
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/timeboundary/TimeBoundaryStrategy.java:
##########
@@ -43,4 +44,12 @@ public interface TimeBoundaryStrategy {
*/
TimeBoundaryInfo computeTimeBoundary(LogicalTableConfig logicalTableConfig,
TableCache tableCache,
RoutingManager routingManager);
+
+
+ /**
+ * Returns the list of physical table names that are part of the time
boundary.
+ * @param logicalTableConfig The logical table configuration
+ * @return The list of physical table names that are part of the time
boundary.
+ */
+ List<String> getTimeBoundaryTableNames(LogicalTableConfig
logicalTableConfig);
Review Comment:
The only other startegy that we know of is `ConstantValueStrategy`. For that
even `cleanup` is not required. Another option is to execute all tasks through
the `RoutingManager` interface. Pass the routing manager to a `cleanup`.
However, on 2nd thought. This is OK for now. The list of tables maybe useful
in another functions as well.
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/BrokerRoutingManager.java:
##########
@@ -653,6 +656,44 @@ public synchronized void removeRouting(String
tableNameWithType) {
}
}
+ public synchronized void removeRoutingForLogicalTable(String
logicalTableName) {
+ LOGGER.info("Removing time boundary manager for logical table: {}",
logicalTableName);
+ LogicalTableConfig logicalTableConfig =
+ ZKMetadataProvider.getLogicalTableConfig(_propertyStore,
logicalTableName);
+ Preconditions.checkState(logicalTableConfig != null, "Failed to find
logical table config for: %s",
+ logicalTableName);
+ if (!logicalTableConfig.isHybridLogicalTable()) {
+ LOGGER.info("Skip removing time boundary manager for non hybrid logical
table: {}", logicalTableName);
+ return;
+ }
+ String strategy =
logicalTableConfig.getTimeBoundaryConfig().getBoundaryStrategy();
+ TimeBoundaryStrategy timeBoundaryStrategy =
+
TimeBoundaryStrategyService.getInstance().getTimeBoundaryStrategy(strategy);
+ List<String> timeBoundaryTableNames =
timeBoundaryStrategy.getTimeBoundaryTableNames(logicalTableConfig);
+ for (String tableNameWithType : timeBoundaryTableNames) {
+
+ if (TableNameBuilder.isRealtimeTableResource(tableNameWithType)) {
Review Comment:
This cannot be true ? A time boundary cannot be associated with a realtime
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]
