Copilot commented on code in PR #16585:
URL: https://github.com/apache/pinot/pull/16585#discussion_r2274801640
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/BrokerRoutingManager.java:
##########
@@ -489,132 +519,152 @@ public synchronized void
buildRoutingForLogicalTable(String logicalTableName) {
* Builds the routing for a table.
* @param tableNameWithType the name of the table
*/
- public synchronized void buildRouting(String tableNameWithType) {
- LOGGER.info("Building routing for table: {}", tableNameWithType);
-
- TableConfig tableConfig =
ZKMetadataProvider.getTableConfig(_propertyStore, tableNameWithType);
- Preconditions.checkState(tableConfig != null, "Failed to find table config
for table: %s", tableNameWithType);
-
- String idealStatePath = getIdealStatePath(tableNameWithType);
- IdealState idealState = getIdealState(idealStatePath);
- Preconditions.checkState(idealState != null, "Failed to find ideal state
for table: %s", tableNameWithType);
- int idealStateVersion = idealState.getRecord().getVersion();
-
- String externalViewPath = getExternalViewPath(tableNameWithType);
- ExternalView externalView = getExternalView(externalViewPath);
- int externalViewVersion;
- // NOTE: External view might be null for new created tables. In such case,
create an empty one and set the version
- // to -1 to ensure the version does not match the next external view
- if (externalView == null) {
- externalView = new ExternalView(tableNameWithType);
- externalViewVersion = -1;
- } else {
- externalViewVersion = externalView.getRecord().getVersion();
- }
-
- Set<String> onlineSegments = getOnlineSegments(idealState);
-
- SegmentPreSelector segmentPreSelector =
- SegmentPreSelectorFactory.getSegmentPreSelector(tableConfig,
_propertyStore);
- Set<String> preSelectedOnlineSegments =
segmentPreSelector.preSelect(onlineSegments);
- SegmentSelector segmentSelector =
SegmentSelectorFactory.getSegmentSelector(tableConfig);
- segmentSelector.init(idealState, externalView, preSelectedOnlineSegments);
+ public void buildRouting(String tableNameWithType) {
+ long buildStartTimeMs = System.currentTimeMillis();
+ Object tableLock =
_routingTableBuildLocks.computeIfAbsent(tableNameWithType, k -> new Object());
+ synchronized (tableLock) {
+ Long lastStartObj = _routingTableBuildStartTimeMs.get(tableNameWithType);
+ long lastRoutingBuildStartTimeMs = lastStartObj != null ? lastStartObj :
Long.MIN_VALUE;
+ if (buildStartTimeMs <= lastRoutingBuildStartTimeMs) {
+ LOGGER.info("Skipping routing build for table: {} because the build
routing request timestamp {} "
+ + "is earlier than the last build start time: {}",
+ tableNameWithType, buildStartTimeMs, lastRoutingBuildStartTimeMs);
+ return;
+ }
- // Register segment pruners and initialize segment zk metadata fetcher.
- List<SegmentPruner> segmentPruners =
SegmentPrunerFactory.getSegmentPruners(tableConfig, _propertyStore);
+ // Record build start time to gate older requests
+ _routingTableBuildStartTimeMs.put(tableNameWithType,
System.currentTimeMillis());
Review Comment:
The build start time is being updated after the initial check, creating a
race condition. The same timestamp used in the comparison (buildStartTimeMs)
should be stored to maintain consistency.
```suggestion
_routingTableBuildStartTimeMs.put(tableNameWithType, buildStartTimeMs);
```
--
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]
