> On June 14, 2016, 1:29 a.m., Mohit Sabharwal wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, > > line 3179 > > <https://reviews.apache.org/r/48233/diff/2/?file=1417191#file1417191line3179> > > > > Since we are moving the functionality from driver to HMS, should we > > deprecate > > hive.limit.query.max.table.partition and introduce a new config called > > hive.metastore.retrieve.max.partitions ? > > > > All metastore configs have "hive.metastore" prefix. > > > > Otherwise: > > 1) The change is backward incompatible for existing users that > > are setting this config at HS2 level and are now expected to set it > > at HMS level to get the same functionality. > > 2) Name would be confusing. > > > > We could do the following: > > 1) Mark hive.limit.query.max.table.partition as deprecated in HiveConf > > and suggest that user > > move to hive.metastore.retrieve.max.partitions > > 2) Do not remove the functionality associated with > > hive.limit.query.max.table.partition in PartitionPruner. > > It does do what the description promises - i.e. fail the query before > > execution stage if number of > > partitions associated with any scan operator exceed configured value. > > 3) Add new config hive.metastore.retrieve.max.partitions to configure > > functionality in this patch. > > > > Makes sense ?
Thanks. It makes sense. I will use "hive.metastore.limit.partition.request". I saw in the hive code that sometimes users can request up to a max # of partitions from the metastore without failing. So I do not want to cause confusion with the name. - Sergio ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48233/#review137434 ----------------------------------------------------------- On June 13, 2016, 6:28 p.m., Sergio Pena wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48233/ > ----------------------------------------------------------- > > (Updated June 13, 2016, 6:28 p.m.) > > > Review request for hive, Mohit Sabharwal and Naveen Gangam. > > > Bugs: HIVE-13884 > https://issues.apache.org/jira/browse/HIVE-13884 > > > Repository: hive-git > > > Description > ------- > > The patch verifies the # of partitions a table has before fetching any from > the metastore. I > t checks that limit from 'hive.limit.query.max.table.partition'. > > A limitation added here is that the variable must be on hive-site.xml in > order to work, and it does not accept to set this through beeline because > HiveMetaStore.java does not read the variables set through beeline. I think > it is better to keep it this way to avoid users changing the value on fly, > and crashing the metastore. > > Another change is that EXPLAIN commands won't be executed either. EXPLAIN > commands need to fetch partitions in order to create the operator tree. If we > allow EXPLAIN to do that, then we may have the same OOM situations for large > partitions. > > > Diffs > ----- > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > c0827ea9d47e569d9697649a7e16d196de3de14d > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java > c135179b97354108f842a5ca2de0c6f0ef28b7fc > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > f98de1326956b19b9d28fc9b1fcdede8d851180d > metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java > a6d3f5385b33b8a4e31ee20ca5cb8f58c97c8702 > metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java > 31f0d7b89670b8a749bbe8a7ff2b4ff9f059a8e2 > > metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java > 3152e77c3c7152ac4dbe7e779ce35f28044fe3c9 > > metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java > 86a243609b23e2ca9bb8849f0da863a95e477d5c > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > cd3c86064df3e7febcc16e03aab6ce407e0dc8a0 > > Diff: https://reviews.apache.org/r/48233/diff/ > > > Testing > ------- > > Waiting for HiveQA. > > > Thanks, > > Sergio Pena > >