Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-10-08 Thread Karthik Manamcheri via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/#review209313
---




standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
Lines 515 (patched)


What happens if there is a comma in the list of exclude patterns?



standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
Lines 939 (patched)


Maybe I am misunderstanding the reduce, but shoudln't you be "AND"ing all 
the predicates?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
Lines 323 (patched)


Is there a reason why this filter is applied only to Table or Partition 
object?


- Karthik Manamcheri


On Oct. 2, 2018, 5:19 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Oct. 2, 2018, 5:19 a.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f81292b0db54f4eb82468191fda38f9a0d4 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c6b10a4f9494e49a42282cf90027ad7 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/MessageFactory.java
>  7ff168f7931f91fe17f7d38df848ba2eed33c463 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b053205f48226da442ce65fcc2d7f6e76763 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
>  30de1c4cfa1cf019186b10583a06da0bf5491634 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-10-01 Thread Bharathkrishna Guruvayoor Murali via Review Board 


> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 918 (patched)
> > 
> >
> > You already have this check when you call this function.
> 
> Bharathkrishna Guruvayoor Murali wrote:
> I thought this could be used separately as a utility method, if the user 
> has just one predicate. Should I just keep the other method which accepts a 
> list of predicates and make this one private?

Closing this issue. Maintaining both methods as they can be used as separate 
utilities if needed.


- Bharathkrishna


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/#review209055
---


On Oct. 2, 2018, 5:19 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Oct. 2, 2018, 5:19 a.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f81292b0db54f4eb82468191fda38f9a0d4 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c6b10a4f9494e49a42282cf90027ad7 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/MessageFactory.java
>  7ff168f7931f91fe17f7d38df848ba2eed33c463 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b053205f48226da442ce65fcc2d7f6e76763 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
>  30de1c4cfa1cf019186b10583a06da0bf5491634 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-10-01 Thread Bharathkrishna Guruvayoor Murali via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/
---

(Updated Oct. 2, 2018, 5:19 a.m.)


Review request for hive and Alexander Kolbasov.


Changes
---

I added init() method to MessageFactory, and overriding it in 
JSONMessageFactory to initialize the static variables. Although the exception 
traces look mostly similar, I guess this adds a bit more clarity and 
information about the exception if the regex is invalid.


Bugs: HIVE-20545
https://issues.apache.org/jira/browse/HIVE-20545


Repository: hive-git


Description
---

Clients can add large-sized parameters in Table/Partition objects. So we need 
to enable adding regex patterns through HiveConf to match parameters to be 
filtered from table and partition objects before serialization in HMS 
notifications.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 30ea7f81292b0db54f4eb82468191fda38f9a0d4 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 c681a87a1c6b10a4f9494e49a42282cf90027ad7 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/MessageFactory.java
 7ff168f7931f91fe17f7d38df848ba2eed33c463 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 2668b053205f48226da442ce65fcc2d7f6e76763 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
 30de1c4cfa1cf019186b10583a06da0bf5491634 


Diff: https://reviews.apache.org/r/68827/diff/5/

Changes: https://reviews.apache.org/r/68827/diff/4-5/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-10-01 Thread Bharathkrishna Guruvayoor Murali via Review Board 


> On Sept. 26, 2018, 5:51 p.m., Andrew Sherman wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
> > Line 297 (original), 310 (patched)
> > 
> >
> > This is used by the notifications that (we think) we understand, but it 
> > is also used by JSONAcidWriteMessage. So what happens if someone uses your 
> > new mechanism to reduce the size of messages, but affects 
> > JSONAcidWriteMessage? In other words there could be multile uses for 
> > notifications in a complex system, and this mechanism affects them all.

For now, I feel this change is reasonable and when there is a specific 
requirement that it should not filter parameters from other types of 
messages(if any), it can be taken up later on as an improvement.


- Bharathkrishna


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/#review209031
---


On Sept. 30, 2018, 6:55 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Sept. 30, 2018, 6:55 p.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f81292b0db54f4eb82468191fda38f9a0d4 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c6b10a4f9494e49a42282cf90027ad7 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b053205f48226da442ce65fcc2d7f6e76763 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
>  30de1c4cfa1cf019186b10583a06da0bf5491634 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-30 Thread Bharathkrishna Guruvayoor Murali via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/
---

(Updated Sept. 30, 2018, 6:55 p.m.)


Review request for hive and Alexander Kolbasov.


Changes
---

corrected the typo.


Bugs: HIVE-20545
https://issues.apache.org/jira/browse/HIVE-20545


Repository: hive-git


Description
---

