You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by xi...@apache.org on 2024/02/27 07:18:09 UTC
(pinot) branch master updated: Add config option for timezone (#12386)
This is an automated email from the ASF dual-hosted git repository.
xiangfu 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 d9ac0efa7d Add config option for timezone (#12386)
d9ac0efa7d is described below
commit d9ac0efa7d92e7e1b5313c8c1f252e5f588a9da2
Author: Dao Thanh Tung <tt...@accountancy.smu.edu.sg>
AuthorDate: Tue Feb 27 07:18:04 2024 +0000
Add config option for timezone (#12386)
* Add config option to allow timezone setting
Signed-off-by: dttung2905 <tt...@accountancy.smu.edu.sg>
* Do not use import all
Signed-off-by: dttung2905 <tt...@accountancy.smu.edu.sg>
* Fix codestyle linting
Signed-off-by: dttung2905 <tt...@accountancy.smu.edu.sg>
* Make changes from code review
Signed-off-by: dttung2905 <tt...@accountancy.smu.edu.sg>
* Add integration test
Signed-off-by: dttung2905 <tt...@accountancy.smu.edu.sg>
* Fix ci lint and remove the method to get timezone
Signed-off-by: dttung2905 <tt...@accountancy.smu.edu.sg>
* break into multiline
Signed-off-by: dttung2905 <tt...@accountancy.smu.edu.sg>
* use local timezone to support backward compatibility
Signed-off-by: dttung2905 <tt...@accountancy.smu.edu.sg>
---------
Signed-off-by: dttung2905 <tt...@accountancy.smu.edu.sg>
---
.../apache/pinot/common/utils/ServiceStartableUtils.java | 13 +++++++++++++
.../org/apache/pinot/controller/helix/ControllerTest.java | 5 ++++-
.../org/apache/pinot/integration/tests/ClusterTest.java | 12 ++++++++++++
.../pinot/server/starter/helix/BaseServerStarter.java | 6 +++---
.../java/org/apache/pinot/spi/utils/CommonConstants.java | 1 +
5 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/utils/ServiceStartableUtils.java b/pinot-common/src/main/java/org/apache/pinot/common/utils/ServiceStartableUtils.java
index 38c9870709..45a791bc9a 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/utils/ServiceStartableUtils.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/utils/ServiceStartableUtils.java
@@ -19,6 +19,7 @@
package org.apache.pinot.common.utils;
import java.util.Map;
+import java.util.TimeZone;
import java.util.concurrent.TimeUnit;
import org.apache.helix.zookeeper.datamodel.ZNRecord;
import org.apache.helix.zookeeper.datamodel.serializer.ZNRecordSerializer;
@@ -29,6 +30,8 @@ import org.apache.pinot.spi.utils.CommonConstants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.apache.pinot.spi.utils.CommonConstants.CONFIG_OF_TIMEZONE;
+
public class ServiceStartableUtils {
private ServiceStartableUtils() {
@@ -38,6 +41,7 @@ public class ServiceStartableUtils {
private static final String CLUSTER_CONFIG_ZK_PATH_TEMPLATE = "/%s/CONFIGS/CLUSTER/%s";
private static final String PINOT_ALL_CONFIG_KEY_PREFIX = "pinot.all.";
private static final String PINOT_INSTANCE_CONFIG_KEY_PREFIX_TEMPLATE = "pinot.%s.";
+ protected static String _timeZone;
/**
* Applies the ZK cluster config to the given instance config if it does not already exist.
@@ -66,6 +70,7 @@ public class ServiceStartableUtils {
zkClient.readData(String.format(CLUSTER_CONFIG_ZK_PATH_TEMPLATE, clusterName, clusterName), true);
if (clusterConfigZNRecord == null) {
LOGGER.warn("Failed to find cluster config for cluster: {}, skipping applying cluster config", clusterName);
+ setupTimezone(instanceConfig);
return;
}
@@ -87,6 +92,7 @@ public class ServiceStartableUtils {
} finally {
zkClient.close();
}
+ setupTimezone(instanceConfig);
}
private static void addConfigIfNotExists(PinotConfiguration instanceConfig, String key, String value) {
@@ -94,4 +100,11 @@ public class ServiceStartableUtils {
instanceConfig.setProperty(key, value);
}
}
+
+ private static void setupTimezone(PinotConfiguration instanceConfig) {
+ TimeZone localTimezone = TimeZone.getDefault();
+ _timeZone = instanceConfig.getProperty(CONFIG_OF_TIMEZONE, localTimezone.getID());
+ System.setProperty("user.timezone", _timeZone);
+ LOGGER.info("Timezone: {}", _timeZone);
+ }
}
diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
index 13d0a4695a..7bbe5eb450 100644
--- a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
@@ -75,6 +75,7 @@ import org.apache.pinot.spi.data.FieldSpec;
import org.apache.pinot.spi.data.MetricFieldSpec;
import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.env.PinotConfiguration;
+import org.apache.pinot.spi.utils.CommonConstants;
import org.apache.pinot.spi.utils.CommonConstants.Helix;
import org.apache.pinot.spi.utils.CommonConstants.Server;
import org.apache.pinot.spi.utils.NetUtils;
@@ -220,6 +221,7 @@ public class ControllerTest {
properties.put(ControllerConf.HELIX_CLUSTER_NAME, getHelixClusterName());
// Enable groovy on the controller
properties.put(ControllerConf.DISABLE_GROOVY, false);
+ properties.put(CommonConstants.CONFIG_OF_TIMEZONE, "UTC");
overrideControllerConf(properties);
return properties;
}
@@ -287,6 +289,7 @@ public class ControllerTest {
configAccessor.set(scope, Helix.ENABLE_CASE_INSENSITIVE_KEY, Boolean.toString(true));
// Set hyperloglog log2m value to 12.
configAccessor.set(scope, Helix.DEFAULT_HYPERLOGLOG_LOG2M_KEY, Integer.toString(12));
+ assertEquals(System.getProperty("user.timezone"), "UTC");
}
public void stopController() {
@@ -1024,7 +1027,7 @@ public class ControllerTest {
public void setupSharedStateAndValidate()
throws Exception {
if (_zookeeperInstance == null || _helixResourceManager == null) {
- // this is expected to happen only when running a single test case outside of testNG group, i.e when test
+ // this is expected to happen only when running a single test case outside testNG group, i.e. when test
// cases are run one at a time within IntelliJ or through maven command line. When running under a testNG
// group, state will have already been setup by @BeforeGroups method in ControllerTestSetup.
startSharedTestSetup();
diff --git a/pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/ClusterTest.java b/pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/ClusterTest.java
index dd16ce1f7f..7a9cf0a860 100644
--- a/pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/ClusterTest.java
+++ b/pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/ClusterTest.java
@@ -162,6 +162,7 @@ public abstract class ClusterTest extends ControllerTest {
brokerConf.setProperty(Helix.KEY_OF_BROKER_QUERY_PORT,
NetUtils.findOpenPort(DEFAULT_BROKER_PORT + brokerId + RandomUtils.nextInt(10000)));
brokerConf.setProperty(Broker.CONFIG_OF_DELAY_SHUTDOWN_TIME_MS, 0);
+ brokerConf.setProperty(CommonConstants.CONFIG_OF_TIMEZONE, "UTC");
overrideBrokerConf(brokerConf);
return brokerConf;
}
@@ -177,6 +178,13 @@ public abstract class ClusterTest extends ControllerTest {
_brokerPorts = new ArrayList<>();
for (int i = 0; i < numBrokers; i++) {
BaseBrokerStarter brokerStarter = startOneBroker(i);
+ assertEquals(
+ brokerStarter.getConfig().getProperty(
+ CommonConstants.CONFIG_OF_TIMEZONE, ""),
+ "UTC"
+ );
+ assertEquals(System.getProperty("user.timezone"), "UTC");
+
_brokerStarters.add(brokerStarter);
_brokerPorts.add(brokerStarter.getPort());
}
@@ -255,6 +263,7 @@ public abstract class ClusterTest extends ControllerTest {
// Thread time measurement is disabled by default, enable it in integration tests.
// TODO: this can be removed when we eventually enable thread time measurement by default.
serverConf.setProperty(Server.CONFIG_OF_ENABLE_THREAD_CPU_TIME_MEASUREMENT, true);
+ serverConf.setProperty(CommonConstants.CONFIG_OF_TIMEZONE, "UTC");
overrideServerConf(serverConf);
return serverConf;
}
@@ -278,6 +287,7 @@ public abstract class ClusterTest extends ControllerTest {
HelixServerStarter serverStarter = new HelixServerStarter();
serverStarter.init(getServerConf(serverId));
serverStarter.start();
+ assertEquals(System.getProperty("user.timezone"), "UTC");
return serverStarter;
}
@@ -319,9 +329,11 @@ public abstract class ClusterTest extends ControllerTest {
minionConf.setProperty(Helix.CONFIG_OF_ZOOKEEPR_SERVER, getZkUrl());
minionConf.setProperty(CommonConstants.Helix.KEY_OF_MINION_PORT,
NetUtils.findOpenPort(CommonConstants.Minion.DEFAULT_HELIX_PORT));
+ minionConf.setProperty(CommonConstants.CONFIG_OF_TIMEZONE, "UTC");
_minionStarter = new MinionStarter();
_minionStarter.init(minionConf);
_minionStarter.start();
+ assertEquals(System.getProperty("user.timezone"), "UTC");
}
protected void stopBroker() {
diff --git a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java
index 98e6038f23..c30e4286aa 100644
--- a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java
+++ b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java
@@ -417,7 +417,7 @@ public abstract class BaseServerStarter implements ServiceStartable {
}
}
- // Update system resource info (CPU, memory, etc)
+ // Update system resource info (CPU, memory, etc.)
Map<String, String> newSystemResourceInfoMap = new SystemResourceInfo().toMap();
Map<String, String> existingSystemResourceInfoMap =
znRecord.getMapField(CommonConstants.Helix.Instance.SYSTEM_RESOURCE_INFO_KEY);
@@ -426,7 +426,7 @@ public abstract class BaseServerStarter implements ServiceStartable {
if (existingSystemResourceInfoMap == null) {
existingSystemResourceInfoMap = newSystemResourceInfoMap;
} else {
- // existingSystemResourceInfoMap may contains more KV pairs than newSystemResourceInfoMap,
+ // existingSystemResourceInfoMap may contain more KV pairs than newSystemResourceInfoMap,
// we need to preserve those KV pairs and only update the different values.
for (Map.Entry<String, String> entry : newSystemResourceInfoMap.entrySet()) {
existingSystemResourceInfoMap.put(entry.getKey(), entry.getValue());
@@ -768,7 +768,7 @@ public abstract class BaseServerStarter implements ServiceStartable {
/**
* When shutting down the server, waits for all the resources turn OFFLINE (all partitions served by the server are
- * neither ONLINE or CONSUMING).
+ * neither ONLINE nor CONSUMING).
*
* @param endTimeMs Timeout for the check
*/
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 6a0831ad1d..2c3b5e8cd2 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
@@ -52,6 +52,7 @@ public class CommonConstants {
public static final String SWAGGER_AUTHORIZATION_KEY = "oauth";
public static final String CONFIG_OF_SWAGGER_RESOURCES_PATH = "META-INF/resources/webjars/swagger-ui/5.1.0/";
+ public static final String CONFIG_OF_TIMEZONE = "pinot.timezone";
/**
* The state of the consumer for a given segment
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org