This is an automated email from the ASF dual-hosted git repository.
jackie 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 3c033a6 Cleanup PinotHelixResourceManager and move the default tag
handling to the starter (#7123)
3c033a6 is described below
commit 3c033a6abd0f67e292a42ec16da7471db6da5d16
Author: Xiaotian (Jackie) Jiang <17555551+jackie-ji...@users.noreply.github.com>
AuthorDate: Fri Jul 2 15:42:44 2021 -0700
Cleanup PinotHelixResourceManager and move the default tag handling to the
starter (#7123)
- Move the default tag handling from PinotHelixResourceManager to
BaseControllerStarter
- Clean up the unused config `controller.upload.onlineToOfflineTimeout`
---
.../pinot/controller/BaseControllerStarter.java | 6 ++++-
.../apache/pinot/controller/ControllerConf.java | 10 --------
.../helix/core/PinotHelixResourceManager.java | 29 +++-------------------
.../pinot/controller/ControllerStarterTest.java | 4 +++
4 files changed, 12 insertions(+), 37 deletions(-)
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java
index 5dcbd65..27fc301 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java
@@ -26,6 +26,7 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -575,7 +576,10 @@ public abstract class BaseControllerStarter implements
ServiceStartable {
private void updateInstanceConfigIfNeeded() {
InstanceConfig instanceConfig =
HelixHelper.getInstanceConfig(_helixParticipantManager,
_helixParticipantInstanceId);
- if (HelixHelper.updateHostnamePort(instanceConfig, _hostname, _port)) {
+ boolean updated = HelixHelper.updateHostnamePort(instanceConfig,
_hostname, _port);
+ updated |= HelixHelper
+ .addDefaultTags(instanceConfig, () ->
Collections.singletonList(CommonConstants.Helix.CONTROLLER_INSTANCE));
+ if (updated) {
HelixHelper.updateInstanceConfig(_helixParticipantManager,
instanceConfig);
}
}
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java
index f1e4f34..c219412 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java
@@ -62,7 +62,6 @@ public class ControllerConf extends PinotConfiguration {
public static final String HELIX_CLUSTER_NAME =
"controller.helix.cluster.name";
public static final String CLUSTER_TENANT_ISOLATION_ENABLE =
"cluster.tenant.isolation.enable";
public static final String CONSOLE_WEBAPP_ROOT_PATH =
"controller.query.console";
- public static final String EXTERNAL_VIEW_ONLINE_TO_OFFLINE_TIMEOUT =
"controller.upload.onlineToOfflineTimeout";
public static final String CONTROLLER_MODE = "controller.mode";
public static final String LEAD_CONTROLLER_RESOURCE_REBALANCE_STRATEGY =
"controller.resource.rebalance.strategy";
@@ -175,7 +174,6 @@ public class ControllerConf extends PinotConfiguration {
// Defines the kind of storage and the underlying PinotFS implementation
private static final String PINOT_FS_FACTORY_CLASS_LOCAL =
"controller.storage.factory.class.file";
- private static final long
DEFAULT_EXTERNAL_VIEW_ONLINE_TO_OFFLINE_TIMEOUT_MILLIS = 120_000L; // 2 minutes
private static final int DEFAULT_SERVER_ADMIN_REQUEST_TIMEOUT_SECONDS = 30;
private static final int DEFAULT_DELETED_SEGMENTS_RETENTION_IN_DAYS = 7;
private static final int DEFAULT_TABLE_MIN_REPLICAS = 1;
@@ -541,14 +539,6 @@ public class ControllerConf extends PinotConfiguration {
Integer.toString(segmentRelocatorFrequencyInSeconds));
}
- public long getExternalViewOnlineToOfflineTimeout() {
- return getProperty(EXTERNAL_VIEW_ONLINE_TO_OFFLINE_TIMEOUT,
DEFAULT_EXTERNAL_VIEW_ONLINE_TO_OFFLINE_TIMEOUT_MILLIS);
- }
-
- public void setExternalViewOnlineToOfflineTimeout(long timeout) {
- setProperty(EXTERNAL_VIEW_ONLINE_TO_OFFLINE_TIMEOUT, timeout);
- }
-
public boolean tenantIsolationEnabled() {
return getProperty(CLUSTER_TENANT_ISOLATION_ENABLE, true);
}
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 bb01ff1..a7b244e 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
@@ -142,7 +142,6 @@ import org.slf4j.LoggerFactory;
public class PinotHelixResourceManager {
private static final Logger LOGGER =
LoggerFactory.getLogger(PinotHelixResourceManager.class);
- private static final long DEFAULT_EXTERNAL_VIEW_UPDATE_RETRY_INTERVAL_MILLIS
= 500L;
private static final long CACHE_ENTRY_EXPIRE_TIME_HOURS = 6L;
private static final RetryPolicy DEFAULT_RETRY_POLICY =
RetryPolicies.exponentialBackoffRetryPolicy(5, 1000L, 2.0f);
public static final String APPEND = "APPEND";
@@ -163,13 +162,11 @@ public class PinotHelixResourceManager {
private final String _helixZkURL;
private final String _helixClusterName;
private final String _dataDir;
- private final long _externalViewOnlineToOfflineTimeoutMillis;
private final boolean _isSingleTenantCluster;
private final boolean _enableBatchMessageMode;
private final boolean _allowHLCTables;
private HelixManager _helixZkManager;
- private String _instanceId;
private HelixAdmin _helixAdmin;
private ZkHelixPropertyStore<ZNRecord> _propertyStore;
private HelixDataAccessor _helixDataAccessor;
@@ -179,12 +176,10 @@ public class PinotHelixResourceManager {
private TableCache _tableCache;
public PinotHelixResourceManager(String zkURL, String helixClusterName,
@Nullable String dataDir,
- long externalViewOnlineToOfflineTimeoutMillis, boolean
isSingleTenantCluster, boolean enableBatchMessageMode,
- boolean allowHLCTables) {
+ boolean isSingleTenantCluster, boolean enableBatchMessageMode, boolean
allowHLCTables) {
_helixZkURL = HelixConfig.getAbsoluteZkPathForHelix(zkURL);
_helixClusterName = helixClusterName;
_dataDir = dataDir;
- _externalViewOnlineToOfflineTimeoutMillis =
externalViewOnlineToOfflineTimeoutMillis;
_isSingleTenantCluster = isSingleTenantCluster;
_enableBatchMessageMode = enableBatchMessageMode;
_allowHLCTables = allowHLCTables;
@@ -228,8 +223,8 @@ public class PinotHelixResourceManager {
public PinotHelixResourceManager(ControllerConf controllerConf) {
this(controllerConf.getZkStr(), controllerConf.getHelixClusterName(),
controllerConf.getDataDir(),
- controllerConf.getExternalViewOnlineToOfflineTimeout(),
controllerConf.tenantIsolationEnabled(),
- controllerConf.getEnableBatchMessageMode(),
controllerConf.getHLCTablesAllowed());
+ controllerConf.tenantIsolationEnabled(),
controllerConf.getEnableBatchMessageMode(),
+ controllerConf.getHLCTablesAllowed());
}
/**
@@ -240,14 +235,10 @@ public class PinotHelixResourceManager {
*/
public synchronized void start(HelixManager helixZkManager) {
_helixZkManager = helixZkManager;
- _instanceId = _helixZkManager.getInstanceName();
_helixAdmin = _helixZkManager.getClusterManagmentTool();
_propertyStore = _helixZkManager.getHelixPropertyStore();
_helixDataAccessor = _helixZkManager.getHelixDataAccessor();
_keyBuilder = _helixDataAccessor.keyBuilder();
-
- // Add instance group tag for controller
- addInstanceGroupTagIfNeeded();
_segmentDeletionManager = new SegmentDeletionManager(_dataDir,
_helixAdmin, _helixClusterName, _propertyStore);
ZKMetadataProvider.setClusterTenantIsolationEnabled(_propertyStore,
_isSingleTenantCluster);
@@ -323,20 +314,6 @@ public class PinotHelixResourceManager {
}
/**
- * Add instance group tag for controller so that pinot controller can be
assigned to lead controller resource.
- */
- private void addInstanceGroupTagIfNeeded() {
- InstanceConfig instanceConfig = getHelixInstanceConfig(_instanceId);
- // The instanceConfig can be null when connecting as a participant while
running from PerfBenchmarkRunner
- if (instanceConfig != null &&
!instanceConfig.containsTag(Helix.CONTROLLER_INSTANCE)) {
- LOGGER.info("Controller: {} doesn't contain group tag: {}. Adding one.",
_instanceId, Helix.CONTROLLER_INSTANCE);
- instanceConfig.addTag(Helix.CONTROLLER_INSTANCE);
- HelixDataAccessor accessor = _helixZkManager.getHelixDataAccessor();
- accessor.setProperty(accessor.keyBuilder().instanceConfig(_instanceId),
instanceConfig);
- }
- }
-
- /**
* Instance related APIs
*/
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/ControllerStarterTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/ControllerStarterTest.java
index 374d344..b5a4061 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/ControllerStarterTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/ControllerStarterTest.java
@@ -18,6 +18,7 @@
*/
package org.apache.pinot.controller;
+import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import org.apache.helix.model.InstanceConfig;
@@ -28,6 +29,7 @@ import org.testng.annotations.Test;
import static org.apache.pinot.controller.ControllerConf.CONTROLLER_HOST;
import static org.apache.pinot.controller.ControllerConf.CONTROLLER_PORT;
import static
org.apache.pinot.spi.utils.CommonConstants.Controller.CONFIG_OF_INSTANCE_ID;
+import static
org.apache.pinot.spi.utils.CommonConstants.Helix.CONTROLLER_INSTANCE;
import static org.testng.Assert.assertEquals;
@@ -57,6 +59,7 @@ public class ControllerStarterTest extends ControllerTest {
assertEquals(instanceConfig.getInstanceName(), instanceId);
assertEquals(instanceConfig.getHostName(), "myHost");
assertEquals(instanceConfig.getPort(), "1234");
+ assertEquals(instanceConfig.getTags(),
Collections.singleton(CONTROLLER_INSTANCE));
stopController();
stopZk();
@@ -77,6 +80,7 @@ public class ControllerStarterTest extends ControllerTest {
assertEquals(instanceConfig.getInstanceName(), instanceId);
assertEquals(instanceConfig.getHostName(), "myHost");
assertEquals(instanceConfig.getPort(), "1234");
+ assertEquals(instanceConfig.getTags(),
Collections.singleton(CONTROLLER_INSTANCE));
stopController();
stopZk();
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
