Re: Review Request 68547: SENTRY-2370: Create a cache of PathUpdates to send to NN

2018-09-27 Thread Arjun Mishra via Review Board 

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

(Updated Sept. 27, 2018, 10:29 p.m.)


Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Changes
---

Post feedback


Bugs: SENTRY-2370
https://issues.apache.org/jira/browse/SENTRY-2370


Repository: sentry


Description
---

When NN requests path updates from sentry and if it exceeds the time threshold, 
on consecutive attempts sentry will attempt to fetch the full update from 
scratch. Instead it should cache it and update the cache before sending it to NN


Diffs (updated)
-

  
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ImageCache.java
 PRE-CREATION 
  
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java
 2d21411e2 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
 08b16a4df 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathsUpdateImageCache.java
 PRE-CREATION 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermissionsUpdateImageCache.java
 PRE-CREATION 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
 b8f5ce7db 
  
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDBUpdateForwarder.java
 f86ce6f83 


Diff: https://reviews.apache.org/r/68547/diff/6/

Changes: https://reviews.apache.org/r/68547/diff/5-6/


Testing
---


Thanks,

Arjun Mishra

Re: Migrating existing loggers to Lombok

2018-09-27 Thread Na Li 
Option 1  works for me

Sent from my iPhone

> On Sep 27, 2018, at 1:04 PM, Brian Towles  
> wrote:
> 
> I feel that since this doesn't really provide any difference in
> functionality, just only a change to how the LOGGER variable gets defined
> and initialized, that it doesn't have to be an in mass change and can be
> done in an incremental and as needed fashion.
> 
> On Tue, Sep 25, 2018 at 1:54 PM Kalyan Kumar Kalvagadda
>  wrote:
> 
>> I prefer option-1.  Changing complete code involves change a ton of class.
>> It's a huge effort. Even if you take option-2 if would take a very only
>> time to change all the classes. Let's be practical.
>> 
>> 
>> *Thanks,Kalyan Kumar Kalvagadda* | Software Engineer
>> t. (469) 279- <00>5732
>> cloudera.com 
>> 
>> [image: Cloudera] 
>> 
>> [image: Cloudera on Twitter]  [image:
>> Cloudera on Facebook]  [image: Cloudera
>> on LinkedIn] 
>> --
>> 
>> 
>> On Tue, Sep 25, 2018 at 1:49 PM Na Li 
>> wrote:
>> 
>>> Stephen,
>>> 
>>> I prefer option 2), so our code can be more consistent.
>>> 
>>> Thanks,
>>> 
>>> Lina
>>> 
>>> On Tue, Sep 25, 2018 at 11:30 AM Stephen Moist
>> >>> 
>>> wrote:
>>> 
 Hey all, with Sentry-2374, we’ve introduced Lombok to create our
>> loggers
 in the SPI portion of Sentry.  For consistency’s sake, we should
>> migrate
 the existing Sentry code to use the Lombok annotations instead of using
>>> the
 existing logger.getLogger(class).  I see 2 ways of doing it.  1) As
>>> classes
 are modified, add in the annotation and remove the existing logger.  2)
 Create an epic in Jira, create tasks under it for each maven module,
>> then
 go through all the classes in each module and convert to it.
 
 I don’t think this is critical or time sensitive to do, but it should
>> be
 more of a code cleanup task than anything else that we can do over the
>>> next
 few months.  Thoughts on this?
>>> 
>> 
> -- 
> *Brian Towles* | Software Engineer
> t. (512) 415- <00>8105 e. btow...@cloudera.com 
> cloudera.com 
> 
> [image: Cloudera] 
> 
> [image: Cloudera on Twitter]  [image:
> Cloudera on Facebook]  [image: Cloudera
> on LinkedIn] 
> --

Re: Migrating existing loggers to Lombok

2018-09-27 Thread Brian Towles 
I feel that since this doesn't really provide any difference in
functionality, just only a change to how the LOGGER variable gets defined
and initialized, that it doesn't have to be an in mass change and can be
done in an incremental and as needed fashion.

On Tue, Sep 25, 2018 at 1:54 PM Kalyan Kumar Kalvagadda
 wrote:

> I prefer option-1.  Changing complete code involves change a ton of class.
> It's a huge effort. Even if you take option-2 if would take a very only
> time to change all the classes. Let's be practical.
>
>
> *Thanks,Kalyan Kumar Kalvagadda* | Software Engineer
> t. (469) 279- <00>5732
> cloudera.com 
>
> [image: Cloudera] 
>
> [image: Cloudera on Twitter]  [image:
> Cloudera on Facebook]  [image: Cloudera
> on LinkedIn] 
> --
>
>
> On Tue, Sep 25, 2018 at 1:49 PM Na Li 
> wrote:
>
> > Stephen,
> >
> > I prefer option 2), so our code can be more consistent.
> >
> > Thanks,
> >
> > Lina
> >
> > On Tue, Sep 25, 2018 at 11:30 AM Stephen Moist
>  > >
> > wrote:
> >
> > > Hey all, with Sentry-2374, we’ve introduced Lombok to create our
> loggers
> > > in the SPI portion of Sentry.  For consistency’s sake, we should
> migrate
> > > the existing Sentry code to use the Lombok annotations instead of using
> > the
> > > existing logger.getLogger(class).  I see 2 ways of doing it.  1) As
> > classes
> > > are modified, add in the annotation and remove the existing logger.  2)
> > > Create an epic in Jira, create tasks under it for each maven module,
> then
> > > go through all the classes in each module and convert to it.
> > >
> > > I don’t think this is critical or time sensitive to do, but it should
> be
> > > more of a code cleanup task than anything else that we can do over the
> > next
> > > few months.  Thoughts on this?
> >
>
-- 
*Brian Towles* | Software Engineer
t. (512) 415- <00>8105 e. btow...@cloudera.com 
cloudera.com 

