krishan1390 commented on code in PR #17842:
URL: https://github.com/apache/pinot/pull/17842#discussion_r2916160166
##########
pinot-controller/src/main/java/org/apache/pinot/controller/validation/BrokerResourceValidationManager.java:
##########
@@ -45,6 +48,17 @@ public BrokerResourceValidationManager(ControllerConf
config, PinotHelixResource
controllerMetrics);
}
+ @Override
+ protected List<String> getTablesToProcess(Properties periodicTaskProperties)
{
+ List<String> tables = super.getTablesToProcess(periodicTaskProperties);
+ if (periodicTaskProperties.get(PeriodicTask.PROPERTY_KEY_TABLE_NAME) !=
null) {
+ return tables;
+ }
+ List<String> combined = new ArrayList<>(tables);
+
combined.addAll(_pinotHelixResourceManager.getBrokerResourceLogicalTablePartitions());
Review Comment:
Can / Should this task run on non-leaders for logical tables ?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -748,6 +748,29 @@ public List<String> getAllTables() {
return
getAllResources().stream().filter(TableNameBuilder::isTableResource).collect(Collectors.toList());
}
+ /**
+ * Returns partition names in broker resource ideal state that are logical
tables (have logical table config
+ * and no physical table config). Used by broker resource validation to
repair logical table broker assignments.
+ * Identifies by config presence rather than name to avoid misclassifying
logical tables named with
+ * _OFFLINE/_REALTIME.
+ */
+ public List<String> getBrokerResourceLogicalTablePartitions() {
Review Comment:
This looks sub optimal and probably edge case prone
1. We are getting table list from ideal state which might not be updated and
is the problem the BrokerResourceValidationManager is trying to fix
2. Doing a ZK read for every table config is also not ideal
Can use getAllLogicalTableNames() instead ?
##########
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerStatelessTest.java:
##########
@@ -373,6 +448,56 @@ public void testRebuildBrokerResourceFromHelixTags()
resetBrokerTags();
}
+ /**
+ * Verifies that rebuildBrokerResourceFromHelixTags succeeds for a logical
table name and
+ * updates the broker resource ideal state correctly (Issue #15751).
+ */
+ @Test
+ public void testRebuildBrokerResourceFromHelixTagsForLogicalTable()
+ throws Exception {
+ // Setup: physical tables (offline + realtime) via manager with test
tenants, then logical table
+ addDummySchema(RAW_TABLE_NAME);
+ TableConfig offlineTableConfig =
+ new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).setBrokerTenant(BROKER_TENANT_NAME)
+ .setServerTenant(SERVER_TENANT_NAME).build();
+ waitForEVToDisappear(offlineTableConfig.getTableName());
+ _helixResourceManager.addTable(offlineTableConfig);
+ waitForEVToDisappear(REALTIME_TABLE_NAME);
+ TableConfig realtimeTableConfig =
+ new
TableConfigBuilder(TableType.REALTIME).setTableName(RAW_TABLE_NAME).setBrokerTenant(BROKER_TENANT_NAME)
+ .setServerTenant(SERVER_TENANT_NAME)
+
.setStreamConfigs(FakeStreamConfigUtils.getDefaultLowLevelStreamConfigs().getStreamConfigsMap()).build();
+ _helixResourceManager.addTable(realtimeTableConfig);
+ List<String> physicalTableNamesWithType = List.of(OFFLINE_TABLE_NAME,
REALTIME_TABLE_NAME);
+ String logicalTableName = "test_logical_table_rebuild";
+ addDummySchema(logicalTableName);
+ LogicalTableConfig logicalTableConfig =
+ ControllerTest.getDummyLogicalTableConfig(logicalTableName,
physicalTableNamesWithType, BROKER_TENANT_NAME);
+ addLogicalTableConfig(logicalTableConfig);
+
+ IdealState idealState = _helixAdmin.getResourceIdealState(_clusterName,
Helix.BROKER_RESOURCE_INSTANCE);
+ assertTrue(idealState.getPartitionSet().contains(logicalTableName));
+ int initialBrokerCount =
idealState.getInstanceSet(logicalTableName).size();
+
+ // Rebuild for logical table name should succeed (not return 400)
+ PinotResourceManagerResponse response =
Review Comment:
Here too, the rebuild is not doing anything and the test will pass even if
rebuild is not called right ? Because, there is no failure in the above path,
and that's why the initial broker count remains the same in the assertion in
line 490.
Either we should inject a failure when a new broker is added. Assert that
initially the ideal state is not updated. And then call rebuild and then
validate that the ideal state is updated.
##########
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerStatelessTest.java:
##########
@@ -326,13 +327,87 @@ public void testUpdateBrokerResource()
resetBrokerTags();
}
+ /**
+ * Verifies that when a new broker is added with the same tenant as a
logical table,
+ * the broker resource ideal state is updated for the logical table
partition (Issue #15751).
+ */
+ @Test
+ public void testUpdateBrokerResourceWithLogicalTable()
+ throws Exception {
+ untagBrokers();
+ Tenant brokerTenant = new Tenant(TenantRole.BROKER, BROKER_TENANT_NAME, 2,
0, 0);
+ _helixResourceManager.createBrokerTenant(brokerTenant);
+
+ String brokerTag = TagNameUtils.getBrokerTagForTenant(BROKER_TENANT_NAME);
+ List<InstanceConfig> instanceConfigs =
HelixHelper.getInstanceConfigs(_helixManager);
+ List<String> taggedBrokers =
HelixHelper.getInstancesWithTag(instanceConfigs, brokerTag);
+ assertEquals(taggedBrokers.size(), 2);
+
+ // Add physical tables (offline + realtime) via manager with test tenants,
then logical table
+ addDummySchema(RAW_TABLE_NAME);
+ TableConfig offlineTableConfig =
+ new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).setBrokerTenant(BROKER_TENANT_NAME)
+ .setServerTenant(SERVER_TENANT_NAME).build();
+ waitForEVToDisappear(offlineTableConfig.getTableName());
+ _helixResourceManager.addTable(offlineTableConfig);
+ waitForEVToDisappear(REALTIME_TABLE_NAME);
+ TableConfig realtimeTableConfig =
+ new
TableConfigBuilder(TableType.REALTIME).setTableName(RAW_TABLE_NAME).setBrokerTenant(BROKER_TENANT_NAME)
+ .setServerTenant(SERVER_TENANT_NAME)
+
.setStreamConfigs(FakeStreamConfigUtils.getDefaultLowLevelStreamConfigs().getStreamConfigsMap()).build();
+ _helixResourceManager.addTable(realtimeTableConfig);
+ List<String> physicalTableNamesWithType = List.of(OFFLINE_TABLE_NAME,
REALTIME_TABLE_NAME);
+ String logicalTableName = "test_logical_table";
+ addDummySchema(logicalTableName);
+ LogicalTableConfig logicalTableConfig =
+ ControllerTest.getDummyLogicalTableConfig(logicalTableName,
physicalTableNamesWithType, BROKER_TENANT_NAME);
+ _helixResourceManager.addLogicalTableConfig(logicalTableConfig);
+
+ IdealState brokerResource = HelixHelper.getBrokerIdealStates(_helixAdmin,
_clusterName);
+ assertTrue(brokerResource.getPartitionSet().contains(logicalTableName));
+ checkBrokerResourceForPartition(logicalTableName, taggedBrokers);
+
+ // Add a new broker instance with same tenant
+ Instance newBrokerInstance =
+ new Instance("localhost", 3, InstanceType.BROKER,
Collections.singletonList(brokerTag), null, 0, 0, 0, 0,
+ false);
+ assertTrue(_helixResourceManager.addInstance(newBrokerInstance,
true).isSuccessful());
+ String newBrokerId = InstanceUtils.getHelixInstanceId(newBrokerInstance);
+ List<String> taggedBrokersAfterAdd = new ArrayList<>(taggedBrokers);
+ taggedBrokersAfterAdd.add(newBrokerId);
+
+ // Logical table partition should include the new broker. If broker add
path did not update it yet,
+ // rebuildBrokerResourceFromHelixTags (e.g. from
BrokerResourceValidationManager) repairs it.
+ PinotResourceManagerResponse rebuildResponse =
+
_helixResourceManager.rebuildBrokerResourceFromHelixTags(logicalTableName);
Review Comment:
We shouldn't be calling rebuild here, right? Rebuild is used only if there
is some failure in the add broker path.
In this particular happy path test, we should not rebuild but verify that
adding a new broker sets the resource correctly without requiring an explicit
rebuild.
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ControllerPeriodicTasksIntegrationTest.java:
##########
@@ -422,6 +422,50 @@ public void testBrokerResourceValidationManager() {
}, 600_000L, "Timeout when waiting for BrokerResourceValidationManager");
}
+ /**
+ * Verifies that BrokerResourceValidationManager also repairs broker
resource for a logical table
+ * when a new broker is added (Issue #15751).
+ */
+ @Test
+ public void testBrokerResourceValidationManagerRepairsLogicalTable()
+ throws IOException {
+ // Add logical table (same broker tenant as physical table)
+ Schema logicalTableSchema = createSchema();
+ logicalTableSchema.setSchemaName(getLogicalTableName());
+ addSchema(logicalTableSchema);
+ createLogicalTable();
+
+ String helixClusterName = getHelixClusterName();
+ String logicalTableName = getLogicalTableName();
+ IdealState idealState = HelixHelper.getBrokerIdealStates(_helixAdmin,
helixClusterName);
+ assertNotNull(idealState);
+ assertTrue(idealState.getPartitionSet().contains(logicalTableName),
"Broker resource should have logical table");
+
+ // Add a new broker so logical table partition is out of sync
+ String brokerId = "Broker_localhost_5678";
+ InstanceConfig instanceConfig = InstanceConfig.toInstanceConfig(brokerId);
+ instanceConfig.addTag(TagNameUtils.getBrokerTagForTenant(TENANT_NAME));
+ _helixAdmin.addInstance(helixClusterName, instanceConfig);
+ Set<String> brokersAfterAdd =
_helixResourceManager.getAllInstancesForBrokerTenant(TENANT_NAME);
+ assertTrue(brokersAfterAdd.contains(brokerId));
Review Comment:
We should assert that right now the idealState doesn't contain the new
broker.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -1150,16 +1187,42 @@ public PinotResourceManagerResponse
rebuildBrokerResource(String tableNameWithTy
}
private void addInstanceToBrokerIdealState(String brokerTenantTag, String
instanceName) {
- IdealState tableIdealState =
_helixAdmin.getResourceIdealState(_helixClusterName,
Helix.BROKER_RESOURCE_INSTANCE);
- for (String tableNameWithType : tableIdealState.getPartitionSet()) {
- TableConfig tableConfig =
ZKMetadataProvider.getTableConfig(_propertyStore, tableNameWithType);
- Preconditions.checkNotNull(tableConfig);
- String brokerTag =
TagNameUtils.extractBrokerTag(tableConfig.getTenantConfig());
- if (brokerTag.equals(brokerTenantTag)) {
- tableIdealState.setPartitionState(tableNameWithType, instanceName,
BrokerResourceStateModel.ONLINE);
+ // Use atomic read-modify-write so updates (including for logical tables)
are persisted and not lost to races.
+ HelixHelper.updateIdealState(getHelixZkManager(),
Helix.BROKER_RESOURCE_INSTANCE, idealState -> {
+ assert idealState != null;
+ for (String partitionName : idealState.getPartitionSet()) {
+ String brokerTag =
resolveBrokerTagForBrokerResourcePartition(partitionName);
+ if (brokerTag == null) {
+ continue;
+ }
+ if (brokerTag.equals(brokerTenantTag)) {
+ idealState.setPartitionState(partitionName, instanceName,
BrokerResourceStateModel.ONLINE);
+ }
}
+ return idealState;
+ }, DEFAULT_RETRY_POLICY);
+ }
+
+ /**
+ * Resolves the broker tag for a partition in the broker resource. Tries
physical table config first,
+ * then logical table config, so both are handled regardless of partition
naming (e.g. a logical table
+ * name ending with _OFFLINE would otherwise be misclassified as physical
and skipped).
+ *
+ * @param partitionName partition name in broker ideal state (physical table
name with type or logical table name)
+ * @return broker tag for the partition, or null if the partition cannot be
resolved (unknown or missing config)
+ */
+ private String resolveBrokerTagForBrokerResourcePartition(String
partitionName) {
+ TableConfig tableConfig =
ZKMetadataProvider.getTableConfig(_propertyStore, partitionName);
+ if (tableConfig != null) {
+ return TagNameUtils.extractBrokerTag(tableConfig.getTenantConfig());
}
- _helixAdmin.setResourceIdealState(_helixClusterName,
Helix.BROKER_RESOURCE_INSTANCE, tableIdealState);
+ LogicalTableConfig logicalTableConfig =
ZKMetadataProvider.getLogicalTableConfig(_propertyStore, partitionName);
+ if (logicalTableConfig != null) {
+ return
TagNameUtils.getBrokerTagForTenant(logicalTableConfig.getBrokerTenant());
+ }
+ LOGGER.debug("Skipping partition {} in broker resource: no table config or
logical table config found",
Review Comment:
This should be a warn rather than debug because its unexpected. previously
we were throwing an exception so either we continue to throw an exception or
update to warning
##########
pinot-controller/src/main/java/org/apache/pinot/controller/validation/BrokerResourceValidationManager.java:
##########
@@ -55,15 +69,21 @@ protected Context preprocess(Properties
periodicTaskProperties) {
@Override
protected void processTable(String tableNameWithType, Context context) {
TableConfig tableConfig =
_pinotHelixResourceManager.getTableConfig(tableNameWithType);
- if (tableConfig == null) {
- LOGGER.warn("Failed to find table config for table: {}, skipping broker
resource validation", tableNameWithType);
+ if (tableConfig != null) {
+ Set<String> brokerInstances = _pinotHelixResourceManager
+ .getAllInstancesForBrokerTenant(context._instanceConfigs,
tableConfig.getTenantConfig().getBroker());
+ _pinotHelixResourceManager.rebuildBrokerResource(tableNameWithType,
brokerInstances);
return;
}
-
- // Rebuild broker resource
- Set<String> brokerInstances = _pinotHelixResourceManager
- .getAllInstancesForBrokerTenant(context._instanceConfigs,
tableConfig.getTenantConfig().getBroker());
- _pinotHelixResourceManager.rebuildBrokerResource(tableNameWithType,
brokerInstances);
+ LogicalTableConfig logicalTableConfig =
_pinotHelixResourceManager.getLogicalTableConfig(tableNameWithType);
+ if (logicalTableConfig != null) {
+ Set<String> brokerInstances = _pinotHelixResourceManager
Review Comment:
Why does logical table config have a broker tenant? What happens if a
physical table's broker tenant is different from a logical table's broker
tenant?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -1150,16 +1187,42 @@ public PinotResourceManagerResponse
rebuildBrokerResource(String tableNameWithTy
}
private void addInstanceToBrokerIdealState(String brokerTenantTag, String
instanceName) {
- IdealState tableIdealState =
_helixAdmin.getResourceIdealState(_helixClusterName,
Helix.BROKER_RESOURCE_INSTANCE);
- for (String tableNameWithType : tableIdealState.getPartitionSet()) {
- TableConfig tableConfig =
ZKMetadataProvider.getTableConfig(_propertyStore, tableNameWithType);
- Preconditions.checkNotNull(tableConfig);
- String brokerTag =
TagNameUtils.extractBrokerTag(tableConfig.getTenantConfig());
- if (brokerTag.equals(brokerTenantTag)) {
- tableIdealState.setPartitionState(tableNameWithType, instanceName,
BrokerResourceStateModel.ONLINE);
+ // Use atomic read-modify-write so updates (including for logical tables)
are persisted and not lost to races.
+ HelixHelper.updateIdealState(getHelixZkManager(),
Helix.BROKER_RESOURCE_INSTANCE, idealState -> {
+ assert idealState != null;
Review Comment:
Instead of assert use Preconditions.checkNotNull as assert isn't production
safe.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -1150,16 +1187,42 @@ public PinotResourceManagerResponse
rebuildBrokerResource(String tableNameWithTy
}
private void addInstanceToBrokerIdealState(String brokerTenantTag, String
instanceName) {
- IdealState tableIdealState =
_helixAdmin.getResourceIdealState(_helixClusterName,
Helix.BROKER_RESOURCE_INSTANCE);
- for (String tableNameWithType : tableIdealState.getPartitionSet()) {
- TableConfig tableConfig =
ZKMetadataProvider.getTableConfig(_propertyStore, tableNameWithType);
- Preconditions.checkNotNull(tableConfig);
- String brokerTag =
TagNameUtils.extractBrokerTag(tableConfig.getTenantConfig());
- if (brokerTag.equals(brokerTenantTag)) {
- tableIdealState.setPartitionState(tableNameWithType, instanceName,
BrokerResourceStateModel.ONLINE);
+ // Use atomic read-modify-write so updates (including for logical tables)
are persisted and not lost to races.
+ HelixHelper.updateIdealState(getHelixZkManager(),
Helix.BROKER_RESOURCE_INSTANCE, idealState -> {
+ assert idealState != null;
+ for (String partitionName : idealState.getPartitionSet()) {
+ String brokerTag =
resolveBrokerTagForBrokerResourcePartition(partitionName);
+ if (brokerTag == null) {
+ continue;
+ }
+ if (brokerTag.equals(brokerTenantTag)) {
+ idealState.setPartitionState(partitionName, instanceName,
BrokerResourceStateModel.ONLINE);
+ }
}
+ return idealState;
+ }, DEFAULT_RETRY_POLICY);
+ }
+
+ /**
+ * Resolves the broker tag for a partition in the broker resource. Tries
physical table config first,
+ * then logical table config, so both are handled regardless of partition
naming (e.g. a logical table
+ * name ending with _OFFLINE would otherwise be misclassified as physical
and skipped).
+ *
+ * @param partitionName partition name in broker ideal state (physical table
name with type or logical table name)
+ * @return broker tag for the partition, or null if the partition cannot be
resolved (unknown or missing config)
+ */
+ private String resolveBrokerTagForBrokerResourcePartition(String
partitionName) {
Review Comment:
probably method name should be resolveBrokerTagForTable(String tableName)
broker resource partition / partition name is too confusing of what the
method is actually doing
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java:
##########
@@ -535,23 +537,43 @@ public static Set<InstanceConfig>
getBrokerInstanceConfigsForTenant(List<Instanc
public static Set<String> getTablesForBrokerTag(HelixManager helixManager,
String brokerTag) {
Set<String> tablesForBrokerTag = new HashSet<>();
- List<TableConfig> tableConfigs =
ZKMetadataProvider.getAllTableConfigs(helixManager.getHelixPropertyStore());
+ ZkHelixPropertyStore<ZNRecord> propertyStore =
helixManager.getHelixPropertyStore();
+ List<TableConfig> tableConfigs =
ZKMetadataProvider.getAllTableConfigs(propertyStore);
for (TableConfig tableConfig : tableConfigs) {
if
(TagNameUtils.getBrokerTagForTenant(tableConfig.getTenantConfig().getBroker()).equals(brokerTag))
{
tablesForBrokerTag.add(tableConfig.getTableName());
}
}
+ // Include logical tables that use this broker tenant (broker resource
partition name = logical table name)
+ for (LogicalTableConfig logicalTableConfig :
ZKMetadataProvider.getAllLogicalTableConfigs(propertyStore)) {
+ String logicalBrokerTag =
+
TagNameUtils.getBrokerTagForTenant(logicalTableConfig.getBrokerTenant() != null
Review Comment:
can getBrokerTenant() return null ? If yes then other places in the PR that
are calling getBrokerTenant() aren't handling null.
Ideally getBrokerTenant() should return DEFAULT if its null so that client
code is simple. Can we make this change instead ?
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java:
##########
@@ -535,23 +537,43 @@ public static Set<InstanceConfig>
getBrokerInstanceConfigsForTenant(List<Instanc
public static Set<String> getTablesForBrokerTag(HelixManager helixManager,
String brokerTag) {
Review Comment:
can this method call the below method with List.of(brokerTag). It will avoid
duplicate code
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ControllerPeriodicTasksIntegrationTest.java:
##########
@@ -422,6 +422,50 @@ public void testBrokerResourceValidationManager() {
}, 600_000L, "Timeout when waiting for BrokerResourceValidationManager");
}
+ /**
+ * Verifies that BrokerResourceValidationManager also repairs broker
resource for a logical table
+ * when a new broker is added (Issue #15751).
+ */
+ @Test
+ public void testBrokerResourceValidationManagerRepairsLogicalTable()
+ throws IOException {
+ // Add logical table (same broker tenant as physical table)
+ Schema logicalTableSchema = createSchema();
+ logicalTableSchema.setSchemaName(getLogicalTableName());
+ addSchema(logicalTableSchema);
+ createLogicalTable();
+
+ String helixClusterName = getHelixClusterName();
+ String logicalTableName = getLogicalTableName();
+ IdealState idealState = HelixHelper.getBrokerIdealStates(_helixAdmin,
helixClusterName);
+ assertNotNull(idealState);
+ assertTrue(idealState.getPartitionSet().contains(logicalTableName),
"Broker resource should have logical table");
+
+ // Add a new broker so logical table partition is out of sync
+ String brokerId = "Broker_localhost_5678";
+ InstanceConfig instanceConfig = InstanceConfig.toInstanceConfig(brokerId);
+ instanceConfig.addTag(TagNameUtils.getBrokerTagForTenant(TENANT_NAME));
+ _helixAdmin.addInstance(helixClusterName, instanceConfig);
+ Set<String> brokersAfterAdd =
_helixResourceManager.getAllInstancesForBrokerTenant(TENANT_NAME);
+ assertTrue(brokersAfterAdd.contains(brokerId));
+
+ // Wait for BrokerResourceValidationManager to repair both physical and
logical table partitions
+ String tableNameWithType =
TableNameBuilder.OFFLINE.tableNameWithType(getTableName());
+ TestUtils.waitForCondition(aVoid -> {
+ IdealState is = HelixHelper.getBrokerIdealStates(_helixAdmin,
helixClusterName);
+ if (is == null) {
+ return false;
+ }
+ return is.getInstanceSet(tableNameWithType).equals(brokersAfterAdd)
+ && is.getInstanceSet(logicalTableName).equals(brokersAfterAdd);
+ }, 600_000L, "Timeout when waiting for BrokerResourceValidationManager to
repair logical table partition");
Review Comment:
Can we reduce the timeout from 10 minutes to 1 minute or something ? How
frequently is the task running? Can we increase the frequency of the task in
this test? And reduce the timeout?
##########
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerStatelessTest.java:
##########
@@ -373,6 +448,56 @@ public void testRebuildBrokerResourceFromHelixTags()
resetBrokerTags();
}
+ /**
+ * Verifies that rebuildBrokerResourceFromHelixTags succeeds for a logical
table name and
+ * updates the broker resource ideal state correctly (Issue #15751).
+ */
+ @Test
+ public void testRebuildBrokerResourceFromHelixTagsForLogicalTable()
+ throws Exception {
+ // Setup: physical tables (offline + realtime) via manager with test
tenants, then logical table
+ addDummySchema(RAW_TABLE_NAME);
+ TableConfig offlineTableConfig =
+ new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).setBrokerTenant(BROKER_TENANT_NAME)
+ .setServerTenant(SERVER_TENANT_NAME).build();
+ waitForEVToDisappear(offlineTableConfig.getTableName());
+ _helixResourceManager.addTable(offlineTableConfig);
+ waitForEVToDisappear(REALTIME_TABLE_NAME);
+ TableConfig realtimeTableConfig =
+ new
TableConfigBuilder(TableType.REALTIME).setTableName(RAW_TABLE_NAME).setBrokerTenant(BROKER_TENANT_NAME)
+ .setServerTenant(SERVER_TENANT_NAME)
+
.setStreamConfigs(FakeStreamConfigUtils.getDefaultLowLevelStreamConfigs().getStreamConfigsMap()).build();
+ _helixResourceManager.addTable(realtimeTableConfig);
+ List<String> physicalTableNamesWithType = List.of(OFFLINE_TABLE_NAME,
REALTIME_TABLE_NAME);
+ String logicalTableName = "test_logical_table_rebuild";
+ addDummySchema(logicalTableName);
+ LogicalTableConfig logicalTableConfig =
+ ControllerTest.getDummyLogicalTableConfig(logicalTableName,
physicalTableNamesWithType, BROKER_TENANT_NAME);
+ addLogicalTableConfig(logicalTableConfig);
+
+ IdealState idealState = _helixAdmin.getResourceIdealState(_clusterName,
Helix.BROKER_RESOURCE_INSTANCE);
+ assertTrue(idealState.getPartitionSet().contains(logicalTableName));
+ int initialBrokerCount =
idealState.getInstanceSet(logicalTableName).size();
+
+ // Rebuild for logical table name should succeed (not return 400)
+ PinotResourceManagerResponse response =
Review Comment:
I see testRebuildBrokerResourceWhenBrokerAddedForLogicalTable() is testing
the rebuild properly. So we can remove this particular test right ?
--
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]
