Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14018 )
Change subject: Prepare for upgrading to Hive 3 ...................................................................... Patch Set 1: (8 comments) http://gerrit.cloudera.org:8080/#/c/14018/1//COMMIT_MSG Commit Message: PS1: > Just curious: why are you wrapping your commit messages so aggressively? Th No specific reason, the edit window was recently adjusted. Switched back. http://gerrit.cloudera.org:8080/#/c/14018/1//COMMIT_MSG@13 PS1, Line 13: initialized > initialize Done http://gerrit.cloudera.org:8080/#/c/14018/1//COMMIT_MSG@17 PS1, Line 17: MiniHMS > Nit: MiniHms (or change MiniHms above to MiniHMS). Done http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/hms/hms_catalog.cc File src/kudu/hms/hms_catalog.cc: http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/hms/hms_catalog.cc@233 PS1, Line 233: // NOTE: LIKE filters are used instead of = filters due to HIVE-21614 > Does this have a tangible impact on performance (on a non-Derby HMS)? Any w I did a bit of research and it seems that the effect of LIKE vs = should be near negligible when special characters like % are not used. In the case of Kudu, this is always the case. http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/hms/mini_hms.cc File src/kudu/hms/mini_hms.cc: http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/hms/mini_hms.cc@158 PS1, Line 158: : // Set HADOOP_OS_TYPE=Linux due to HADOOP-8719. : // Remove after HADOOP-15966 is available (Hadoop 3.1.3+) : env_vars.insert({ "HADOOP_OS_TYPE", "Linux" }); > Can this be combined into the env_vars declaration above? Done http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/hms/mini_hms.cc@310 PS1, Line 310: > Please rationalize these new parameters above. Done http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/hms/mini_hms.cc@320 PS1, Line 320: : <property> : <name>hive.async.log.enabled</name> : <value>false</value> : </property> > I experimented with this but didn't find that it made any difference w.r.t. I could remove it, it was precautionary more than anything. http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/util/subprocess.cc File src/kudu/util/subprocess.cc: http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/util/subprocess.cc@726 PS1, Line 726: p.SetEnvVars(env_vars); > Could you pass env_vars by value and std::move() it here? Done -- To view, visit http://gerrit.cloudera.org:8080/14018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If43ae2330b3d99374c68bae313a3f8bc070f9c69 Gerrit-Change-Number: 14018 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 07 Aug 2019 18:52:54 +0000 Gerrit-HasComments: Yes
