You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Ashutosh Chauhan <ha...@apache.org> on 2020/05/05 05:04:34 UTC
Re: Review Request 72437: Reduce number of DB calls in
ObjectStore::getPartitionsByExprInternal
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72437/#review220616
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
Line 1548 (original), 1548 (patched)
<https://reviews.apache.org/r/72437/#comment309172>
assert table != null ?
At this point there is no advantage of passing null for partition keys, since if its null then retrieving partitions of this table won't make sense.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
Line 832 (original), 830 (patched)
<https://reviews.apache.org/r/72437/#comment309177>
isView only used for this check here, which can be eliminated.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
Lines 1017 (patched)
<https://reviews.apache.org/r/72437/#comment309173>
assert partitonKeys != null
If they are null, retrieving partitions won't make sense.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 3473-3474 (patched)
<https://reviews.apache.org/r/72437/#comment309174>
This needs to be avoided, else it will generate additional RDBMS queries. It's only needed for getPartitionsViaSqlFilter() and not on other paths. And even there its needed only to make checks which can be avoided altogether. so, there is no need for isViewTable anywhere.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 3832 (patched)
<https://reviews.apache.org/r/72437/#comment309175>
may remove this.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 4077-4078 (patched)
<https://reviews.apache.org/r/72437/#comment309176>
This needs to be avoided, else it will generate additional RDBMS queries. It's only needed for one path and even there its only needed to make checks. isViewTable can be eliminated entirely.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 4287 (patched)
<https://reviews.apache.org/r/72437/#comment309178>
LOG.debug
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 4114 (original), 4307 (patched)
<https://reviews.apache.org/r/72437/#comment309179>
LOG.debug
- Ashutosh Chauhan
On April 27, 2020, 9:15 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72437/
> -----------------------------------------------------------
>
> (Updated April 27, 2020, 9:15 a.m.)
>
>
> Review request for hive, Ashutosh Chauhan, Rajesh Balamohan, and Vineet Garg.
>
>
> Bugs: HIVE-23282
> https://issues.apache.org/jira/browse/HIVE-23282
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> ObjectStore::getPartitionsByExprInternal internally uses Table information for getting partitionKeys, table, catalog name.
>
>
>
> For this, it ends up populating entire table data from DB (including skew column, parameters, sort, bucket cols etc). This makes it a lot more expensive call. It would be good to check if MTable itself can be used instead of Table.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java 4f58cd91efc
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java d1558876f14
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java 53b7a67a429
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java 9834883f00f
>
>
> Diff: https://reviews.apache.org/r/72437/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 72437: Reduce number of DB calls in
ObjectStore::getPartitionsByExprInternal
Posted by Attila Magyar <am...@hortonworks.com>.
> On May 5, 2020, 5:04 a.m., Ashutosh Chauhan wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
> > Line 832 (original), 830 (patched)
> > <https://reviews.apache.org/r/72437/diff/3/?file=2230109#file2230109line839>
> >
> > isView only used for this check here, which can be eliminated.
Not sure what you mean by eliminating it? Removing it altogeather?
- Attila
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72437/#review220616
-----------------------------------------------------------
On April 27, 2020, 9:15 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72437/
> -----------------------------------------------------------
>
> (Updated April 27, 2020, 9:15 a.m.)
>
>
> Review request for hive, Ashutosh Chauhan, Rajesh Balamohan, and Vineet Garg.
>
>
> Bugs: HIVE-23282
> https://issues.apache.org/jira/browse/HIVE-23282
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> ObjectStore::getPartitionsByExprInternal internally uses Table information for getting partitionKeys, table, catalog name.
>
>
>
> For this, it ends up populating entire table data from DB (including skew column, parameters, sort, bucket cols etc). This makes it a lot more expensive call. It would be good to check if MTable itself can be used instead of Table.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java 4f58cd91efc
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java d1558876f14
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java 53b7a67a429
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java 9834883f00f
>
>
> Diff: https://reviews.apache.org/r/72437/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 72437: Reduce number of DB calls in
ObjectStore::getPartitionsByExprInternal
Posted by Ashutosh Chauhan <ha...@apache.org>.
> On May 5, 2020, 5:04 a.m., Ashutosh Chauhan wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
> > Line 832 (original), 830 (patched)
> > <https://reviews.apache.org/r/72437/diff/3/?file=2230109#file2230109line839>
> >
> > isView only used for this check here, which can be eliminated.
>
> Attila Magyar wrote:
> Not sure what you mean by eliminating it? Removing it altogeather?
yes remove this if(), its only purpose is to throw exception. We can remove this to save RDBMS query for isViewTable()
- Ashutosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72437/#review220616
-----------------------------------------------------------
On April 27, 2020, 9:15 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72437/
> -----------------------------------------------------------
>
> (Updated April 27, 2020, 9:15 a.m.)
>
>
> Review request for hive, Ashutosh Chauhan, Rajesh Balamohan, and Vineet Garg.
>
>
> Bugs: HIVE-23282
> https://issues.apache.org/jira/browse/HIVE-23282
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> ObjectStore::getPartitionsByExprInternal internally uses Table information for getting partitionKeys, table, catalog name.
>
>
>
> For this, it ends up populating entire table data from DB (including skew column, parameters, sort, bucket cols etc). This makes it a lot more expensive call. It would be good to check if MTable itself can be used instead of Table.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java 4f58cd91efc
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java d1558876f14
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java 53b7a67a429
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java 9834883f00f
>
>
> Diff: https://reviews.apache.org/r/72437/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Attila Magyar
>
>