You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ne...@apache.org on 2019/01/17 17:11:53 UTC
[incubator-pinot] branch master updated: Fix SegmentStatusChecker
exception (#3704)
This is an automated email from the ASF dual-hosted git repository.
nehapawar 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 d0d5434 Fix SegmentStatusChecker exception (#3704)
d0d5434 is described below
commit d0d543427c12d3dcc971278e687d8355f3dd54c9
Author: Neha Pawar <np...@linkedin.com>
AuthorDate: Thu Jan 17 09:11:48 2019 -0800
Fix SegmentStatusChecker exception (#3704)
SegmentStatusChecker is instantiated in the ControllerStarter constructor, right after PinotHelixResourceManager is instantiated. The SegmentStatusChecker (and all other periodic tasks) receive the _pinotHelixResourceManager object as a parameter in their constructor. However, the _pinotHelixResourceManager is not supposed to be used until PinotHelixResourceManager::start() is invoked. Only after invoking start() does the _pinotHelixResourceManager properly get initialized i.e. the _h [...]
As a result, even though we pass _pinotHelixResourceManager to all periodic tasks in their constructor, we cannot be accessing these objects, until after the invocation of start().
---
.../controller/helix/SegmentStatusChecker.java | 19 +---
.../helix/core/PinotHelixResourceManager.java | 18 ++++
.../controller/helix/SegmentStatusCheckerTest.java | 118 +++++----------------
3 files changed, 50 insertions(+), 105 deletions(-)
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java
index ee9b7de..64d8aac 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java
@@ -21,13 +21,9 @@ package org.apache.pinot.controller.helix;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
-import org.apache.helix.HelixAdmin;
-import org.apache.helix.ZNRecord;
import org.apache.helix.model.ExternalView;
import org.apache.helix.model.IdealState;
-import org.apache.helix.store.zk.ZkHelixPropertyStore;
import org.apache.pinot.common.config.TableNameBuilder;
-import org.apache.pinot.common.metadata.ZKMetadataProvider;
import org.apache.pinot.common.metadata.segment.OfflineSegmentZKMetadata;
import org.apache.pinot.common.metrics.ControllerGauge;
import org.apache.pinot.common.metrics.ControllerMetrics;
@@ -50,9 +46,6 @@ public class SegmentStatusChecker extends ControllerPeriodicTask {
public static final String ERROR = "ERROR";
public static final String CONSUMING = "CONSUMING";
private final ControllerMetrics _metricsRegistry;
- private final HelixAdmin _helixAdmin;
- private final String _helixClusterName;
- private final ZkHelixPropertyStore<ZNRecord> _propertyStore;
private final int _waitForPushTimeSeconds;
// log messages about disabled tables atmost once a day
@@ -72,9 +65,6 @@ public class SegmentStatusChecker extends ControllerPeriodicTask {
public SegmentStatusChecker(PinotHelixResourceManager pinotHelixResourceManager, ControllerConf config,
ControllerMetrics metricsRegistry) {
super("SegmentStatusChecker", config.getStatusCheckerFrequencyInSeconds(), pinotHelixResourceManager);
- _helixAdmin = pinotHelixResourceManager.getHelixAdmin();
- _helixClusterName = pinotHelixResourceManager.getHelixClusterName();
- _propertyStore = _pinotHelixResourceManager.getPropertyStore();
_waitForPushTimeSeconds = config.getStatusCheckerWaitForPushTimeInSeconds();
_metricsRegistry = metricsRegistry;
@@ -128,7 +118,7 @@ public class SegmentStatusChecker extends ControllerPeriodicTask {
} else {
_realTimeTableCount++;
}
- IdealState idealState = _helixAdmin.getResourceIdealState(_helixClusterName, tableNameWithType);
+ IdealState idealState = _pinotHelixResourceManager.getTableIdealState(tableNameWithType);
if ((idealState == null) || (idealState.getPartitionSet().isEmpty())) {
int nReplicasFromIdealState = 1;
try {
@@ -157,7 +147,7 @@ public class SegmentStatusChecker extends ControllerPeriodicTask {
idealState.toString().length());
_metricsRegistry.setValueOfTableGauge(tableNameWithType, ControllerGauge.SEGMENT_COUNT,
(long) (idealState.getPartitionSet().size()));
- ExternalView externalView = _helixAdmin.getResourceExternalView(_helixClusterName, tableNameWithType);
+ ExternalView externalView = _pinotHelixResourceManager.getTableExternalView(tableNameWithType);
int nReplicasIdealMax = 0; // Keeps track of maximum number of replicas in ideal state
int nReplicasExternal = -1; // Keeps track of minimum number of replicas in external view
@@ -190,7 +180,8 @@ public class SegmentStatusChecker extends ControllerPeriodicTask {
TableType tableType = TableNameBuilder.getTableTypeFromTableName(tableNameWithType);
if ((tableType != null) && (tableType.equals(TableType.OFFLINE))) {
OfflineSegmentZKMetadata segmentZKMetadata =
- ZKMetadataProvider.getOfflineSegmentZKMetadata(_propertyStore, tableNameWithType, partitionName);
+ _pinotHelixResourceManager.getOfflineSegmentZKMetadata(tableNameWithType, partitionName);
+
if (segmentZKMetadata != null
&& segmentZKMetadata.getPushTime() > System.currentTimeMillis() - _waitForPushTimeSeconds * 1000) {
// push not yet finished, skip
@@ -243,7 +234,7 @@ public class SegmentStatusChecker extends ControllerPeriodicTask {
nReplicasIdealMax);
}
} catch (Exception e) {
- LOGGER.warn("Caught exception while updating segment status for table {}", tableNameWithType, e);
+ LOGGER.error("Caught exception while updating segment status for table {}", tableNameWithType, e);
// Remove the metric for this table
resetTableMetrics(tableNameWithType);
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 1f38cab..b6d59b2 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
@@ -1927,6 +1927,24 @@ public class PinotHelixResourceManager {
}
/**
+ * Gets the ideal state of the table
+ * @param tableNameWithType Table name with suffix
+ * @return IdealState of tableNameWithType
+ */
+ public IdealState getTableIdealState(String tableNameWithType) {
+ return _helixAdmin.getResourceIdealState(_helixClusterName, tableNameWithType);
+ }
+
+ /**
+ * Gets the external view of the table
+ * @param tableNameWithType Table name with suffix
+ * @return ExternalView of tableNameWithType
+ */
+ public ExternalView getTableExternalView(String tableNameWithType) {
+ return _helixAdmin.getResourceExternalView(_helixClusterName, tableNameWithType);
+ }
+
+ /**
* Get the table config for the given table name with type suffix.
*
* @param tableNameWithType Table name with type suffix
diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/SegmentStatusCheckerTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/SegmentStatusCheckerTest.java
index 79d4d43..db33605 100644
--- a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/SegmentStatusCheckerTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/SegmentStatusCheckerTest.java
@@ -23,12 +23,12 @@ import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
import org.apache.helix.AccessOption;
-import org.apache.helix.HelixAdmin;
import org.apache.helix.ZNRecord;
import org.apache.helix.model.ExternalView;
import org.apache.helix.model.IdealState;
import org.apache.helix.store.zk.ZkHelixPropertyStore;
import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.metadata.segment.OfflineSegmentZKMetadata;
import org.apache.pinot.common.metrics.ControllerGauge;
import org.apache.pinot.common.metrics.ControllerMetrics;
import org.apache.pinot.common.utils.CommonConstants;
@@ -38,8 +38,7 @@ import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
import org.testng.Assert;
import org.testng.annotations.Test;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
public class SegmentStatusCheckerTest {
@@ -71,17 +70,11 @@ public class SegmentStatusCheckerTest {
externalView.setState("myTable_1","pinot1","ERROR");
externalView.setState("myTable_1","pinot2","ONLINE");
- HelixAdmin helixAdmin;
- {
- helixAdmin = mock(HelixAdmin.class);
- when(helixAdmin.getResourceIdealState("StatusChecker",tableName)).thenReturn(idealState);
- when(helixAdmin.getResourceExternalView("StatusChecker",tableName)).thenReturn(externalView);
- }
{
helixResourceManager = mock(PinotHelixResourceManager.class);
when(helixResourceManager.getAllTables()).thenReturn(allTableNames);
- when(helixResourceManager.getHelixClusterName()).thenReturn("StatusChecker");
- when(helixResourceManager.getHelixAdmin()).thenReturn(helixAdmin);
+ when(helixResourceManager.getTableIdealState(tableName)).thenReturn(idealState);
+ when(helixResourceManager.getTableExternalView(tableName)).thenReturn(externalView);
}
{
config = mock(ControllerConf.class);
@@ -136,17 +129,11 @@ public class SegmentStatusCheckerTest {
externalView.setState(seg3.getSegmentName(),"pinot2","CONSUMING");
externalView.setState(seg3.getSegmentName(),"pinot3","OFFLINE");
- HelixAdmin helixAdmin;
- {
- helixAdmin = mock(HelixAdmin.class);
- when(helixAdmin.getResourceIdealState("StatusChecker",tableName)).thenReturn(idealState);
- when(helixAdmin.getResourceExternalView("StatusChecker",tableName)).thenReturn(externalView);
- }
{
helixResourceManager = mock(PinotHelixResourceManager.class);
when(helixResourceManager.getAllTables()).thenReturn(allTableNames);
- when(helixResourceManager.getHelixClusterName()).thenReturn("StatusChecker");
- when(helixResourceManager.getHelixAdmin()).thenReturn(helixAdmin);
+ when(helixResourceManager.getTableIdealState(tableName)).thenReturn(idealState);
+ when(helixResourceManager.getTableExternalView(tableName)).thenReturn(externalView);
}
{
config = mock(ControllerConf.class);
@@ -212,18 +199,13 @@ public class SegmentStatusCheckerTest {
when(propertyStore.get("/SEGMENTS/myTable_OFFLINE/myTable_3",null, AccessOption.PERSISTENT)).thenReturn(znrecord);
}
- HelixAdmin helixAdmin;
- {
- helixAdmin = mock(HelixAdmin.class);
- when(helixAdmin.getResourceIdealState("StatusChecker",tableName)).thenReturn(idealState);
- when(helixAdmin.getResourceExternalView("StatusChecker",tableName)).thenReturn(externalView);
- }
{
helixResourceManager = mock(PinotHelixResourceManager.class);
when(helixResourceManager.getAllTables()).thenReturn(allTableNames);
- when(helixResourceManager.getHelixClusterName()).thenReturn("StatusChecker");
- when(helixResourceManager.getHelixAdmin()).thenReturn(helixAdmin);
- when(helixResourceManager.getPropertyStore()).thenReturn(propertyStore);
+ when(helixResourceManager.getTableIdealState(tableName)).thenReturn(idealState);
+ when(helixResourceManager.getTableExternalView(tableName)).thenReturn(externalView);
+ when(helixResourceManager.getOfflineSegmentZKMetadata(tableName, "myTable_3")).thenReturn(
+ new OfflineSegmentZKMetadata(znrecord));
}
{
config = mock(ControllerConf.class);
@@ -259,17 +241,11 @@ public class SegmentStatusCheckerTest {
idealState.setReplicas("2");
idealState.setRebalanceMode(IdealState.RebalanceMode.CUSTOMIZED);
- HelixAdmin helixAdmin;
- {
- helixAdmin = mock(HelixAdmin.class);
- when(helixAdmin.getResourceIdealState("StatusChecker",tableName)).thenReturn(idealState);
- when(helixAdmin.getResourceExternalView("StatusChecker",tableName)).thenReturn(null);
- }
{
helixResourceManager = mock(PinotHelixResourceManager.class);
when(helixResourceManager.getAllTables()).thenReturn(allTableNames);
- when(helixResourceManager.getHelixClusterName()).thenReturn("StatusChecker");
- when(helixResourceManager.getHelixAdmin()).thenReturn(helixAdmin);
+ when(helixResourceManager.getTableIdealState(tableName)).thenReturn(idealState);
+ when(helixResourceManager.getTableExternalView(tableName)).thenReturn(null);
}
{
config = mock(ControllerConf.class);
@@ -293,17 +269,11 @@ public class SegmentStatusCheckerTest {
List<String> allTableNames = new ArrayList<String>();
allTableNames.add(tableName);
- HelixAdmin helixAdmin;
- {
- helixAdmin = mock(HelixAdmin.class);
- when(helixAdmin.getResourceIdealState("StatusChecker",tableName)).thenReturn(null);
- when(helixAdmin.getResourceExternalView("StatusChecker",tableName)).thenReturn(null);
- }
{
helixResourceManager = mock(PinotHelixResourceManager.class);
when(helixResourceManager.getAllTables()).thenReturn(allTableNames);
- when(helixResourceManager.getHelixClusterName()).thenReturn("StatusChecker");
- when(helixResourceManager.getHelixAdmin()).thenReturn(helixAdmin);
+ when(helixResourceManager.getTableIdealState(tableName)).thenReturn(null);
+ when(helixResourceManager.getTableExternalView(tableName)).thenReturn(null);
}
{
config = mock(ControllerConf.class);
@@ -339,12 +309,6 @@ public class SegmentStatusCheckerTest {
externalView.setState("myTable_1","pinot1","ONLINE");
externalView.setState("myTable_1","pinot2","ONLINE");
- HelixAdmin helixAdmin;
- {
- helixAdmin = mock(HelixAdmin.class);
- when(helixAdmin.getResourceIdealState("StatusChecker","myTable_OFFLINE")).thenReturn(idealState);
- when(helixAdmin.getResourceExternalView("StatusChecker","myTable_OFFLINE")).thenReturn(externalView);
- }
ZNRecord znrecord = new ZNRecord("myTable_0");
znrecord.setSimpleField(CommonConstants.Segment.SEGMENT_NAME,"myTable_0");
znrecord.setSimpleField(CommonConstants.Segment.TABLE_NAME, "myTable_OFFLINE");
@@ -360,18 +324,13 @@ public class SegmentStatusCheckerTest {
znrecord.setLongField(CommonConstants.Segment.Offline.PUSH_TIME, System.currentTimeMillis());
znrecord.setLongField(CommonConstants.Segment.Offline.REFRESH_TIME,System.currentTimeMillis());
- ZkHelixPropertyStore<ZNRecord> propertyStore;
- {
- propertyStore = (ZkHelixPropertyStore<ZNRecord>) mock(ZkHelixPropertyStore.class);
- when(propertyStore.get("/SEGMENTS/myTable_OFFLINE/myTable_0",null, AccessOption.PERSISTENT)).thenReturn(znrecord);
- }
-
{
helixResourceManager = mock(PinotHelixResourceManager.class);
when(helixResourceManager.getAllTables()).thenReturn(allTableNames);
- when(helixResourceManager.getHelixClusterName()).thenReturn("StatusChecker");
- when(helixResourceManager.getHelixAdmin()).thenReturn(helixAdmin);
- when(helixResourceManager.getPropertyStore()).thenReturn(propertyStore);
+ when(helixResourceManager.getTableIdealState(tableName)).thenReturn(idealState);
+ when(helixResourceManager.getTableExternalView(tableName)).thenReturn(externalView);
+ when(helixResourceManager.getOfflineSegmentZKMetadata(tableName, "myTable_0")).thenReturn(
+ new OfflineSegmentZKMetadata(znrecord));
}
{
config = mock(ControllerConf.class);
@@ -405,17 +364,11 @@ public class SegmentStatusCheckerTest {
idealState.setReplicas("0");
idealState.setRebalanceMode(IdealState.RebalanceMode.CUSTOMIZED);
- HelixAdmin helixAdmin;
- {
- helixAdmin = mock(HelixAdmin.class);
- when(helixAdmin.getResourceIdealState("StatusChecker",tableName)).thenReturn(idealState);
- when(helixAdmin.getResourceExternalView("StatusChecker",tableName)).thenReturn(null);
- }
{
helixResourceManager = mock(PinotHelixResourceManager.class);
when(helixResourceManager.getAllTables()).thenReturn(allTableNames);
- when(helixResourceManager.getHelixClusterName()).thenReturn("StatusChecker");
- when(helixResourceManager.getHelixAdmin()).thenReturn(helixAdmin);
+ when(helixResourceManager.getTableIdealState(tableName)).thenReturn(idealState);
+ when(helixResourceManager.getTableExternalView(tableName)).thenReturn(null);
}
{
config = mock(ControllerConf.class);
@@ -443,17 +396,11 @@ public class SegmentStatusCheckerTest {
List<String> allTableNames = new ArrayList<String>();
allTableNames.add(tableName);
IdealState idealState = null;
- HelixAdmin helixAdmin;
- {
- helixAdmin = mock(HelixAdmin.class);
- when(helixAdmin.getResourceIdealState("StatusChecker",tableName)).thenReturn(idealState);
- when(helixAdmin.getResourceExternalView("StatusChecker",tableName)).thenReturn(null);
- }
{
helixResourceManager = mock(PinotHelixResourceManager.class);
when(helixResourceManager.getAllTables()).thenReturn(allTableNames);
- when(helixResourceManager.getHelixClusterName()).thenReturn("StatusChecker");
- when(helixResourceManager.getHelixAdmin()).thenReturn(helixAdmin);
+ when(helixResourceManager.getTableIdealState(tableName)).thenReturn(idealState);
+ when(helixResourceManager.getTableExternalView(tableName)).thenReturn(null);
}
{
config = mock(ControllerConf.class);
@@ -489,17 +436,12 @@ public class SegmentStatusCheckerTest {
idealState.setPartitionState("myTable_OFFLINE", "pinot3", "OFFLINE");
idealState.setReplicas("1");
idealState.setRebalanceMode(IdealState.RebalanceMode.CUSTOMIZED);
- HelixAdmin helixAdmin;
- {
- helixAdmin = mock(HelixAdmin.class);
- when(helixAdmin.getResourceIdealState("StatusChecker",tableName)).thenReturn(idealState);
- when(helixAdmin.getResourceExternalView("StatusChecker",tableName)).thenReturn(null);
- }
+
{
helixResourceManager = mock(PinotHelixResourceManager.class);
when(helixResourceManager.getAllTables()).thenReturn(allTableNames);
- when(helixResourceManager.getHelixClusterName()).thenReturn("StatusChecker");
- when(helixResourceManager.getHelixAdmin()).thenReturn(helixAdmin);
+ when(helixResourceManager.getTableIdealState(tableName)).thenReturn(idealState);
+ when(helixResourceManager.getTableExternalView(tableName)).thenReturn(null);
}
{
config = mock(ControllerConf.class);
@@ -540,17 +482,11 @@ public class SegmentStatusCheckerTest {
idealState.setReplicas(nReplicasStr);
idealState.setRebalanceMode(IdealState.RebalanceMode.CUSTOMIZED);
- HelixAdmin helixAdmin;
- {
- helixAdmin = mock(HelixAdmin.class);
- when(helixAdmin.getResourceIdealState("StatusChecker",tableName)).thenReturn(idealState);
- when(helixAdmin.getResourceExternalView("StatusChecker",tableName)).thenReturn(null);
- }
{
helixResourceManager = mock(PinotHelixResourceManager.class);
when(helixResourceManager.getAllTables()).thenReturn(allTableNames);
- when(helixResourceManager.getHelixClusterName()).thenReturn("StatusChecker");
- when(helixResourceManager.getHelixAdmin()).thenReturn(helixAdmin);
+ when(helixResourceManager.getTableIdealState(tableName)).thenReturn(idealState);
+ when(helixResourceManager.getTableExternalView(tableName)).thenReturn(null);
}
{
config = mock(ControllerConf.class);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org