Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11798 )

Change subject: [docs] add Hive Metastore integration
......................................................................


Patch Set 6:

(8 comments)

Looking good, just a few more nits.

http://gerrit.cloudera.org:8080/#/c/11798/6/docs/hive_metastore.adoc
File docs/hive_metastore.adoc:

http://gerrit.cloudera.org:8080/#/c/11798/6/docs/hive_metastore.adoc@76
PS6, Line 76: The Hive Metastore does not enforce case sensitivity for table 
names,
            : consequently Kudu is also case-insensitive when integrated with 
the Hive
            : Metastore. Kudu will prevent tables from being created when a 
table only differs
            : by case. Additionally, operations that open, alter, or drop 
tables are
            : case-insensitive for both the database and table name portion of 
the Kudu
            : table name when the integration is enabled.
nit reword a bit: "Additionally, the HMS does not enforce case sensitivity for 
table name identifiers. As such, when enabled, Kudu will follow suit and 
disallow tables from being created when one already exists whose table name 
identifier differs only by case. Operations that open, alter, or drop tables 
will also be case-insensitive for the table name identifiers."


http://gerrit.cloudera.org:8080/#/c/11798/6/docs/hive_metastore.adoc@86
PS6, Line 86: as well as allow
tiny nit: "and to allow"


http://gerrit.cloudera.org:8080/#/c/11798/6/docs/hive_metastore.adoc@87
PS6, Line 87: `hive-site.xml`
nit: missing space


http://gerrit.cloudera.org:8080/#/c/11798/6/docs/hive_metastore.adoc@119
PS6, Line 119: note: Kudu tables with names that conflict only by case will 
prevent Kudu master(s)
             : from starting up when the integration is enabled. Be sure to 
rename one of the
             : conflicting tables before enabling the Hive Metastore 
integration.
Can you make this a warning, and move this up to just before "## Enabling the 
Hive Metastore Integration"? It'd be nice to have this warning _before_ going 
through all the steps that might end in a crash.

also nit reword it slightly: "WARNING: Given the case insensitivity upon 
enabling the integration, if multiple Kudu tables exist whose names only differ 
by case, the Kudu master(s) will fail to start up. Be sure to rename such 
conflicting tables before enabling the Hive Metastore integration."


http://gerrit.cloudera.org:8080/#/c/11798/6/docs/hive_metastore.adoc@125
PS6, Line 125: When the Hive Metastore integration is enabled on a Kudu cluster 
with existing
             : tables, an additional process, as a first step, must be 
completed to ensure the
             : tables are appropriately added to the Hive Metastore catalog, 
via running the
             : administrative tool described in the next section. This will 
also require
             : renaming the table(s) to conform to the Hive table name 
identifier constraints
             : described above. Failure to do so may result in metadata 
inconsistencies
             : between Kudu and the HMS, such as existing Kudu tables not being 
present in the
             : HMS and thus not being discoverable by external HMS-aware 
components (e.g. Sentry).
             : Moreover, the existing Impala tables will have outdated metadata 
in their HMS
             : entries and may be rendered unusable when Hive Metastore 
integration is enabled
             : in Kudu.
I think here might be a good place to call out what the integration is doing, 
since it kind of justifies the next couple sections:

"When the Hive Metastore integration is enabled, Kudu will automatically 
synchronize changes to Kudu tables between Kudu and the HMS. As such, it is 
important that the Kudu and HMS start with a consistent view of existing 
tables, using the administrative tools described in the next section. This may 
entail renaming Kudu tables to conform to the Hive naming constraints described 
above. Failure to do so may result in metadata inconsistencies between Kudu and 
the HMS, such as existing Kudu tables not being present in the HMS and, thus, 
not being discoverable or useable by external, HMS-aware components (e.g. 
Sentry, Impala)."


http://gerrit.cloudera.org:8080/#/c/11798/6/docs/hive_metastore.adoc@156
PS6, Line 156: automatically-fixable issues. For instance, by creating a Hive 
Metastore
             : table for each Kudu table which is missing one.
nit: make this one sentence: "...automatically-fixable issues, for instance, by 
creating a table entry in the HMS for each Kudu table that doesn't already have 
one."


http://gerrit.cloudera.org:8080/#/c/11798/6/docs/hive_metastore.adoc@159
PS6, Line 159: how manual fix can help.
             :
nit: "how a manual fix can help."


http://gerrit.cloudera.org:8080/#/c/11798/6/docs/hive_metastore.adoc@161
PS6, Line 161: granied
nit: "grained"



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12939c8f2245450ad46898c2050451b090c7ea01
Gerrit-Change-Number: 11798
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alex Rodoni <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 14 Dec 2018 01:26:47 +0000
Gerrit-HasComments: Yes

Reply via email to