Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13757 )
Change subject: [docs] update Hive Metastore integration and Impala integration docs ...................................................................... Patch Set 4: (31 comments) http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc File docs/hive_metastore.adoc: http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@92 PS2, Line 92: (as such, the Hive Metastore : will create directories for Kudu tables that will be empty) Without knowing more about how the HMS works, this doesn't follow logically from the previous sentence. In any case, it's probably better as a NOTE or WARNING. http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@93 PS2, Line 93: Though Kudu and the HMS : catalog could go out of sync from time to time (due to : link:https://issues.apache.org/jira/browse/KUDU-2475[KUDU-2475]). Probably better as a WARNING, and also describe what user-facing things can lead to this. Crashed master? What else? http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@99 PS2, Line 99: ntry). nit: remove http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@102 PS2, Line 102: table tables http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@102 PS2, Line 102: Dropping When dropping http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@103 PS2, Line 103: the data and the table truly are dropped maybe: the table's data is dropped at its source? dropped in Kudu? http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@104 PS2, Line 104: Moreover, external table can be named differently : as the underlying Kudu table, while internal table always named the same as the Kudu : table. How about: External tables may refer to tables by a name that is different from the name of the table stored in Kudu, while internal tables must use the same name as that stored in Kudu. http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@106 PS2, Line 106: with Hive Metastore integration, Kudu only automatically synchronizes : metadata for internal tables. maybe: since external tables do not necessarily need to be synchonrized with Kudu, the Hive Metastore integration and tooling will only automatically synchronized metadata for internal tables. http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@108 PS2, Line 108: information. information about table types in Impala. http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@143 PS2, Line 143: (Kerberos) nit: remove http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@143 PS2, Line 143: which `hive_metastore_sasl_enabled` in which `--hive_metastore_sasl_enabled` http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@144 PS2, Line 144: `hive_metastore_kerberos_principal` `--hive_metastore_kerberos_principal` http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@144 PS2, Line 144: primary portion What is the primary portion? Maybe give some example configs. http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@144 PS2, Line 144: to true, `hive_metastore_kerberos_principal` must match the primary portion of : hive.metastore.kerberos.principal backticks http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@154 PS2, Line 154: or during the normal operation of a Kudu cluster. http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@163 PS2, Line 163: require the full list of master addresses to be specified: : Is the issue about ports, or is it about what the values of the master addresses field in the HMS entries? http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@166 PS2, Line 166: How about "the Kudu HMS tooling". Or "`kudu hms` tool"? Same elsewhere http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@168 PS2, Line 168: maybe: the Kudu admin user (commonly "kudu") http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@168 PS2, Line 168: drop http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@170 PS2, Line 170: cluster is specified. : done by configuring the user as a trusted user via the `--trusted_user_acl` Kudu Master configuration. http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@176 PS2, Line 176: user as trusted in the masters (via : Maybe clarify what is deemed a Kudu table, e.g. "...for all Kudu tables, as indicated by their presence in the Kudu catalog." E.g. to clarify that this doesn't take into account the storage handler? Or does it? http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@204 PS2, Line 204: any automatically-fixable issues, for instance, by creating a table entry in : the HMS for each Kudu table that How about, "Before enabling the Kudu-HMS integration" http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@206 PS2, Line 206: cuting it. When no automatic fix is : available, it will make suggestions on remove parens and start a new sentence instead http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@210 PS2, Line 210: the reasons documente Prepare for the Upgrade http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@215 PS2, Line 215: e Hive Metastores. It is disco using the hms list tool? http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@218 PS2, Line 218: pgrading Existing Tab Perform the Upgrade http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@228 PS2, Line 228: available, IMO optional is pretty different from conditional. This must be done, and you mention the condition, so maybe drop "Optional" http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@237 PS2, Line 237: Same here, I don't think we should say the below steps are optional. Instead, how about "skip to step 7" or something. http://gerrit.cloudera.org:8080/#/c/13757/2/docs/hive_metastore.adoc@291 PS2, Line 291: nit: maybe link to hive_metastore.adoc#enabling-the-hive-metastore-integration http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc File docs/hive_metastore.adoc: http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@149 PS4, Line 149: needs to have read and write privileges on directories the HMS created for Kudu tables. How about: needs to be able to access directories created by the HMS http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@151 PS4, Line 151: command remove -- To view, visit http://gerrit.cloudera.org:8080/13757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8726a39d56c4e9954f208700e99e7bcf2bbc290d Gerrit-Change-Number: 13757 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Alex Rodoni <[email protected]> Gerrit-Reviewer: Alexey Serbin <[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-Reviewer: Priyanka Chheda <[email protected]> Gerrit-Comment-Date: Tue, 02 Jul 2019 01:10:56 +0000 Gerrit-HasComments: Yes
