Re: Review Request 69076: SENTRY-2301: Log where sentry stands in the snapshot fetching process, periodically

2019-02-05 Thread Na Li via Review Board 

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


Ship it!




Ship It!

- Na Li


On Jan. 29, 2019, 9:09 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69076/
> ---
> 
> (Updated Jan. 29, 2019, 9:09 p.m.)
> 
> 
> Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
> Sergio Pena.
> 
> 
> Bugs: SENTRY-2301
> https://issues.apache.org/jira/browse/SENTRY-2301
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> When sentry is fetching snapshot from HMS, it should log periodically on 
> where it stands in the snapshot process. This will help person debugging it 
> and help him understand the progress.
> 
>  
> 
> This is important as this process could take magnitude of minutes when the 
> HMS data is huge.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java
>  2d21411e2 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  534bb51c1 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
>  4ff3dc91a 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  4baeb6725 
> 
> 
> Diff: https://reviews.apache.org/r/69076/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69573: SENTRY-2477: When requesting for deltas check if nn seq num is 1 more than latest sequence num

2019-02-05 Thread Na Li via Review Board 

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




sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Line 136 (original), 136 (patched)


Can you change the comment to include the situation that "seqNum > 
curSeqNum +1", and under what situation, it may happen? 

You don't need to wait for my addition approval before commiting the change


- Na Li


On Dec. 17, 2018, 5:07 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69573/
> ---
> 
> (Updated Dec. 17, 2018, 5:07 p.m.)
> 
> 
> Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
> Sergio Pena.
> 
> 
> Bugs: SENTRY-2477
> https://issues.apache.org/jira/browse/SENTRY-2477
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> If NN seq number and latest sentry sequence number is larger than 1 we need 
> to request a full update
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
>  08b16a4df 
>   
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDBUpdateForwarder.java
>  f86ce6f83 
> 
> 
> Diff: https://reviews.apache.org/r/69573/diff/1/
> 
> 
> Testing
> ---
> 
> $ mvn -f sentry-hdfs/sentry-hdfs-service/pom.xml test 
> -Dtest=TestDBUpdateForwarder
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69573: SENTRY-2477: When requesting for deltas check if nn seq num is 1 more than latest sequence num

2019-02-05 Thread Na Li via Review Board 

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


Ship it!




Ship It!

- Na Li


On Dec. 17, 2018, 5:07 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69573/
> ---
> 
> (Updated Dec. 17, 2018, 5:07 p.m.)
> 
> 
> Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
> Sergio Pena.
> 
> 
> Bugs: SENTRY-2477
> https://issues.apache.org/jira/browse/SENTRY-2477
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> If NN seq number and latest sentry sequence number is larger than 1 we need 
> to request a full update
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
>  08b16a4df 
>   
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDBUpdateForwarder.java
>  f86ce6f83 
> 
> 
> Diff: https://reviews.apache.org/r/69573/diff/1/
> 
> 
> Testing
> ---
> 
> $ mvn -f sentry-hdfs/sentry-hdfs-service/pom.xml test 
> -Dtest=TestDBUpdateForwarder
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69875: SENTRY-2205: Improve Sentry NN Logging

2019-02-05 Thread kalyan kumar kalvagadda via Review Board 

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


Ship it!




- kalyan kumar kalvagadda


On Feb. 5, 2019, 12:01 a.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Feb. 5, 2019, 12:01 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Haley Reeve, and Na Li.
> 
> 
> Bugs: SENTRY-2205
> https://issues.apache.org/jira/browse/SENTRY-2205
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Logging will help us identify below
> 1. Track the updates from Sentry
> 2. What the path update is?
> 3. What the perm update is?
> 4. How the path change is effecting the exizting cache and what is the result 
> of the update?
> 5. How the perm change is effecting the exizting cache and what is the result 
> of the update?
> 6. Changes to the path tree stored as part of the cache.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
>  97a04d9eb5a1e0c5feb97b038b8fc0edec9b0d38 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryAuthzUpdate.java
>  db11c1e1ad7d11d936bcb5f5d29ad105471394d7 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
>  bbf3f13e44ca3ee6ecb1d635abb9048942b26ac3 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
>  c967f4e2ee0a811e04362aef452b1cfddfd973f3 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
>  9dae03562aff82574dcda5339df415193655d5a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
>  5726c0e834e6e5ad47f47f3337d9317ee1366955 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
>  9115697088cab8fd1732086963c7c3f982069380 
> 
> 
> Diff: https://reviews.apache.org/r/69875/diff/3/
> 
> 
> Testing
> ---
> 
> As there is no functiuonal change I have not added any tests. I made sure 
> that existing tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 69875: SENTRY-2205: Improve Sentry NN Logging

