[
https://issues.apache.org/jira/browse/HIVE-23965?focusedWorklogId=466323&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-466323
]
ASF GitHub Bot logged work on HIVE-23965:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 04/Aug/20 16:52
Start Date: 04/Aug/20 16:52
Worklog Time Spent: 10m
Work Description: kgyrtkirk commented on a change in pull request #1347:
URL: https://github.com/apache/hive/pull/1347#discussion_r465151500
##########
File path:
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/AbstractCliConfig.java
##########
@@ -405,4 +409,11 @@ private String getAbsolutePath(String dir) {
return new File(new File(HIVE_ROOT), dir).getAbsolutePath();
}
+ public String getMetastoreType() {
+ return metastoreType;
+ }
+
+ public void setMetastoreType(String metastoreType) {
Review comment:
setters should be protected in this class
##########
File path: data/conf/tpcds30tb/tez/hive-site.xml
##########
@@ -0,0 +1,214 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="configuration.xsl"?>
+<!--
+ 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.
+-->
+<configuration>
+ <property>
+ <name>hive.in.test</name>
+ <value>true</value>
+ <description>Internal marker for test. Used for masking env-dependent
values</description>
+ </property>
+ <property>
+ <name>hive.rpc.query.plan</name>
+ <value>true</value>
+ </property>
+ <property>
+ <name>hive.llap.execution.mode</name>
+ <value>only</value>
+ </property>
+ <property>
+ <name>hive.exec.post.hooks</name>
+ <value>
+ org.apache.hadoop.hive.ql.hooks.HiveProtoLoggingHook
+ </value>
+ </property>
+ <property>
+ <name>hive.zookeeper.kerberos.enabled</name>
+ <value>false</value>
+ </property>
+ <property>
+ <name>hive.privilege.synchronizer</name>
+ <value>false</value>
+ </property>
+ <property>
+ <name>javax.jdo.option.ConnectionDriverName</name>
+ <value>org.postgresql.Driver</value>
+ </property>
+ <property>
+ <name>hive.exec.scratchdir</name>
+ <value>${test.tmp.dir}/scratchdir</value>
+ <description>Scratch space for Hive jobs</description>
+ </property>
+ <property>
+ <name>hive.server2.tez.sessions.init.threads</name>
+ <value>1</value>
+ </property>
+ <property>
+ <name>hive.optimize.dynamic.partition.hashjoin</name>
+ <value>true</value>
+ </property>
+ <property>
+ <name>hive.exec.orc.split.strategy</name>
+ <value>BI</value>
+ </property>
+ <property>
+ <name>hive.mapjoin.bucket.cache.size</name>
+ <value>10000</value>
+ </property>
+ <property>
+ <name>hive.current.database</name>
+ <value>tpcds_bin_partitioned_orc_30000</value>
+ </property>
+ <property>
+ <name>hive.optimize.metadataonly</name>
+ <value>true</value>
+ </property>
+ <property>
+
<name>hive.vectorized.execution.mapjoin.native.fast.hashtable.enabled</name>
+ <value>true</value>
+ </property>
+ <property>
+ <name>hive.execution.mode</name>
+ <value>llap</value>
+ </property>
+ <property>
+ <name>hive.prewarm.numcontainers</name>
+ <value>3</value>
+ </property>
+ <property>
+ <name>hive.tez.dynamic.partition.pruning</name>
+ <value>true</value>
+ </property>
+ <property>
+ <name>hive.support.concurrency</name>
+ <value>true</value>
+ </property>
+ <property>
+ <name>hive.txn.manager</name>
+ <value>org.apache.hadoop.hive.ql.lockmgr.DbTxnManager</value>
+ </property>
+ <property>
+ <name>hive.llap.kerberos.enabled</name>
+ <value>false</value>
+ </property>
+ <property>
+ <name>hive.load.data.owner</name>
+ <value>hive</value>
+ </property>
+ <property>
+ <name>hive.enforce.sortmergebucketmapjoin</name>
+ <value>true</value>
+ </property>
+ <property>
+ <name>hive.limit.optimize.enable</name>
+ <value>true</value>
+ </property>
+ <property>
+ <name>hive.auto.convert.sortmerge.join.to.mapjoin</name>
+ <value>true</value>
+ </property>
+ <property>
+ <name>hive.txn.acid.dir.cache.duration</name>
+ <value>0</value>
+ </property>
+ <property>
+ <name>hive.optimize.bucketmapjoin</name>
+ <value>true</value>
+ </property>
+ <property>
+ <name>hive.tez.container.size</name>
+ <value>4096</value>
+ </property>
+ <property>
+ <name>hive.stats.fetch.bitvector</name>
+ <value>true</value>
Review comment:
I think we should probably change the defaults for some of these settings
##########
File path:
itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestTezTPCDS30TBCliDriver.java
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.hadoop.hive.cli;
+
+import org.apache.hadoop.hive.cli.control.CliAdapter;
+import org.apache.hadoop.hive.cli.control.CliConfigs;
+import org.apache.hadoop.hive.cli.control.SplitSupport;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TestRule;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+import org.junit.runners.model.Statement;
+
+import java.io.File;
+import java.util.Comparator;
+import java.util.List;
+
+@RunWith(Parameterized.class)
+public class TestTezTPCDS30TBCliDriver {
+
+ static CliAdapter adapter = new
CliConfigs.TezTPCDS30TBCliConfig().getCliAdapter();
+
+ @Parameters(name = "{0}") public static List<Object[]> getParameters()
throws Exception {
Review comment:
nit: newline after annotation
##########
File path: standalone-metastore/pom.xml
##########
@@ -360,7 +360,7 @@
<dependency>
<groupId>org.postgresql</groupId>
<artifactId>postgresql</artifactId>
- <version>9.3-1102-jdbc41</version>
Review comment:
please use a property to set the version
##########
File path:
itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestTezPerfCliDriver.java
##########
@@ -34,7 +34,7 @@
@RunWith(Parameterized.class)
public class TestTezPerfCliDriver {
- static CliAdapter adapter = new
CliConfigs.TezPerfCliConfig(false).getCliAdapter();
+ static CliAdapter adapter = new
CliConfigs.TezCustomTPCDSCliConfig(false).getCliAdapter();
Review comment:
this is changing the naming contract between the configs and the perf
I think because we will have a new set of `PerfCliDriver`s we could just
remove the old ones - or retain the conf names as well..
##########
File path:
itests/util/src/main/java/org/apache/hadoop/hive/ql/MetaStoreDumpUtility.java
##########
@@ -56,6 +56,8 @@
static final Logger LOG =
LoggerFactory.getLogger(MetaStoreDumpUtility.class);
+ // TODO: Change the name of the method or document appropriately
Review comment:
I think we should remove the old stuff ....in that case explanation is
also unneccessary :D
##########
File path: ql/src/test/queries/clientpositive/perf/cbo_query74.q
##########
@@ -1,3 +1,4 @@
+--! qt:disabled:HIVE-23963
Review comment:
this will disable this test in all drivers - in case we will only use
the new driver it will be clearer...
##########
File path: itests/qtest/pom.xml
##########
@@ -420,7 +420,7 @@
<dependency>
<groupId>org.postgresql</groupId>
<artifactId>postgresql</artifactId>
- <version>9.3-1102-jdbc41</version>
+ <version>42.2.14</version>
Review comment:
could we use a property for this version?
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
##########
@@ -1743,7 +1743,9 @@ public void setLocalMapRedErrors(Map<String,
List<String>> localMapRedErrors) {
public String getCurrentDatabase() {
if (currentDatabase == null) {
- currentDatabase = DEFAULT_DATABASE_NAME;
+ currentDatabase = sessionConf.getVar(ConfVars.HIVE_CURRENT_DATABASE);
Review comment:
I don't think we should do this - why do we need this change?
##########
File path:
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/TPCDSMetastoreCliDriver.java
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.hadoop.hive.cli.control;
+
+import org.apache.hadoop.hive.ql.MetaStoreDumpUtility;
+import org.apache.hadoop.hive.ql.QTestSystemProperties;
+
+/**
+ * CliDriver initialising the metastore with basic table and column statistics
from various TPC-DS datasets.
+ *
+ * The table statistics do not reflect cardinalities from a specific TPC-DS
scale factor (SF). Below we outline the
+ * tables and the TPC-DS SF to which it corresponds.
+ *
+ * Tables in 30TB dataset (SF=30000):
+ * <ul>
+ * <li>call_center</li>
+ * <li>catalog_page</li>
+ * <li>customer</li>
+ * <li>customer_address</li>
+ * <li>item</li>
+ * <li>promotions</li>
+ * <li>store</li>
+ * <li>warehouse</li>
+ * <li>web_page</li>
+ * <li>web_site</li>
+ * </ul>
+ *
+ * Tables in 200GB dataset (SF=300):
+ * <ul>
+ * <li>catalog_returns</li>
+ * <li>catalog_sales</li>
+ * <li>store_returns</li>
+ * <li>store_sales</li>
+ * <li>web_sales</li>
+ * </ul>
+ *
+ * Table in 3GB dataset (SF=3): inventory
+ * Table with zero rows: ship_mode
+ *
+ * The fact that tables are in datasets with different scale factors is most
likely a bug but given that this driver
+ * has been in use for quite some time now it may make sense to keep around
for a while.
+ */
+public class TPCDSMetastoreCliDriver extends CorePerfCliDriver {
Review comment:
could we add `Core` to the name of the class?
I think the important thing here is that it works with tpcds - so I would
also recommend to remove the "Metastore" keyword from the name of the class
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
##########
@@ -1743,7 +1743,9 @@ public void setLocalMapRedErrors(Map<String,
List<String>> localMapRedErrors) {
public String getCurrentDatabase() {
if (currentDatabase == null) {
- currentDatabase = DEFAULT_DATABASE_NAME;
+ currentDatabase = sessionConf.getVar(ConfVars.HIVE_CURRENT_DATABASE);
+ if (StringUtils.isEmpty(currentDatabase))
Review comment:
nit: braces
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 466323)
Time Spent: 20m (was: 10m)
> Improve plan regression tests using TPCDS30TB metastore dump and custom
> configs
> -------------------------------------------------------------------------------
>
> Key: HIVE-23965
> URL: https://issues.apache.org/jira/browse/HIVE-23965
> Project: Hive
> Issue Type: Improvement
> Reporter: Stamatis Zampetakis
> Assignee: Stamatis Zampetakis
> Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The existing regression tests (HIVE-12586) based on TPC-DS have certain
> shortcomings:
> The table statistics do not reflect cardinalities from a specific TPC-DS
> scale factor (SF). Some tables are from a 30TB dataset, others from 200GB
> dataset, and others from a 3GB dataset. This mix leads to plans that may
> never appear when using an actual TPC-DS dataset.
> The existing statistics do not contain information about partitions something
> that can have a big impact on the resulting plans.
> The existing regression tests rely on more or less on the default
> configuration (hive-site.xml). In real-life scenarios though some of the
> configurations differ and may impact the choices of the optimizer.
> This issue aims to address the above shortcomings by using a curated
> TPCDS30TB metastore dump along with some custom hive configurations.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
