This is an automated email from the ASF dual-hosted git repository.
snlee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 78bfe3e Fix 500 issue with 'GET /tenants' API call (#4061)
78bfe3e is described below
commit 78bfe3e2dea4a8cdacf425fda416e1969378006b
Author: Seunghyun Lee <[email protected]>
AuthorDate: Thu Apr 4 20:34:41 2019 -0700
Fix 500 issue with 'GET /tenants' API call (#4061)
When a server has a wrong formatted tag, current implemantation
would return 500 error for GET /tenants call.
In that case, we should not fail and return the list of valid
tenants.
For the wrong formatted tags, this pr adds the log so that one can
look up the logs when cleaning up the tags.
---
.../apache/pinot/common/config/TagNameUtils.java | 5 +-
.../helix/core/PinotHelixResourceManager.java | 22 +++++++--
.../helix/core/PinotHelixResourceManagerTest.java | 54 +++++++++++++++++++---
3 files changed, 68 insertions(+), 13 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/config/TagNameUtils.java
b/pinot-common/src/main/java/org/apache/pinot/common/config/TagNameUtils.java
index 0b75d5d..878c438 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/config/TagNameUtils.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/config/TagNameUtils.java
@@ -18,6 +18,7 @@
*/
package org.apache.pinot.common.config;
+import org.apache.pinot.common.exception.InvalidConfigException;
import org.apache.pinot.common.utils.ServerType;
import org.apache.pinot.common.utils.TenantRole;
@@ -44,7 +45,7 @@ public class TagNameUtils {
return false;
}
- public static TenantRole getTenantRoleFromTag(String tagName) {
+ public static TenantRole getTenantRoleFromTag(String tagName) throws
InvalidConfigException {
if (tagName.endsWith(ServerType.REALTIME.toString())) {
return TenantRole.SERVER;
}
@@ -54,7 +55,7 @@ public class TagNameUtils {
if (tagName.endsWith(TenantRole.BROKER.toString())) {
return TenantRole.BROKER;
}
- throw new RuntimeException("Cannot identify tenant type from tag name : "
+ tagName);
+ throw new InvalidConfigException("Cannot identify tenant type from tag
name : " + tagName);
}
public static String getTagFromTenantAndServerType(String tenantName,
ServerType type) {
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 04b6f9b..1ad19fc 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
@@ -817,8 +817,15 @@ public class PinotHelixResourceManager {
.equals(CommonConstants.Minion.UNTAGGED_INSTANCE)) {
continue;
}
- if (TagNameUtils.getTenantRoleFromTag(tag) == TenantRole.BROKER) {
- tenantSet.add(TagNameUtils.getTenantNameFromTag(tag));
+ TenantRole tenantRole;
+ try {
+ tenantRole = TagNameUtils.getTenantRoleFromTag(tag);
+ if (tenantRole == TenantRole.BROKER) {
+ tenantSet.add(TagNameUtils.getTenantNameFromTag(tag));
+ }
+ } catch (InvalidConfigException e) {
+ LOGGER.warn("Instance {} contains an invalid tag: {}", instanceName,
tag);
+ continue;
}
}
}
@@ -836,8 +843,15 @@ public class PinotHelixResourceManager {
.equals(CommonConstants.Minion.UNTAGGED_INSTANCE)) {
continue;
}
- if (TagNameUtils.getTenantRoleFromTag(tag) == TenantRole.SERVER) {
- tenantSet.add(TagNameUtils.getTenantNameFromTag(tag));
+ TenantRole tenantRole;
+ try {
+ tenantRole = TagNameUtils.getTenantRoleFromTag(tag);
+ if (tenantRole == TenantRole.SERVER) {
+ tenantSet.add(TagNameUtils.getTenantNameFromTag(tag));
+ }
+ } catch (InvalidConfigException e) {
+ LOGGER.warn("Instance {} contains an invalid tag: {}", instanceName,
tag);
+ continue;
}
}
}
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerTest.java
index 6d3a76f..11459fd 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerTest.java
@@ -44,6 +44,7 @@ import
org.apache.pinot.controller.helix.ControllerRequestBuilderUtil;
import org.apache.pinot.controller.helix.ControllerTest;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
+import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
@@ -217,13 +218,6 @@ public class PinotHelixResourceManagerTest extends
ControllerTest {
idealState = _helixAdmin.getResourceIdealState(_helixClusterName,
CommonConstants.Helix.BROKER_RESOURCE_INSTANCE);
Assert.assertEquals(idealState.getInstanceStateMap(OFFLINE_TABLE_NAME).size(),
5);
- // Untag all Brokers for other tests
- for (String brokerInstance :
_helixResourceManager.getAllInstancesForBrokerTenant(BROKER_TENANT_NAME)) {
- _helixAdmin
- .removeInstanceTag(_helixClusterName, brokerInstance,
TagNameUtils.getBrokerTagForTenant(BROKER_TENANT_NAME));
- _helixAdmin.addInstanceTag(_helixClusterName, brokerInstance,
CommonConstants.Helix.UNTAGGED_BROKER_INSTANCE);
- }
-
// Delete the table
_helixResourceManager.deleteOfflineTable(TABLE_NAME);
}
@@ -264,6 +258,52 @@ public class PinotHelixResourceManagerTest extends
ControllerTest {
}
}
+ @Test void testRetrieveTenantNames() {
+ // Create broker tenant on 1 Broker
+ Tenant brokerTenant =
+ new
Tenant.TenantBuilder(BROKER_TENANT_NAME).setRole(TenantRole.BROKER).setTotalInstances(1).build();
+ _helixResourceManager.createBrokerTenant(brokerTenant);
+
+ Set<String> brokerTenantNames =
_helixResourceManager.getAllBrokerTenantNames();
+ Assert.assertEquals(brokerTenantNames.size(), 1);
+ Assert.assertEquals(brokerTenantNames.iterator().next(),
BROKER_TENANT_NAME);
+
+ String testBrokerInstance =
+
_helixResourceManager.getAllInstancesForBrokerTenant(BROKER_TENANT_NAME).iterator().next();
+ _helixAdmin.addInstanceTag(_helixClusterName, testBrokerInstance,
"wrong_tag");
+
+ brokerTenantNames = _helixResourceManager.getAllBrokerTenantNames();
+ Assert.assertEquals(brokerTenantNames.size(), 1);
+ Assert.assertEquals(brokerTenantNames.iterator().next(),
BROKER_TENANT_NAME);
+
+ _helixAdmin.removeInstanceTag(_helixClusterName, testBrokerInstance,
"wrong_tag");
+
+ // Server tenant is already created during setup.
+ Set<String> serverTenantNames =
_helixResourceManager.getAllServerTenantNames();
+ Assert.assertEquals(serverTenantNames.size(), 1);
+ Assert.assertEquals(serverTenantNames.iterator().next(),
SERVER_TENANT_NAME);
+
+ String testServerInstance =
+
_helixResourceManager.getAllInstancesForServerTenant(SERVER_TENANT_NAME).iterator().next();
+ _helixAdmin.addInstanceTag(_helixClusterName, testServerInstance,
"wrong_tag");
+
+ serverTenantNames = _helixResourceManager.getAllServerTenantNames();
+ Assert.assertEquals(serverTenantNames.size(), 1);
+ Assert.assertEquals(serverTenantNames.iterator().next(),
SERVER_TENANT_NAME);
+
+ _helixAdmin.removeInstanceTag(_helixClusterName, testServerInstance,
"wrong_tag");
+ }
+
+ @AfterMethod
+ public void cleanUpBrokerTags() {
+ // Untag all Brokers for other tests
+ for (String brokerInstance :
_helixResourceManager.getAllInstancesForBrokerTenant(BROKER_TENANT_NAME)) {
+ _helixAdmin
+ .removeInstanceTag(_helixClusterName, brokerInstance,
TagNameUtils.getBrokerTagForTenant(BROKER_TENANT_NAME));
+ _helixAdmin.addInstanceTag(_helixClusterName, brokerInstance,
CommonConstants.Helix.UNTAGGED_BROKER_INSTANCE);
+ }
+ }
+
@AfterClass
public void tearDown() {
stopController();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
