ankitsultana commented on code in PR #16777:
URL: https://github.com/apache/pinot/pull/16777#discussion_r2353107941
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/generator/TaskGeneratorUtils.java:
##########
@@ -125,4 +129,65 @@ public static String extractMinionInstanceTag(TableConfig
tableConfig, String ta
}
return CommonConstants.Helix.UNTAGGED_MINION_INSTANCE;
}
+
+ /**
+ * Generic method to get configuration values from ZK cluster config with
fallback hierarchy:
+ * 1. Check minion tenant specific config (if minionTag provided)
+ * 2. Check task type specific config
+ * 3. Return default value
+ *
+ * @param taskType The task type to build config keys
+ * @param configKeySuffix The suffix to append to task type for config key
+ * @param minionTag The minion tag for tenant-specific config (can be null)
+ * @param defaultValue The default value to return if no config is found
+ * @param returnType The class type for parsing the config value. Currently
supported: Integer, Long, String
+ * @param clusterInfoAccessor The cluster info accessor to get config values
+ * @param <T> The return type
+ * @return The parsed configuration value or default value
+ */
+ public static <T> T getClusterMinionConfigValue(String taskType, String
configKeySuffix, String minionTag,
+ T defaultValue, Class<T> returnType, ClusterInfoAccessor
clusterInfoAccessor) {
+ // Priority 1: Check minion tenant specific cluster config
+ if (minionTag != null) {
+ String tenantSpecificConfigKey = taskType + "." + minionTag +
configKeySuffix;
+ String configValue =
clusterInfoAccessor.getClusterConfig(tenantSpecificConfigKey);
+ if (configValue != null) {
+ try {
+ return parseConfigValue(configValue, returnType);
+ } catch (Exception e) {
+ LOGGER.error("Invalid config {}: '{}'", tenantSpecificConfigKey,
configValue, e);
+ }
+ }
+ }
+
+ // Priority 2: Check task type specific cluster config
+ String taskTypeConfigKey = taskType + configKeySuffix;
+ String configValue =
clusterInfoAccessor.getClusterConfig(taskTypeConfigKey);
+ if (configValue != null) {
+ try {
+ return parseConfigValue(configValue, returnType);
+ } catch (Exception e) {
+ LOGGER.error("Invalid config {}: '{}'", taskTypeConfigKey,
configValue, e);
+ }
+ }
+
+ // Priority 3: Default value
+ return defaultValue;
+ }
+
+ /**
+ * Helper method to parse config values to the appropriate type
+ */
+ @SuppressWarnings("unchecked")
+ private static <T> T parseConfigValue(String configValue, Class<T>
returnType) {
Review Comment:
nit: this means that if one were to add a new config that takes in a
different type, it would fail at Runtime, unless there's UT coverage for it.
Just to be safe, maybe you can also cover the Double and Float types 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
