This is an automated email from the ASF dual-hosted git repository.
mayanks pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 601c5b7354 Handling of empty server tags in controller's availability
check (#13954)
601c5b7354 is described below
commit 601c5b73540b34dd159fc5c12521e37706c0d31e
Author: Anand Kr Shaw <[email protected]>
AuthorDate: Wed Sep 11 00:56:09 2024 +0530
Handling of empty server tags in controller's availability check (#13954)
* Fix for untag in server
* Fix for typo
* Unit test cases for un-tagged
* Unit test cases for un-tagged
* Add unit test case for negtive scenario
* Improve the code for duplicate config load
* Improve the code for duplicate config load
* Improve the code for duplicate config load
---------
Co-authored-by: anandheritage <[email protected]>
---
.../pinot/common/utils/helix/HelixHelper.java | 24 ++++++++++++++++
.../helix/core/PinotHelixResourceManager.java | 8 ++++--
.../pinot/controller/helix/ControllerTest.java | 21 ++++++++++++++
.../PinotHelixResourceManagerStatelessTest.java | 32 ++++++++++++++++++++++
4 files changed, 82 insertions(+), 3 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java
index 9129c7b1fa..4ffad84d61 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java
@@ -531,6 +531,13 @@ public class HelixHelper {
return getInstancesWithTag(getInstanceConfigs(helixManager), tag);
}
+ /**
+ * Returns the instances in the cluster without any tag.
+ */
+ public static List<String> getInstancesWithoutTag(HelixManager helixManager,
String tag) {
+ return getInstancesWithoutTag(getInstanceConfigs(helixManager), tag);
+ }
+
/**
* Returns the instances in the cluster with the given tag.
*
@@ -542,6 +549,12 @@ public class HelixHelper {
return
instancesWithTag.stream().map(InstanceConfig::getInstanceName).collect(Collectors.toList());
}
+ public static List<String> getInstancesWithoutTag(List<InstanceConfig>
instanceConfigs, String tag) {
+ List<InstanceConfig> instancesWithoutTag =
getInstancesConfigsWithoutTag(instanceConfigs, tag);
+ return
instancesWithoutTag.stream().map(InstanceConfig::getInstanceName).collect(Collectors.toList());
+ }
+
+
public static List<InstanceConfig>
getInstancesConfigsWithTag(List<InstanceConfig> instanceConfigs, String tag) {
List<InstanceConfig> instancesWithTag = new ArrayList<>();
for (InstanceConfig instanceConfig : instanceConfigs) {
@@ -552,6 +565,17 @@ public class HelixHelper {
return instancesWithTag;
}
+ public static List<InstanceConfig>
getInstancesConfigsWithoutTag(List<InstanceConfig> instanceConfigs, String tag)
{
+ List<InstanceConfig> instancesWithoutTag = new ArrayList<>();
+ for (InstanceConfig instanceConfig : instanceConfigs) {
+ // instanceConfig.getTags() never returns null
+ if (instanceConfig.getTags().isEmpty() ||
instanceConfig.containsTag(tag)) {
+ instancesWithoutTag.add(instanceConfig);
+ }
+ }
+ return instancesWithoutTag;
+ }
+
/**
* Returns the enabled instances in the cluster with the given tag.
*/
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
index f727ea5dbc..c7eeb341e1 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
@@ -3438,10 +3438,12 @@ public class PinotHelixResourceManager {
* @return List of untagged online server instances.
*/
public List<String> getOnlineUnTaggedServerInstanceList() {
- List<String> instanceList =
HelixHelper.getInstancesWithTag(_helixZkManager,
Helix.UNTAGGED_SERVER_INSTANCE);
+ List<String> instanceListWithoutTags =
HelixHelper.getInstancesWithoutTag(_helixZkManager,
+ Helix.UNTAGGED_SERVER_INSTANCE);
List<String> liveInstances =
_helixDataAccessor.getChildNames(_keyBuilder.liveInstances());
- instanceList.retainAll(liveInstances);
- return instanceList;
+
+ instanceListWithoutTags.retainAll(liveInstances);
+ return instanceListWithoutTags;
}
public List<String> getOnlineInstanceList() {
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
index 9af48dbf49..1d0048cf1e 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
@@ -422,6 +422,27 @@ public class ControllerTest {
_fakeInstanceHelixManagers.add(helixManager);
}
+ public void addFakeServerInstanceToAutoJoinHelixClusterWithEmptyTag(String
instanceId, boolean isSingleTenant)
+ throws Exception {
+ HelixManager helixManager =
+ HelixManagerFactory.getZKHelixManager(getHelixClusterName(),
instanceId, InstanceType.PARTICIPANT, getZkUrl());
+ helixManager.getStateMachineEngine()
+
.registerStateModelFactory(FakeSegmentOnlineOfflineStateModelFactory.STATE_MODEL_DEF,
+ new FakeSegmentOnlineOfflineStateModelFactory());
+ helixManager.connect();
+ HelixAdmin helixAdmin = helixManager.getClusterManagmentTool();
+ if (isSingleTenant) {
+ helixAdmin.addInstanceTag(getHelixClusterName(), instanceId,
TagNameUtils.getOfflineTagForTenant(null));
+ helixAdmin.addInstanceTag(getHelixClusterName(), instanceId,
TagNameUtils.getRealtimeTagForTenant(null));
+ }
+ HelixConfigScope configScope = new
HelixConfigScopeBuilder(HelixConfigScope.ConfigScopeProperty.PARTICIPANT,
+ getHelixClusterName()).forParticipant(instanceId).build();
+ int adminPort = NetUtils.findOpenPort(_nextServerPort);
+ helixAdmin.setConfig(configScope, Map.of(Helix.Instance.ADMIN_PORT_KEY,
Integer.toString(adminPort)));
+ _nextServerPort = adminPort + 1;
+ _fakeInstanceHelixManagers.add(helixManager);
+ }
+
/** Add fake server instances until total number of server instances reaches
maxCount */
public void addMoreFakeServerInstancesToAutoJoinHelixCluster(int maxCount,
boolean isSingleTenant)
throws Exception {
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerStatelessTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerStatelessTest.java
index 5af87ce0e5..feedaa8c32 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerStatelessTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerStatelessTest.java
@@ -96,6 +96,7 @@ public class PinotHelixResourceManagerStatelessTest extends
ControllerTest {
private static final int NUM_SERVER_INSTANCES = NUM_OFFLINE_SERVER_INSTANCES
+ NUM_REALTIME_SERVER_INSTANCES;
private static final String BROKER_TENANT_NAME = "brokerTenant";
private static final String SERVER_TENANT_NAME = "serverTenant";
+ private static final String SERVER_NAME_TAGGED = "Server_localhost_0";
private static final String RAW_TABLE_NAME = "testTable";
private static final String OFFLINE_TABLE_NAME =
TableNameBuilder.OFFLINE.tableNameWithType(RAW_TABLE_NAME);
@@ -702,6 +703,37 @@ public class PinotHelixResourceManagerStatelessTest
extends ControllerTest {
resetServerTags();
}
+ @Test
+ public void testHandleEmptyServerTags()
+ throws Exception {
+ // Create an instance with no tags
+ String serverName = "Server_localhost_" + NUM_SERVER_INSTANCES;
+ Instance instance = new Instance("localhost", NUM_SERVER_INSTANCES,
InstanceType.SERVER,
+ Collections.emptyList(), null, 0, 12345, 0, 0, false);
+ _helixResourceManager.addInstance(instance, false);
+ addFakeServerInstanceToAutoJoinHelixClusterWithEmptyTag(serverName, false);
+
+ List<String> allInstances = _helixResourceManager.getAllInstances();
+ assertTrue(allInstances.contains(serverName));
+ List<String> allLiveInstances =
_helixResourceManager.getAllLiveInstances();
+ assertTrue(allLiveInstances.contains(serverName));
+
+ // Verify that the server is considered untagged
+ List<String> untaggedServers =
_helixResourceManager.getOnlineUnTaggedServerInstanceList();
+ assertTrue(untaggedServers.contains(serverName), "Server with empty tags
should be considered untagged");
+
+ // Takes care of the negative case
+ assertFalse(untaggedServers.contains(SERVER_NAME_TAGGED), "Server with
tags should not be considered untagged");
+
+
+ stopAndDropFakeInstance(serverName);
+
+ allInstances = _helixResourceManager.getAllInstances();
+ assertFalse(allInstances.contains(serverName));
+ allLiveInstances = _helixResourceManager.getAllLiveInstances();
+ assertFalse(allLiveInstances.contains(serverName));
+ }
+
@Test
public void testLeadControllerResource() {
IdealState leadControllerResourceIdealState =
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
