Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance

2019-01-25 Thread Peter Vary via Review Board 

---
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

2019-01-04 Thread Peter Vary via Review Board 


> 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)
> > 
> >
> > 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

2019-01-03 Thread Karthik Manamcheri via Review Board 


> 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)
> > 
> >
> > 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

2019-01-03 Thread Karthik Manamcheri via Review Board 


> 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)
> > 
> >
> > 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;

  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

2019-01-03 Thread Sergio Pena via Review Board 

---
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)


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

2019-01-03 Thread Peter Vary via Review Board 


> 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)
> > 
> >
> > 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

2019-01-02 Thread Karthik Manamcheri via Review Board 

---
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)


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

2019-01-02 Thread Karthik Manamcheri via Review Board 

---
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

2019-01-02 Thread Karthik Manamcheri via Review Board 

---
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

2019-01-02 Thread Karthik Manamcheri via Review Board 

---
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)


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
> 
>

Re: Review Request 69642: HIVE-20977: Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance

2019-01-02 Thread Karthik Manamcheri via Review Board 

---
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