abhishekbafna commented on code in PR #15776:
URL: https://github.com/apache/pinot/pull/15776#discussion_r2094811351


##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/BrokerRoutingManager.java:
##########
@@ -418,17 +420,74 @@ public synchronized void includeServerToRouting(String 
instanceId) {
   }
 
   /**
-   * Builds/rebuilds the routing for the physical table, for logical tables it 
is skipped.
-   * @param physicalOrLogicalTable a physical table with type or logical table 
name
+   * Builds the routing for a logical table. This method is called when a 
logical table is created or updated.
+   * @param logicalTableName the name of the logical table
    */
-  public synchronized void buildRouting(String physicalOrLogicalTable) {
-    // skip route building for logical tables
-    if (ZKMetadataProvider.isLogicalTableExists(_propertyStore, 
physicalOrLogicalTable)) {
-      LOGGER.info("Skipping route building for logical table: {}", 
physicalOrLogicalTable);
+  public synchronized void buildRoutingForLogicalTable(String 
logicalTableName) {
+    LogicalTableConfig logicalTableConfig = 
ZKMetadataProvider.getLogicalTableConfig(_propertyStore, logicalTableName);
+    Preconditions.checkState(logicalTableConfig != null, "Failed to find 
logical table config for: %s",
+        logicalTableConfig);
+    if (!logicalTableConfig.isHybridLogicalTable()) {
+      LOGGER.info("Skip time boundary manager setting for non hybrid logical 
table: {}", logicalTableName);
       return;
     }
 
-    String tableNameWithType = physicalOrLogicalTable;
+    LOGGER.info("Setting time boundary manager for logical table: {}", 
logicalTableName);
+
+    TimeBoundaryConfig timeBoundaryConfig = 
logicalTableConfig.getTimeBoundaryConfig();
+    
Preconditions.checkArgument(timeBoundaryConfig.getBoundaryStrategy().equals("min"),
+        "Invalid time boundary strategy: %s", 
timeBoundaryConfig.getBoundaryStrategy());
+    List<String> includedTables =
+        (List<String>) 
timeBoundaryConfig.getParameters().getOrDefault("includedTables", List.of());
+
+    for (String tableNameWithType : includedTables) {
+      // skip time boundary manager init for realtime table
+      if (TableNameBuilder.isRealtimeTableResource(tableNameWithType)) {
+        LOGGER.info("Skip time boundary manager init for realtime table: {}", 
tableNameWithType);
+        continue;
+      }
+
+      // skip hybrid tables, time boundary for such offline table already 
exists
+      String realtimeTableName = TableNameBuilder.REALTIME.tableNameWithType(
+          TableNameBuilder.extractRawTableName(tableNameWithType));
+      if (ZKMetadataProvider.isTableConfigExists(_propertyStore, 
realtimeTableName)) {
+        LOGGER.info("Skip time boundary manager init for for hybrid table: 
{}", tableNameWithType);
+        continue;
+      }
+
+      // build routing if it does not exist for the offline table
+      if (!_routingEntryMap.containsKey(tableNameWithType)) {
+        buildRouting(tableNameWithType);

Review Comment:
   The `buildRouting` does not have any exception declaration and handling so 
it there is no exception handling.
   
   The intention is not to build the routing multiple time but ensure it exists 
before we access it from the routing map. If there is any issue with build 
routing for table that would surface again and should prevent further 
processing for the logical table build routing too.
   



-- 
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]

Reply via email to