You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Karthik Manamcheri via Review Board <no...@reviews.apache.org> on 2019/01/02 20:15:46 UTC
Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object
in PreReadTableEvent to improve get_partition performance
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69642/
-----------------------------------------------------------
(Updated Jan. 2, 2019, 8:15 p.m.)
Review request for hive, Adam Holley, Na Li, Morio Ramdenbourg, Naveen Gangam, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
Repository: hive-git
Description
-------
HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
Diffs
-----
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java a9398ae1e7
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/events/PreReadTableEvent.java beec72bc12
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/ThrowingSupplier.java PRE-CREATION
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java fe64a91b56
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/events/TestPreReadTableEvent.java PRE-CREATION
Diff: https://reviews.apache.org/r/69642/diff/2/
Testing
-------
Unit tests.
Manual performance test with Cloudera BDR to notice improved backup performance.
Thanks,
Karthik Manamcheri
Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object
in PreReadTableEvent to improve get_partition performance
Posted by Karthik Manamcheri via Review Board <no...@reviews.apache.org>.
> On Jan. 3, 2019, 1:42 a.m., Karthik Manamcheri wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java
> > Lines 374 (patched)
> > <https://reviews.apache.org/r/69642/diff/4/?file=2117205#file2117205line374>
> >
> > The behavior of the getPartitions\* changed to be more inline with how the other getTable/getDatabase calls work.
> >
> > Before this change, if you issue a getPartitionsByNames with an empty database, we threw an exception. After this change, we will return an empty list of partitions instead. This behavior is similar to what happens if you issue a getTablesByNames call (an empty list of tables are returned)
>
> Peter Vary wrote:
> This change is also worrying. This is also an API change which might cause backward incompatibility problems with customers expecting empty list.
> We proposed this kind of changes a way back, and the community consensus was that we should not change even the Exception types that have been thrown.
Actually I am fixing a regression bug. In the master branch (without my change), if you remove the TransactionalValidationListener from the list of pre-listeners, the GetPartitions API tests fail! The getPartitions API test depends on the fact there is a a pre-event listener. Customers already expect the API to throw an exception (or return an empty list) depending on if there is a listener attached or not. So the API contract itself is that it can throw an exception (OR return an empty list). This is how earlier versions of Hive are. HIVE-12064 introduced a bug which changed the behavior of clients. My change makes it so that it behaves similar to how earlier versions of Hive behaved (before HIVE-12064).
We should also fix the tests so that they don't depend of the existence (or non-existence) of listeners or plugins.
Basically what I am saying is that this API change is not new and is how Hive 1.x/2.x behaved (before HIVE-12064). We wrote the unit tests around the bug!
- Karthik
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69642/#review211625
-----------------------------------------------------------
On Jan. 3, 2019, 1:40 a.m., Karthik Manamcheri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69642/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2019, 1:40 a.m.)
>
>
> Review request for hive, Adam Holley, Na Li, Morio Ramdenbourg, Naveen Gangam, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
>
>
> Diffs
> -----
>
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java a9398ae1e7
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/events/PreReadTableEvent.java beec72bc12
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/ThrowingSupplier.java PRE-CREATION
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java 7429d18226
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java fe64a91b56
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java 4d7f7c1220
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java a338bd4032
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/events/TestPreReadTableEvent.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69642/diff/4/
>
>
> Testing
> -------
>
> Unit tests.
> Manual performance test with Cloudera BDR to notice improved backup performance.
>
>
> Thanks,
>
> Karthik Manamcheri
>
>
Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object
in PreReadTableEvent to improve get_partition performance
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
> On jan. 3, 2019, 1:42 de, Karthik Manamcheri wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java
> > Lines 374 (patched)
> > <https://reviews.apache.org/r/69642/diff/4/?file=2117205#file2117205line374>
> >
> > The behavior of the getPartitions\* changed to be more inline with how the other getTable/getDatabase calls work.
> >
> > Before this change, if you issue a getPartitionsByNames with an empty database, we threw an exception. After this change, we will return an empty list of partitions instead. This behavior is similar to what happens if you issue a getTablesByNames call (an empty list of tables are returned)
This change is also worrying. This is also an API change which might cause backward incompatibility problems with customers expecting empty list.
We proposed this kind of changes a way back, and the community consensus was that we should not change even the Exception types that have been thrown.
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69642/#review211625
-----------------------------------------------------------
On jan. 3, 2019, 1:40 de, Karthik Manamcheri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69642/
> -----------------------------------------------------------
>
> (Updated jan. 3, 2019, 1:40 de)
>
>
> Review request for hive, Adam Holley, Na Li, Morio Ramdenbourg, Naveen Gangam, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
>
>
> Diffs
> -----
>
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java a9398ae1e7
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/events/PreReadTableEvent.java beec72bc12
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/ThrowingSupplier.java PRE-CREATION
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java 7429d18226
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java fe64a91b56
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java 4d7f7c1220
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java a338bd4032
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/events/TestPreReadTableEvent.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69642/diff/4/
>
>
> Testing
> -------
>
> Unit tests.
> Manual performance test with Cloudera BDR to notice improved backup performance.
>
>
> Thanks,
>
> Karthik Manamcheri
>
>
Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object
in PreReadTableEvent to improve get_partition performance
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
> On jan. 3, 2019, 1:42 de, Karthik Manamcheri wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java
> > Lines 374 (patched)
> > <https://reviews.apache.org/r/69642/diff/4/?file=2117205#file2117205line374>
> >
> > The behavior of the getPartitions\* changed to be more inline with how the other getTable/getDatabase calls work.
> >
> > Before this change, if you issue a getPartitionsByNames with an empty database, we threw an exception. After this change, we will return an empty list of partitions instead. This behavior is similar to what happens if you issue a getTablesByNames call (an empty list of tables are returned)
>
> Peter Vary wrote:
> This change is also worrying. This is also an API change which might cause backward incompatibility problems with customers expecting empty list.
> We proposed this kind of changes a way back, and the community consensus was that we should not change even the Exception types that have been thrown.
>
> Karthik Manamcheri wrote:
> Actually I am fixing a regression bug. In the master branch (without my change), if you remove the TransactionalValidationListener from the list of pre-listeners, the GetPartitions API tests fail! The getPartitions API test depends on the fact there is a a pre-event listener. Customers already expect the API to throw an exception (or return an empty list) depending on if there is a listener attached or not. So the API contract itself is that it can throw an exception (OR return an empty list). This is how earlier versions of Hive are. HIVE-12064 introduced a bug which changed the behavior of clients. My change makes it so that it behaves similar to how earlier versions of Hive behaved (before HIVE-12064).
>
> We should also fix the tests so that they don't depend of the existence (or non-existence) of listeners or plugins.
>
> Basically what I am saying is that this API change is not new and is how Hive 1.x/2.x behaved (before HIVE-12064). We wrote the unit tests around the bug!
Bugs which are exist for too long prone to become features :D
I personally agree with you. but I would be more confortible if we could find 1 more committer to validate our view - maybe Alan Gates, Thejas Nair who were the ones who actively participated in our "IMetaStoreClient and HMS Thrift API exception handling" discussion. You might want to ping them.
Thanks,
Peter
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69642/#review211625
-----------------------------------------------------------
On jan. 3, 2019, 1:40 de, Karthik Manamcheri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69642/
> -----------------------------------------------------------
>
> (Updated jan. 3, 2019, 1:40 de)
>
>
> Review request for hive, Adam Holley, Na Li, Morio Ramdenbourg, Naveen Gangam, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
>
>
> Diffs
> -----
>
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java a9398ae1e7
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/events/PreReadTableEvent.java beec72bc12
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/ThrowingSupplier.java PRE-CREATION
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java 7429d18226
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java fe64a91b56
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java 4d7f7c1220
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java a338bd4032
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/events/TestPreReadTableEvent.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69642/diff/4/
>
>
> Testing
> -------
>
> Unit tests.
> Manual performance test with Cloudera BDR to notice improved backup performance.
>
>
> Thanks,
>
> Karthik Manamcheri
>
>
Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object
in PreReadTableEvent to improve get_partition performance
Posted by Karthik Manamcheri via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69642/#review211625
-----------------------------------------------------------
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java
Lines 374 (patched)
<https://reviews.apache.org/r/69642/#comment297012>
The behavior of the getPartitions\* changed to be more inline with how the other getTable/getDatabase calls work.
Before this change, if you issue a getPartitionsByNames with an empty database, we threw an exception. After this change, we will return an empty list of partitions instead. This behavior is similar to what happens if you issue a getTablesByNames call (an empty list of tables are returned)
- Karthik Manamcheri
On Jan. 3, 2019, 1:40 a.m., Karthik Manamcheri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69642/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2019, 1:40 a.m.)
>
>
> Review request for hive, Adam Holley, Na Li, Morio Ramdenbourg, Naveen Gangam, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
>
>
> Diffs
> -----
>
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java a9398ae1e7
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/events/PreReadTableEvent.java beec72bc12
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/ThrowingSupplier.java PRE-CREATION
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java 7429d18226
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java fe64a91b56
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java 4d7f7c1220
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java a338bd4032
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/events/TestPreReadTableEvent.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69642/diff/4/
>
>
> Testing
> -------
>
> Unit tests.
> Manual performance test with Cloudera BDR to notice improved backup performance.
>
>
> Thanks,
>
> Karthik Manamcheri
>
>
Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object
in PreReadTableEvent to improve get_partition performance
Posted by Karthik Manamcheri via Review Board <no...@reviews.apache.org>.
> On Jan. 3, 2019, 6 p.m., Sergio Pena wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 4549-4555 (original), 4560-4569 (patched)
> > <https://reviews.apache.org/r/69642/diff/4/?file=2117200#file2117200line4560>
> >
> > Isn't simpler to have a constructor that accepts the (catName, dbName, tblName, this) instead of using a Supplier? It will reduce code and make the code more readable.
So I tried implementing with a constrcutor which accepts the catName, dbName, tblName. That actually looks more difficult to understand.
The PreReadTableEvent would become the following
```java
public class PreReadTableEvent extends PreEventContext {
private final AtomicReference<Table> table;
private final String catName;
private final String dbName;
private final String tblName;
/**
* event with pre-fetched table object
* @param table the table object
* @param handler the HMS handler
*/
public PreReadTableEvent(Table table, IHMSHandler handler) {
super(PreEventType.READ_TABLE, handler);
this.table = new AtomicReference<>(table);
this.catName = null;
this.dbName = null;
this.tblName = null;
}
public PreReadTableEvent(String catName, String dbName, String tblName, IHMSHandler handler) {
super(PreEventType.READ_TABLE, handler);
this.table = new AtomicReference<>(null);
this.catName = catName;
this.dbName = dbName;
this.tblName = tblName;
}
private Table getTableFromMs() {
try {
Table t = super.getHandler().getMS().getTable(catName, dbName, tblName);
if (t == null) {
throw new NoSuchObjectException(TableName.getQualified(catName, dbName, tblName)
+ " table not found");
}
return t;
} catch (NoSuchObjectException | MetaException e) {
throw new RuntimeException(e);
}
}
/**
* @return the table
*/
public Table getTable() {
Table retVal = table.get();
if (retVal == null) {
retVal = getTableFromMs();
if (!table.compareAndSet(null, getTableFromMs())) {
return table.get();
}
}
return retVal;
}
}
```
In my opinion, this is harder to read and understand and prone to many errors. We have multiple properties (catName, dbName and tblName) which can either be null or not and depending on that the behavior is different. We also need to have the slightly confusing implementation of getTable() to do the checks and compareAndSet to ensure thread-safety. Suppliers.memoize(..) abstracts many of this from us. I think my current approach is more readable and maintainable.
- Karthik
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69642/#review211643
-----------------------------------------------------------
On Jan. 3, 2019, 1:40 a.m., Karthik Manamcheri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69642/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2019, 1:40 a.m.)
>
>
> Review request for hive, Adam Holley, Na Li, Morio Ramdenbourg, Naveen Gangam, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
>
>
> Diffs
> -----
>
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java a9398ae1e7
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/events/PreReadTableEvent.java beec72bc12
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/ThrowingSupplier.java PRE-CREATION
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java 7429d18226
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java fe64a91b56
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java 4d7f7c1220
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java a338bd4032
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/events/TestPreReadTableEvent.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69642/diff/4/
>
>
> Testing
> -------
>
> Unit tests.
> Manual performance test with Cloudera BDR to notice improved backup performance.
>
>
> Thanks,
>
> Karthik Manamcheri
>
>
Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object
in PreReadTableEvent to improve get_partition performance
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69642/#review211643
-----------------------------------------------------------
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
Lines 4549-4555 (original), 4560-4569 (patched)
<https://reviews.apache.org/r/69642/#comment297044>
Isn't simpler to have a constructor that accepts the (catName, dbName, tblName, this) instead of using a Supplier? It will reduce code and make the code more readable.
- Sergio Pena
On Jan. 3, 2019, 1:40 a.m., Karthik Manamcheri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69642/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2019, 1:40 a.m.)
>
>
> Review request for hive, Adam Holley, Na Li, Morio Ramdenbourg, Naveen Gangam, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
>
>
> Diffs
> -----
>
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java a9398ae1e7
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/events/PreReadTableEvent.java beec72bc12
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/ThrowingSupplier.java PRE-CREATION
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java 7429d18226
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java fe64a91b56
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java 4d7f7c1220
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java a338bd4032
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/events/TestPreReadTableEvent.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69642/diff/4/
>
>
> Testing
> -------
>
> Unit tests.
> Manual performance test with Cloudera BDR to notice improved backup performance.
>
>
> Thanks,
>
> Karthik Manamcheri
>
>
Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object
in PreReadTableEvent to improve get_partition performance
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69642/#review212343
-----------------------------------------------------------
Ship it!
Ship It!
- Peter Vary
On jan. 3, 2019, 1:40 de, Karthik Manamcheri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69642/
> -----------------------------------------------------------
>
> (Updated jan. 3, 2019, 1:40 de)
>
>
> Review request for hive, Adam Holley, Na Li, Morio Ramdenbourg, Naveen Gangam, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
>
>
> Diffs
> -----
>
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java a9398ae1e7
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/events/PreReadTableEvent.java beec72bc12
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/ThrowingSupplier.java PRE-CREATION
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java 7429d18226
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java fe64a91b56
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java 4d7f7c1220
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java a338bd4032
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/events/TestPreReadTableEvent.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69642/diff/4/
>
>
> Testing
> -------
>
> Unit tests.
> Manual performance test with Cloudera BDR to notice improved backup performance.
>
>
> Thanks,
>
> Karthik Manamcheri
>
>
Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object
in PreReadTableEvent to improve get_partition performance
Posted by Karthik Manamcheri via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69642/
-----------------------------------------------------------
(Updated Jan. 3, 2019, 1:40 a.m.)
Review request for hive, Adam Holley, Na Li, Morio Ramdenbourg, Naveen Gangam, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
Repository: hive-git
Description
-------
HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
Diffs (updated)
-----
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java a9398ae1e7
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/events/PreReadTableEvent.java beec72bc12
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/ThrowingSupplier.java PRE-CREATION
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java 7429d18226
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java fe64a91b56
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java 4d7f7c1220
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java a338bd4032
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/events/TestPreReadTableEvent.java PRE-CREATION
Diff: https://reviews.apache.org/r/69642/diff/4/
Changes: https://reviews.apache.org/r/69642/diff/3-4/
Testing
-------
Unit tests.
Manual performance test with Cloudera BDR to notice improved backup performance.
Thanks,
Karthik Manamcheri
Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object
in PreReadTableEvent to improve get_partition performance
Posted by Karthik Manamcheri via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69642/
-----------------------------------------------------------
(Updated Jan. 2, 2019, 8:32 p.m.)
Review request for hive, Adam Holley, Na Li, Morio Ramdenbourg, Naveen Gangam, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
Repository: hive-git
Description
-------
HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
Diffs (updated)
-----
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java a9398ae1e7
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/events/PreReadTableEvent.java beec72bc12
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/ThrowingSupplier.java PRE-CREATION
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java fe64a91b56
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/events/TestPreReadTableEvent.java PRE-CREATION
Diff: https://reviews.apache.org/r/69642/diff/3/
Changes: https://reviews.apache.org/r/69642/diff/2-3/
Testing
-------
Unit tests.
Manual performance test with Cloudera BDR to notice improved backup performance.
Thanks,
Karthik Manamcheri
Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object
in PreReadTableEvent to improve get_partition performance
Posted by Karthik Manamcheri via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69642/#review211612
-----------------------------------------------------------
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
Line 4551 (original), 4554 (patched)
<https://reviews.apache.org/r/69642/#comment296966>
Changed it to NoSuchObjectException. I don't recall why I changed it to a MetaException, but it doesn't need to change.
Regarding the API change, I thought about it more and changed code to not actually change any semantics. With the new change, we'll throw a NoSuchObjectException here which will get wrapped into a RuntimeException to be thrown. This will be called when the pre-event listener is fired. At that point, we can catch the RuntimeException, and rethrow the NoSuchObjectException (as it was doing before this change).
- Karthik Manamcheri
On Jan. 2, 2019, 8:15 p.m., Karthik Manamcheri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69642/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2019, 8:15 p.m.)
>
>
> Review request for hive, Adam Holley, Na Li, Morio Ramdenbourg, Naveen Gangam, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
>
>
> Diffs
> -----
>
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java a9398ae1e7
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/events/PreReadTableEvent.java beec72bc12
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/ThrowingSupplier.java PRE-CREATION
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java fe64a91b56
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/events/TestPreReadTableEvent.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69642/diff/2/
>
>
> Testing
> -------
>
> Unit tests.
> Manual performance test with Cloudera BDR to notice improved backup performance.
>
>
> Thanks,
>
> Karthik Manamcheri
>
>