Jackie-Jiang commented on a change in pull request #6468:
URL: https://github.com/apache/incubator-pinot/pull/6468#discussion_r562119547
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/ClusterInfoAccessor.java
##########
@@ -157,4 +160,20 @@ public void setRealtimeToOfflineSegmentsTaskMetadata(
public String getVipUrl() {
return _controllerConf.generateVipUrl();
}
+
+ /**
+ * Get the cluster config for a given config name, return null if not found.
+ *
+ * @return cluster config
+ */
+ public String getClusterConfig(String configName) {
+ HelixConfigScope helixConfigScope = new
HelixConfigScopeBuilder(HelixConfigScope.ConfigScopeProperty.CLUSTER)
+ .forCluster(_pinotHelixResourceManager.getHelixClusterName()).build();
+ Map<String, String> configMap =
+ _pinotHelixResourceManager.getHelixAdmin().getConfig(helixConfigScope,
Arrays.asList(configName));
Review comment:
(nit)
```suggestion
_pinotHelixResourceManager.getHelixAdmin().getConfig(helixConfigScope,
Collections.singletonList(configName));
```
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/common/MinionConstants.java
##########
@@ -92,6 +92,8 @@ private MinionConstants() {
// Generate segment and push to controller based on batch ingestion configs
public static class SegmentGenerationAndPushTask {
public static final String TASK_TYPE = "SegmentGenerationAndPushTask";
+ public static final String CONFIG_NUMBER_CONCURRENT_TASKS_PER_INSTANCE =
+
"pinot.minion.task.generator.SegmentGenerationAndPushTask.numConcurrentTasksPerInstance";
Review comment:
Simplify this key to
`SegmentGenerationAndPushTask.numConcurrentTasksPerInstance`? The current one
looks like an instance config key
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/ClusterInfoAccessor.java
##########
@@ -157,4 +160,20 @@ public void setRealtimeToOfflineSegmentsTaskMetadata(
public String getVipUrl() {
return _controllerConf.generateVipUrl();
}
+
+ /**
+ * Get the cluster config for a given config name, return null if not found.
+ *
+ * @return cluster config
+ */
+ public String getClusterConfig(String configName) {
+ HelixConfigScope helixConfigScope = new
HelixConfigScopeBuilder(HelixConfigScope.ConfigScopeProperty.CLUSTER)
+ .forCluster(_pinotHelixResourceManager.getHelixClusterName()).build();
+ Map<String, String> configMap =
+ _pinotHelixResourceManager.getHelixAdmin().getConfig(helixConfigScope,
Arrays.asList(configName));
+ if (configMap == null || !configMap.containsKey(configName)) {
+ return null;
+ }
+ return configMap.get(configName);
Review comment:
(nit) Can be simplified to
```suggestion
return configMap != null ? configMap.get(configName) : null;
```
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
