[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r314085360
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##
@@ -147,6 +150,9 @@ public ControllerStarter(ControllerConf conf) {
} else {
_adminApp =
new
ControllerAdminApiApplication(_config.getQueryConsoleWebappPath(),
_config.getQueryConsoleUseHttps());
+ // Helix instance type should be explicitly set to PARTICIPANT ONLY in
{@link ControllerStarter}.
Review comment:
Updated.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r314085308 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java ## @@ -0,0 +1,222 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.controller; + +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import org.apache.helix.HelixDataAccessor; +import org.apache.helix.HelixManager; +import org.apache.helix.PropertyKey; +import org.apache.helix.model.LiveInstance; +import org.apache.helix.model.ResourceConfig; +import org.apache.pinot.common.config.TableNameBuilder; +import org.apache.pinot.common.metrics.ControllerMeter; +import org.apache.pinot.common.metrics.ControllerMetrics; +import org.apache.pinot.common.utils.CommonConstants; +import org.apache.pinot.common.utils.helix.LeadControllerUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_ENABLED_KEY; Review comment: Done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r314085242
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,222 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.ResourceConfig;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.metrics.ControllerMeter;
+import org.apache.pinot.common.metrics.ControllerMetrics;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_ENABLED_KEY;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+ private static final long CONTROLLER_LEADERSHIP_FETCH_INTERVAL = 60_000L;
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r314085195
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,222 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.ResourceConfig;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.metrics.ControllerMeter;
+import org.apache.pinot.common.metrics.ControllerMetrics;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_ENABLED_KEY;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+ private static final long CONTROLLER_LEADERSHIP_FETCH_INTERVAL = 60_000L;
+
+ private final Set _partitionIdCache;
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313992748
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,222 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.ResourceConfig;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.metrics.ControllerMeter;
+import org.apache.pinot.common.metrics.ControllerMetrics;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_ENABLED_KEY;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+ private static final long CONTROLLER_LEADERSHIP_FETCH_INTERVAL = 60_000L;
+
+ private final Set _partitionIdCache;
+ private final String _instanceId;
+ private final HelixManager _helixManager;
+ private final ControllerMetrics _controllerMetrics;
+ private final Thread _controllerLeadershipFetchingThread;
+
+ private volatile boolean _isLeadControllerResourceEnabled = false;
+ private volatile boolean _amIHelixLeader = false;
+ private volatile boolean _isShuttingDown = false;
+
+ public LeadControllerManager(HelixManager helixManager, ControllerMetrics
controllerMetrics) {
Review comment:
And I checked the logic of `isLeader()` in HelixManager. The additional
check is whether sessionId matches, whereas in Pinot it's unnecessary to check
session id for finding a leader for tables.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313989001
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,222 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.ResourceConfig;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.metrics.ControllerMeter;
+import org.apache.pinot.common.metrics.ControllerMetrics;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_ENABLED_KEY;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+ private static final long CONTROLLER_LEADERSHIP_FETCH_INTERVAL = 60_000L;
+
+ private final Set _partitionIdCache;
+ private final String _instanceId;
+ private final HelixManager _helixManager;
+ private final ControllerMetrics _controllerMetrics;
+ private final Thread _controllerLeadershipFetchingThread;
+
+ private volatile boolean _isLeadControllerResourceEnabled = false;
+ private volatile boolean _amIHelixLeader = false;
+ private volatile boolean _isShuttingDown = false;
+
+ public LeadControllerManager(HelixManager helixManager, ControllerMetrics
controllerMetrics) {
Review comment:
For Pinot-only mode, `controllerHelixManager` is null. So we shouldn't pass
`controllerHelixManager` in here.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313520599 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java ## @@ -155,21 +160,32 @@ public PinotHelixResourceManager(@Nonnull String zkURL, @Nonnull String helixClu _isSingleTenantCluster = isSingleTenantCluster; _enableBatchMessageMode = enableBatchMessageMode; _allowHLCTables = allowHLCTables; +_helixInstanceType = helixInstanceType; + +_helixZkManager = Review comment: Done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313189629
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,111 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtil;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+ public static final String RESOURCE_ENABLED = "RESOURCE_ENABLED";
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtil.murmur2(StringUtil.encodeUtf8(rawTableName)) &
Integer.MAX_VALUE);
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
+return getHashCodeForTable(rawTableName) % numPartitions;
+ }
+
+ /**
+ * Gets lead controller participant id for table from lead controller
resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return controller participant id for partition leader, e.g.
Controller_localhost_9000. Null if not found or resource is disabled.
+ */
+ public static String getLeadControllerInstanceForTable(ExternalView
leadControllerResourceExternalView,
+ String rawTableName) {
+if (leadControllerResourceExternalView == null) {
+ return null;
+}
+Set partitionSet =
leadControllerResourceExternalView.getPartitionSet();
+if (partitionSet == null || partitionSet.isEmpty()) {
+ return null;
+}
+int numPartitions = partitionSet.size();
+int partitionIndex = getPartitionIdForTable(rawTableName, numPartitions);
+String partitionName = generatePartitionName(partitionIndex);
+Map partitionStateMap =
leadControllerResourceExternalView.getStateMap(partitionName);
+
+// Get master host from partition map. Return null if no master found.
+for (Map.Entry entry : partitionStateMap.entrySet()) {
+ if (MasterSlaveSMD.States.MASTER.name().equals(entry.getValue())) {
+return entry.getKey();
+ }
+}
+return null;
+ }
+
+ /**
+ * Generates controller participant id, e.g. returns
Controller_localhost_9000 given localhost as hostname and 9000 as port.
+ */
+ public static String generateControllerParticipantId(String controllerHost,
String controllerPort) {
+return
org.apache.pinot.common.utils.CommonConstants.Helix.PREFIX_OF_CONTROLLER_INSTANCE
+ controllerHost + "_"
++ controllerPort;
+ }
+
+ /**
+ * Extracts lead controller hostname and port from controller participant
id, e.g. returns localhost_9000 given Controller_localhost_9000 as controller
participant id.
+ */
+ public static String extractLeadControllerHostNameAndPort(String
controllerParticipantId) {
Review comment:
Updated the code to return the host-port pair instead of a string.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313189741
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,111 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtil;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+ public static final String RESOURCE_ENABLED = "RESOURCE_ENABLED";
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtil.murmur2(StringUtil.encodeUtf8(rawTableName)) &
Integer.MAX_VALUE);
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
+return getHashCodeForTable(rawTableName) % numPartitions;
+ }
+
+ /**
+ * Gets lead controller participant id for table from lead controller
resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return controller participant id for partition leader, e.g.
Controller_localhost_9000. Null if not found or resource is disabled.
+ */
+ public static String getLeadControllerInstanceForTable(ExternalView
leadControllerResourceExternalView,
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313058968
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##
@@ -146,7 +150,8 @@
public PinotHelixResourceManager(@Nonnull String zkURL, @Nonnull String
helixClusterName,
@Nonnull String controllerInstanceId, String dataDir, long
externalViewOnlineToOfflineTimeoutMillis,
- boolean isSingleTenantCluster, boolean enableBatchMessageMode, boolean
allowHLCTables) {
+ boolean isSingleTenantCluster, boolean enableBatchMessageMode, boolean
allowHLCTables,
+ InstanceType helixInstanceType) {
Review comment:
No need to keep this instance type any more.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313059200 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java ## @@ -155,21 +160,32 @@ public PinotHelixResourceManager(@Nonnull String zkURL, @Nonnull String helixClu _isSingleTenantCluster = isSingleTenantCluster; _enableBatchMessageMode = enableBatchMessageMode; _allowHLCTables = allowHLCTables; +_helixInstanceType = helixInstanceType; + +_helixZkManager = Review comment: Pass HelixManager as param in start() method. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313058999
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##
@@ -155,21 +160,32 @@ public PinotHelixResourceManager(@Nonnull String zkURL,
@Nonnull String helixClu
_isSingleTenantCluster = isSingleTenantCluster;
_enableBatchMessageMode = enableBatchMessageMode;
_allowHLCTables = allowHLCTables;
+_helixInstanceType = helixInstanceType;
+
+_helixZkManager =
+HelixManagerFactory.getZKHelixManager(_helixClusterName, _instanceId,
_helixInstanceType, _helixZkURL);
+// LeadControllerManager needs to be initialized before registering as
Helix participant.
+_leadControllerManager = new LeadControllerManager(_instanceId,
_helixZkManager);
}
- public PinotHelixResourceManager(@Nonnull ControllerConf controllerConf) {
-this(controllerConf.getZkStr(), controllerConf.getHelixClusterName(),
-CommonConstants.Helix.PREFIX_OF_CONTROLLER_INSTANCE +
controllerConf.getControllerHost() + "_" + controllerConf
-.getControllerPort(), controllerConf.getDataDir(),
controllerConf.getExternalViewOnlineToOfflineTimeout(),
+ // This constructor should only be explicitly called by {@link
ControllerStarter}.
+ public PinotHelixResourceManager(@Nonnull ControllerConf controllerConf,
InstanceType instanceType) {
+this(controllerConf.getZkStr(), controllerConf.getHelixClusterName(),
LeadControllerUtils
+
.generateControllerParticipantId(controllerConf.getControllerHost(),
controllerConf.getControllerPort()),
+controllerConf.getDataDir(),
controllerConf.getExternalViewOnlineToOfflineTimeout(),
controllerConf.tenantIsolationEnabled(),
controllerConf.getEnableBatchMessageMode(),
-controllerConf.getHLCTablesAllowed());
+controllerConf.getHLCTablesAllowed(), instanceType);
+ }
+
+ public PinotHelixResourceManager(@Nonnull ControllerConf controllerConf) {
+this(controllerConf, InstanceType.ADMINISTRATOR);
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313058813
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##
@@ -155,21 +160,32 @@ public PinotHelixResourceManager(@Nonnull String zkURL,
@Nonnull String helixClu
_isSingleTenantCluster = isSingleTenantCluster;
_enableBatchMessageMode = enableBatchMessageMode;
_allowHLCTables = allowHLCTables;
+_helixInstanceType = helixInstanceType;
+
+_helixZkManager =
+HelixManagerFactory.getZKHelixManager(_helixClusterName, _instanceId,
_helixInstanceType, _helixZkURL);
+// LeadControllerManager needs to be initialized before registering as
Helix participant.
+_leadControllerManager = new LeadControllerManager(_instanceId,
_helixZkManager);
}
- public PinotHelixResourceManager(@Nonnull ControllerConf controllerConf) {
-this(controllerConf.getZkStr(), controllerConf.getHelixClusterName(),
-CommonConstants.Helix.PREFIX_OF_CONTROLLER_INSTANCE +
controllerConf.getControllerHost() + "_" + controllerConf
-.getControllerPort(), controllerConf.getDataDir(),
controllerConf.getExternalViewOnlineToOfflineTimeout(),
+ // This constructor should only be explicitly called by {@link
ControllerStarter}.
+ public PinotHelixResourceManager(@Nonnull ControllerConf controllerConf,
InstanceType instanceType) {
+this(controllerConf.getZkStr(), controllerConf.getHelixClusterName(),
LeadControllerUtils
+
.generateControllerParticipantId(controllerConf.getControllerHost(),
controllerConf.getControllerPort()),
+controllerConf.getDataDir(),
controllerConf.getExternalViewOnlineToOfflineTimeout(),
controllerConf.tenantIsolationEnabled(),
controllerConf.getEnableBatchMessageMode(),
-controllerConf.getHLCTablesAllowed());
+controllerConf.getHLCTablesAllowed(), instanceType);
+ }
+
+ public PinotHelixResourceManager(@Nonnull ControllerConf controllerConf) {
+this(controllerConf, InstanceType.ADMINISTRATOR);
Review comment:
Removed the instance type from resource manager and put the logic to the
caller instead.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313058363
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,111 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtil;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+ public static final String RESOURCE_ENABLED = "RESOURCE_ENABLED";
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtil.murmur2(StringUtil.encodeUtf8(rawTableName)) &
Integer.MAX_VALUE);
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
+return getHashCodeForTable(rawTableName) % numPartitions;
+ }
+
+ /**
+ * Gets lead controller participant id for table from lead controller
resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return controller participant id for partition leader, e.g.
Controller_localhost_9000. Null if not found or resource is disabled.
+ */
+ public static String getLeadControllerInstanceForTable(ExternalView
leadControllerResourceExternalView,
+ String rawTableName) {
+if (leadControllerResourceExternalView == null) {
+ return null;
+}
+Set partitionSet =
leadControllerResourceExternalView.getPartitionSet();
+if (partitionSet == null || partitionSet.isEmpty()) {
+ return null;
+}
+int numPartitions = partitionSet.size();
+int partitionIndex = getPartitionIdForTable(rawTableName, numPartitions);
+String partitionName = generatePartitionName(partitionIndex);
+Map partitionStateMap =
leadControllerResourceExternalView.getStateMap(partitionName);
+
+// Get master host from partition map. Return null if no master found.
+for (Map.Entry entry : partitionStateMap.entrySet()) {
+ if (MasterSlaveSMD.States.MASTER.name().equals(entry.getValue())) {
+return entry.getKey();
+ }
+}
+return null;
+ }
+
+ /**
+ * Generates controller participant id, e.g. returns
Controller_localhost_9000 given localhost as hostname and 9000 as port.
+ */
+ public static String generateControllerParticipantId(String controllerHost,
String controllerPort) {
+return
org.apache.pinot.common.utils.CommonConstants.Helix.PREFIX_OF_CONTROLLER_INSTANCE
+ controllerHost + "_"
++ controllerPort;
+ }
+
+ /**
+ * Extracts lead controller hostname and port from controller participant
id, e.g. returns localhost_9000 given Controller_localhost_9000 as controller
participant id.
+ */
+ public static String extractLeadControllerHostNameAndPort(String
controllerParticipantId) {
+return
controllerParticipantId.substring(controllerParticipantId.indexOf("_") + 1);
+ }
+
+ /**
+ * Generates partition name, e.g. returns leadControllerResource_0 given 0
as partition index.
+ */
+ public static String generatePartitionName(int partiti
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313058394
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,111 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtil;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+ public static final String RESOURCE_ENABLED = "RESOURCE_ENABLED";
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtil.murmur2(StringUtil.encodeUtf8(rawTableName)) &
Integer.MAX_VALUE);
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
+return getHashCodeForTable(rawTableName) % numPartitions;
+ }
+
+ /**
+ * Gets lead controller participant id for table from lead controller
resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return controller participant id for partition leader, e.g.
Controller_localhost_9000. Null if not found or resource is disabled.
+ */
+ public static String getLeadControllerInstanceForTable(ExternalView
leadControllerResourceExternalView,
+ String rawTableName) {
+if (leadControllerResourceExternalView == null) {
+ return null;
+}
+Set partitionSet =
leadControllerResourceExternalView.getPartitionSet();
+if (partitionSet == null || partitionSet.isEmpty()) {
+ return null;
+}
+int numPartitions = partitionSet.size();
+int partitionIndex = getPartitionIdForTable(rawTableName, numPartitions);
+String partitionName = generatePartitionName(partitionIndex);
+Map partitionStateMap =
leadControllerResourceExternalView.getStateMap(partitionName);
+
+// Get master host from partition map. Return null if no master found.
+for (Map.Entry entry : partitionStateMap.entrySet()) {
+ if (MasterSlaveSMD.States.MASTER.name().equals(entry.getValue())) {
+return entry.getKey();
+ }
+}
+return null;
+ }
+
+ /**
+ * Generates controller participant id, e.g. returns
Controller_localhost_9000 given localhost as hostname and 9000 as port.
+ */
+ public static String generateControllerParticipantId(String controllerHost,
String controllerPort) {
+return
org.apache.pinot.common.utils.CommonConstants.Helix.PREFIX_OF_CONTROLLER_INSTANCE
+ controllerHost + "_"
++ controllerPort;
+ }
+
+ /**
+ * Extracts lead controller hostname and port from controller participant
id, e.g. returns localhost_9000 given Controller_localhost_9000 as controller
participant id.
+ */
+ public static String extractLeadControllerHostNameAndPort(String
controllerParticipantId) {
+return
controllerParticipantId.substring(controllerParticipantId.indexOf("_") + 1);
+ }
+
+ /**
+ * Generates partition name, e.g. returns leadControllerResource_0 given 0
as partition index.
+ */
+ public static String generatePartitionName(int partiti
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313058483 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java ## @@ -70,11 +69,14 @@ import org.apache.pinot.controller.validation.RealtimeSegmentValidationManager; import org.apache.pinot.core.crypt.PinotCrypterFactory; import org.apache.pinot.core.periodictask.PeriodicTask; +import org.apache.pinot.core.periodictask.PeriodicTaskScheduler; import org.apache.pinot.filesystem.PinotFSFactory; import org.glassfish.hk2.utilities.binding.AbstractBinder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME; Review comment: Done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313058293
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,111 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtil;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+ public static final String RESOURCE_ENABLED = "RESOURCE_ENABLED";
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtil.murmur2(StringUtil.encodeUtf8(rawTableName)) &
Integer.MAX_VALUE);
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
+return getHashCodeForTable(rawTableName) % numPartitions;
+ }
+
+ /**
+ * Gets lead controller participant id for table from lead controller
resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return controller participant id for partition leader, e.g.
Controller_localhost_9000. Null if not found or resource is disabled.
+ */
+ public static String getLeadControllerInstanceForTable(ExternalView
leadControllerResourceExternalView,
+ String rawTableName) {
+if (leadControllerResourceExternalView == null) {
+ return null;
+}
+Set partitionSet =
leadControllerResourceExternalView.getPartitionSet();
+if (partitionSet == null || partitionSet.isEmpty()) {
+ return null;
+}
+int numPartitions = partitionSet.size();
+int partitionIndex = getPartitionIdForTable(rawTableName, numPartitions);
+String partitionName = generatePartitionName(partitionIndex);
+Map partitionStateMap =
leadControllerResourceExternalView.getStateMap(partitionName);
+
+// Get master host from partition map. Return null if no master found.
+for (Map.Entry entry : partitionStateMap.entrySet()) {
+ if (MasterSlaveSMD.States.MASTER.name().equals(entry.getValue())) {
+return entry.getKey();
+ }
+}
+return null;
+ }
+
+ /**
+ * Generates controller participant id, e.g. returns
Controller_localhost_9000 given localhost as hostname and 9000 as port.
+ */
+ public static String generateControllerParticipantId(String controllerHost,
String controllerPort) {
+return
org.apache.pinot.common.utils.CommonConstants.Helix.PREFIX_OF_CONTROLLER_INSTANCE
+ controllerHost + "_"
++ controllerPort;
+ }
+
+ /**
+ * Extracts lead controller hostname and port from controller participant
id, e.g. returns localhost_9000 given Controller_localhost_9000 as controller
participant id.
+ */
+ public static String extractLeadControllerHostNameAndPort(String
controllerParticipantId) {
+return
controllerParticipantId.substring(controllerParticipantId.indexOf("_") + 1);
+ }
+
+ /**
+ * Generates partition name, e.g. returns leadControllerResource_0 given 0
as partition index.
+ */
+ public static String generatePartitionName(int partiti
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313058262
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,111 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtil;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+ public static final String RESOURCE_ENABLED = "RESOURCE_ENABLED";
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtil.murmur2(StringUtil.encodeUtf8(rawTableName)) &
Integer.MAX_VALUE);
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
+return getHashCodeForTable(rawTableName) % numPartitions;
+ }
+
+ /**
+ * Gets lead controller participant id for table from lead controller
resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return controller participant id for partition leader, e.g.
Controller_localhost_9000. Null if not found or resource is disabled.
+ */
+ public static String getLeadControllerInstanceForTable(ExternalView
leadControllerResourceExternalView,
+ String rawTableName) {
+if (leadControllerResourceExternalView == null) {
+ return null;
+}
+Set partitionSet =
leadControllerResourceExternalView.getPartitionSet();
+if (partitionSet == null || partitionSet.isEmpty()) {
+ return null;
+}
+int numPartitions = partitionSet.size();
+int partitionIndex = getPartitionIdForTable(rawTableName, numPartitions);
+String partitionName = generatePartitionName(partitionIndex);
+Map partitionStateMap =
leadControllerResourceExternalView.getStateMap(partitionName);
+
+// Get master host from partition map. Return null if no master found.
+for (Map.Entry entry : partitionStateMap.entrySet()) {
+ if (MasterSlaveSMD.States.MASTER.name().equals(entry.getValue())) {
+return entry.getKey();
+ }
+}
+return null;
+ }
+
+ /**
+ * Generates controller participant id, e.g. returns
Controller_localhost_9000 given localhost as hostname and 9000 as port.
+ */
+ public static String generateControllerParticipantId(String controllerHost,
String controllerPort) {
+return
org.apache.pinot.common.utils.CommonConstants.Helix.PREFIX_OF_CONTROLLER_INSTANCE
+ controllerHost + "_"
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313056981
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,111 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtil;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+ public static final String RESOURCE_ENABLED = "RESOURCE_ENABLED";
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtil.murmur2(StringUtil.encodeUtf8(rawTableName)) &
Integer.MAX_VALUE);
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
Review comment:
Updated.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313057058
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,111 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtil;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+ public static final String RESOURCE_ENABLED = "RESOURCE_ENABLED";
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtil.murmur2(StringUtil.encodeUtf8(rawTableName)) &
Integer.MAX_VALUE);
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
+return getHashCodeForTable(rawTableName) % numPartitions;
+ }
+
+ /**
+ * Gets lead controller participant id for table from lead controller
resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return controller participant id for partition leader, e.g.
Controller_localhost_9000. Null if not found or resource is disabled.
+ */
+ public static String getLeadControllerInstanceForTable(ExternalView
leadControllerResourceExternalView,
+ String rawTableName) {
+if (leadControllerResourceExternalView == null) {
+ return null;
+}
+Set partitionSet =
leadControllerResourceExternalView.getPartitionSet();
+if (partitionSet == null || partitionSet.isEmpty()) {
+ return null;
+}
+int numPartitions = partitionSet.size();
+int partitionIndex = getPartitionIdForTable(rawTableName, numPartitions);
+String partitionName = generatePartitionName(partitionIndex);
+Map partitionStateMap =
leadControllerResourceExternalView.getStateMap(partitionName);
+
+// Get master host from partition map. Return null if no master found.
+for (Map.Entry entry : partitionStateMap.entrySet()) {
+ if (MasterSlaveSMD.States.MASTER.name().equals(entry.getValue())) {
+return entry.getKey();
+ }
+}
+return null;
+ }
+
+ /**
+ * Generates controller participant id, e.g. returns
Controller_localhost_9000 given localhost as hostname and 9000 as port.
+ */
+ public static String generateControllerParticipantId(String controllerHost,
String controllerPort) {
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313056862
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,111 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtil;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+ public static final String RESOURCE_ENABLED = "RESOURCE_ENABLED";
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtil.murmur2(StringUtil.encodeUtf8(rawTableName)) &
Integer.MAX_VALUE);
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r313056816
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,111 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtil;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+ public static final String RESOURCE_ENABLED = "RESOURCE_ENABLED";
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r307051249
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,160 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.ResourceConfig;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+
+ private Set _partitionIndexCache;
+ private String _instanceId;
+ private HelixManager _helixManager;
+ private volatile boolean _isLeadControllerResourceEnabled = false;
+ private volatile boolean _amIHelixLeader = false;
+ private volatile boolean _isShuttingDown = false;
+
+ public LeadControllerManager(String instanceId, HelixManager helixManager) {
+_instanceId = instanceId;
+_helixManager = helixManager;
+_partitionIndexCache = ConcurrentHashMap.newKeySet();
+ }
+
+ /**
+ * Marks the cached indices invalid and unregisters {@link
PinotHelixResourceManager}.
Review comment:
Added.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r307051141
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,163 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.model.ExternalView;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+
+ private Map _partitionIndexCache;
+ private PinotHelixResourceManager _pinotHelixResourceManager;
Review comment:
Refactored the code to pass `HelixManager` to the constructor of
`LeadControllerManager`. No need to have methods to register/unregister
`PinotHelxResourceManager`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r307050822
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,163 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.model.ExternalView;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+
+ private Map _partitionIndexCache;
+ private PinotHelixResourceManager _pinotHelixResourceManager;
+ private volatile boolean _amIHelixLeader = false;
+
+ public LeadControllerManager() {
+_partitionIndexCache = new ConcurrentHashMap<>();
+ }
+
+ public void registerResourceManager(PinotHelixResourceManager
pinotHelixResourceManager) {
+_pinotHelixResourceManager = pinotHelixResourceManager;
+ }
+
+ /**
+ * Check whether the current controller is the leader for the given table.
Return true if current controller is the leader for this table.
+ * Otherwise check whether the current controller is helix leader.
+ * @param tableName table name with/without table type.
+ */
+ public boolean isLeaderForTable(String tableName) {
+String rawTableName = TableNameBuilder.extractRawTableName(tableName);
+int partitionIndex =
+LeadControllerUtils.getPartitionIdForTable(rawTableName,
NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE);
+if (_partitionIndexCache.containsKey(partitionIndex)) {
Review comment:
Refactored the code to use resource config to determine whether the resource
is enabled.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r307050463
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java
##
@@ -500,4 +503,55 @@ public IdealState apply(IdealState idealState) {
public static Set getBrokerInstancesForTenant(List
instanceConfigs, String tenant) {
return new HashSet<>(HelixHelper.getInstancesWithTag(instanceConfigs,
TagNameUtils.getBrokerTagForTenant(tenant)));
}
+
+ /**
+ * Gets hash code for table.
+ * Note: This method CANNOT be changed when lead controller resource has
been enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return rawTableName.hashCode();
Review comment:
Put `murmur2` function to `HashUtil` in pinot-common module.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r306618557
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##
@@ -425,6 +416,23 @@ private void initPinotCrypterFactory() {
}
}
+ private void registerListeners(HelixManager helixParticipantManager) {
+// This registration is not needed when the resource is enabled.
+// However, the resource can be disabled sometime while the cluster is in
operation, so we keep it here. Plus, it does not add much overhead.
+// At some point in future when we stop supporting the disabled resource,
we will remove this line altogether and the logic that goes with it.
+LOGGER.info("Registering helix controller listener");
+helixParticipantManager.addControllerListener(
+(ControllerChangeListener) changeContext ->
_leadControllerManager.onHelixControllerChange());
+
+LOGGER.info("Registering resource config listener");
+try {
+ helixParticipantManager.addResourceConfigChangeListener(
+ (resourceConfigList, changeContext) ->
_leadControllerManager.onResourceConfigChange());
+} catch (Exception e) {
+ throw new RuntimeException("Error registering resource config listener",
e);
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r306618567
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##
@@ -425,6 +416,23 @@ private void initPinotCrypterFactory() {
}
}
+ private void registerListeners(HelixManager helixParticipantManager) {
+// This registration is not needed when the resource is enabled.
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r306618492 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadRestletResource.java ## @@ -311,7 +311,7 @@ private SuccessResponse uploadSegment(FormDataMultiPart multiPart, boolean enabl // Validate segment SegmentValidatorResponse segmentValidatorResponse = new SegmentValidator(_pinotHelixResourceManager, _controllerConf, _executor, _connectionManager, - _controllerMetrics, _controllerLeadershipManager) + _controllerMetrics, _leadControllerManager) Review comment: Done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r306616766
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,138 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+
+ private Set _partitionIndexCache;
+ private PinotHelixResourceManager _pinotHelixResourceManager;
+ private volatile boolean _isLeadControllerResourceEnabled = false;
+ private volatile boolean _amIHelixLeader = false;
+
+ public LeadControllerManager() {
+_partitionIndexCache = ConcurrentHashMap.newKeySet();
+ }
+
+ /**
+ * Registers {@link PinotHelixResourceManager} in LeadControllerManager,
which is needed to get the external view of lead controller resource.
+ */
+ public void registerResourceManager(PinotHelixResourceManager
pinotHelixResourceManager) {
+_pinotHelixResourceManager = pinotHelixResourceManager;
+ }
+
+ /**
+ * Marks the cached indices invalid and unregisters {@link
PinotHelixResourceManager}.
+ */
+ public void stop() {
+_partitionIndexCache.clear();
+_pinotHelixResourceManager = null;
+ }
+
+ /**
+ * Checks whether the current controller is the leader for the given table.
Return true if current controller is the leader for this table.
+ * Otherwise check whether the current controller is helix leader if the
resource is disabled.
+ * @param tableName table name with/without table type.
+ */
+ public boolean isLeaderForTable(String tableName) {
+if (isLeadControllerResourceEnabled()) {
+ String rawTableName = TableNameBuilder.extractRawTableName(tableName);
+ int partitionIndex =
LeadControllerUtils.getPartitionIdForTable(rawTableName,
NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE);
+ return _partitionIndexCache.contains(partitionIndex);
+} else {
+ // Checks if it's Helix leader if lead controller resource is disabled.
+ return isHelixLeader();
+}
+ }
+
+ /**
+ * Given a partition name, marks current controller as lead controller for
this partition by caching the partition index to current controller.
+ * @param partitionName partition name in lead controller resource, e.g.
leadControllerResource_0.
+ */
+ public synchronized void addPartitionLeader(String partitionName) {
+LOGGER.info("Add Partition: {} to LeadControllerManager", partitionName);
+int partitionIndex =
LeadControllerUtils.extractPartitionIndex(partitionName);
+_partitionIndexCache.add(partitionIndex);
+ }
+
+ /**
+ * Given a partition name, removes current controller as lead controller for
this partition by removing the partition index from current controller.
+ * @param partitionName partition name in lead controller resource, e.g.
leadControllerResource_0.
+ */
+ public synchronized void removePartitionLeader(String partitionName) {
+LOGGER.info("Remove Partition: {} from LeadControllerManager",
partitionName);
+int partitionIndex =
LeadControllerUtils.extractPartitionIndex(partitionName);
+_partitionIndexCache.remove(partitionIndex);
+ }
+
+ /**
+ * Checks if the current controller host is Helix cluster leader.
+ */
+ private boolean isHelixLeader() {
+return _amIHelixLeader;
+ }
+
+ public boolean isLeadControllerResourceEnabled() {
+return _isLeadControllerResourceEnabled;
+ }
+
+ /**
+ * Callback on changes in the controller. Should be registered to the
controller callback. This callback is
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305554916
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/HashUtil.java
##
@@ -105,4 +106,52 @@ public static long hash64(final byte[] data, int length,
int seed) {
h ^= h >>> r;
return h;
}
+
+ /**
+ * Generates 32 bit murmur2 hash from byte array
+ * @param data byte array to hash
+ * @return 32 bit hash of the given array
+ */
+ @VisibleForTesting
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305554900
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,109 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtil;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtil.murmur2(StringUtil.encodeUtf8(rawTableName)) &
0x7fff);
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305083234
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,163 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.model.ExternalView;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+
+ private Map _partitionIndexCache;
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305043418
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtils;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtils.murmur2(StringUtil.encodeUtf8(rawTableName)) &
0x7fff);
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
+return getHashCodeForTable(rawTableName) % numPartitions;
+ }
+
+ /**
+ * Gets lead controller for table from lead controller resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return leader of partition, null if not found or resource is disabled.
+ */
+ public static String getLeadControllerForTable(ExternalView
leadControllerResourceExternalView, String rawTableName) {
+if (leadControllerResourceExternalView == null) {
+ return null;
+}
+Set partitionSet =
leadControllerResourceExternalView.getPartitionSet();
+if (partitionSet == null || partitionSet.isEmpty()) {
+ return null;
+}
+int numPartitions = partitionSet.size();
+int partitionIndex = getPartitionIdForTable(rawTableName, numPartitions);
+String partitionName = formPartitionName(partitionIndex);
+Map partitionStateMap =
leadControllerResourceExternalView.getStateMap(partitionName);
+
+// Get master host from partition map. Return null if no master found.
+for (Map.Entry entry : partitionStateMap.entrySet()) {
+ if (MasterSlaveSMD.States.MASTER.name().equals(entry.getValue())) {
+return entry.getKey();
+ }
+}
+return null;
+ }
+
+ public static String generateControllerParticipantId(String controllerHost,
String controllerPort) {
+return
org.apache.pinot.common.utils.CommonConstants.Helix.PREFIX_OF_CONTROLLER_INSTANCE
+ controllerHost + "_"
++ controllerPort;
+ }
+
+ public static String formPartitionName(int partitionIndex) {
+return
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME
+ "_" + partitionIndex;
+ }
+
+ public static int extractPartitionIndex(String partitionName) {
+return
Integer.parseInt(partitionName.substring(partitionName.lastIndexOf("_") + 1));
+ }
+
+ public static String extractLeadControllerHostNameAndPort(String
controllerParticipantId) {
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
--
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305043353
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,163 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.model.ExternalView;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+
+ private Map _partitionIndexCache;
+ private PinotHelixResourceManager _pinotHelixResourceManager;
+ private volatile boolean _amIHelixLeader = false;
+
+ public LeadControllerManager() {
+_partitionIndexCache = new ConcurrentHashMap<>();
+ }
+
+ public void registerResourceManager(PinotHelixResourceManager
pinotHelixResourceManager) {
+_pinotHelixResourceManager = pinotHelixResourceManager;
+ }
+
+ /**
+ * Check whether the current controller is the leader for the given table.
Return true if current controller is the leader for this table.
+ * Otherwise check whether the current controller is helix leader.
+ * @param tableName table name with/without table type.
+ */
+ public boolean isLeaderForTable(String tableName) {
+String rawTableName = TableNameBuilder.extractRawTableName(tableName);
+int partitionIndex =
+LeadControllerUtils.getPartitionIdForTable(rawTableName,
NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE);
+if (_partitionIndexCache.containsKey(partitionIndex)) {
+ return true;
+} else {
+ return isHelixLeader();
+}
+ }
+
+ /**
+ * Get lead controller id and partition index for given table. Returns -1 as
partition index and null as lead controller id if not found.
+ * @param tableName table name with/without table type.
+ * @return lead controller id and partition index
+ */
+ public LeadControllerResponse getLeadControllerAndPartitionIdForTable(String
tableName) {
Review comment:
Removed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305043502
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/HashUtils.java
##
@@ -0,0 +1,75 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import com.google.common.annotations.VisibleForTesting;
+
+
+public class HashUtils {
+
+ /**
+ * NOTE: This code has been copied over from
org.apache.kafka.common.utils.Utils::murmur2
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305043464
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtils;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtils.murmur2(StringUtil.encodeUtf8(rawTableName)) &
0x7fff);
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
+return getHashCodeForTable(rawTableName) % numPartitions;
+ }
+
+ /**
+ * Gets lead controller for table from lead controller resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return leader of partition, null if not found or resource is disabled.
+ */
+ public static String getLeadControllerForTable(ExternalView
leadControllerResourceExternalView, String rawTableName) {
+if (leadControllerResourceExternalView == null) {
+ return null;
+}
+Set partitionSet =
leadControllerResourceExternalView.getPartitionSet();
+if (partitionSet == null || partitionSet.isEmpty()) {
+ return null;
+}
+int numPartitions = partitionSet.size();
+int partitionIndex = getPartitionIdForTable(rawTableName, numPartitions);
+String partitionName = formPartitionName(partitionIndex);
+Map partitionStateMap =
leadControllerResourceExternalView.getStateMap(partitionName);
+
+// Get master host from partition map. Return null if no master found.
+for (Map.Entry entry : partitionStateMap.entrySet()) {
+ if (MasterSlaveSMD.States.MASTER.name().equals(entry.getValue())) {
+return entry.getKey();
+ }
+}
+return null;
+ }
+
+ public static String generateControllerParticipantId(String controllerHost,
String controllerPort) {
+return
org.apache.pinot.common.utils.CommonConstants.Helix.PREFIX_OF_CONTROLLER_INSTANCE
+ controllerHost + "_"
++ controllerPort;
+ }
+
+ public static String formPartitionName(int partitionIndex) {
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305043145
##
File path:
pinot-core/src/main/java/org/apache/pinot/server/realtime/ControllerLeaderLocator.java
##
@@ -78,35 +82,81 @@ public static ControllerLeaderLocator getInstance() {
/**
* Locate the controller leader so that we can send LLC segment completion
requests to it.
* Checks the {@link
ControllerLeaderLocator::_cachedControllerLeaderInvalid} flag and fetches the
leader from helix if cached value is invalid
- *
+ * @param rawTableName table name without type.
* @return The host:port string of the current controller leader.
*/
- public synchronized Pair getControllerLeader() {
+ public synchronized Pair getControllerLeader(String
rawTableName) {
if (!_cachedControllerLeaderInvalid) {
return _controllerLeaderHostPort;
}
+String leaderForTable = getLeaderForTable(rawTableName);
+if (leaderForTable == null) {
+ LOGGER.warn("Failed to find a leader for Table: {}", rawTableName);
+ _cachedControllerLeaderInvalid = true;
+ return null;
+} else {
+ _controllerLeaderHostPort =
generateControllerLeaderHostPortPair(leaderForTable);
+ _cachedControllerLeaderInvalid = false;
+ LOGGER.info("Setting controller leader to be {}:{}",
_controllerLeaderHostPort.getFirst(),
+ _controllerLeaderHostPort.getSecond());
+ return _controllerLeaderHostPort;
+}
+ }
+
+ /**
+ * If partition leader exists, use this as the leader for realtime segment
completion.
+ * Otherwise, try to use Helix leader.
+ * @param rawTableName table name without type
+ * @return the controller leader id with hostname and port for this table,
e.g. localhost_9000
+ */
+ private String getLeaderForTable(String rawTableName) {
+String leaderForTable;
+ExternalView leadControllerResourceExternalView =
+
_helixManager.getClusterManagmentTool().getResourceExternalView(_clusterName,
LEAD_CONTROLLER_RESOURCE_NAME);
+String partitionLeader =
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305043106
##
File path:
pinot-core/src/main/java/org/apache/pinot/server/realtime/ControllerLeaderLocator.java
##
@@ -78,35 +82,81 @@ public static ControllerLeaderLocator getInstance() {
/**
* Locate the controller leader so that we can send LLC segment completion
requests to it.
* Checks the {@link
ControllerLeaderLocator::_cachedControllerLeaderInvalid} flag and fetches the
leader from helix if cached value is invalid
- *
+ * @param rawTableName table name without type.
* @return The host:port string of the current controller leader.
*/
- public synchronized Pair getControllerLeader() {
+ public synchronized Pair getControllerLeader(String
rawTableName) {
if (!_cachedControllerLeaderInvalid) {
return _controllerLeaderHostPort;
}
+String leaderForTable = getLeaderForTable(rawTableName);
+if (leaderForTable == null) {
+ LOGGER.warn("Failed to find a leader for Table: {}", rawTableName);
+ _cachedControllerLeaderInvalid = true;
+ return null;
+} else {
+ _controllerLeaderHostPort =
generateControllerLeaderHostPortPair(leaderForTable);
+ _cachedControllerLeaderInvalid = false;
+ LOGGER.info("Setting controller leader to be {}:{}",
_controllerLeaderHostPort.getFirst(),
+ _controllerLeaderHostPort.getSecond());
+ return _controllerLeaderHostPort;
+}
+ }
+
+ /**
+ * If partition leader exists, use this as the leader for realtime segment
completion.
+ * Otherwise, try to use Helix leader.
+ * @param rawTableName table name without type
+ * @return the controller leader id with hostname and port for this table,
e.g. localhost_9000
+ */
+ private String getLeaderForTable(String rawTableName) {
+String leaderForTable;
+ExternalView leadControllerResourceExternalView =
+
_helixManager.getClusterManagmentTool().getResourceExternalView(_clusterName,
LEAD_CONTROLLER_RESOURCE_NAME);
+String partitionLeader =
+
LeadControllerUtils.getLeadControllerForTable(leadControllerResourceExternalView,
rawTableName);
+if (partitionLeader != null) {
+ // Converts participant id (with Prefix "Controller_") to controller id
and assigns it as the leader.
+ leaderForTable =
LeadControllerUtils.extractLeadControllerHostNameAndPort(partitionLeader);
+} else {
+ // Gets Helix leader to be the leader to this table, otherwise returns
null.
+ leaderForTable = getHelixClusterLeader();
+}
+return leaderForTable;
+ }
+
+ /**
+ * Gets Helix leader in the cluster. Null if there is no leader.
+ * @return Helix leader.
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305035262
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtils;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtils.murmur2(StringUtil.encodeUtf8(rawTableName)) &
0x7fff);
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
+return getHashCodeForTable(rawTableName) % numPartitions;
+ }
+
+ /**
+ * Gets lead controller for table from lead controller resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return leader of partition, null if not found or resource is disabled.
Review comment:
Yes, it's controller participant id like `Controller_localhost_9000`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305035262
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
##
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.helix;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.utils.HashUtils;
+import org.apache.pinot.common.utils.StringUtil;
+
+
+public class LeadControllerUtils {
+
+ /**
+ * Gets hash code for table using murmur2 function, ignores the most
significant bit.
+ * Note: This method CANNOT be changed when lead controller resource is
enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code ignoring the most significant bit.
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return (HashUtils.murmur2(StringUtil.encodeUtf8(rawTableName)) &
0x7fff);
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
+return getHashCodeForTable(rawTableName) % numPartitions;
+ }
+
+ /**
+ * Gets lead controller for table from lead controller resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return leader of partition, null if not found or resource is disabled.
Review comment:
Yes, it's controller partition id like `Controller_localhost_9000`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305032488
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,163 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.model.ExternalView;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+
+ private Map _partitionIndexCache;
+ private PinotHelixResourceManager _pinotHelixResourceManager;
Review comment:
The purpose of bringing `PinotHelxResourceManager` here is to fetch the EV
of leadControllerResource, where all the requirements like `KeyBuilder` and
`DataAccessor` live.
And `PinotHelxResourceManager` needs `LeadControllerManager` to register the
cache for MasterSlaveStateModelFactory before connecting to Cluster as a
participant.
So it's hard to break that. That's why I added a method
`registerResourceManager()` in `LeadControllerManager`. One thing we can do is
add an `unregisterResourceManager()` in stop method.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305032488
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,163 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.model.ExternalView;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+
+ private Map _partitionIndexCache;
+ private PinotHelixResourceManager _pinotHelixResourceManager;
Review comment:
The purpose of bringing `PinotHelxResourceManager` here is to fetch the EV
of leadControllerResource, where all the requirements like `KeyBuilder` and
`DataAccessor` live.
And `PinotHelxResourceManager` needs `LeadControllerManager` to register the
cache for MasterSlaveStateModelFactory when connecting to Cluster as a
participant.
So it's hard to break that. That's why I added a method
`registerResourceManager()` in `LeadControllerManager`. One thing we can do is
add an `unregisterResourceManager()` in stop method.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305032488
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,163 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.model.ExternalView;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+
+ private Map _partitionIndexCache;
+ private PinotHelixResourceManager _pinotHelixResourceManager;
Review comment:
The purpose of bringing `PinotHelxResourceManager` here is to fetch the EV
of leadControllerResource, where all the requirements like `KeyBuilder` and
`DataAccessor` live.
And `PinotHelxResourceManager` needs `LeadControllerManager` to register the
cache for MasterSlaveStateModelFactory.
So it's hard to break that. That's why I added a method
`registerResourceManager()` in `LeadControllerManager`. One thing we can do is
add an `unregisterResourceManager()` in stop method.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r305026100
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,163 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.helix.model.ExternalView;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+
+ private Map _partitionIndexCache;
+ private PinotHelixResourceManager _pinotHelixResourceManager;
+ private volatile boolean _amIHelixLeader = false;
+
+ public LeadControllerManager() {
+_partitionIndexCache = new ConcurrentHashMap<>();
+ }
+
+ public void registerResourceManager(PinotHelixResourceManager
pinotHelixResourceManager) {
+_pinotHelixResourceManager = pinotHelixResourceManager;
+ }
+
+ /**
+ * Check whether the current controller is the leader for the given table.
Return true if current controller is the leader for this table.
+ * Otherwise check whether the current controller is helix leader.
+ * @param tableName table name with/without table type.
+ */
+ public boolean isLeaderForTable(String tableName) {
+String rawTableName = TableNameBuilder.extractRawTableName(tableName);
+int partitionIndex =
+LeadControllerUtils.getPartitionIdForTable(rawTableName,
NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE);
+if (_partitionIndexCache.containsKey(partitionIndex)) {
Review comment:
If that's the case, the current controller returns `false` since it isn't
the leader for the given table, which is mentioned in the javadoc of this
method.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304684480
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##
@@ -139,6 +139,9 @@ public ControllerStarter(ControllerConf conf) {
} else {
_adminApp =
new
ControllerAdminApiApplication(_config.getQueryConsoleWebappPath(),
_config.getQueryConsoleUseHttps());
+ // Helix instance type should explicitly be set to PARTICIPANT ONLY in
{@link ControllerStarter}.
+ // Other places like {@link PerfBenchmarkDriver} which directly call
{@link PinotHelixResourceManager} should NOT register as PARTICIPANT, which
would be put to lead controller resource and mess up the leadership assignment.
Those places should use ADMINISTRATOR other than PARTICIPANT.
+ _config.setHelixInstanceType(InstanceType.PARTICIPANT);
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304684386
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java
##
@@ -500,4 +503,55 @@ public IdealState apply(IdealState idealState) {
public static Set getBrokerInstancesForTenant(List
instanceConfigs, String tenant) {
return new HashSet<>(HelixHelper.getInstancesWithTag(instanceConfigs,
TagNameUtils.getBrokerTagForTenant(tenant)));
}
+
+ /**
+ * Gets hash code for table.
+ * Note: This method CANNOT be changed when lead controller resource has
been enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return rawTableName.hashCode();
Review comment:
Murmur2 is in pinot-core module whereas this class is in pinot-common. It'll
require us to refactor the code though.
I think using `(rawTableName.hashCode() & Integer.MAX_VALUE)` would be
enough.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304684440
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java
##
@@ -133,6 +134,7 @@ private static long getRandomInitialDelayInSeconds() {
// If it's set to false, existing HLC realtime tables will stop consumption,
and creation of new HLC tables will be disallowed.
// Please make sure there is no HLC table running in the cluster before
disallowing it.
private static final String ALLOW_HLC_TABLES = "controller.allow.hlc.tables";
+ private static final String HELIX_INSTANCE_TYPE =
"controller.helix.instance.type";
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304680780
##
File path:
pinot-core/src/main/java/org/apache/pinot/server/realtime/ControllerLeaderLocator.java
##
@@ -78,35 +82,76 @@ public static ControllerLeaderLocator getInstance() {
/**
* Locate the controller leader so that we can send LLC segment completion
requests to it.
* Checks the {@link
ControllerLeaderLocator::_cachedControllerLeaderInvalid} flag and fetches the
leader from helix if cached value is invalid
- *
+ * @param rawTableName table name without type.
* @return The host:port string of the current controller leader.
*/
- public synchronized Pair getControllerLeader() {
+ public synchronized Pair getControllerLeader(String
rawTableName) {
if (!_cachedControllerLeaderInvalid) {
return _controllerLeaderHostPort;
}
+String leaderForTable = getLeaderForTable(rawTableName);
+if (leaderForTable == null) {
+ LOGGER.warn("Failed to find a leader for Table: {}", rawTableName);
+ _cachedControllerLeaderInvalid = true;
+ return null;
+} else {
+ _controllerLeaderHostPort =
generateControllerLeaderHostPortPair(leaderForTable);
+ _cachedControllerLeaderInvalid = false;
+ LOGGER.info("Setting controller leader to be {}:{}",
_controllerLeaderHostPort.getFirst(),
+ _controllerLeaderHostPort.getSecond());
+ return _controllerLeaderHostPort;
+}
+ }
+
+ /**
+ * If partition leader exists, use this as the leader for realtime segment
completion.
+ * Otherwise, try to use Helix leader.
+ * @param rawTableName table name without type
+ * @return the leader for this table.
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304680791
##
File path:
pinot-core/src/main/java/org/apache/pinot/server/realtime/ControllerLeaderLocator.java
##
@@ -78,35 +82,76 @@ public static ControllerLeaderLocator getInstance() {
/**
* Locate the controller leader so that we can send LLC segment completion
requests to it.
* Checks the {@link
ControllerLeaderLocator::_cachedControllerLeaderInvalid} flag and fetches the
leader from helix if cached value is invalid
- *
+ * @param rawTableName table name without type.
* @return The host:port string of the current controller leader.
*/
- public synchronized Pair getControllerLeader() {
+ public synchronized Pair getControllerLeader(String
rawTableName) {
if (!_cachedControllerLeaderInvalid) {
return _controllerLeaderHostPort;
}
+String leaderForTable = getLeaderForTable(rawTableName);
+if (leaderForTable == null) {
+ LOGGER.warn("Failed to find a leader for Table: {}", rawTableName);
+ _cachedControllerLeaderInvalid = true;
+ return null;
+} else {
+ _controllerLeaderHostPort =
generateControllerLeaderHostPortPair(leaderForTable);
+ _cachedControllerLeaderInvalid = false;
+ LOGGER.info("Setting controller leader to be {}:{}",
_controllerLeaderHostPort.getFirst(),
+ _controllerLeaderHostPort.getSecond());
+ return _controllerLeaderHostPort;
+}
+ }
+
+ /**
+ * If partition leader exists, use this as the leader for realtime segment
completion.
+ * Otherwise, try to use Helix leader.
+ * @param rawTableName table name without type
+ * @return the leader for this table.
+ */
+ private String getLeaderForTable(String rawTableName) {
+String leaderForTable;
+ExternalView leadControllerResourceExternalView =
+
_helixManager.getClusterManagmentTool().getResourceExternalView(_clusterName,
LEAD_CONTROLLER_RESOURCE_NAME);
+String partitionLeader =
HelixHelper.getLeadControllerForTable(leadControllerResourceExternalView,
rawTableName);
+if (partitionLeader != null) {
+ // Converts participant id (with Prefix "Controller_") to controller id
and assigns it as the leader.
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304680753 ## File path: pinot-core/src/test/java/org/apache/pinot/server/realtime/ControllerLeaderLocatorTest.java ## @@ -19,6 +19,7 @@ package org.apache.pinot.server.realtime; import org.apache.helix.BaseDataAccessor; +import org.apache.helix.HelixAdmin; Review comment: We have such tests in `PinotControllerModeTest` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304672238
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/statemodel/LeadControllerChecker.java
##
@@ -0,0 +1,57 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.helix.core.statemodel;
+
+import com.google.common.base.Preconditions;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE;
+
+
+public class LeadControllerChecker {
Review comment:
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304671325
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##
@@ -255,30 +246,29 @@ private void setUpPinotController() {
_helixResourceManager.start();
HelixManager helixParticipantManager =
_helixResourceManager.getHelixZkManager();
-LOGGER.info("Registering controller leadership manager");
-// TODO: when Helix separation is completed, leadership only depends on
the master in leadControllerResource, remove
-// ControllerLeadershipManager and this callback.
+// Get lead controller manager from resource manager.
+_leadControllerManager = _helixResourceManager.getLeadControllerManager();
+
+LOGGER.info("Registering helix controller listener");
helixParticipantManager.addControllerListener(
-(ControllerChangeListener) changeContext ->
_controllerLeadershipManager.onControllerChange());
+(ControllerChangeListener) changeContext ->
_leadControllerManager.onHelixControllerChange());
Review comment:
Added.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304670816
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java
##
@@ -500,4 +503,55 @@ public IdealState apply(IdealState idealState) {
public static Set getBrokerInstancesForTenant(List
instanceConfigs, String tenant) {
return new HashSet<>(HelixHelper.getInstancesWithTag(instanceConfigs,
TagNameUtils.getBrokerTagForTenant(tenant)));
}
+
+ /**
+ * Gets hash code for table.
+ * Note: This method CANNOT be changed when lead controller resource has
been enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return rawTableName.hashCode();
+ }
+
+ /**
+ * Given a raw table name and number of partitions, returns the partition id
in lead controller resource.
+ * @param rawTableName raw table name
+ * @param numPartitions number of partitions
+ * @return partition id in lead controller resource.
+ */
+ public static int getPartitionIdForTable(String rawTableName, int
numPartitions) {
+return getHashCodeForTable(rawTableName) % numPartitions;
+ }
+
+ /**
+ * Gets lead controller for table from lead controller resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return leader of partition, null if not found or resource is disabled.
+ */
+ public static String getLeadControllerForTable(ExternalView
leadControllerResourceExternalView, String rawTableName) {
+if (leadControllerResourceExternalView == null) {
+ return null;
+}
+Set partitionSet =
leadControllerResourceExternalView.getPartitionSet();
+if (partitionSet == null || partitionSet.isEmpty()) {
+ return null;
+}
+int numPartitions = partitionSet.size();
+int partitionIndex = getPartitionIdForTable(rawTableName, numPartitions);
+String partitionName = LEAD_CONTROLLER_RESOURCE_NAME + "_" +
partitionIndex;
+Map partitionStateMap =
leadControllerResourceExternalView.getStateMap(partitionName);
+
+// Get master host from partition map. Return null if no master found.
+for (Map.Entry entry : partitionStateMap.entrySet()) {
+ if (MasterSlaveSMD.States.MASTER.name().equals(entry.getValue())) {
+return entry.getKey();
+ }
+}
+LOGGER.info("No host in MASTER state in Partition: {} for Table: {}",
partitionIndex, rawTableName);
Review comment:
Removed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304570461
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/statemodel/LeadControllerResourceMasterSlaveStateModelFactory.java
##
@@ -0,0 +1,65 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.helix.core.statemodel;
+
+import org.apache.helix.NotificationContext;
+import org.apache.helix.examples.MasterSlaveStateModelFactory;
+import org.apache.helix.model.Message;
+import org.apache.helix.participant.statemachine.StateModel;
+import org.apache.pinot.controller.LeadControllerManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+public class LeadControllerResourceMasterSlaveStateModelFactory extends
MasterSlaveStateModelFactory {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerResourceMasterSlaveStateModelFactory.class);
+
+ private final LeadControllerManager _leadControllerManager;
+
+ public
LeadControllerResourceMasterSlaveStateModelFactory(LeadControllerManager
leadControllerManager) {
+super();
+_leadControllerManager = leadControllerManager;
+ }
+
+ @Override
+ public StateModel createNewStateModel(String resourceName, String
partitionName) {
+MasterSlaveStateModel stateModel = new
LeadControllerResourceMasterSlaveStateModel();
+stateModel.setPartitionName(partitionName);
+return stateModel;
+ }
+
+ public class LeadControllerResourceMasterSlaveStateModel extends
MasterSlaveStateModel {
+
+@Override
+public void onBecomeMasterFromSlave(Message message, NotificationContext
context) {
Review comment:
Not necessary. All the cached partition number will be dropped after the
shutdown.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304560398
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##
@@ -288,8 +278,10 @@ private void setUpPinotController() {
// Setting up periodic tasks
List controllerPeriodicTasks =
setupControllerPeriodicTasks();
LOGGER.info("Init controller periodic tasks scheduler");
-_controllerPeriodicTaskScheduler = new ControllerPeriodicTaskScheduler();
-_controllerPeriodicTaskScheduler.init(controllerPeriodicTasks,
_controllerLeadershipManager);
+_periodicTaskScheduler = new PeriodicTaskScheduler();
Review comment:
We don't need all the methods in `ControllerPeriodicTaskScheduler`. Thus, we
can just remove this class.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304563279
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##
@@ -478,9 +475,10 @@ private void stopHelixController() {
private void stopPinotController() {
try {
- // Stopping ControllerLeadershipManager has to be done before stopping
HelixResourceManager.
- LOGGER.info("Stopping controller leadership manager");
- _controllerLeadershipManager.stop();
+ // Stopping periodic tasks has to be done before stopping
HelixResourceManager.
+ // Stop controller periodic task.
+ LOGGER.info("Stopping controller periodic tasks");
Review comment:
No, stopping controller will mark all the periodic tasks to stop. Each
periodic task will firstly check whether to stop/run, and then check whether to
run the task for a given table.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304560398
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##
@@ -288,8 +278,10 @@ private void setUpPinotController() {
// Setting up periodic tasks
List controllerPeriodicTasks =
setupControllerPeriodicTasks();
LOGGER.info("Init controller periodic tasks scheduler");
-_controllerPeriodicTaskScheduler = new ControllerPeriodicTaskScheduler();
-_controllerPeriodicTaskScheduler.init(controllerPeriodicTasks,
_controllerLeadershipManager);
+_periodicTaskScheduler = new PeriodicTaskScheduler();
Review comment:
We don't need all the method in `ControllerPeriodicTaskScheduler`. Thus, we
can just remove this class.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304182545
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
##
@@ -0,0 +1,157 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller;
+
+import java.util.Set;
+import org.apache.helix.model.ExternalView;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.utils.helix.HelixHelper;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.apache.pinot.controller.helix.core.statemodel.LeadControllerChecker;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE;
+
+
+public class LeadControllerManager {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(LeadControllerManager.class);
+
+ private final LeadControllerChecker _leadControllerChecker;
+ private PinotHelixResourceManager _pinotHelixResourceManager;
+ private boolean _amIHelixLeader = false;
+
+ public LeadControllerManager() {
+_leadControllerChecker = new LeadControllerChecker();
+ }
+
+ public void registerResourceManager(PinotHelixResourceManager
pinotHelixResourceManager) {
+_pinotHelixResourceManager = pinotHelixResourceManager;
+ }
+
+ /**
+ * Check whether the current controller is the leader for the given table.
Return true if current controller is the leader for this table.
+ * Otherwise check whether the current controller is helix leader.
+ * @param tableName table name with/without table type.
+ */
+ public boolean isLeaderForTable(String tableName) {
+String rawTableName = TableNameBuilder.extractRawTableName(tableName);
+int partitionIndex =
+HelixHelper.getPartitionIdForTable(rawTableName,
NUMBER_OF_PARTITIONS_IN_LEAD_CONTROLLER_RESOURCE);
+if (_leadControllerChecker.isPartitionLeader(partitionIndex)) {
+ return true;
+} else {
+ return isHelixLeader();
+}
+ }
+
+ /**
+ * Get lead controller id and partition index for given table. Returns -1 as
partition index and null as lead controller id if not found.
+ * @param tableName table name with/without table type.
+ * @return lead controller id and partition index
+ */
+ public LeadControllerResponse getLeadControllerAndPartitionIdForTable(String
tableName) {
+String rawTableName = TableNameBuilder.extractRawTableName(tableName);
+ExternalView leadControllerResourceExternalView =
_pinotHelixResourceManager.getHelixAdmin()
+
.getResourceExternalView(_pinotHelixResourceManager.getHelixClusterName(),
LEAD_CONTROLLER_RESOURCE_NAME);
+if (leadControllerResourceExternalView == null) {
+ return new LeadControllerResponse();
+}
+Set partitionSet =
leadControllerResourceExternalView.getPartitionSet();
+if (partitionSet == null || partitionSet.isEmpty()) {
+ return new LeadControllerResponse();
+}
+
+int numPartitions = partitionSet.size();
+int partitionIndex = HelixHelper.getPartitionIdForTable(rawTableName,
numPartitions);
+String leadControllerId =
HelixHelper.getLeadControllerForTable(leadControllerResourceExternalView,
rawTableName);
+return new LeadControllerResponse(partitionIndex, leadControllerId);
+ }
+
+ public synchronized void addPartitionLeader(String partitionName) {
+_leadControllerChecker.addPartitionLeader(partitionName);
+ }
+
+ public synchronized void removePartitionLeader(String partitionName) {
+_leadControllerChecker.removePartitionLeader(partitionName);
+ }
+
+ /**
+ * Checks if the current controller host is Helix leader.
+ */
+ private boolean isHelixLeader() {
+return _amIHelixLeader;
+ }
+
+ /**
+ * Response class to return lead controller id and partition index.
+ */
+ public class LeadControllerResponse {
+private int _partitionIndex;
+
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r304181910
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java
##
@@ -500,4 +503,55 @@ public IdealState apply(IdealState idealState) {
public static Set getBrokerInstancesForTenant(List
instanceConfigs, String tenant) {
return new HashSet<>(HelixHelper.getInstancesWithTag(instanceConfigs,
TagNameUtils.getBrokerTagForTenant(tenant)));
}
+
+ /**
+ * Gets hash code for table.
+ * Note: This method CANNOT be changed when lead controller resource has
been enabled.
+ * Otherwise it will assign different controller for the same table, which
will mess up the controller periodic tasks and realtime segment completion.
+ * @param rawTableName table name
+ * @return hash code
+ */
+ private static int getHashCodeForTable(String rawTableName) {
+return rawTableName.hashCode();
Review comment:
Just did benchmark different partition functions and put the result in the
description of this PR.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r296102994
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java
##
@@ -500,4 +503,42 @@ public IdealState apply(IdealState idealState) {
public static Set getBrokerInstancesForTenant(List
instanceConfigs, String tenant) {
return new HashSet<>(HelixHelper.getInstancesWithTag(instanceConfigs,
TagNameUtils.getBrokerTagForTenant(tenant)));
}
+
+ /**
+ * Gets hash code for table.
+ * @param rawTableName table name
+ * @return hash code
+ */
+ public static int getHashCodeForTable(String rawTableName) {
Review comment:
Actually this hash function can be changed **only when** lead controller
resource is disabled. It cannot be changed when the resource is enabled. I'll
add it to the comment.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r296101383
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/HelixHelper.java
##
@@ -500,4 +503,42 @@ public IdealState apply(IdealState idealState) {
public static Set getBrokerInstancesForTenant(List
instanceConfigs, String tenant) {
return new HashSet<>(HelixHelper.getInstancesWithTag(instanceConfigs,
TagNameUtils.getBrokerTagForTenant(tenant)));
}
+
+ /**
+ * Gets hash code for table.
+ * @param rawTableName table name
+ * @return hash code
+ */
+ public static int getHashCodeForTable(String rawTableName) {
+return rawTableName.hashCode();
+ }
+
+ /**
+ * Gets lead controller for table from lead controller resource.
+ * If the resource is disabled or no controller registered as participant,
there is no instance in "MASTER" state.
+ * @param leadControllerResourceExternalView external view of lead
controller resource
+ * @param rawTableName table name without type
+ * @return leader of partition, null if not found.
Review comment:
There's no need to throw exception. There'll be no difference between
throwing an exception and logging a message saying partition leader is not
found.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli commented on a change in pull request #4323: [Controller Separation]
Add logic for lead controller resource
URL: https://github.com/apache/incubator-pinot/pull/4323#discussion_r296101035
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerLeadershipManager.java
##
@@ -1,117 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.pinot.controller;
-
-import java.util.HashMap;
-import java.util.Map;
-import javax.annotation.concurrent.ThreadSafe;
-import org.apache.helix.HelixManager;
-import org.apache.pinot.common.metrics.ControllerGauge;
-import org.apache.pinot.common.metrics.ControllerMetrics;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-
-/**
- * Single place for listening on controller changes.
- * This should be created at controller startup and everyone who wants to
listen to controller changes should subscribe.
- */
-@ThreadSafe
-public class ControllerLeadershipManager {
Review comment:
The behavior of running periodic tasks has changed. Previous it depends on
the notification from controller listener. When it's Helix leader, we start
periodic task scheduler; when it isn't, we stop periodic task scheduler.
The code change here will always run the scheduler. Inside the for loop we
call `_leadControllerManager.isLeaderForTable(tableNameWithType)` to determine
whether we should use Partition leader or Helix leader.
This class can still be useful for caching helix leader (which reduces ZK
access) and emitting metric. I'll bring this class back and remove the logic
with subscribers in the next push.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