Clients can add large-sized parameters in Table/Partition objects. So we need 
to enable adding regex patterns through HiveConf to match parameters to be 
filtered from table and partition objects before serialization in HMS 
notifications.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 30ea7f81292b0db54f4eb82468191fda38f9a0d4 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 c681a87a1c6b10a4f9494e49a42282cf90027ad7 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 2668b053205f48226da442ce65fcc2d7f6e76763 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
 30de1c4cfa1cf019186b10583a06da0bf5491634 


Diff: https://reviews.apache.org/r/68827/diff/4/

Changes: https://reviews.apache.org/r/68827/diff/3-4/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-28 Thread Andrew Sherman via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/#review209102
---




standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
Lines 517 (patched)


typo: th


- Andrew Sherman


On Sept. 28, 2018, 4:59 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Sept. 28, 2018, 4:59 p.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f81292b0db54f4eb82468191fda38f9a0d4 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c6b10a4f9494e49a42282cf90027ad7 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b053205f48226da442ce65fcc2d7f6e76763 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
>  30de1c4cfa1cf019186b10583a06da0bf5491634 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-28 Thread Bharathkrishna Guruvayoor Murali via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/
---

(Updated Sept. 28, 2018, 4:59 p.m.)


Review request for hive and Alexander Kolbasov.


Changes
---

Added unit-tests which uses MetaStoreConf To make sure it works with default 
values


Bugs: HIVE-20545
https://issues.apache.org/jira/browse/HIVE-20545


Repository: hive-git


Description
---

Clients can add large-sized parameters in Table/Partition objects. So we need 
to enable adding regex patterns through HiveConf to match parameters to be 
filtered from table and partition objects before serialization in HMS 
notifications.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 30ea7f81292b0db54f4eb82468191fda38f9a0d4 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 c681a87a1c6b10a4f9494e49a42282cf90027ad7 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 2668b053205f48226da442ce65fcc2d7f6e76763 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
 30de1c4cfa1cf019186b10583a06da0bf5491634 


Diff: https://reviews.apache.org/r/68827/diff/3/

Changes: https://reviews.apache.org/r/68827/diff/2-3/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-28 Thread Andrew Sherman via Review Board 


> On Sept. 26, 2018, 5:51 p.m., Andrew Sherman wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 917 (patched)
> > 
> >
> > make private
> 
> Bharathkrishna Guruvayoor Murali wrote:
> I thought this could be used separately as a utility method, if the user 
> has just one predicate. Should I just keep the other method which accepts a 
> list of predicates and make this one private?

IDK, not super important


> On Sept. 26, 2018, 5:51 p.m., Andrew Sherman wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
> > Lines 66 (patched)
> > 
> >
> > add test with the default param map from MetastoreConf
> 
> Bharathkrishna Guruvayoor Murali wrote:
> An empty exclude pattern will remove all elements from the map! But when 
> it is done through configuration, the default value returns an empty array, 
> like new String[0].
> Hence, the predicates become an empty List. What can I do if needed to 
> add a test for this.

What I mean is a test that works from a default  MetaConf object. This make the 
test more close to reality.


- Andrew


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/#review209031
---


On Sept. 28, 2018, 10:17 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Sept. 28, 2018, 10:17 a.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f81292b0db54f4eb82468191fda38f9a0d4 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c6b10a4f9494e49a42282cf90027ad7 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b053205f48226da442ce65fcc2d7f6e76763 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
>  30de1c4cfa1cf019186b10583a06da0bf5491634 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-28 Thread Bharathkrishna Guruvayoor Murali via Review Board 


> On Sept. 26, 2018, 5:51 p.m., Andrew Sherman wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
> > Lines 66 (patched)
> > 
> >
> > add test with the default param map from MetastoreConf

An empty exclude pattern will remove all elements from the map! But when it is 
done through configuration, the default value returns an empty array, like new 
String[0].
Hence, the predicates become an empty List. What can I do if needed to add a 
test for this.


- Bharathkrishna


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/#review209031
---


On Sept. 28, 2018, 10:17 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Sept. 28, 2018, 10:17 a.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f81292b0db54f4eb82468191fda38f9a0d4 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c6b10a4f9494e49a42282cf90027ad7 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b053205f48226da442ce65fcc2d7f6e76763 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
>  30de1c4cfa1cf019186b10583a06da0bf5491634 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-28 Thread Bharathkrishna Guruvayoor Murali via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/
---

(Updated Sept. 28, 2018, 10:17 a.m.)


Review request for hive and Alexander Kolbasov.


Bugs: HIVE-20545
https://issues.apache.org/jira/browse/HIVE-20545


Repository: hive-git


Description
---

