This is an automated email from the ASF dual-hosted git repository. boroknagyz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 3f74171a271b9e5614ce1752221517a12dae0c2f Author: Yongzhi Chen <yc...@cloudera.com> AuthorDate: Tue Jun 25 11:32:13 2019 -0400 IMPALA-8681: Only show ValidWriteIdLists for Acid tables Lists ValidWriteIds for transactional tables in profile. If a query does not trigger any transactional table loading, the query profile will not have the "Loaded ValidWriteIdLists" timeline. Tests: Manual tests. Fixed StmtMetadataLoaderTest. Added acid_profile test Sample output: Query Compilation: 3s525ms - Metadata load started: 37.369ms (37.369ms) - Metadata load finished. loaded-tables=1/1 ... - Loaded ValidWriteIdLists for transactional tables: functional.insert_only_transactional_table:0:9223372036854775807:: : 3s312ms (551.463us) - Analysis finished: 3s370ms (58.110ms) ... Change-Id: Ifcc31c7ddcfc471b0e5308f7e4aaadfa8189a905 Reviewed-on: http://gerrit.cloudera.org:8080/13736 Reviewed-by: Csaba Ringhofer <csringho...@cloudera.com> Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com> --- .../org/apache/impala/analysis/StmtMetadataLoader.java | 17 +++++++++++------ .../apache/impala/analysis/StmtMetadataLoaderTest.java | 5 ++--- .../queries/QueryTest/acid-profile.test | 18 ++++++++++++++++++ tests/common/skip.py | 6 ++++++ tests/query_test/test_acid.py | 10 +++++++++- 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java b/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java index fd11af1..1ae8f59 100644 --- a/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java +++ b/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java @@ -31,6 +31,7 @@ import org.apache.impala.catalog.FeView; import org.apache.impala.common.InternalException; import org.apache.impala.compat.MetastoreShim; import org.apache.impala.service.Frontend; +import org.apache.impala.util.AcidUtils; import org.apache.impala.util.EventSequence; import org.apache.impala.util.TUniqueIdUtil; import org.slf4j.Logger; @@ -231,19 +232,23 @@ public class StmtMetadataLoader { numCatalogUpdatesReceived_)); if (MetastoreShim.getMajorVersion() > 2) { - StringBuilder validIdsBuf = new StringBuilder("Loaded ValidWriteIdLists: "); + StringBuilder validIdsBuf = new StringBuilder("Loaded ValidWriteIdLists"); + validIdsBuf.append(" for transactional tables: "); + boolean hasAcidTbls = false; for (FeTable iTbl : loadedTbls_.values()) { - validIdsBuf.append("\n"); - validIdsBuf.append(" "); - validIdsBuf.append(iTbl.getValidWriteIds()); + if (AcidUtils.isTransactionalTable(iTbl.getMetaStoreTable().getParameters())) { + validIdsBuf.append("\n"); + validIdsBuf.append(" "); + validIdsBuf.append(iTbl.getValidWriteIds()); + hasAcidTbls = true; + } } validIdsBuf.append("\n"); validIdsBuf.append(" "); - timeline_.markEvent(validIdsBuf.toString()); + if (hasAcidTbls) timeline_.markEvent(validIdsBuf.toString()); } } fe_.getImpaladTableUsageTracker().recordTableUsage(loadedTbls_.keySet()); - return new StmtTableCache(catalog, dbs_, loadedTbls_); } diff --git a/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java b/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java index 9f65022..4632de0 100644 --- a/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java @@ -102,7 +102,7 @@ public class StmtMetadataLoaderTest { } } - + // Assume tables in the stmt are not acid tables. private void validateUncached(StatementBase stmt, Frontend fe, int expectedNumLoadRequests, int expectedNumCatalogUpdates, String[] expectedDbs, String[] expectedTables) throws InternalException { @@ -116,8 +116,7 @@ public class StmtMetadataLoaderTest { Assert.assertEquals(expectedNumCatalogUpdates, mdLoader.getNumCatalogUpdatesReceived()); // Validate timeline. - long expectedNumEvents = MetastoreShim.getMajorVersion() >= 3 ? 3 : 2; - Assert.assertEquals(expectedNumEvents, mdLoader.getTimeline().getNumEvents()); + Assert.assertEquals(2, mdLoader.getTimeline().getNumEvents()); // Validate dbs and tables. validateDbs(stmtTableCache, expectedDbs); validateTables(stmtTableCache, expectedTables); diff --git a/testdata/workloads/functional-query/queries/QueryTest/acid-profile.test b/testdata/workloads/functional-query/queries/QueryTest/acid-profile.test new file mode 100644 index 0000000..598a02b --- /dev/null +++ b/testdata/workloads/functional-query/queries/QueryTest/acid-profile.test @@ -0,0 +1,18 @@ +==== +---- HIVE_QUERY +use $DATABASE; +create table tbl_ld (x int) tblproperties ( + 'transactional'='true', + 'transactional_properties'='insert_only'); + +insert into tbl_ld values (1); +==== +---- QUERY +invalidate metadata tbl_ld; +select * from tbl_ld +---- RESULTS +1 +---- RUNTIME_PROFILE +# Verify that ValidWriteIdLists is in the profile +row_regex: .*Loaded ValidWriteIdLists for transactional tables:.* +==== diff --git a/tests/common/skip.py b/tests/common/skip.py index 335ecc0..c4d03ca 100644 --- a/tests/common/skip.py +++ b/tests/common/skip.py @@ -267,3 +267,9 @@ class SkipIfCatalogV2: return pytest.mark.skipif( IMPALA_TEST_CLUSTER_PROPERTIES.is_catalog_v2_cluster(), reason="IMPALA-7539: support HDFS permission checks for LocalCatalog") + + @classmethod + def hms_event_polling_enabled(self): + return pytest.mark.skipif( + IMPALA_TEST_CLUSTER_PROPERTIES.is_catalog_v2_cluster(), + reason="Table isn't invalidated with Local catalog and enabled hms_event_polling.") diff --git a/tests/query_test/test_acid.py b/tests/query_test/test_acid.py index 74adc5d..d39ba9f 100644 --- a/tests/query_test/test_acid.py +++ b/tests/query_test/test_acid.py @@ -17,7 +17,7 @@ # Functional tests for ACID integration with Hive. from tests.common.impala_test_suite import ImpalaTestSuite -from tests.common.skip import SkipIfHive2 +from tests.common.skip import (SkipIfHive2, SkipIfCatalogV2) from tests.common.test_dimensions import create_single_exec_option_dimension @@ -51,6 +51,14 @@ class TestAcid(ImpalaTestSuite): def test_acid_partitioned(self, vector, unique_database): self.run_test_case('QueryTest/acid-partitioned', vector, use_db=unique_database) + # When local CatalogV2 combines with hms_enent_polling enabled, it seems + # that Catalog loads tables by itself, the query statement cannot trigger + # loading tables. As the ValidWriteIdlists is part of table loading profile, + # it can not be shown in the query profile. Skip CatalogV2 to avoid flaky tests. + @SkipIfHive2.acid + @SkipIfCatalogV2.hms_event_polling_enabled() + def test_acid_profile(self, vector, unique_database): + self.run_test_case('QueryTest/acid-profile', vector, use_db=unique_database) # TODO(todd): further tests to write: # TRUNCATE, once HIVE-20137 is implemented. # INSERT OVERWRITE with empty result set, once HIVE-21750 is fixed.