You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2022/02/02 22:03:59 UTC
[pinot] branch master updated: add pinot.minion prefix on minion configs for consistency (#8109)
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new f4a8d9d add pinot.minion prefix on minion configs for consistency (#8109)
f4a8d9d is described below
commit f4a8d9d592add5399e8c5c86926dff3043f35696
Author: Xiaobing <61...@users.noreply.github.com>
AuthorDate: Wed Feb 2 14:03:33 2022 -0800
add pinot.minion prefix on minion configs for consistency (#8109)
---
.../org/apache/pinot/minion/BaseMinionStarter.java | 23 +++++--
.../java/org/apache/pinot/minion/MinionConf.java | 7 +++
.../org/apache/pinot/minion/MinionConfTest.java | 72 ++++++++++++++++++++++
.../pinot-configuration-new-minion.properties | 24 ++++++++
.../pinot-configuration-old-minion.properties | 24 ++++++++
.../apache/pinot/spi/env/PinotConfiguration.java | 4 ++
.../apache/pinot/spi/utils/CommonConstants.java | 23 +++++--
7 files changed, 167 insertions(+), 10 deletions(-)
diff --git a/pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java b/pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java
index 86e74f2..4cbf37c 100644
--- a/pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java
+++ b/pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java
@@ -174,9 +174,7 @@ public abstract class BaseMinionStarter implements ServiceStartable {
// TODO: put all the metrics related configs down to "pinot.server.metrics"
PinotMetricsRegistry metricsRegistry = PinotMetricUtils.getPinotMetricsRegistry(_config.getMetricsConfig());
- MinionMetrics minionMetrics = new MinionMetrics(_config
- .getProperty(CommonConstants.Minion.CONFIG_OF_METRICS_PREFIX_KEY,
- CommonConstants.Minion.CONFIG_OF_METRICS_PREFIX), metricsRegistry);
+ MinionMetrics minionMetrics = new MinionMetrics(_config.getMetricsPrefix(), metricsRegistry);
minionMetrics.initializeGlobalMeters();
minionContext.setMinionMetrics(minionMetrics);
@@ -194,6 +192,9 @@ public abstract class BaseMinionStarter implements ServiceStartable {
// Start all components
LOGGER.info("Initializing PinotFSFactory");
PinotConfiguration pinotFSConfig = _config.subset(CommonConstants.Minion.PREFIX_OF_CONFIG_OF_PINOT_FS_FACTORY);
+ if (pinotFSConfig.isEmpty()) {
+ pinotFSConfig = _config.subset(CommonConstants.Minion.DEPRECATED_PREFIX_OF_CONFIG_OF_PINOT_FS_FACTORY);
+ }
PinotFSFactory.init(pinotFSConfig);
LOGGER.info("Initializing QueryRewriterFactory");
@@ -202,16 +203,28 @@ public abstract class BaseMinionStarter implements ServiceStartable {
LOGGER.info("Initializing segment fetchers for all protocols");
PinotConfiguration segmentFetcherFactoryConfig =
_config.subset(CommonConstants.Minion.PREFIX_OF_CONFIG_OF_SEGMENT_FETCHER_FACTORY);
+ if (segmentFetcherFactoryConfig.isEmpty()) {
+ segmentFetcherFactoryConfig =
+ _config.subset(CommonConstants.Minion.DEPRECATED_PREFIX_OF_CONFIG_OF_SEGMENT_FETCHER_FACTORY);
+ }
SegmentFetcherFactory.init(segmentFetcherFactoryConfig);
LOGGER.info("Initializing pinot crypter");
PinotConfiguration pinotCrypterConfig = _config.subset(CommonConstants.Minion.PREFIX_OF_CONFIG_OF_PINOT_CRYPTER);
+ if (pinotCrypterConfig.isEmpty()) {
+ pinotCrypterConfig = _config.subset(CommonConstants.Minion.DEPRECATED_PREFIX_OF_CONFIG_OF_PINOT_CRYPTER);
+ }
PinotCrypterFactory.init(pinotCrypterConfig);
// Need to do this before we start receiving state transitions.
LOGGER.info("Initializing ssl context for segment uploader");
- PinotConfiguration httpsConfig = _config.subset(CommonConstants.Minion.PREFIX_OF_CONFIG_OF_SEGMENT_UPLOADER)
- .subset(CommonConstants.HTTPS_PROTOCOL);
+ PinotConfiguration segmentUploaderConfig =
+ _config.subset(CommonConstants.Minion.PREFIX_OF_CONFIG_OF_SEGMENT_UPLOADER);
+ if (segmentUploaderConfig.isEmpty()) {
+ segmentUploaderConfig =
+ _config.subset(CommonConstants.Minion.DEPRECATED_PREFIX_OF_CONFIG_OF_SEGMENT_UPLOADER);
+ }
+ PinotConfiguration httpsConfig = segmentUploaderConfig.subset(CommonConstants.HTTPS_PROTOCOL);
if (httpsConfig.getProperty(HTTPS_ENABLED, false)) {
SSLContext sslContext =
new ClientSSLContextGenerator(httpsConfig.subset(CommonConstants.PREFIX_OF_SSL_SUBSET)).generate();
diff --git a/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java b/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java
index 0d365b8..0981612 100644
--- a/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java
+++ b/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java
@@ -20,6 +20,7 @@ package org.apache.pinot.minion;
import java.util.HashMap;
import java.util.Map;
+import java.util.Optional;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.utils.CommonConstants;
import org.apache.pinot.spi.utils.NetUtils;
@@ -67,4 +68,10 @@ public class MinionConf extends PinotConfiguration {
public PinotConfiguration getMetricsConfig() {
return subset(CommonConstants.Minion.METRICS_CONFIG_PREFIX);
}
+
+ public String getMetricsPrefix() {
+ return Optional.ofNullable(getProperty(CommonConstants.Minion.CONFIG_OF_METRICS_PREFIX_KEY))
+ .orElseGet(() -> getProperty(CommonConstants.Minion.DEPRECATED_CONFIG_OF_METRICS_PREFIX_KEY,
+ CommonConstants.Minion.CONFIG_OF_METRICS_PREFIX));
+ }
}
diff --git a/pinot-minion/src/test/java/org/apache/pinot/minion/MinionConfTest.java b/pinot-minion/src/test/java/org/apache/pinot/minion/MinionConfTest.java
new file mode 100644
index 0000000..1f33840
--- /dev/null
+++ b/pinot-minion/src/test/java/org/apache/pinot/minion/MinionConfTest.java
@@ -0,0 +1,72 @@
+/**
+ * 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.minion;
+
+import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.pinot.spi.env.PinotConfiguration;
+import org.apache.pinot.spi.utils.CommonConstants;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+
+public class MinionConfTest {
+
+ @Test
+ public void testDeprecatedConfigs()
+ throws ConfigurationException {
+ // Check configs with old names that have no pinot.minion prefix.
+ String[] cfgKeys = new String[]{
+ CommonConstants.Minion.DEPRECATED_PREFIX_OF_CONFIG_OF_PINOT_FS_FACTORY,
+ CommonConstants.Minion.DEPRECATED_PREFIX_OF_CONFIG_OF_SEGMENT_FETCHER_FACTORY,
+ CommonConstants.Minion.DEPRECATED_PREFIX_OF_CONFIG_OF_SEGMENT_UPLOADER,
+ CommonConstants.Minion.DEPRECATED_PREFIX_OF_CONFIG_OF_PINOT_CRYPTER
+ };
+ PinotConfiguration rawCfg = new PinotConfiguration(new PropertiesConfiguration(
+ PropertiesConfiguration.class.getClassLoader().getResource("pinot-configuration-old-minion.properties")
+ .getFile()));
+ final MinionConf oldConfig = new MinionConf(rawCfg.toMap());
+ Assert.assertEquals(oldConfig.getMetricsPrefix(), "pinot.minion.old.custom.metrics.");
+ for (String cfgKey : cfgKeys) {
+ Assert.assertFalse(oldConfig.subset(cfgKey).isEmpty(), cfgKey);
+ Assert.assertTrue(oldConfig.subset("pinot.minion." + cfgKey).isEmpty(), cfgKey);
+ }
+
+ // Check configs with new names that have the pinot.minion prefix.
+ rawCfg = new PinotConfiguration(new PropertiesConfiguration(
+ PropertiesConfiguration.class.getClassLoader().getResource("pinot-configuration-new-minion.properties")
+ .getFile()));
+ final MinionConf newConfig = new MinionConf(rawCfg.toMap());
+ for (String cfgKey : cfgKeys) {
+ Assert.assertTrue(newConfig.subset(cfgKey).isEmpty(), cfgKey);
+ Assert.assertFalse(newConfig.subset("pinot.minion." + cfgKey).isEmpty(), cfgKey);
+ }
+ // Check the config values.
+ Assert.assertEquals(newConfig.getMetricsPrefix(), "pinot.minion.new.custom.metrics.");
+ PinotConfiguration subcfg = newConfig.subset(CommonConstants.Minion.PREFIX_OF_CONFIG_OF_PINOT_FS_FACTORY);
+ Assert.assertEquals(subcfg.subset("class").getProperty("s3"), "org.apache.pinot.plugin.filesystem.S3PinotFS");
+ subcfg = newConfig.subset(CommonConstants.Minion.PREFIX_OF_CONFIG_OF_SEGMENT_FETCHER_FACTORY);
+ Assert.assertEquals(subcfg.getProperty("protocols"), "file,http,s3");
+ subcfg = newConfig.subset(CommonConstants.Minion.PREFIX_OF_CONFIG_OF_SEGMENT_UPLOADER);
+ Assert.assertEquals(subcfg.subset("https").getProperty("enabled"), "true");
+ subcfg = newConfig.subset(CommonConstants.Minion.PREFIX_OF_CONFIG_OF_PINOT_CRYPTER);
+ Assert.assertEquals(subcfg.subset("class").getProperty("nooppinotcrypter"),
+ "org.apache.pinot.core.crypt.NoOpPinotCrypter");
+ }
+}
diff --git a/pinot-minion/src/test/resources/pinot-configuration-new-minion.properties b/pinot-minion/src/test/resources/pinot-configuration-new-minion.properties
new file mode 100644
index 0000000..67b9a18
--- /dev/null
+++ b/pinot-minion/src/test/resources/pinot-configuration-new-minion.properties
@@ -0,0 +1,24 @@
+#
+# 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.
+#
+
+pinot.minion.storage.factory.class.s3=org.apache.pinot.plugin.filesystem.S3PinotFS
+pinot.minion.segment.fetcher.protocols=file,http,s3
+pinot.minion.segment.uploader.https.enabled=true
+pinot.minion.crypter.class.nooppinotcrypter=org.apache.pinot.core.crypt.NoOpPinotCrypter
+pinot.minion.metrics.prefix=pinot.minion.new.custom.metrics.
diff --git a/pinot-minion/src/test/resources/pinot-configuration-old-minion.properties b/pinot-minion/src/test/resources/pinot-configuration-old-minion.properties
new file mode 100644
index 0000000..be0b2c7
--- /dev/null
+++ b/pinot-minion/src/test/resources/pinot-configuration-old-minion.properties
@@ -0,0 +1,24 @@
+#
+# 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.
+#
+
+storage.factory.class.s3=org.apache.pinot.plugin.filesystem.S3PinotFS
+segment.fetcher.protocols=file,http,s3
+segment.uploader.https.enabled=true
+crypter.class.nooppinotcrypter=org.apache.pinot.core.crypt.NoOpPinotCrypter
+metricsPrefix=pinot.minion.old.custom.metrics.
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java b/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java
index d2724b8..f6ec539 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java
@@ -453,4 +453,8 @@ public class PinotConfiguration {
public String toString() {
return new Obfuscator().toJsonString(this);
}
+
+ public boolean isEmpty() {
+ return _configuration.isEmpty();
+ }
}
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
index 7882f35..a85c757 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
@@ -78,6 +78,7 @@ public class CommonConstants {
public static final String LEAD_CONTROLLER_RESOURCE_ENABLED_KEY = "RESOURCE_ENABLED";
public static final String ENABLE_CASE_INSENSITIVE_KEY = "enable.case.insensitive";
+ @Deprecated
public static final String DEPRECATED_ENABLE_CASE_INSENSITIVE_KEY = "enable.case.insensitive.pql";
public static final String DEFAULT_HYPERLOGLOG_LOG2M_KEY = "default.hyperloglog.log2m";
@@ -446,7 +447,9 @@ public class CommonConstants {
public static final String METADATA_EVENT_OBSERVER_PREFIX = "metadata.event.notifier";
// Config keys
- public static final String CONFIG_OF_METRICS_PREFIX_KEY = "metricsPrefix";
+ public static final String CONFIG_OF_METRICS_PREFIX_KEY = "pinot.minion.metrics.prefix";
+ @Deprecated
+ public static final String DEPRECATED_CONFIG_OF_METRICS_PREFIX_KEY = "metricsPrefix";
public static final String METRICS_REGISTRY_REGISTRATION_LISTENERS_KEY = "metricsRegistryRegistrationListeners";
public static final String METRICS_CONFIG_PREFIX = "pinot.minion.metrics";
@@ -455,10 +458,20 @@ public class CommonConstants {
public static final String DEFAULT_INSTANCE_BASE_DIR =
System.getProperty("java.io.tmpdir") + File.separator + "PinotMinion";
public static final String DEFAULT_INSTANCE_DATA_DIR = DEFAULT_INSTANCE_BASE_DIR + File.separator + "data";
- public static final String PREFIX_OF_CONFIG_OF_PINOT_FS_FACTORY = "storage.factory";
- public static final String PREFIX_OF_CONFIG_OF_SEGMENT_FETCHER_FACTORY = "segment.fetcher";
- public static final String PREFIX_OF_CONFIG_OF_SEGMENT_UPLOADER = "segment.uploader";
- public static final String PREFIX_OF_CONFIG_OF_PINOT_CRYPTER = "crypter";
+
+ // Add pinot.minion prefix on those configs to be consistent with configs of controller and server.
+ public static final String PREFIX_OF_CONFIG_OF_PINOT_FS_FACTORY = "pinot.minion.storage.factory";
+ public static final String PREFIX_OF_CONFIG_OF_SEGMENT_FETCHER_FACTORY = "pinot.minion.segment.fetcher";
+ public static final String PREFIX_OF_CONFIG_OF_SEGMENT_UPLOADER = "pinot.minion.segment.uploader";
+ public static final String PREFIX_OF_CONFIG_OF_PINOT_CRYPTER = "pinot.minion.crypter";
+ @Deprecated
+ public static final String DEPRECATED_PREFIX_OF_CONFIG_OF_PINOT_FS_FACTORY = "storage.factory";
+ @Deprecated
+ public static final String DEPRECATED_PREFIX_OF_CONFIG_OF_SEGMENT_FETCHER_FACTORY = "segment.fetcher";
+ @Deprecated
+ public static final String DEPRECATED_PREFIX_OF_CONFIG_OF_SEGMENT_UPLOADER = "segment.uploader";
+ @Deprecated
+ public static final String DEPRECATED_PREFIX_OF_CONFIG_OF_PINOT_CRYPTER = "crypter";
/**
* Service token for accessing protected controller APIs.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org