You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by jl...@apache.org on 2019/01/29 23:01:47 UTC
[incubator-pinot] branch master updated: Better handle NPE from
getting instance config (#3758)
This is an automated email from the ASF dual-hosted git repository.
jlli 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 f8a1ff2 Better handle NPE from getting instance config (#3758)
f8a1ff2 is described below
commit f8a1ff2c106cb41bd1e1d89dadc2edc177f6c518
Author: Jialiang Li <jl...@linkedin.com>
AuthorDate: Tue Jan 29 15:01:42 2019 -0800
Better handle NPE from getting instance config (#3758)
* Better handle NPE from getting instance config
---
.../api/resources/PinotInstanceRestletResource.java | 7 ++++---
.../pinot/controller/api/resources/PqlQueryResource.java | 4 ++++
.../controller/helix/core/PinotHelixResourceManager.java | 15 +++++++++------
.../helix/core/PinotHelixResourceManagerTest.java | 7 -------
4 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
index c3672d9..6ff9f16 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
@@ -85,12 +85,13 @@ public class PinotInstanceRestletResource {
@ApiOperation(value = "Get instance information", produces = MediaType.APPLICATION_JSON)
@ApiResponses(value = {@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 404, message = "Instance not found"), @ApiResponse(code = 500, message = "Internal error")})
public String getInstance(
- @ApiParam(value = "Instance name", required = true, example = "Server_a.b.com_20000 | Broker_my.broker.com_30000") @PathParam("instanceName") String instanceName) {
- if (!pinotHelixResourceManager.instanceExists(instanceName)) {
+ @ApiParam(value = "Instance name", required = true, example = "Server_a.b.com_20000 | Broker_my.broker.com_30000")
+ @PathParam("instanceName") String instanceName) {
+ InstanceConfig instanceConfig = pinotHelixResourceManager.getHelixInstanceConfig(instanceName);
+ if (instanceConfig == null) {
throw new ControllerApplicationException(LOGGER, "Instance " + instanceName + " not found",
Response.Status.NOT_FOUND);
}
- InstanceConfig instanceConfig = pinotHelixResourceManager.getHelixInstanceConfig(instanceName);
ObjectNode response = JsonUtils.newObjectNode();
response.put("instanceName", instanceConfig.getInstanceName());
response.put("hostName", instanceConfig.getHostName());
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PqlQueryResource.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PqlQueryResource.java
index e1f6c37..325dc04 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PqlQueryResource.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PqlQueryResource.java
@@ -129,6 +129,10 @@ public class PqlQueryResource {
// Send query to a random broker.
String instanceId = instanceIds.get(RANDOM.nextInt(instanceIds.size()));
InstanceConfig instanceConfig = _pinotHelixResourceManager.getHelixInstanceConfig(instanceId);
+ if (instanceConfig == null) {
+ LOGGER.error("Instance {} not found", instanceId);
+ return QueryException.INTERNAL_ERROR.toString();
+ }
String hostNameWithPrefix = instanceConfig.getHostName();
String url =
"http://" + hostNameWithPrefix.substring(hostNameWithPrefix.indexOf("_") + 1) + ":" + instanceConfig.getPort()
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 652ebce..ed9b604 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
@@ -283,10 +283,9 @@ public class PinotHelixResourceManager {
* @param instanceId Instance Id
* @return Helix instance config
*/
- @Nonnull
public InstanceConfig getHelixInstanceConfig(@Nonnull String instanceId) {
ZNRecord znRecord = _cacheInstanceConfigsDataAccessor.get("/" + instanceId, null, AccessOption.PERSISTENT);
- return new InstanceConfig(znRecord);
+ return znRecord != null ? new InstanceConfig(znRecord) : null;
}
/**
@@ -2157,9 +2156,8 @@ public class PinotHelixResourceManager {
* @return True if instance exists in the Helix cluster, False otherwise.
*/
public boolean instanceExists(String instanceName) {
- HelixDataAccessor helixDataAccessor = _helixZkManager.getHelixDataAccessor();
- InstanceConfig config = helixDataAccessor.getProperty(_keyBuilder.instanceConfig(instanceName));
- return (config != null);
+ ZNRecord znRecord = _cacheInstanceConfigsDataAccessor.get("/" + instanceName, null, AccessOption.PERSISTENT);
+ return (znRecord != null);
}
public boolean isSingleTenantCluster() {
@@ -2210,6 +2208,10 @@ public class PinotHelixResourceManager {
BiMap<String, String> endpointToInstance = HashBiMap.create(instances.size());
for (String instance : instances) {
InstanceConfig helixInstanceConfig = getHelixInstanceConfig(instance);
+ if (helixInstanceConfig == null) {
+ LOGGER.warn("Instance {} not found", instance);
+ continue;
+ }
ZNRecord record = helixInstanceConfig.getRecord();
String[] hostnameSplit = helixInstanceConfig.getHostName().split("_");
Preconditions.checkState(hostnameSplit.length >= 2);
@@ -2239,13 +2241,14 @@ public class PinotHelixResourceManager {
final long externalViewOnlineToOfflineTimeoutMillis = 100L;
final boolean isSingleTenantCluster = false;
final boolean isUpdateStateModel = false;
+ final boolean enableBatchMessageMode = false;
MetricsRegistry metricsRegistry = new MetricsRegistry();
final boolean dryRun = true;
final String tableName = "testTable";
final TableType tableType = TableType.OFFLINE;
PinotHelixResourceManager helixResourceManager =
new PinotHelixResourceManager(zkURL, helixClusterName, controllerInstanceId, localDiskDir,
- externalViewOnlineToOfflineTimeoutMillis, isSingleTenantCluster, isUpdateStateModel);
+ externalViewOnlineToOfflineTimeoutMillis, isSingleTenantCluster, isUpdateStateModel, enableBatchMessageMode);
helixResourceManager.start();
ZNRecord record = helixResourceManager.rebalanceTable(tableName, dryRun, tableType);
ObjectMapper mapper = new ObjectMapper();
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 3d5f944..6d3a76f 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
@@ -19,7 +19,6 @@
package org.apache.pinot.controller.helix.core;
import com.google.common.collect.BiMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Random;
import java.util.Set;
@@ -265,12 +264,6 @@ public class PinotHelixResourceManagerTest extends ControllerTest {
}
}
- @Test
- public void testGetDataInstanceAdminEndpoints() {
- Set<String> fakeInstances = new HashSet<>();
- new Random().nextInt(NUM_INSTANCES);
- }
-
@AfterClass
public void tearDown() {
stopController();
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org