Alexey Serbin 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:

(20 comments)

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@102
PS4, Line 102: table
Kudu tables ?


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@102
PS4, Line 102: Dropping an internal table from
             : Impala, the data and the table truly are dropped
maybe rephrase as: Dropping an internal Kudu table from Impala also drops the 
underlying Kudu table as well


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@141
PS4, Line 141: match hive.metastore.sasl.enabled
Match that property from what source?


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@143
PS4, Line 143: hive_metastore_sasl_enabled
Is this hive.metastore.sasl.enabled property from some XML file or this is 
about --hive_metastore_sasl_enabled master's run-time flag?


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@144
PS4, Line 144: hive_metastore_kerberos_principal
--hive_metastore_kerberos_principal master's run-time flag ?


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@153
PS4, Line 153: * Restart the Kudu master(s).
Does it make sense to mention that Impala also need to be restarted if using 
it?  Or that's not required?

If the latter, then I remember I saw some issues with turning HMS support on 
when Impala was not restarted, so probably I need to figure out what went wrong 
there (at least to clarify what was wrong).


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@159
PS4, Line 159: any
Drop 'any': I know there are some issues which cannot be fixed with those tools.


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@162
PS4, Line 162: `hms` tools
nit: The `hms` tools ?


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@178
PS4, Line 178: read and write
nit: the read and the write ?


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@179
PS4, Line 179: which can be done in the Hive Metastore using
             : `sentry.metastore.service.users` flag
nit: for clarity, move into a separate sentence, something like

To grant ... , add the `kudu` user into the HMS's 
`sentry.metastore.service.users` property.


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@185
PS4, Line 185: all
I'm not sure what 'all' means here, but the tool doesn't list anything for Kudu 
tables that were created via Kudu API when HMS integration was disabled.

Maybe, just drop 'all' ?


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@206
PS4, Line 206: before
instead of


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@209
PS4, Line 209: The `kudu hms fix` tool will not automatically fix Impala 
external tables
             : for the reasons documented above
Then maybe it makes sense to provide detailed instructions w.r.t. Kudu external 
tables in Impala?


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@259
PS4, Line 259: does not report
             :   any inconsistent catalog warnings
nit: does not report any catalog inconsistencies ?


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@277
PS4, Line 277: fix
Is this about 'kudu hms fix' or it's about 'kudu hms check', actually?  I 
thought this was about 'kudu hms check'.  Correct me if I'm wrong.


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@277
PS4, Line 277: . (Optional) If `kudu hms fix` tool reports inconsistent 
catalog, perform a dry-run of the
             :   `kudu hms fix` tool to understand how the tool will attempt to 
address the automatically-fixable
             :    issues.
How does the operator know which issues are fixable and which are not in 
general?  For example, it seems the `hms fix` tool has the option to 
automatically rename the tables: `--fix_inconsistent_tables`.  However, the 
'hms check' tool recommends to rename the tables manually.


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@306
PS4, Line 306: --ignore_other_clusters=true
Here and elsehwhere: why to add the options which are set to the specified 
values by default?


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@309
PS4, Line 309: drop_orphan_hms_tables
nit: --drop_orphan_hms_tables


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@311
PS4, Line 311: The workaround is to downgrade
             : the tables using `kudu hms downgrade` tool and then rerun `kudu 
hms fix` tool.
This assumes the new tables are using the new Kudu table storage handler.  
However, those talbes might be using the legacy handler instead.

Would dropping those tables via impala-shell be a more appropriate workaround, 
regardless of the version of Kudu handler?


http://gerrit.cloudera.org:8080/#/c/13757/4/docs/hive_metastore.adoc@314
PS4, Line 314: . Recreate external tables manually using Impala Shell.
             :
             : . Enable the Hive Metastore Integration as described in the 
section above.
Should these two be swapped?



--
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 00:12:52 +0000
Gerrit-HasComments: Yes

Reply via email to