2019-02-05 Thread kalyan kumar kalvagadda via Review Board 


> On Feb. 5, 2019, 11:13 p.m., Arjun Mishra wrote:
> > Left last bits of comments. Please fix them and shit it

Ok. I will fix them and push the changes.


- kalyan kumar


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


On Feb. 5, 2019, 12:01 a.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Feb. 5, 2019, 12:01 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Haley Reeve, and Na Li.
> 
> 
> Bugs: SENTRY-2205
> https://issues.apache.org/jira/browse/SENTRY-2205
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Logging will help us identify below
> 1. Track the updates from Sentry
> 2. What the path update is?
> 3. What the perm update is?
> 4. How the path change is effecting the exizting cache and what is the result 
> of the update?
> 5. How the perm change is effecting the exizting cache and what is the result 
> of the update?
> 6. Changes to the path tree stored as part of the cache.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
>  97a04d9eb5a1e0c5feb97b038b8fc0edec9b0d38 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryAuthzUpdate.java
>  db11c1e1ad7d11d936bcb5f5d29ad105471394d7 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
>  bbf3f13e44ca3ee6ecb1d635abb9048942b26ac3 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
>  c967f4e2ee0a811e04362aef452b1cfddfd973f3 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
>  9dae03562aff82574dcda5339df415193655d5a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
>  5726c0e834e6e5ad47f47f3337d9317ee1366955 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
>  9115697088cab8fd1732086963c7c3f982069380 
> 
> 
> Diff: https://reviews.apache.org/r/69875/diff/3/
> 
> 
> Testing
> ---
> 
> As there is no functiuonal change I have not added any tests. I made sure 
> that existing tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 69875: SENTRY-2205: Improve Sentry NN Logging

2019-02-05 Thread Arjun Mishra via Review Board 

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




sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 519 (original), 542 (patched)