Clients can add large-sized parameters in Table/Partition objects. So we need 
to enable adding regex patterns through HiveConf to match parameters to be 
filtered from table and partition objects before serialization in HMS 
notifications.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 30ea7f81292b0db54f4eb82468191fda38f9a0d4 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 c681a87a1c6b10a4f9494e49a42282cf90027ad7 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 2668b053205f48226da442ce65fcc2d7f6e76763 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
 30de1c4cfa1cf019186b10583a06da0bf5491634 


Diff: https://reviews.apache.org/r/68827/diff/2/

Changes: https://reviews.apache.org/r/68827/diff/1-2/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-28 Thread Bharathkrishna Guruvayoor Murali via Review Board 


> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
> > Lines 44 (patched)
> > 
> >
> > It would be better to use hamcrest asserts and just check that you map 
> > matches your expected map with a single assert.

Fixed.


- Bharathkrishna


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/#review209055
---


On Sept. 28, 2018, 9:38 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Sept. 28, 2018, 9:38 a.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f8129 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b05320 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-28 Thread Bharathkrishna Guruvayoor Murali via Review Board 


> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 917 (patched)
> > 
> >
> > I think a better name would be something like truncateMapByKey.
> > 
> > Can you guarantee that it always receives non-null map? Then you don't 
> > need to check for null.
> > 
> > Since you don't care about values, can this be a generic map ` > T>`?

Since this is a utility method, I wanted to check for null as it could be 
called from other classes in future.
Making the Map


> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 918 (patched)
> > 
> >
> > You already have this check when you call this function.

I thought this could be used separately as a utility method, if the user has 
just one predicate. Should I just keep the other method which accepts a list of 
predicates and make this one private?


> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 931 (patched)
> > 
> >
> > Naming - may be something like 'truncateMapByKeys'?

filter sounds better to me than truncate! I changed method name to 
filterMapKeys. Let me know if it is fine.


> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
> > Line 297 (original), 310 (patched)
> > 
> >
> > The default exclude filter is empty. When someone sets this variable 
> > they should understand what they are doing.

I added a better description for variable in MetastoreConf. Please let know if 
that is sufficient.


> On Sept. 26, 2018, 11:20 p.m., Alexander Kolbasov wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
> > Lines 37 (patched)
> > 
> >
> > Can you add a test with an empty exclude pattern?

An empty exclude pattern will remove all elements from the map! But when it is 
done through configuration, the default value returns an empty array, like new 
String[0].
Hence, the predicates become an empty List. What can I do if needed to add a 
test for this.


- Bharathkrishna


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/#review209055
---


On Sept. 28, 2018, 9:38 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Sept. 28, 2018, 9:38 a.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f8129 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b05320 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-28 Thread Bharathkrishna Guruvayoor Murali via Review Board 


> On Sept. 26, 2018, 5:51 p.m., Andrew Sherman wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 913 (patched)
> > 
> >
> > Good doumentation!
> > Are you 100% sure that this Map is never sused by anyone else? What 
> > about future code?

By future code, do you mean for someone who might use this in future?
Currently, I have added a note in CreateTableObjJson to warn that the Map could 
be filtered based on regexes provided.


> On Sept. 26, 2018, 5:51 p.m., Andrew Sherman wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
> > Lines 917 (patched)
> > 
> >
> > make private

I thought this could be used separately as a utility method, if the user has 
just one predicate. Should I just keep the other method which accepts a list of 
predicates and make this one private?


- Bharathkrishna


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/#review209031
---


On Sept. 28, 2018, 9:38 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68827/
> ---
> 
> (Updated Sept. 28, 2018, 9:38 a.m.)
> 
> 
> Review request for hive and Alexander Kolbasov.
> 
> 
> Bugs: HIVE-20545
> https://issues.apache.org/jira/browse/HIVE-20545
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Clients can add large-sized parameters in Table/Partition objects. So we need 
> to enable adding regex patterns through HiveConf to match parameters to be 
> filtered from table and partition objects before serialization in HMS 
> notifications.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  30ea7f8129 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
>  c681a87a1c 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  2668b05320 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68827/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-28 Thread Bharathkrishna Guruvayoor Murali via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68827/
---

(Updated Sept. 28, 2018, 9:38 a.m.)


Review request for hive and Alexander Kolbasov.


Summary (updated)
-

HIVE-20545 : Exclude large-sized parameters from serialization of Table and 
Partition thrift objects in HMS notifications


Bugs: HIVE-20545
https://issues.apache.org/jira/browse/HIVE-20545


Repository: hive-git


Description
---

Clients can add large-sized parameters in Table/Partition objects. So we need 
to enable adding regex patterns through HiveConf to match parameters to be 
filtered from table and partition objects before serialization in HMS 
notifications.


Diffs
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 30ea7f8129 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 c681a87a1c 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 2668b05320 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/68827/diff/1/


Testing
---


Thanks,

Bharathkrishna Guruvayoor Murali