You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2019/11/04 02:54:33 UTC

[Impala-ASF-CR] IMPALA-8974: Fixed a bug when create kudu managed table without HMS config

Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14398 )

Change subject: IMPALA-8974: Fixed a bug when create kudu managed table without HMS config
......................................................................


Patch Set 8:

(7 comments)

Sorry for my late reply. I've also added some logics for using customized hive-site.xml in https://gerrit.cloudera.org/c/14527 (you may need a rebase to see the latest changes), which can simplify some works here, e.g. don't need TEST_PREPEND_CLASSPATH.

The changes are small but it's a big topic that configuring catalogd without HMS (not sure if I understand correctly). Does it work as expected after this patch, or there're other relative broken preconditions also need to fix? I'm quite curious about this scenario and would be appreciated if you could share more!

http://gerrit.cloudera.org:8080/#/c/14398/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14398/8//COMMIT_MSG@7
PS8, Line 7: without HMS
           : config
This looks like a new scenario that we haven't thought about. Does it mean the HMS clients (in catalogd) can work as normal without HMS in all APIs? Looking forward to see if we can deploy Impala+Kudu without HMS and HDFS in the future!

BTW, may worth to test with Hive-3 by exporting USE_CDP_HIVE=true and rebuilding Impala.


http://gerrit.cloudera.org:8080/#/c/14398/8/bin/start-daemon.sh
File bin/start-daemon.sh:

http://gerrit.cloudera.org:8080/#/c/14398/8/bin/start-daemon.sh@32
PS8, Line 32:   CLASSPATH=$TEST_PREPEND_CLASSPATH:$CLASSPATH
We can use CUSTOM_CLASSPATH which is used in bin/set-classpath.sh


http://gerrit.cloudera.org:8080/#/c/14398/8/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/14398/8/bin/start-impala-cluster.py@136
PS8, Line 136: parser.add_option("--test_prepend_classpath", dest="test_prepend_classpath", default="",
Can be replacted by using --env_vars="CUSTOM_CLASSPATH=xxx" after rebasing this patch onto master.


http://gerrit.cloudera.org:8080/#/c/14398/8/fe/src/test/java/org/apache/impala/customcluster/CreateKuduTableWithoutHMSTest.java
File fe/src/test/java/org/apache/impala/customcluster/CreateKuduTableWithoutHMSTest.java:

http://gerrit.cloudera.org:8080/#/c/14398/8/fe/src/test/java/org/apache/impala/customcluster/CreateKuduTableWithoutHMSTest.java@40
PS8, Line 40:     
nit: the indention in this file should be two spaces instead of four.


http://gerrit.cloudera.org:8080/#/c/14398/8/fe/src/test/java/org/apache/impala/customcluster/CreateKuduTableWithoutHMSTest.java@43
PS8, Line 43: cmds
nit: cmds_. Class field names should end with "_" in our code style.


http://gerrit.cloudera.org:8080/#/c/14398/8/fe/src/test/java/org/apache/impala/customcluster/CreateKuduTableWithoutHMSTest.java@100
PS8, Line 100:         cmds.add(restart_cmd);
nit: Use CustomClusterRunner.StartImpalaCluster(args) at last instead?


http://gerrit.cloudera.org:8080/#/c/14398/8/fe/src/test/resources/hive-site-without-hms.xml.py
File fe/src/test/resources/hive-site-without-hms.xml.py:

http://gerrit.cloudera.org:8080/#/c/14398/8/fe/src/test/resources/hive-site-without-hms.xml.py@20
PS8, Line 20: # This python script is used for CreateKuduTableWithoutHMSTest.java
It'd be better to use this script in bin/create-test-configuration.sh too. So future tests can reuse the generated hive-site.xml.



-- 
To view, visit http://gerrit.cloudera.org:8080/14398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacc53801a660c033869cb4747910c98a80e08297
Gerrit-Change-Number: 14398
Gerrit-PatchSet: 8
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Mon, 04 Nov 2019 02:54:33 +0000
Gerrit-HasComments: Yes