clearAuthzObjs() is setting authzObjs to null. Can we add that to the log 
message: 
LOG.debug("Entry with path: {} is changed to DIR. Setting authzObjs to 
null", this.getFullPath());



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Lines 211 (patched)


This is not part of the logging. Please add. Right now you are just 
returning the sequenceInfo but not logging it


- Arjun Mishra


On Feb. 5, 2019, 12:01 a.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Feb. 5, 2019, 12:01 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Haley Reeve, and Na Li.
> 
> 
> Bugs: SENTRY-2205
> https://issues.apache.org/jira/browse/SENTRY-2205
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Logging will help us identify below
> 1. Track the updates from Sentry
> 2. What the path update is?
> 3. What the perm update is?
> 4. How the path change is effecting the exizting cache and what is the result 
> of the update?
> 5. How the perm change is effecting the exizting cache and what is the result 
> of the update?
> 6. Changes to the path tree stored as part of the cache.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
>  97a04d9eb5a1e0c5feb97b038b8fc0edec9b0d38 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryAuthzUpdate.java
>  db11c1e1ad7d11d936bcb5f5d29ad105471394d7 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
>  bbf3f13e44ca3ee6ecb1d635abb9048942b26ac3 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
>  c967f4e2ee0a811e04362aef452b1cfddfd973f3 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
>  9dae03562aff82574dcda5339df415193655d5a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
>  5726c0e834e6e5ad47f47f3337d9317ee1366955 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
>  9115697088cab8fd1732086963c7c3f982069380 
> 
> 
> Diff: https://reviews.apache.org/r/69875/diff/3/
> 
> 
> Testing
> ---
> 
> As there is no functiuonal change I have not added any tests. I made sure 
> that existing tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 69875: SENTRY-2205: Improve Sentry NN Logging

2019-02-05 Thread Arjun Mishra via Review Board 

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


Ship it!




- Arjun Mishra


On Feb. 5, 2019, 12:01 a.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Feb. 5, 2019, 12:01 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Haley Reeve, and Na Li.
> 
> 
> Bugs: SENTRY-2205
> https://issues.apache.org/jira/browse/SENTRY-2205
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Logging will help us identify below
> 1. Track the updates from Sentry
> 2. What the path update is?
> 3. What the perm update is?
> 4. How the path change is effecting the exizting cache and what is the result 
> of the update?
> 5. How the perm change is effecting the exizting cache and what is the result 
> of the update?
> 6. Changes to the path tree stored as part of the cache.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
>  97a04d9eb5a1e0c5feb97b038b8fc0edec9b0d38 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryAuthzUpdate.java
>  db11c1e1ad7d11d936bcb5f5d29ad105471394d7 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
>  bbf3f13e44ca3ee6ecb1d635abb9048942b26ac3 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
>  c967f4e2ee0a811e04362aef452b1cfddfd973f3 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
>  9dae03562aff82574dcda5339df415193655d5a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
>  5726c0e834e6e5ad47f47f3337d9317ee1366955 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
>  9115697088cab8fd1732086963c7c3f982069380 
> 
> 
> Diff: https://reviews.apache.org/r/69875/diff/3/
> 
> 
> Testing
> ---
> 
> As there is no functiuonal change I have not added any tests. I made sure 
> that existing tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 69875: SENTRY-2205: Improve Sentry NN Logging

2019-02-05 Thread Arjun Mishra via Review Board 

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


Ship it!




Left last bits of comments. Please fix them and shit it

- Arjun Mishra


On Feb. 5, 2019, 12:01 a.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Feb. 5, 2019, 12:01 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Haley Reeve, and Na Li.
> 
> 
> Bugs: SENTRY-2205
> https://issues.apache.org/jira/browse/SENTRY-2205
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Logging will help us identify below
> 1. Track the updates from Sentry
> 2. What the path update is?
> 3. What the perm update is?
> 4. How the path change is effecting the exizting cache and what is the result 
> of the update?
> 5. How the perm change is effecting the exizting cache and what is the result 
> of the update?
> 6. Changes to the path tree stored as part of the cache.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
>  97a04d9eb5a1e0c5feb97b038b8fc0edec9b0d38 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryAuthzUpdate.java
>  db11c1e1ad7d11d936bcb5f5d29ad105471394d7 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
>  bbf3f13e44ca3ee6ecb1d635abb9048942b26ac3 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
>  c967f4e2ee0a811e04362aef452b1cfddfd973f3 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
>  9dae03562aff82574dcda5339df415193655d5a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
>  5726c0e834e6e5ad47f47f3337d9317ee1366955 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
>  9115697088cab8fd1732086963c7c3f982069380 
> 
> 
> Diff: https://reviews.apache.org/r/69875/diff/3/
> 
> 
> Testing
> ---
> 
> As there is no functiuonal change I have not added any tests. I made sure 
> that existing tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 69875: SENTRY-2205: Improve Sentry NN Logging

2019-02-05 Thread kalyan kumar kalvagadda via Review Board 


> On Feb. 5, 2019, 8:25 p.m., kalyan kumar kalvagadda wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
> > Line 124 (original), 124 (patched)
> > 
> >
> > why? What is missing new the one?
> > Looking at the previous one can not know what numbers mean. With new 
> > one, you can see what is the perm sequence number ,path sequence number and 
> > path image number.
> 
> Arjun Mishra wrote:
> There are 3 {} but 4 arguments. You need one more {} for perm updates 
> size. Also nit pick: Can you change NUmber to Number

I see, Will fix.


- kalyan kumar


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


On Feb. 5, 2019, 12:01 a.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Feb. 5, 2019, 12:01 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Haley Reeve, and Na Li.
> 
> 
> Bugs: SENTRY-2205
> https://issues.apache.org/jira/browse/SENTRY-2205
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Logging will help us identify below
> 1. Track the updates from Sentry
> 2. What the path update is?
> 3. What the perm update is?
> 4. How the path change is effecting the exizting cache and what is the result 
> of the update?
> 5. How the perm change is effecting the exizting cache and what is the result 
> of the update?
> 6. Changes to the path tree stored as part of the cache.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
>  97a04d9eb5a1e0c5feb97b038b8fc0edec9b0d38 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryAuthzUpdate.java
>  db11c1e1ad7d11d936bcb5f5d29ad105471394d7 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
>  bbf3f13e44ca3ee6ecb1d635abb9048942b26ac3 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
>  c967f4e2ee0a811e04362aef452b1cfddfd973f3 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
>  9dae03562aff82574dcda5339df415193655d5a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
>  5726c0e834e6e5ad47f47f3337d9317ee1366955 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
>  9115697088cab8fd1732086963c7c3f982069380 
> 
> 
> Diff: https://reviews.apache.org/r/69875/diff/3/
> 
> 
> Testing
> ---
> 
> As there is no functiuonal change I have not added any tests. I made sure 
> that existing tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 69875: SENTRY-2205: Improve Sentry NN Logging

2019-02-05 Thread Arjun Mishra via Review Board 

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




sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
Line 123 (original), 125 (patched)


Why do you need both isDebugEnabled and isTraceEnabled?


- Arjun Mishra


On Feb. 5, 2019, 12:01 a.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Feb. 5, 2019, 12:01 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Haley Reeve, and Na Li.
> 
> 
> Bugs: SENTRY-2205
> https://issues.apache.org/jira/browse/SENTRY-2205
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Logging will help us identify below
> 1. Track the updates from Sentry
> 2. What the path update is?
> 3. What the perm update is?
> 4. How the path change is effecting the exizting cache and what is the result 
> of the update?
> 5. How the perm change is effecting the exizting cache and what is the result 
> of the update?
> 6. Changes to the path tree stored as part of the cache.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
>  97a04d9eb5a1e0c5feb97b038b8fc0edec9b0d38 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryAuthzUpdate.java
>  db11c1e1ad7d11d936bcb5f5d29ad105471394d7 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
>  bbf3f13e44ca3ee6ecb1d635abb9048942b26ac3 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
>  c967f4e2ee0a811e04362aef452b1cfddfd973f3 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
>  9dae03562aff82574dcda5339df415193655d5a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
>  5726c0e834e6e5ad47f47f3337d9317ee1366955 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
>  9115697088cab8fd1732086963c7c3f982069380 
> 
> 
> Diff: https://reviews.apache.org/r/69875/diff/3/
> 
> 
> Testing
> ---
> 
> As there is no functiuonal change I have not added any tests. I made sure 
> that existing tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 69875: SENTRY-2205: Improve Sentry NN Logging

2019-02-05 Thread Arjun Mishra via Review Board 


> On Feb. 5, 2019, 8:25 p.m., kalyan kumar kalvagadda wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
> > Line 124 (original), 124 (patched)
> > 
> >
> > why? What is missing new the one?
> > Looking at the previous one can not know what numbers mean. With new 
> > one, you can see what is the perm sequence number ,path sequence number and 
> > path image number.

There are 3 {} but 4 arguments. You need one more {} for perm updates size. 
Also nit pick: Can you change NUmber to Number


- Arjun


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


On Feb. 5, 2019, 12:01 a.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Feb. 5, 2019, 12:01 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Haley Reeve, and Na Li.
> 
> 
> Bugs: SENTRY-2205
> https://issues.apache.org/jira/browse/SENTRY-2205
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Logging will help us identify below
> 1. Track the updates from Sentry
> 2. What the path update is?
> 3. What the perm update is?
> 4. How the path change is effecting the exizting cache and what is the result 
> of the update?
> 5. How the perm change is effecting the exizting cache and what is the result 
> of the update?
> 6. Changes to the path tree stored as part of the cache.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
>  97a04d9eb5a1e0c5feb97b038b8fc0edec9b0d38 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryAuthzUpdate.java
>  db11c1e1ad7d11d936bcb5f5d29ad105471394d7 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
>  bbf3f13e44ca3ee6ecb1d635abb9048942b26ac3 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
>  c967f4e2ee0a811e04362aef452b1cfddfd973f3 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
>  9dae03562aff82574dcda5339df415193655d5a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
>  5726c0e834e6e5ad47f47f3337d9317ee1366955 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
>  9115697088cab8fd1732086963c7c3f982069380 
> 
> 
> Diff: https://reviews.apache.org/r/69875/diff/3/
> 
> 
> Testing
> ---
> 
> As there is no functiuonal change I have not added any tests. I made sure 
> that existing tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 69875: SENTRY-2205: Improve Sentry NN Logging

2019-02-05 Thread kalyan kumar kalvagadda via Review Board 

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




sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
Line 124 (original), 124 (patched)


why? What is missing new the one?
Looking at the previous one can not know what numbers mean. With new one, 
you can see what is the perm sequence number ,path sequence number and path 
image number.


- kalyan kumar kalvagadda


On Feb. 5, 2019, 12:01 a.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Feb. 5, 2019, 12:01 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Haley Reeve, and Na Li.
> 
> 
> Bugs: SENTRY-2205
> https://issues.apache.org/jira/browse/SENTRY-2205
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Logging will help us identify below
> 1. Track the updates from Sentry
> 2. What the path update is?
> 3. What the perm update is?
> 4. How the path change is effecting the exizting cache and what is the result 
> of the update?
> 5. How the perm change is effecting the exizting cache and what is the result 
> of the update?
> 6. Changes to the path tree stored as part of the cache.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
>  97a04d9eb5a1e0c5feb97b038b8fc0edec9b0d38 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryAuthzUpdate.java
>  db11c1e1ad7d11d936bcb5f5d29ad105471394d7 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
>  bbf3f13e44ca3ee6ecb1d635abb9048942b26ac3 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
>  c967f4e2ee0a811e04362aef452b1cfddfd973f3 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
>  9dae03562aff82574dcda5339df415193655d5a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
>  5726c0e834e6e5ad47f47f3337d9317ee1366955 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
>  9115697088cab8fd1732086963c7c3f982069380 
> 
> 
> Diff: https://reviews.apache.org/r/69875/diff/3/
> 
> 
> Testing
> ---
> 
> As there is no functiuonal change I have not added any tests. I made sure 
> that existing tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>