You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by sh...@apache.org on 2018/04/24 03:49:27 UTC
[kylin] 05/07: KYLIN-3337 Use an auto-closeable style API to
replace KylinConfig.setKylinConfigThreadLocal()
This is an automated email from the ASF dual-hosted git repository.
shaofengshi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kylin.git
commit a9dcad1a859158ffeecceff614ed864695788a0d
Author: Li Yang <li...@apache.org>
AuthorDate: Sat Apr 14 07:17:32 2018 +0800
KYLIN-3337 Use an auto-closeable style API to replace KylinConfig.setKylinConfigThreadLocal()
---
.../java/org/apache/kylin/common/KylinConfig.java | 29 ++++++++---
.../org/apache/kylin/common/KylinConfigTest.java | 32 +++++++-----
.../kylin/engine/mr/common/AbstractHadoopJob.java | 8 ++-
.../kylin/rest/service/AdminServiceTest.java | 58 ++++++++++++----------
.../kylin/rest/service/QueryServiceTest.java | 28 ++++++-----
.../apache/kylin/source/hive/HiveMRInputTest.java | 6 +--
.../v2/coprocessor/endpoint/CubeVisitService.java | 12 +++--
7 files changed, 103 insertions(+), 70 deletions(-)
diff --git a/core-common/src/main/java/org/apache/kylin/common/KylinConfig.java b/core-common/src/main/java/org/apache/kylin/common/KylinConfig.java
index 105cd9e..81c9b89 100644
--- a/core-common/src/main/java/org/apache/kylin/common/KylinConfig.java
+++ b/core-common/src/main/java/org/apache/kylin/common/KylinConfig.java
@@ -239,18 +239,31 @@ public class KylinConfig extends KylinConfigBase {
setKylinConfigInEnvIfMissing(props);
}
- public static void setKylinConfigThreadLocal(KylinConfig config) {
- THREAD_ENV_INSTANCE.set(config);
+ // auto-closeable API to remind that a thread local config must always be removed
+ public static SetAndUnsetThreadLocalConfig setAndUnsetThreadLocalConfig(KylinConfig config) {
+ return new SetAndUnsetThreadLocalConfig(config);
}
+
+ public static class SetAndUnsetThreadLocalConfig implements AutoCloseable {
+
+ public SetAndUnsetThreadLocalConfig(KylinConfig config) {
+ THREAD_ENV_INSTANCE.set(config);
+ }
- public static void removeKylinConfigThreadLocal() {
- THREAD_ENV_INSTANCE.remove();
+ @Override
+ public void close() {
+ THREAD_ENV_INSTANCE.remove();
+ }
}
- public static KylinConfig createKylinConfig(String propsInStr) throws IOException {
- Properties props = new Properties();
- props.load(new StringReader(propsInStr));
- return createKylinConfig(props);
+ public static KylinConfig createKylinConfig(String propsInStr) {
+ try {
+ Properties props = new Properties();
+ props.load(new StringReader(propsInStr));
+ return createKylinConfig(props);
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to create KylinConfig from string: " + propsInStr, e);
+ }
}
public static KylinConfig createKylinConfig(KylinConfig another) {
diff --git a/core-common/src/test/java/org/apache/kylin/common/KylinConfigTest.java b/core-common/src/test/java/org/apache/kylin/common/KylinConfigTest.java
index 92c730a..811c574 100644
--- a/core-common/src/test/java/org/apache/kylin/common/KylinConfigTest.java
+++ b/core-common/src/test/java/org/apache/kylin/common/KylinConfigTest.java
@@ -27,6 +27,7 @@ import static org.junit.Assert.assertTrue;
import java.util.Map;
import java.util.Properties;
+import org.apache.kylin.common.KylinConfig.SetAndUnsetThreadLocalConfig;
import org.junit.Test;
import com.google.common.collect.Maps;
@@ -98,7 +99,7 @@ public class KylinConfigTest extends HotLoadKylinPropertiesTestCase {
}
@Test
- public void testThreadLocalOverride() {
+ public void testThreadLocalOverride() throws InterruptedException {
final String metadata1 = "meta1";
final String metadata2 = "meta2";
@@ -111,18 +112,23 @@ public class KylinConfigTest extends HotLoadKylinPropertiesTestCase {
// test thread-local override
KylinConfig threadConfig = KylinConfig.createKylinConfig(new Properties());
threadConfig.setMetadataUrl(metadata2);
- KylinConfig.setKylinConfigThreadLocal(threadConfig);
-
- assertEquals(metadata2, KylinConfig.getInstanceFromEnv().getMetadataUrl().toString());
-
- // other threads still use system KylinConfig
- new Thread(new Runnable() {
- @Override
- public void run() {
- System.out.println("Started new thread.");
- assertEquals(metadata1, KylinConfig.getInstanceFromEnv().getMetadataUrl().toString());
- }
- }).start();
+
+ try (SetAndUnsetThreadLocalConfig autoUnset = KylinConfig.setAndUnsetThreadLocalConfig(threadConfig)) {
+
+ assertEquals(metadata2, KylinConfig.getInstanceFromEnv().getMetadataUrl().toString());
+
+ // other threads still use system KylinConfig
+ final String[] holder = new String[1];
+ Thread child = new Thread(new Runnable() {
+ @Override
+ public void run() {
+ holder[0] = KylinConfig.getInstanceFromEnv().getMetadataUrl().toString();
+ }
+ });
+ child.start();
+ child.join();
+ assertEquals(metadata1, holder[0]);
+ }
}
@Test
diff --git a/engine-mr/src/main/java/org/apache/kylin/engine/mr/common/AbstractHadoopJob.java b/engine-mr/src/main/java/org/apache/kylin/engine/mr/common/AbstractHadoopJob.java
index 8925a8e..7b25354 100644
--- a/engine-mr/src/main/java/org/apache/kylin/engine/mr/common/AbstractHadoopJob.java
+++ b/engine-mr/src/main/java/org/apache/kylin/engine/mr/common/AbstractHadoopJob.java
@@ -59,6 +59,7 @@ import org.apache.hadoop.util.ReflectionUtils;
import org.apache.hadoop.util.Tool;
import org.apache.hadoop.util.ToolRunner;
import org.apache.kylin.common.KylinConfig;
+import org.apache.kylin.common.KylinConfig.SetAndUnsetThreadLocalConfig;
import org.apache.kylin.common.StorageURL;
import org.apache.kylin.common.util.CliCommandExecutor;
import org.apache.kylin.common.util.HadoopUtil;
@@ -495,7 +496,12 @@ public abstract class AbstractHadoopJob extends Configured implements Tool {
} catch (IOException e) {
throw new RuntimeException(e);
}
- KylinConfig.setKylinConfigThreadLocal(config);
+
+ // This is a bad example where the thread local KylinConfig cannot be auto-closed due to
+ // limitation of MR API. It works because MR task runs its own process. Do not copy.
+ @SuppressWarnings("unused")
+ SetAndUnsetThreadLocalConfig shouldAutoClose = KylinConfig.setAndUnsetThreadLocalConfig(config);
+
return config;
}
diff --git a/server/src/test/java/org/apache/kylin/rest/service/AdminServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/AdminServiceTest.java
index 8d49abc..cc0421a 100644
--- a/server/src/test/java/org/apache/kylin/rest/service/AdminServiceTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/service/AdminServiceTest.java
@@ -23,6 +23,7 @@ import java.io.IOException;
import org.apache.commons.io.FileUtils;
import org.apache.kylin.common.KylinConfig;
+import org.apache.kylin.common.KylinConfig.SetAndUnsetThreadLocalConfig;
import org.junit.Assert;
import org.junit.Test;
import org.springframework.beans.factory.annotation.Autowired;
@@ -44,33 +45,36 @@ public class AdminServiceTest extends ServiceTestBase {
FileUtils.deleteQuietly(file);
FileUtils.touch(file);
String path = Thread.currentThread().getContextClassLoader().getResource("kylin.properties").getPath();
- KylinConfig.setKylinConfigThreadLocal(KylinConfig.createInstanceFromUri(path));
- String expected = "kylin.web.link-streaming-guide=http://kylin.apache.org/\n" +
- "kylin.web.dashboard-enabled=\n" +
- "kylin.web.contact-mail=\n" +
- "kylin.query.cache-enabled=true\n" +
- "kylin.web.link-diagnostic=\n" +
- "kylin.web.help.length=4\n" +
- "kylin.web.timezone=GMT+8\n" +
- "kylin.server.external-acl-provider=\n" +
- "kylin.tool.auto-migrate-cube.enabled=\n" +
- "kylin.storage.default=2\n" +
- "kylin.cube.cubeplanner.enabled=false\n" +
- "kylin.web.help=\n" +
- "kylin.web.export-allow-other=true\n" +
- "kylin.web.link-hadoop=\n" +
- "kylin.web.hide-measures=RAW\n" +
- "kylin.htrace.show-gui-trace-toggle=false\n" +
- "kylin.web.export-allow-admin=true\n" +
- "kylin.env=QA\n" +
- "kylin.web.hive-limit=20\n" +
- "kylin.engine.default=2\n" +
- "kylin.web.help.3=onboard|Cube Design Tutorial|http://kylin.apache.org/docs21/howto/howto_optimize_cubes.html\n" +
- "kylin.web.help.2=tableau|Tableau Guide|http://kylin.apache.org/docs21/tutorial/tableau_91.html\n" +
- "kylin.web.help.1=odbc|ODBC Driver|http://kylin.apache.org/docs21/tutorial/odbc.html\n" +
- "kylin.web.help.0=start|Getting Started|http://kylin.apache.org/docs21/tutorial/kylin_sample.html\n" +
- "kylin.security.profile=testing\n";
- Assert.assertEquals(expected, adminService.getPublicConfig());
+ KylinConfig config = KylinConfig.createInstanceFromUri(path);
+ try (SetAndUnsetThreadLocalConfig autoUnset = KylinConfig.setAndUnsetThreadLocalConfig(config)) {
+
+ String expected = "kylin.web.link-streaming-guide=http://kylin.apache.org/\n" +
+ "kylin.web.dashboard-enabled=\n" +
+ "kylin.web.contact-mail=\n" +
+ "kylin.query.cache-enabled=true\n" +
+ "kylin.web.link-diagnostic=\n" +
+ "kylin.web.help.length=4\n" +
+ "kylin.web.timezone=GMT+8\n" +
+ "kylin.server.external-acl-provider=\n" +
+ "kylin.tool.auto-migrate-cube.enabled=\n" +
+ "kylin.storage.default=2\n" +
+ "kylin.cube.cubeplanner.enabled=false\n" +
+ "kylin.web.help=\n" +
+ "kylin.web.export-allow-other=true\n" +
+ "kylin.web.link-hadoop=\n" +
+ "kylin.web.hide-measures=RAW\n" +
+ "kylin.htrace.show-gui-trace-toggle=false\n" +
+ "kylin.web.export-allow-admin=true\n" +
+ "kylin.env=QA\n" +
+ "kylin.web.hive-limit=20\n" +
+ "kylin.engine.default=2\n" +
+ "kylin.web.help.3=onboard|Cube Design Tutorial|http://kylin.apache.org/docs21/howto/howto_optimize_cubes.html\n" +
+ "kylin.web.help.2=tableau|Tableau Guide|http://kylin.apache.org/docs21/tutorial/tableau_91.html\n" +
+ "kylin.web.help.1=odbc|ODBC Driver|http://kylin.apache.org/docs21/tutorial/odbc.html\n" +
+ "kylin.web.help.0=start|Getting Started|http://kylin.apache.org/docs21/tutorial/kylin_sample.html\n" +
+ "kylin.security.profile=testing\n";
+ Assert.assertEquals(expected, adminService.getPublicConfig());
+ }
}
}
diff --git a/server/src/test/java/org/apache/kylin/rest/service/QueryServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/QueryServiceTest.java
index 5c633a3..f3dc6b1 100644
--- a/server/src/test/java/org/apache/kylin/rest/service/QueryServiceTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/service/QueryServiceTest.java
@@ -21,8 +21,9 @@ package org.apache.kylin.rest.service;
import java.io.IOException;
import java.sql.SQLException;
-import org.apache.kylin.common.QueryContext;
import org.apache.kylin.common.KylinConfig;
+import org.apache.kylin.common.KylinConfig.SetAndUnsetThreadLocalConfig;
+import org.apache.kylin.common.QueryContext;
import org.apache.kylin.common.QueryContextFacade;
import org.apache.kylin.job.exception.JobException;
import org.apache.kylin.metadata.project.ProjectInstance;
@@ -79,21 +80,22 @@ public class QueryServiceTest extends ServiceTestBase {
KylinConfig config = KylinConfig.getInstanceFromEnv();
config.setProperty("kylin.query.convert-create-table-to-with", "true");
- KylinConfig.setKylinConfigThreadLocal(config);
+ try (SetAndUnsetThreadLocalConfig autoUnset = KylinConfig.setAndUnsetThreadLocalConfig(config)) {
- SQLRequest request = new SQLRequest();
- request.setProject("default");
- request.setSql(create_table1);
- queryService.doQueryWithCache(request);
+ SQLRequest request = new SQLRequest();
+ request.setProject("default");
+ request.setSql(create_table1);
+ queryService.doQueryWithCache(request);
- request.setSql(create_table2);
- queryService.doQueryWithCache(request);
+ request.setSql(create_table2);
+ queryService.doQueryWithCache(request);
- request.setSql(select_table);
- SQLResponse response = queryService.doQueryWithCache(request, true);
+ request.setSql(select_table);
+ SQLResponse response = queryService.doQueryWithCache(request, true);
- Assert.assertEquals(
- "WITH tableId as (select * from some_table1) , tableId2 AS (select * FROM some_table2) select * from tableId join tableId2 on tableId.a = tableId2.b;",
- response.getExceptionMessage());
+ Assert.assertEquals(
+ "WITH tableId as (select * from some_table1) , tableId2 AS (select * FROM some_table2) select * from tableId join tableId2 on tableId.a = tableId2.b;",
+ response.getExceptionMessage());
+ }
}
}
diff --git a/source-hive/src/test/java/org/apache/kylin/source/hive/HiveMRInputTest.java b/source-hive/src/test/java/org/apache/kylin/source/hive/HiveMRInputTest.java
index f33ee74..8df3174 100644
--- a/source-hive/src/test/java/org/apache/kylin/source/hive/HiveMRInputTest.java
+++ b/source-hive/src/test/java/org/apache/kylin/source/hive/HiveMRInputTest.java
@@ -28,6 +28,7 @@ import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.kylin.common.KylinConfig;
+import org.apache.kylin.common.KylinConfig.SetAndUnsetThreadLocalConfig;
import org.apache.kylin.job.execution.DefaultChainedExecutable;
import org.junit.Assert;
import org.junit.Test;
@@ -38,9 +39,8 @@ public class HiveMRInputTest {
public void TestGetJobWorkingDir() throws IOException {
FileSystem fileSystem = FileSystem.get(new Configuration());
Path jobWorkDirPath = null;
- try {
- KylinConfig kylinConfig = mock(KylinConfig.class);
- KylinConfig.setKylinConfigThreadLocal(kylinConfig);
+ KylinConfig kylinConfig = mock(KylinConfig.class);
+ try (SetAndUnsetThreadLocalConfig autoUnset = KylinConfig.setAndUnsetThreadLocalConfig(kylinConfig)) {
when(kylinConfig.getHiveTableDirCreateFirst()).thenReturn(true);
when(kylinConfig.getHdfsWorkingDirectory()).thenReturn("/tmp/kylin/");
DefaultChainedExecutable defaultChainedExecutable = mock(DefaultChainedExecutable.class);
diff --git a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/cube/v2/coprocessor/endpoint/CubeVisitService.java b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/cube/v2/coprocessor/endpoint/CubeVisitService.java
index b784ab9..fd54e2b 100644
--- a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/cube/v2/coprocessor/endpoint/CubeVisitService.java
+++ b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/cube/v2/coprocessor/endpoint/CubeVisitService.java
@@ -42,6 +42,7 @@ import org.apache.hadoop.hbase.protobuf.ResponseConverter;
import org.apache.hadoop.hbase.regionserver.HRegion;
import org.apache.hadoop.hbase.regionserver.RegionScanner;
import org.apache.kylin.common.KylinConfig;
+import org.apache.kylin.common.KylinConfig.SetAndUnsetThreadLocalConfig;
import org.apache.kylin.common.exceptions.KylinTimeoutException;
import org.apache.kylin.common.exceptions.ResourceLimitExceededException;
import org.apache.kylin.common.util.Bytes;
@@ -239,18 +240,19 @@ public class CubeVisitService extends CubeVisitProtos.CubeVisitService implement
CubeVisitProtos.CubeVisitResponse.ErrorInfo errorInfo = null;
+ // if user change kylin.properties on kylin server, need to manually redeploy coprocessor jar to update KylinConfig of Env.
+ KylinConfig kylinConfig = KylinConfig.createKylinConfig(request.getKylinProperties());
+
String queryId = request.hasQueryId() ? request.getQueryId() : "UnknownId";
logger.info("start query {} in thread {}", queryId, Thread.currentThread().getName());
- try (SetThreadName ignored = new SetThreadName("Query %s", queryId)) {
+ try (SetAndUnsetThreadLocalConfig autoUnset = KylinConfig.setAndUnsetThreadLocalConfig(kylinConfig);
+ SetThreadName ignored = new SetThreadName("Query %s", queryId)) {
+
final long serviceStartTime = System.currentTimeMillis();
region = (HRegion) env.getRegion();
region.startRegionOperation();
- // if user change kylin.properties on kylin server, need to manually redeploy coprocessor jar to update KylinConfig of Env.
- KylinConfig kylinConfig = KylinConfig.createKylinConfig(request.getKylinProperties());
- KylinConfig.setKylinConfigThreadLocal(kylinConfig);
-
debugGitTag = region.getTableDesc().getValue(IRealizationConstants.HTableGitTag);
final GTScanRequest scanReq = GTScanRequest.serializer
--
To stop receiving notification emails like this one, please contact
shaofengshi@apache.org.