vrajat commented on code in PR #15862:
URL: https://github.com/apache/pinot/pull/15862#discussion_r2102876342
##########
pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BrokerResourceOnlineOfflineStateModelFactory.java:
##########
@@ -97,14 +97,19 @@ public void onBecomeOnlineFromOffline(Message message,
NotificationContext conte
@Transition(from = "ONLINE", to = "OFFLINE")
public void onBecomeOfflineFromOnline(Message message, NotificationContext
context) {
- String tableNameWithType = message.getPartitionName();
- LOGGER.info("Processing transition from ONLINE to OFFLINE for table:
{}", tableNameWithType);
+ String physicalOrLogicalTable = message.getPartitionName();
Review Comment:
Why is this method called for logical table ? Is it because an entry is
removed from `brokerResource` ?
##########
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:
This is specific to `MinTimeBoundaryStrategy`. Other strategies may not need
it. Is a `cleanup(RouteEntryMap)` a better interface method ?
##########
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)) {
+ LOGGER.warn("Skipping removing time boundary manager for real-time
table: {}", tableNameWithType);
Review Comment:
This is should be an INFO.
##########
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)) {
+ LOGGER.warn("Skipping removing time boundary manager for real-time
table: {}", tableNameWithType);
+ continue;
+ }
+
+ String rawTableName =
TableNameBuilder.extractRawTableName(tableNameWithType);
+ String realtimeTableName =
TableNameBuilder.REALTIME.tableNameWithType(rawTableName);
+ if (_routingEntryMap.containsKey(realtimeTableName)) {
+ LOGGER.warn("Skipping removing time boundary manager for hybrid
physical table: {}", rawTableName);
Review Comment:
Same here. It is OK for it to be a hybrid 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: 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