[image: Cloudera] 

[image: Cloudera on Twitter]  [image:
Cloudera on Facebook]  [image: Cloudera
on LinkedIn] 
--

Re: [VOTE] Release Sentry version 2.1.0

2018-09-27 Thread Na Li 
Sergio,

It is at https://dist.apache.org/repos/dist/dev/sentry/2.1.0/

Sorry, I did not list it in previous email.

Thanks,

Lina

On Thu, Sep 27, 2018 at 9:20 AM Sergio Pena
 wrote:

> Lina, where are the .tar.gz packages for 2.1.0 located?
>
> - Sergio
>
> On Tue, Sep 25, 2018 at 1:24 PM Na Li 
> wrote:
>
> > This is the release of Apache Sentry, version 2.1.0.
> > It fixes the following issues:
> >
> > https://issues.apache.org/jira/projects/SENTRY/versions/12342213
> >
> >
> > Maven artifacts are available
> >
> > https://repository.apache.org/content/repositories/orgapachesentry-1015/
> >
> > Tag to be voted
> > onhttps://
> >
> git-wip-us.apache.org/repos/asf/sentry/?p=sentry.git;a=tag;h=refs/tags/release-2.1.0
> >
> >
> > Sentry's KEYS containing the PGP key we used to sign the release:
> > http://www.apache.org/dist/sentry/KEYS
> >
> >  We are voting on the source:tag=release-2.1.0, SHA=
> >
> > c5af6f0b81481b640b29d6ed9dde2ad1a399e882 (You can get the hash of the
> > tag by doing
> >
> >  "git rev-list release-2.1.0 | head -n 1" )
> >
> >
> > Vote will be open for 72 hours.
> >
> > [ ] +1 approve
> > [ ] +0 no opinion
> > [ ] -1 disapprove (and reason why)
> >
> > Thanks,
> >
> >
> > Lina
> >
>

Re: Review Request 68727: SENTRY-2306: Reduce the HMS snapshot size by ignoring partitions at default locations

2018-09-27 Thread Sergio Pena via Review Board 

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



Changes look good. But I'd like to know two things before approving it:

1. Could you verify Lina's concerns (see JIRA) about what would happen if a 
partition or table location changes in the NotificationProcessor? If a 
partition location that has the same prefix as the table changes to be out of 
the table location, would this affect the HDFS ACLs?
2. Could you run a benchmark with and without this patch to see the improvement 
in numbers?

- Sergio Pena


On Sept. 24, 2018, 10:28 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68727/
> ---
> 
> (Updated Sept. 24, 2018, 10:28 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Na Li, and 
> Sergio Pena.
> 
> 
> Bugs: SENTRY-2306
> https://issues.apache.org/jira/browse/SENTRY-2306
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Ignore the partitions entries which are located in default location as they 
> share the same permissions as the table. This will reduce the snapshot size 
> decreasing the time to persist the snapshot and also time to send the full 
> update to Namenode. This is important as the partition information has lion’s 
> share in a HMS Full snapshot.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  1eda41b2b6bd940a404cc1ba09a861fe783ead04 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java
>  0b4f4aa24bd3002c50bf4d80a6fa361f66052973 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
>  3e27d1bbee9bea9a60e7f7e012a367957610826c 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  b3a4934dfc523d14eec216df00a6b7597c66c166 
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestFullUpdateInitializer.java
>  589acbed12855ff09309a04c9214f8daf87ea1de 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  3d7fbe3fea333d58e46cd721d610c7d8050c9de4 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  574bc4b0432824abc81fe3c0dd30d1fe01f012e5 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentryService.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
>  9fa42f2de41b6700a18a358f8d5e442104dd0585 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java
>  0e82b86a727f578013a63c005aa05279790344f1 
> 
> 
> Diff: https://reviews.apache.org/r/68727/diff/4/
> 
> 
> Testing
> ---
> 
> Made sure all the existing tests passed and also added unit and e2e tests to 
> verify the new behavior.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: [VOTE] Release Sentry version 2.1.0

2018-09-27 Thread Sergio Pena 
Lina, where are the .tar.gz packages for 2.1.0 located?

- Sergio

On Tue, Sep 25, 2018 at 1:24 PM Na Li  wrote:

> This is the release of Apache Sentry, version 2.1.0.
> It fixes the following issues:
>
> https://issues.apache.org/jira/projects/SENTRY/versions/12342213
>
>
> Maven artifacts are available
>
> https://repository.apache.org/content/repositories/orgapachesentry-1015/
>
> Tag to be voted
> onhttps://
> git-wip-us.apache.org/repos/asf/sentry/?p=sentry.git;a=tag;h=refs/tags/release-2.1.0
>
>
> Sentry's KEYS containing the PGP key we used to sign the release:
> http://www.apache.org/dist/sentry/KEYS
>
>  We are voting on the source:tag=release-2.1.0, SHA=
>
> c5af6f0b81481b640b29d6ed9dde2ad1a399e882 (You can get the hash of the
> tag by doing
>
>  "git rev-list release-2.1.0 | head -n 1" )
>
>
> Vote will be open for 72 hours.
>
> [ ] +1 approve
> [ ] +0 no opinion
> [ ] -1 disapprove (and reason why)
>
> Thanks,
>
>
> Lina
>