Jackie-Jiang commented on code in PR #15726:
URL: https://github.com/apache/pinot/pull/15726#discussion_r2080196255
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/BrokerRoutingManager.java:
##########
@@ -417,6 +417,12 @@ public synchronized void includeServerToRouting(String
instanceId) {
* Builds/rebuilds the routing for the given table.
*/
public synchronized void buildRouting(String tableNameWithType) {
+ // skip route building for logical tables
+ if (ZKMetadataProvider.isLogicalTableExists(_propertyStore,
tableNameWithType)) {
Review Comment:
I don't think this handling is needed. There are 2 triggering point for this
method:
1. BrokerResponse Helix resource (logical table shouldn't be added into it)
2. Physical table config update (logical table config update shouldn't
trigger it)
The argument contains type suffix, which most likely won't be the actual
logical table name. I'd suggest removing it and simply let table config
existence check fail if it is really triggered by a logical 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
