You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/06/15 05:07:59 UTC
[1/4] impala git commit: IMPALA-5604: document
DISABLE_CODEGEN_ROWS_THRESHOLD
Repository: impala
Updated Branches:
refs/heads/master b98fb8c2a -> 853b6a9c8
IMPALA-5604: document DISABLE_CODEGEN_ROWS_THRESHOLD
Also fix a couple of nits in EXEC_SINGLE_NODE_ROWS_THRESHOLD.
Change-Id: I709cd55e3869888feb645f85e61a99901d41d479
Reviewed-on: http://gerrit.cloudera.org:8080/10727
Reviewed-by: Alex Rodoni <ar...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/1b0a027f
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/1b0a027f
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/1b0a027f
Branch: refs/heads/master
Commit: 1b0a027ffb216d729fe68e7240313af77550ae08
Parents: b98fb8c
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Thu Jun 14 16:13:58 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Fri Jun 15 02:00:58 2018 +0000
----------------------------------------------------------------------
docs/impala.ditamap | 1 +
docs/impala_keydefs.ditamap | 1 +
.../impala_disable_codegen_rows_threshold.xml | 97 ++++++++++++++++++++
.../impala_exec_single_node_rows_threshold.xml | 11 ++-
4 files changed, 105 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/1b0a027f/docs/impala.ditamap
----------------------------------------------------------------------
diff --git a/docs/impala.ditamap b/docs/impala.ditamap
index 8e6d078..060a819 100644
--- a/docs/impala.ditamap
+++ b/docs/impala.ditamap
@@ -182,6 +182,7 @@ under the License.
<topicref rev="2.9.0 IMPALA-5381" href="topics/impala_default_join_distribution_mode.xml"/>
<topicref rev="2.10.0 IMPALA-3200" href="topics/impala_default_spillable_buffer_size.xml"/>
<topicref href="topics/impala_disable_codegen.xml"/>
+ <topicref rev="2.10.0 IMPALA-5483" href="topics/impala_disable_codegen_rows_threshold.xml"/>
<topicref audience="hidden" href="topics/impala_disable_outermost_topn.xml"/>
<topicref rev="2.5.0" href="topics/impala_disable_row_runtime_filtering.xml"/>
<topicref rev="2.5.0" href="topics/impala_disable_streaming_preaggregations.xml"/>
http://git-wip-us.apache.org/repos/asf/impala/blob/1b0a027f/docs/impala_keydefs.ditamap
----------------------------------------------------------------------
diff --git a/docs/impala_keydefs.ditamap b/docs/impala_keydefs.ditamap
index 08a53ff..056af35 100644
--- a/docs/impala_keydefs.ditamap
+++ b/docs/impala_keydefs.ditamap
@@ -10780,6 +10780,7 @@ under the License.
<keydef href="topics/impala_default_join_distribution_mode.xml" keys="default_join_distribution_mode"/>
<keydef rev="2.10.0 IMPALA-3200" href="topics/impala_default_spillable_buffer_size.xml" keys="default_spillable_buffer_size"/>
<keydef href="topics/impala_disable_codegen.xml" keys="disable_codegen"/>
+ <keydef href="topics/impala_disable_codegen_rows_threshold.xml" keys="disable_codegen_rows_threshold"/>
<keydef href="topics/impala_disable_outermost_topn.xml" keys="disable_outermost_topn"/>
<keydef href="topics/impala_disable_row_runtime_filtering.xml" keys="disable_row_runtime_filtering"/>
<keydef href="topics/impala_disable_streaming_preaggregations.xml" keys="disable_streaming_preaggregations"/>
http://git-wip-us.apache.org/repos/asf/impala/blob/1b0a027f/docs/topics/impala_disable_codegen_rows_threshold.xml
----------------------------------------------------------------------
diff --git a/docs/topics/impala_disable_codegen_rows_threshold.xml b/docs/topics/impala_disable_codegen_rows_threshold.xml
new file mode 100644
index 0000000..b16a691
--- /dev/null
+++ b/docs/topics/impala_disable_codegen_rows_threshold.xml
@@ -0,0 +1,97 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+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.
+-->
+<!DOCTYPE concept PUBLIC "-//OASIS//DTD DITA Concept//EN" "concept.dtd">
+<concept rev="2.0.0" id="exec_single_node_rows_threshold">
+
+ <title>DISABLE_CODEGEN_ROWS_THRESHOLD Query Option (<keyword keyref="impala210_full"/> or higher only)</title>
+ <titlealts audience="PDF"><navtitle>DISABLE_CODEGEN_ROWS_THRESHOLD</navtitle></titlealts>
+ <prolog>
+ <metadata>
+ <data name="Category" value="Impala"/>
+ <data name="Category" value="Impala Query Options"/>
+ <data name="Category" value="Scalability"/>
+ <data name="Category" value="Performance"/>
+ <data name="Category" value="Developers"/>
+ <data name="Category" value="Data Analysts"/>
+ </metadata>
+ </prolog>
+
+ <conbody>
+
+ <p rev="2.0.0">
+ <indexterm audience="hidden">DISABLE_CODEGEN_ROWS_THRESHOLD query option</indexterm>
+ This setting controls the cutoff point (in terms of number of rows processed per Impala daemon) below which
+ Impala disables native code generation for the whole query.
+
+ Native code generation is very beneficial for queries that process many rows because
+ it reduces the time taken to process of each row. However, generating the native code
+ adds latency to query startup. Therefore, automatically disabling codegen for
+ queries that process relatively small amounts of data can improve query response time.
+ </p>
+
+ <p conref="../shared/impala_common.xml#common/syntax_blurb"/>
+
+<codeblock>SET DISABLE_CODEGEN_ROWS_THRESHOLD=<varname>number_of_rows</varname></codeblock>
+
+ <p>
+ <b>Type:</b> numeric
+ </p>
+
+ <p>
+ <b>Default:</b> 50000
+ </p>
+
+ <p>
+ <b>Usage notes:</b> Typically, you increase the default value to make this optimization apply to more queries.
+ If incorrect or corrupted table and column statistics cause Impala to apply this optimization incorrectly to
+ queries that actually involve substantial work, you might see the queries being slower as a result of codegen
+ being disabled. In that case, recompute statistics with the <codeph>COMPUTE STATS</codeph> or
+ <codeph>COMPUTE INCREMENTAL STATS</codeph> statement. If there is a problem collecting accurate statistics,
+ you can turn this feature off by setting the value to 0.
+ </p>
+
+ <p conref="../shared/impala_common.xml#common/internals_blurb"/>
+
+ <p>
+ This setting applies to queries where the number of rows processed can be accurately
+ determined, either through table and column statistics, or by the presence of a
+ <codeph>LIMIT</codeph> clause. If Impala cannot accurately estimate the number of rows,
+ then this setting does not apply.
+ </p>
+
+ <p rev="2.3.0">
+ If a query uses the complex data types <codeph>STRUCT</codeph>, <codeph>ARRAY</codeph>,
+ or <codeph>MAP</codeph>, then codegen is never automatically disabled regardless of the
+ <codeph>DISABLE_CODEGEN_ROWS_THRESHOLD</codeph> setting.
+ </p>
+
+ <p conref="../shared/impala_common.xml#common/added_in_2100"/>
+
+<!-- Don't have any other places that tie into this particular optimization technique yet.
+Potentially: conceptual topics about code generation, distributed queries
+
+<p conref="../shared/impala_common.xml#common/related_info"/>
+<p>
+</p>
+-->
+
+ </conbody>
+
+</concept>
http://git-wip-us.apache.org/repos/asf/impala/blob/1b0a027f/docs/topics/impala_exec_single_node_rows_threshold.xml
----------------------------------------------------------------------
diff --git a/docs/topics/impala_exec_single_node_rows_threshold.xml b/docs/topics/impala_exec_single_node_rows_threshold.xml
index 4822712..62f7988 100644
--- a/docs/topics/impala_exec_single_node_rows_threshold.xml
+++ b/docs/topics/impala_exec_single_node_rows_threshold.xml
@@ -41,8 +41,8 @@ under the License.
as a <q>small</q> query, turning off optimizations such as parallel execution and native code generation. The
overhead for these optimizations is applicable for queries involving substantial amounts of data, but it
makes sense to skip them for queries involving tiny amounts of data. Reducing the overhead for small queries
- allows Impala to complete them more quickly, keeping YARN resources, admission control slots, and so on
- available for data-intensive queries.
+ allows Impala to complete them more quickly, keeping admission control slots, CPU, memory, and so on
+ available for resource-intensive queries.
</p>
<p conref="../shared/impala_common.xml#common/syntax_blurb"/>
@@ -69,9 +69,10 @@ under the License.
<p conref="../shared/impala_common.xml#common/internals_blurb"/>
<p>
- This setting applies to query fragments where the amount of data to scan can be accurately determined, either
- through table and column statistics, or by the presence of a <codeph>LIMIT</codeph> clause. If Impala cannot
- accurately estimate the size of the input data, this setting does not apply.
+ This setting applies to queries where the number of rows processed can be accurately
+ determined, either through table and column statistics, or by the presence of a
+ <codeph>LIMIT</codeph> clause. If Impala cannot accurately estimate the number of rows,
+ then this setting does not apply.
</p>
<p rev="2.3.0">
[4/4] impala git commit: IMPALA-7135. Skeleton implementation of
LocalCatalog
Posted by ta...@apache.org.
IMPALA-7135. Skeleton implementation of LocalCatalog
This adds some of the high level classes for implementing the local
catalog:
- LocalCatalog is the top level implementation. The plan is to
instantiate this once per query, so that no thread safety is required.
- It loads metadata from a MetaProvider interface. The current
implementation fetches directly from HMS and provides no caching. A
future subtask will add a CachingMetaProvider implementation.
Separating out caching will make it easier to experiment with
different policies or storage mechanisms.
- It instantiates LocalDb and LocalTable objects to implement FeDb and
FeTable. These are mostly stubbed out except for the most basic
functionality. Functionality will be filled in incrementally in
further patches.
Since it's not yet possible to hook this up to most of the existing
tests, a very simple new unit test is included to cover the bits of
functionality that are not stubbed out. I didn't concentrate on too much
test coverage here, since once we've implemented more functionality we
can switch over all of the existing tests to get coverage of the new
implementation.
Change-Id: Iab653371188b21c72f50ee1ec4e94950aa6fb9ee
Reviewed-on: http://gerrit.cloudera.org:8080/10627
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/853b6a9c
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/853b6a9c
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/853b6a9c
Branch: refs/heads/master
Commit: 853b6a9c83b7f136e5df200a579ff39b3db42b8c
Parents: 547bc57
Author: Todd Lipcon <to...@cloudera.com>
Authored: Wed Jun 6 15:00:13 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Fri Jun 15 04:10:48 2018 +0000
----------------------------------------------------------------------
.../java/org/apache/impala/catalog/Catalog.java | 8 +-
.../apache/impala/catalog/CatalogObject.java | 3 +-
.../java/org/apache/impala/catalog/FeDb.java | 3 +-
.../java/org/apache/impala/catalog/HasName.java | 29 +++
.../catalog/local/DirectMetaProvider.java | 83 ++++++++
.../impala/catalog/local/LocalCatalog.java | 212 +++++++++++++++++++
.../catalog/local/LocalCatalogException.java | 34 +++
.../apache/impala/catalog/local/LocalDb.java | 173 +++++++++++++++
.../apache/impala/catalog/local/LocalTable.java | 160 ++++++++++++++
.../impala/catalog/local/MetaProvider.java | 48 +++++
.../impala/catalog/local/LocalCatalogTest.java | 76 +++++++
11 files changed, 823 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/853b6a9c/fe/src/main/java/org/apache/impala/catalog/Catalog.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/Catalog.java b/fe/src/main/java/org/apache/impala/catalog/Catalog.java
index 82b411a..7403bc2 100644
--- a/fe/src/main/java/org/apache/impala/catalog/Catalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/Catalog.java
@@ -332,7 +332,7 @@ public abstract class Catalog {
* The results are sorted in String.CASE_INSENSITIVE_ORDER.
* matcher must not be null.
*/
- private List<String> filterStringsByPattern(Iterable<String> candidates,
+ public static List<String> filterStringsByPattern(Iterable<String> candidates,
PatternMatcher matcher) {
Preconditions.checkNotNull(matcher);
List<String> filtered = Lists.newArrayList();
@@ -343,9 +343,9 @@ public abstract class Catalog {
return filtered;
}
- private static class CatalogObjectOrder implements Comparator<CatalogObject> {
+ private static class CatalogObjectOrder implements Comparator<HasName> {
@Override
- public int compare(CatalogObject o1, CatalogObject o2) {
+ public int compare(HasName o1, HasName o2) {
return String.CASE_INSENSITIVE_ORDER.compare(o1.getName(), o2.getName());
}
}
@@ -357,7 +357,7 @@ public abstract class Catalog {
* The results are sorted in CATALOG_OBJECT_ORDER.
* matcher must not be null.
*/
- private <T extends CatalogObject> List<T> filterCatalogObjectsByPattern(
+ public static <T extends HasName> List<T> filterCatalogObjectsByPattern(
Iterable<? extends T> candidates, PatternMatcher matcher) {
Preconditions.checkNotNull(matcher);
List<T> filtered = Lists.newArrayList();
http://git-wip-us.apache.org/repos/asf/impala/blob/853b6a9c/fe/src/main/java/org/apache/impala/catalog/CatalogObject.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/CatalogObject.java b/fe/src/main/java/org/apache/impala/catalog/CatalogObject.java
index cc4c495..34310c3 100644
--- a/fe/src/main/java/org/apache/impala/catalog/CatalogObject.java
+++ b/fe/src/main/java/org/apache/impala/catalog/CatalogObject.java
@@ -22,11 +22,12 @@ import org.apache.impala.thrift.TCatalogObjectType;
/**
* Interface that all catalog objects implement.
*/
-public interface CatalogObject {
+public interface CatalogObject extends HasName {
// Returns the TCatalogObject type of this Catalog object.
public TCatalogObjectType getCatalogObjectType();
// Returns the unqualified object name.
+ @Override
public String getName();
// Returns the unique name of this catalog object.
http://git-wip-us.apache.org/repos/asf/impala/blob/853b6a9c/fe/src/main/java/org/apache/impala/catalog/FeDb.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/FeDb.java b/fe/src/main/java/org/apache/impala/catalog/FeDb.java
index 8de877f..111fbd0 100644
--- a/fe/src/main/java/org/apache/impala/catalog/FeDb.java
+++ b/fe/src/main/java/org/apache/impala/catalog/FeDb.java
@@ -26,10 +26,11 @@ import org.apache.impala.util.PatternMatcher;
/**
* Frontend interface for interacting with a database.
*/
-public interface FeDb {
+public interface FeDb extends HasName {
/**
* @return the name of the database
*/
+ @Override
String getName();
/**
http://git-wip-us.apache.org/repos/asf/impala/blob/853b6a9c/fe/src/main/java/org/apache/impala/catalog/HasName.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/HasName.java b/fe/src/main/java/org/apache/impala/catalog/HasName.java
new file mode 100644
index 0000000..594d541
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/catalog/HasName.java
@@ -0,0 +1,29 @@
+// 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.impala.catalog;
+
+/**
+ * Interface for named catalog objects, used for sorting and pattern
+ * matching.
+ */
+public interface HasName {
+ /**
+ * @return the unqualified name of the object
+ */
+ public String getName();
+}
http://git-wip-us.apache.org/repos/asf/impala/blob/853b6a9c/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java b/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
new file mode 100644
index 0000000..0867091
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
@@ -0,0 +1,83 @@
+// 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.impala.catalog.local;
+
+import org.apache.hadoop.hive.metastore.api.Database;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.NoSuchObjectException;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.api.UnknownDBException;
+import org.apache.impala.catalog.MetaStoreClientPool;
+import org.apache.impala.catalog.MetaStoreClientPool.MetaStoreClient;
+import org.apache.impala.service.BackendConfig;
+import org.apache.impala.thrift.TBackendGflags;
+import org.apache.thrift.TException;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Metadata provider which calls out directly to the source systems
+ * (filesystem, HMS, etc) with no caching.
+ */
+class DirectMetaProvider implements MetaProvider {
+ private static MetaStoreClientPool msClientPool_;
+
+ DirectMetaProvider() {
+ initMsClientPool();
+ }
+
+ private static synchronized void initMsClientPool() {
+ // Lazy-init the metastore client pool based on the backend configuration.
+ // TODO(todd): this should probably be a process-wide singleton.
+ if (msClientPool_ == null) {
+ TBackendGflags cfg = BackendConfig.INSTANCE.getBackendCfg();
+ msClientPool_ = new MetaStoreClientPool(cfg.num_metadata_loading_threads,
+ cfg.initial_hms_cnxn_timeout_s);
+ }
+ }
+
+ @Override
+ public ImmutableList<String> loadDbList() throws TException {
+ try (MetaStoreClient c = msClientPool_.getClient()) {
+ return ImmutableList.copyOf(c.getHiveClient().getAllDatabases());
+ }
+ }
+
+ @Override
+ public Database loadDb(String dbName) throws TException {
+ try (MetaStoreClient c = msClientPool_.getClient()) {
+ return c.getHiveClient().getDatabase(dbName);
+ }
+ }
+
+ @Override
+ public ImmutableList<String> loadTableNames(String dbName)
+ throws MetaException, UnknownDBException, TException {
+ try (MetaStoreClient c = msClientPool_.getClient()) {
+ return ImmutableList.copyOf(c.getHiveClient().getAllTables(dbName));
+ }
+ }
+
+ @Override
+ public Table loadTable(String dbName, String tableName)
+ throws MetaException, NoSuchObjectException, TException {
+ try (MetaStoreClient c = msClientPool_.getClient()) {
+ return c.getHiveClient().getTable(dbName, tableName);
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/impala/blob/853b6a9c/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java b/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
new file mode 100644
index 0000000..beac563
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
@@ -0,0 +1,212 @@
+// 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.impala.catalog.local;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.impala.analysis.TableName;
+import org.apache.impala.catalog.AuthorizationPolicy;
+import org.apache.impala.catalog.BuiltinsDb;
+import org.apache.impala.catalog.Catalog;
+import org.apache.impala.catalog.CatalogException;
+import org.apache.impala.catalog.DatabaseNotFoundException;
+import org.apache.impala.catalog.Db;
+import org.apache.impala.catalog.FeCatalog;
+import org.apache.impala.catalog.FeDataSource;
+import org.apache.impala.catalog.FeDb;
+import org.apache.impala.catalog.FeFsPartition;
+import org.apache.impala.catalog.FeTable;
+import org.apache.impala.catalog.Function;
+import org.apache.impala.catalog.Function.CompareMode;
+import org.apache.impala.catalog.HdfsCachePool;
+import org.apache.impala.catalog.ImpaladCatalog;
+import org.apache.impala.thrift.TCatalogObject;
+import org.apache.impala.thrift.TPartitionKeyValue;
+import org.apache.impala.thrift.TUniqueId;
+import org.apache.impala.util.PatternMatcher;
+import org.apache.thrift.TException;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Maps;
+
+/**
+ * Implementation of FeCatalog which runs within the impalad and fetches metadata
+ * on-demand using a {@link MetaProvider} instance.
+ *
+ * This class should be instantiated once per query. As the catalog is queried,
+ * it lazy-loads those pieces of metadata into the instance and instantiates
+ * appropriate catalog object implementations. This provides caching within a
+ * query -- multiple calls to fetch a given database, table, etc, will return
+ * the same instance. However, this class does not inherently provide any caching
+ * outside the scope of a single query: that caching should be performed at the
+ * level of the MetaProvider.
+ *
+ * This class is not thread-safe, nor are any of the catalog object implementations
+ * returned from its methods.
+ */
+public class LocalCatalog implements FeCatalog {
+ private final MetaProvider metaProvider_;
+ private Map<String, FeDb> dbs_ = Maps.newHashMap();
+ private static final Db builtinsDb_ = new BuiltinsDb(ImpaladCatalog.BUILTINS_DB);
+
+ public LocalCatalog(MetaProvider metaProvider) {
+ metaProvider_ = Preconditions.checkNotNull(metaProvider);
+ }
+
+ @Override
+ public List<? extends FeDb> getDbs(PatternMatcher matcher) {
+ loadDbs();
+ return Catalog.filterCatalogObjectsByPattern(dbs_.values(), matcher);
+ }
+
+ private void loadDbs() {
+ if (!dbs_.isEmpty()) return;
+ Map<String, FeDb> dbs = Maps.newHashMap();
+ List<String> names;
+ try {
+ names = metaProvider_.loadDbList();
+ } catch (TException e) {
+ throw new LocalCatalogException("Unable to load database names", e);
+ }
+ for (String dbName : names) {
+ dbName = dbName.toLowerCase();
+ if (dbs_.containsKey(dbName)) {
+ dbs.put(dbName, dbs_.get(dbName));
+ } else {
+ dbs.put(dbName, new LocalDb(this, dbName));
+ }
+ }
+ dbs.put(builtinsDb_.getName(), builtinsDb_);
+ dbs_ = dbs;
+ }
+
+
+ @Override
+ public List<String> getTableNames(String dbName, PatternMatcher matcher)
+ throws DatabaseNotFoundException {
+ return Catalog.filterStringsByPattern(getDbOrThrow(dbName).getAllTableNames(), matcher);
+ }
+
+ @Override
+ public FeTable getTable(String dbName, String tableName)
+ throws DatabaseNotFoundException {
+ return getDbOrThrow(dbName).getTable(tableName);
+ }
+
+ @Override
+ public TCatalogObject getTCatalogObject(TCatalogObject objectDesc)
+ throws CatalogException {
+ // TODO(todd): this probably makes the /catalog page not load with an error.
+ // We should probably disable that page in local-catalog mode.
+ throw new UnsupportedOperationException("LocalCatalog.getTCatalogObject");
+ }
+
+ @Override
+ public FeDb getDb(String db) {
+ loadDbs();
+ return dbs_.get(db);
+ }
+
+ private FeDb getDbOrThrow(String dbName) throws DatabaseNotFoundException {
+ Preconditions.checkNotNull(dbName);
+ FeDb db = getDb(dbName);
+ if (db == null) {
+ throw new DatabaseNotFoundException("Database '" + dbName + "' not found");
+ }
+ return db;
+ }
+
+ @Override
+ public FeFsPartition getHdfsPartition(
+ String db, String tbl, List<TPartitionKeyValue> partition_spec)
+ throws CatalogException {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public List<? extends FeDataSource> getDataSources(
+ PatternMatcher createHivePatternMatcher) {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public FeDataSource getDataSource(String dataSourceName) {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public Function getFunction(Function desc, CompareMode mode) {
+ FeDb db = getDb(desc.dbName());
+ if (db == null) return null;
+ return db.getFunction(desc, mode);
+ }
+
+ @Override
+ public HdfsCachePool getHdfsCachePool(String poolName) {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public void prioritizeLoad(Set<TableName> tableNames) {
+ // No-op for local catalog.
+ }
+
+ @Override
+ public void waitForCatalogUpdate(long timeoutMs) {
+ // No-op for local catalog.
+ }
+
+ @Override
+ public Path getTablePath(Table msTbl) {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public TUniqueId getCatalogServiceId() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public AuthorizationPolicy getAuthPolicy() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public String getDefaultKuduMasterHosts() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public boolean isReady() {
+ // We are always ready.
+ return true;
+ }
+
+ @Override
+ public void setIsReady(boolean isReady) {
+ // No-op for local catalog.
+ }
+
+ MetaProvider getMetaProvider() {
+ return metaProvider_;
+ }
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/impala/blob/853b6a9c/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalogException.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalogException.java b/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalogException.java
new file mode 100644
index 0000000..59e0be4
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalogException.java
@@ -0,0 +1,34 @@
+// 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.impala.catalog.local;
+
+
+/**
+ * Exception indicating an error loading catalog information into
+ * the impalad.
+ *
+ * TODO(todd): should this be changed to a checked exception? The frontend
+ * interfaces don't have appropriate "throws" clauses to do so without some
+ * significant virality.
+ */
+public class LocalCatalogException extends RuntimeException {
+ private static final long serialVersionUID = -429271377417249918L;
+
+ public LocalCatalogException(String msg, Throwable cause) {
+ super(msg, cause);
+ }
+}
http://git-wip-us.apache.org/repos/asf/impala/blob/853b6a9c/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java b/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
new file mode 100644
index 0000000..1bbe9b8
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
@@ -0,0 +1,173 @@
+// 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.impala.catalog.local;
+
+import java.util.List;
+import java.util.Map;
+
+import org.apache.hadoop.hive.metastore.api.Database;
+import org.apache.impala.catalog.FeDb;
+import org.apache.impala.catalog.FeTable;
+import org.apache.impala.catalog.Function;
+import org.apache.impala.catalog.Function.CompareMode;
+import org.apache.impala.thrift.TDatabase;
+import org.apache.impala.thrift.TFunctionCategory;
+import org.apache.impala.util.PatternMatcher;
+import org.apache.thrift.TException;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Maps;
+
+/**
+ * Database instance loaded from {@link LocalCatalog}.
+ *
+ * This class is not thread-safe. A new instance is created for
+ * each catalog instance.
+ */
+class LocalDb implements FeDb {
+ private final LocalCatalog catalog_;
+ /** The lower-case name of the database. */
+ private final String name_;
+ private Database msDb_;
+
+ /**
+ * Map from lower-cased table name to table object. Values will be
+ * null for tables which have not yet been loaded.
+ */
+ private Map<String, LocalTable> tables_;
+
+ public LocalDb(LocalCatalog catalog, String dbName) {
+ Preconditions.checkNotNull(catalog);
+ Preconditions.checkNotNull(dbName);
+ Preconditions.checkArgument(dbName.toLowerCase().equals(dbName));
+ this.catalog_ = catalog;
+ this.name_ = dbName;
+ }
+
+ @Override
+ public String getName() {
+ return name_;
+ }
+
+ @Override
+ public Database getMetaStoreDb() {
+ if (msDb_ == null) {
+ try {
+ msDb_ = catalog_.getMetaProvider().loadDb(name_);
+ } catch (TException e) {
+ throw new LocalCatalogException(String.format(
+ "Could not load database '%s' from HMS", name_), e);
+ }
+ }
+ return msDb_;
+ }
+
+ @Override
+ public boolean containsTable(String tableName) {
+ loadTableNames();
+ return tables_.containsKey(tableName.toLowerCase());
+ }
+
+ @Override
+ public FeTable getTable(String tblName) {
+ Preconditions.checkNotNull(tblName);
+ Preconditions.checkArgument(tblName.toLowerCase().equals(tblName));
+ loadTableNames();
+ if (!tables_.containsKey(tblName)) {
+ // Table doesn't exist.
+ return null;
+ }
+ LocalTable tbl = tables_.get(tblName);
+ if (tbl == null) {
+ // The table exists but hasn't been loaded yet.
+ tbl = LocalTable.load(this, tblName);
+ tables_.put(tblName, tbl);
+ }
+ return tbl;
+ }
+
+ @Override
+ public List<String> getAllTableNames() {
+ loadTableNames();
+ return ImmutableList.copyOf(tables_.keySet());
+ }
+
+ /**
+ * Populate the 'tables_' map if it is not already populated.
+ *
+ * The map is populated with appropriate keys but null values which
+ * will be replaced on-demand.
+ */
+ private void loadTableNames() {
+ if (tables_ != null) return;
+ Map<String, LocalTable> newMap = Maps.newHashMap();
+ try {
+ List<String> names = catalog_.getMetaProvider().loadTableNames(name_);
+ for (String tableName : names) {
+ newMap.put(tableName.toLowerCase(), null);
+ }
+ } catch (TException e) {
+ throw new LocalCatalogException(String.format(
+ "Could not load table names for database '%s' from HMS", name_), e);
+ }
+ tables_ = newMap;
+ }
+
+ @Override
+ public boolean isSystemDb() {
+ return false;
+ }
+
+ @Override
+ public Function getFunction(Function desc, CompareMode mode) {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public List<Function> getFunctions(String functionName) {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public List<Function> getFunctions(
+ TFunctionCategory category, String function) {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public List<Function> getFunctions(
+ TFunctionCategory category, PatternMatcher patternMatcher) {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public int numFunctions() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public boolean containsFunction(String function) {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public TDatabase toThrift() {
+ throw new UnsupportedOperationException("TODO");
+ }
+}
http://git-wip-us.apache.org/repos/asf/impala/blob/853b6a9c/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java b/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
new file mode 100644
index 0000000..58c0580
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
@@ -0,0 +1,160 @@
+// 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.impala.catalog.local;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.impala.analysis.TableName;
+import org.apache.impala.catalog.ArrayType;
+import org.apache.impala.catalog.Column;
+import org.apache.impala.catalog.FeDb;
+import org.apache.impala.catalog.FeTable;
+import org.apache.impala.thrift.TCatalogObjectType;
+import org.apache.impala.thrift.TTableDescriptor;
+import org.apache.impala.thrift.TTableStats;
+
+import com.google.common.base.Preconditions;
+
+/**
+ * Table instance loaded from {@link LocalCatalog}.
+ *
+ * This class is not thread-safe. A new instance is created for
+ * each catalog instance.
+ */
+class LocalTable implements FeTable {
+ private final LocalDb db_;
+ /** The lower-case name of the table. */
+ private final String name_;
+
+ public static LocalTable load(LocalDb db, String tblName) {
+ // TODO: change this function to instantiate the appropriate
+ // subclass based on the table type (eg view, hbase table, etc)
+ return new LocalTable(db, tblName);
+ }
+
+ public LocalTable(LocalDb db, String tblName) {
+ Preconditions.checkNotNull(db);
+ Preconditions.checkNotNull(tblName);
+ Preconditions.checkArgument(tblName.toLowerCase().equals(tblName));
+ this.db_ = db;
+ this.name_ = tblName;
+ }
+
+ @Override
+ public boolean isLoaded() {
+ return true;
+ }
+
+ @Override
+ public Table getMetaStoreTable() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public String getStorageHandlerClassName() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public TCatalogObjectType getCatalogObjectType() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public String getName() {
+ return name_;
+ }
+
+ @Override
+ public String getFullName() {
+ return db_.getName() + "." + name_;
+ }
+
+ @Override
+ public TableName getTableName() {
+ return new TableName(db_.getName(), name_);
+ }
+
+ @Override
+ public ArrayList<Column> getColumns() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public List<Column> getColumnsInHiveOrder() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public List<String> getColumnNames() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public List<Column> getClusteringColumns() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public List<Column> getNonClusteringColumns() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public int getNumClusteringCols() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public boolean isClusteringColumn(Column c) {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public Column getColumn(String name) {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public ArrayType getType() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public FeDb getDb() {
+ return db_;
+ }
+
+ @Override
+ public long getNumRows() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public TTableStats getTTableStats() {
+ throw new UnsupportedOperationException("TODO");
+ }
+
+ @Override
+ public TTableDescriptor toThriftDescriptor(int tableId, Set<Long> referencedPartitions) {
+ throw new UnsupportedOperationException("TODO");
+ }
+}
http://git-wip-us.apache.org/repos/asf/impala/blob/853b6a9c/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java b/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
new file mode 100644
index 0000000..cd8d3b9
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
@@ -0,0 +1,48 @@
+// 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.impala.catalog.local;
+
+import org.apache.hadoop.hive.metastore.api.Database;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.NoSuchObjectException;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.api.UnknownDBException;
+import org.apache.thrift.TException;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Interface for loading metadata. See {@link LocalCatalog} for an example.
+ *
+ * Implementations may directly access the metadata from the source systems
+ * or may include caching, etc.
+ *
+ * TODO(IMPALA-7127): expand this to include file metadata, sentry metadata,
+ * etc.
+ */
+interface MetaProvider {
+ ImmutableList<String> loadDbList() throws TException;
+
+ Database loadDb(String dbName) throws TException;
+
+ ImmutableList<String> loadTableNames(String dbName)
+ throws MetaException, UnknownDBException, TException;
+
+ Table loadTable(String dbName, String tableName)
+ throws NoSuchObjectException, MetaException, TException;
+}
http://git-wip-us.apache.org/repos/asf/impala/blob/853b6a9c/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java b/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
new file mode 100644
index 0000000..52ff255
--- /dev/null
+++ b/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
@@ -0,0 +1,76 @@
+// 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.impala.catalog.local;
+
+import static org.junit.Assert.*;
+
+import java.util.Set;
+
+import org.apache.impala.catalog.FeDb;
+import org.apache.impala.catalog.FeTable;
+import org.apache.impala.util.PatternMatcher;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.google.common.collect.ImmutableSet;
+
+public class LocalCatalogTest {
+
+ private LocalCatalog catalog_;
+
+ @Before
+ public void setupCatalog() {
+ catalog_ = new LocalCatalog(new DirectMetaProvider());
+ }
+
+ @Test
+ public void testDbs() throws Exception {
+ FeDb functionalDb = catalog_.getDb("functional");
+ assertNotNull(functionalDb);
+ FeDb functionalSeqDb = catalog_.getDb("functional_seq");
+ assertNotNull(functionalSeqDb);
+
+ Set<FeDb> dbs = ImmutableSet.copyOf(
+ catalog_.getDbs(PatternMatcher.MATCHER_MATCH_ALL));
+ assertTrue(dbs.contains(functionalDb));
+ assertTrue(dbs.contains(functionalSeqDb));
+
+ dbs = ImmutableSet.copyOf(
+ catalog_.getDbs(PatternMatcher.createHivePatternMatcher("*_seq")));
+ assertFalse(dbs.contains(functionalDb));
+ assertTrue(dbs.contains(functionalSeqDb));
+ }
+
+ @Test
+ public void testTables() throws Exception {
+ Set<String> names = ImmutableSet.copyOf(catalog_.getTableNames(
+ "functional", PatternMatcher.MATCHER_MATCH_ALL));
+ assertTrue(names.contains("alltypes"));
+
+ FeDb db = catalog_.getDb("functional");
+ assertNotNull(db);
+ FeTable t = catalog_.getTable("functional", "alltypes");
+ assertNotNull(t);
+ assertSame(t, db.getTable("alltypes"));
+ assertSame(db, t.getDb());
+ assertEquals("alltypes", t.getName());
+ assertEquals("functional", t.getDb().getName());
+ assertEquals("functional.alltypes", t.getFullName());
+ }
+
+}
[3/4] impala git commit: IMPALA-7174: fix test_cancellation for
RELEASE builds
Posted by ta...@apache.org.
IMPALA-7174: fix test_cancellation for RELEASE builds
The test was DOA when run against a release build because the debug
actions that it depends on were disabled. The fix is to enable the
debug actions for release builds, similar to other debug actions.
I assume the original motivation of the NDEBUG checks was to avoid
adding overhead to release builds. The cost is minimised by quickly
checking whether the string is empty before proceeding with any
further work.
Also remove wonky exception handling - the test was swallowing
exceptions but we don't expect that code to throw exceptions.
Testing:
Looped the test on a release build.
Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Reviewed-on: http://gerrit.cloudera.org:8080/10722
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/547bc571
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/547bc571
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/547bc571
Branch: refs/heads/master
Commit: 547bc5712137d089432b277b4cb2960e14199b6a
Parents: 76d40f9
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Thu Jun 14 14:21:06 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Fri Jun 15 03:21:27 2018 +0000
----------------------------------------------------------------------
be/src/scheduling/admission-controller.cc | 4 ----
be/src/service/client-request-state.cc | 6 ------
be/src/util/debug-util.cc | 4 +---
be/src/util/debug-util.h | 15 +++++++++++----
tests/custom_cluster/test_admission_controller.py | 6 ++----
5 files changed, 14 insertions(+), 21 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/547bc571/be/src/scheduling/admission-controller.cc
----------------------------------------------------------------------
diff --git a/be/src/scheduling/admission-controller.cc b/be/src/scheduling/admission-controller.cc
index 43cc05c..f39eacf 100644
--- a/be/src/scheduling/admission-controller.cc
+++ b/be/src/scheduling/admission-controller.cc
@@ -41,10 +41,8 @@ DEFINE_int64(queue_wait_timeout_ms, 60 * 1000, "Maximum amount of time (in "
namespace impala {
-#ifndef NDEBUG
// Available 'sleep_label' string that can be used in the debug_action query option.
static const string SLEEP_AFTER_ADMISSION_OUTCOME_MS = "SLEEP_AFTER_ADMISSION_OUTCOME_MS";
-#endif
/// Convenience method.
std::string PrintBytes(int64_t value) {
@@ -626,9 +624,7 @@ Status AdmissionController::AdmitQuery(QuerySchedule* schedule,
schedule->summary_profile()->AddInfoString(PROFILE_INFO_KEY_QUEUE_DETAIL,
Substitute(PROFILE_INFO_VAL_QUEUE_DETAIL, wait_time_ms, not_admitted_reason));
-#ifndef NDEBUG
SleepIfSetInDebugOptions(schedule->query_options(), SLEEP_AFTER_ADMISSION_OUTCOME_MS);
-#endif
{
lock_guard<mutex> lock(admission_ctrl_lock_);
http://git-wip-us.apache.org/repos/asf/impala/blob/547bc571/be/src/service/client-request-state.cc
----------------------------------------------------------------------
diff --git a/be/src/service/client-request-state.cc b/be/src/service/client-request-state.cc
index 27ae77e..0c3910d 100644
--- a/be/src/service/client-request-state.cc
+++ b/be/src/service/client-request-state.cc
@@ -64,12 +64,10 @@ static const string TABLES_MISSING_STATS_KEY = "Tables Missing Stats";
static const string TABLES_WITH_CORRUPT_STATS_KEY = "Tables With Corrupt Table Stats";
static const string TABLES_WITH_MISSING_DISK_IDS_KEY = "Tables With Missing Disk Ids";
-#ifndef NDEBUG
// Available 'sleep_label' strings that can be used in the debug_action query option.
static const string SLEEP_BEFORE_ADMISSION_MS = "SLEEP_BEFORE_ADMISSION_MS";
static const string SLEEP_AFTER_COORDINATOR_STARTS_MS =
"SLEEP_AFTER_COORDINATOR_STARTS_MS";
-#endif
ClientRequestState::ClientRequestState(
const TQueryCtx& query_ctx, ExecEnv* exec_env, Frontend* frontend,
@@ -491,9 +489,7 @@ Status ClientRequestState::ExecAsyncQueryOrDmlRequest(
}
void ClientRequestState::FinishExecQueryOrDmlRequest() {
-#ifndef NDEBUG
SleepIfSetInDebugOptions(schedule_->query_options(), SLEEP_BEFORE_ADMISSION_MS);
-#endif
DCHECK(exec_env_->admission_controller() != nullptr);
Status admit_status = ExecEnv::GetInstance()->admission_controller()->AdmitQuery(
schedule_.get(), &admit_outcome_);
@@ -504,9 +500,7 @@ void ClientRequestState::FinishExecQueryOrDmlRequest() {
coord_.reset(new Coordinator(*schedule_, query_events_));
Status exec_status = coord_->Exec();
-#ifndef NDEBUG
SleepIfSetInDebugOptions(schedule_->query_options(), SLEEP_AFTER_COORDINATOR_STARTS_MS);
-#endif
bool cancelled = false;
Status cancellation_status;
http://git-wip-us.apache.org/repos/asf/impala/blob/547bc571/be/src/util/debug-util.cc
----------------------------------------------------------------------
diff --git a/be/src/util/debug-util.cc b/be/src/util/debug-util.cc
index 3113703..eb55a7c 100644
--- a/be/src/util/debug-util.cc
+++ b/be/src/util/debug-util.cc
@@ -285,8 +285,7 @@ string GetBackendString() {
return Substitute("$0:$1", FLAGS_hostname, FLAGS_be_port);
}
-#ifndef NDEBUG
-void SleepIfSetInDebugOptions(
+void SleepIfSetInDebugOptionsImpl(
const TQueryOptions& query_options, const string& sleep_label) {
vector<string> components;
boost::split(components, query_options.debug_action, boost::is_any_of(":"));
@@ -294,6 +293,5 @@ void SleepIfSetInDebugOptions(
SleepForMs(atoi(components[1].c_str()));
}
}
-#endif
}
http://git-wip-us.apache.org/repos/asf/impala/blob/547bc571/be/src/util/debug-util.h
----------------------------------------------------------------------
diff --git a/be/src/util/debug-util.h b/be/src/util/debug-util.h
index 9a01228..813f460 100644
--- a/be/src/util/debug-util.h
+++ b/be/src/util/debug-util.h
@@ -107,15 +107,22 @@ std::string GetStackTrace();
/// Returns the backend name in "host:port" form suitable for human consumption.
std::string GetBackendString();
-#ifndef NDEBUG
+/// Slow path implementing SleepIfSetInDebugOptions() for the case where 'query_options'
+/// is non-empty.
+void SleepIfSetInDebugOptionsImpl(
+ const TQueryOptions& query_options, const string& sleep_label);
+
/// If sleep time is specified in the debug_action query option in the format
/// <sleep_label>:<sleep_time_ms>, where <sleep_label> is a string and <sleep_time_ms>
/// is an integer, and 'sleep_label' matches the one specified in the query option, then
/// this methods extracts the corresponding <sleep_time_ms> and initiates a sleep for that
/// many milliseconds.
-void SleepIfSetInDebugOptions(
- const TQueryOptions& query_options, const string& sleep_label);
-#endif
+static inline void SleepIfSetInDebugOptions(
+ const TQueryOptions& query_options, const string& sleep_label) {
+ // Make sure this is very cheap if debug actions are not in use.
+ if (LIKELY(query_options.debug_action.empty())) return;
+ return SleepIfSetInDebugOptionsImpl(query_options, sleep_label);
+}
// FILE_CHECKs are conditions that we expect to be true but could fail due to a malformed
// input file. They differentiate these cases from DCHECKs, which indicate conditions that
http://git-wip-us.apache.org/repos/asf/impala/blob/547bc571/tests/custom_cluster/test_admission_controller.py
----------------------------------------------------------------------
diff --git a/tests/custom_cluster/test_admission_controller.py b/tests/custom_cluster/test_admission_controller.py
index 6fb0ae5..8a15c46 100644
--- a/tests/custom_cluster/test_admission_controller.py
+++ b/tests/custom_cluster/test_admission_controller.py
@@ -546,10 +546,6 @@ class TestAdmissionController(TestAdmissionControllerBase, HS2TestSuite):
client.close_query(handle)
queued_profile = client.get_runtime_profile(queued_query_handle)
assert "Admission result: Cancelled (queued)" in queued_profile
- except Exception as e:
- print e.args
- finally:
- client.close()
for i in self.cluster.impalads:
i.service.wait_for_metric_value("impala-server.num-fragments-in-flight", 0)
assert self.cluster.impalads[0].service.get_metric_value(
@@ -558,6 +554,8 @@ class TestAdmissionController(TestAdmissionControllerBase, HS2TestSuite):
"admission-controller.total-admitted.default-pool") == 3
assert self.cluster.impalads[0].service.get_metric_value(
"admission-controller.total-queued.default-pool") == 2
+ finally:
+ client.close()
class TestAdmissionControllerStress(TestAdmissionControllerBase):
"""Submits a number of queries (parameterized) with some delay between submissions
[2/4] impala git commit: IMPALA-7111: avoid use of boost::split in
CheckPluginEnabled
Posted by ta...@apache.org.
IMPALA-7111: avoid use of boost::split in CheckPluginEnabled
This is an attempt to either avoid the bug or make it easier to diagnose
if it reoccurs. My suspicion is that somehow boost::split() is
accessing the input string in a non-thread-safe manner, but the
implementation is opaque enough that it's not obvious how.
Change-Id: I17b7f083731a33b832035f24900e351e2cb2feb8
Reviewed-on: http://gerrit.cloudera.org:8080/10709
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/76d40f92
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/76d40f92
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/76d40f92
Branch: refs/heads/master
Commit: 76d40f92ad9214053602518adaf00df8b7477a37
Parents: 1b0a027
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Wed Jun 13 13:48:48 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Fri Jun 15 02:07:40 2018 +0000
----------------------------------------------------------------------
be/src/exec/hdfs-plugin-text-scanner.cc | 9 +++-----
be/src/util/string-util-test.cc | 31 ++++++++++++++++++++++++++++
be/src/util/string-util.cc | 11 ++++++++++
be/src/util/string-util.h | 3 +++
4 files changed, 48 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/76d40f92/be/src/exec/hdfs-plugin-text-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-plugin-text-scanner.cc b/be/src/exec/hdfs-plugin-text-scanner.cc
index 19e3313..638c7bc 100644
--- a/be/src/exec/hdfs-plugin-text-scanner.cc
+++ b/be/src/exec/hdfs-plugin-text-scanner.cc
@@ -27,8 +27,9 @@
#include "runtime/runtime-state.h"
#include "runtime/hdfs-fs-cache.h"
#include "util/debug-util.h"
-#include "util/hdfs-util.h"
#include "util/dynamic-util.h"
+#include "util/hdfs-util.h"
+#include "util/string-util.h"
#include "common/names.h"
@@ -103,11 +104,7 @@ Status HdfsPluginTextScanner::IssueInitialRanges(HdfsScanNodeBase* scan_node,
}
Status HdfsPluginTextScanner::CheckPluginEnabled(const string& plugin_name) {
- vector<string> enabled_plugins;
- boost::split(enabled_plugins, FLAGS_enabled_hdfs_text_scanner_plugins,
- boost::is_any_of(","));
- if (find(enabled_plugins.begin(), enabled_plugins.end(), plugin_name)
- == enabled_plugins.end()) {
+ if (!CommaSeparatedContains(FLAGS_enabled_hdfs_text_scanner_plugins, plugin_name)) {
return Status(Substitute("Scanner plugin '$0' is not one of the enabled plugins: '$1'",
plugin_name, FLAGS_enabled_hdfs_text_scanner_plugins));
}
http://git-wip-us.apache.org/repos/asf/impala/blob/76d40f92/be/src/util/string-util-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/string-util-test.cc b/be/src/util/string-util-test.cc
index 979eb9f..2c268dd 100644
--- a/be/src/util/string-util-test.cc
+++ b/be/src/util/string-util-test.cc
@@ -79,6 +79,37 @@ TEST(TruncateUpTest, Basic) {
EvalTruncation(a, b, 2, UP);
}
+TEST(CommaSeparatedContainsTest, Basic) {
+ // Basic tests with string present.
+ EXPECT_TRUE(CommaSeparatedContains("LZO", "LZO"));
+ EXPECT_TRUE(CommaSeparatedContains("foo,LZO", "LZO"));
+ EXPECT_TRUE(CommaSeparatedContains("LZO,bar", "LZO"));
+ EXPECT_TRUE(CommaSeparatedContains("foo,LZO,bar", "LZO"));
+
+ // Handles zero-length entries.
+ EXPECT_FALSE(CommaSeparatedContains("", "LZO"));
+ EXPECT_FALSE(CommaSeparatedContains(",", "LZO"));
+ EXPECT_FALSE(CommaSeparatedContains(",,", "LZO"));
+ EXPECT_TRUE(CommaSeparatedContains("foo,LZO,", "LZO"));
+ EXPECT_TRUE(CommaSeparatedContains(",foo,LZO,", "LZO"));
+ EXPECT_TRUE(CommaSeparatedContains(",foo,,LZO,", "LZO"));
+
+ // Basic tests with string absent.
+ EXPECT_FALSE(CommaSeparatedContains("foo,bar", "LZO"));
+ EXPECT_FALSE(CommaSeparatedContains("foo", "LZO"));
+ EXPECT_FALSE(CommaSeparatedContains("foo,", "LZO"));
+ EXPECT_FALSE(CommaSeparatedContains("foo,bar,baz", "LZO"));
+ EXPECT_FALSE(CommaSeparatedContains(",foo,LzO,", "LZO"));
+
+ // Pattern is longer than token.
+ EXPECT_FALSE(CommaSeparatedContains(",foo,LzO,", "ZZZZZ"));
+ // Pattern is longer than string.
+ EXPECT_FALSE(CommaSeparatedContains("foo", "ZZZZZ"));
+
+ // Whitespace is included in tokens alone.
+ EXPECT_FALSE(CommaSeparatedContains("foo , foo, foo,\nfoo,\tfoo", "foo"));
+}
+
}
IMPALA_TEST_MAIN();
http://git-wip-us.apache.org/repos/asf/impala/blob/76d40f92/be/src/util/string-util.cc
----------------------------------------------------------------------
diff --git a/be/src/util/string-util.cc b/be/src/util/string-util.cc
index b6c8fb7..f8e284f 100644
--- a/be/src/util/string-util.cc
+++ b/be/src/util/string-util.cc
@@ -54,4 +54,15 @@ Status TruncateUp(const string& str, int32_t max_length, string* result) {
return Status::OK();
}
+bool CommaSeparatedContains(const std::string& cs_list, const std::string& item) {
+ size_t pos = 0;
+ while (pos < cs_list.size()) {
+ size_t comma_pos = cs_list.find(",", pos);
+ if (comma_pos == string::npos) return cs_list.compare(pos, string::npos, item) == 0;
+ if (cs_list.compare(pos, comma_pos - pos, item) == 0) return true;
+ pos = comma_pos + 1;
+ }
+ return false;
+}
+
}
http://git-wip-us.apache.org/repos/asf/impala/blob/76d40f92/be/src/util/string-util.h
----------------------------------------------------------------------
diff --git a/be/src/util/string-util.h b/be/src/util/string-util.h
index 7e7ab12..2811a3c 100644
--- a/be/src/util/string-util.h
+++ b/be/src/util/string-util.h
@@ -37,6 +37,9 @@ Status TruncateDown(const std::string& str, int32_t max_length, std::string* res
WARN_UNUSED_RESULT
Status TruncateUp(const std::string& str, int32_t max_length, std::string* result);
+/// Return true if the comma-separated string 'cs_list' contains 'item' as one of
+/// the comma-separated values.
+bool CommaSeparatedContains(const std::string& cs_list, const std::string& item);
}
#endif