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

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

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

(Updated Feb. 1, 2019, 10:22 p.m.)


Review request for sentry, Arjun Mishra, Haley Reeve, and Na Li.


Changes
---

addressed review comments.


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

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

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


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-01 Thread kalyan kumar kalvagadda via Review Board


> On Feb. 1, 2019, 4:34 p.m., Arjun Mishra wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
> > Line 468 (original), 472 (patched)
> > 
> >
> > Log here?

added log to removeAuthzObj.


> On Feb. 1, 2019, 4:34 p.m., Arjun Mishra wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
> > Line 481 (original), 487 (patched)
> > 
> >
> > Log here?

added log to removeAuthzObj.


> On Feb. 1, 2019, 4:34 p.m., Arjun Mishra wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
> > Line 511 (original), 518 (patched)
> > 
> >
> > Log here?

Only place it is called is deleteIfDangling. We are logging there. These is no 
need to log here again.


- kalyan kumar


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


On Jan. 31, 2019, 4:46 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Jan. 31, 2019, 4:46 p.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/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/1/
> 
> 
> 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-01 Thread kalyan kumar kalvagadda via Review Board


> On Feb. 1, 2019, 4:48 p.m., Arjun Mishra wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 199 (original), 203 (patched)
> > 
> >
> > Why not keep seq num and image num also class name?

This is logged using getSequenceInfo. Which is invoked below.


> On Feb. 1, 2019, 4:48 p.m., Arjun Mishra wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 209 (original), 213 (patched)
> > 
> >
> > Same as above

This is logged using getSequenceInfo. Which is invoked below.


- kalyan kumar


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


On Jan. 31, 2019, 4:46 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Jan. 31, 2019, 4:46 p.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/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/1/
> 
> 
> 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-01 Thread Haley Reeve via Review Board

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




sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 454 (patched)


What exactly is the purpose of this log? If the following "if" statement is 
false, then nothing will happen, but this line will be logged regardless, which 
makes it look like something did happen. I think it would be better to either 
combine this log message with the one inside the if statement, or to change it 
to something like, "Attempting to remove path..." and make sure we have a log 
message that shows whether or not it was successful.


- Haley Reeve


On Jan. 31, 2019, 4:46 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Jan. 31, 2019, 4:46 p.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/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/1/
> 
> 
> 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-01 Thread Arjun Mishra via Review Board

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




sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
Lines 118 (patched)


This is not required as renameAuthzObject has the same log message



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 199 (original), 203 (patched)


Why not keep seq num and image num also class name?



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 209 (original), 213 (patched)


Same as above



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 216 (original), 218 (patched)


Same as above



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


I prefer the old way of logging


- Arjun Mishra


On Jan. 31, 2019, 4:46 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Jan. 31, 2019, 4:46 p.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/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/1/
> 
> 
> 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-01 Thread Arjun Mishra via Review Board

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



We definitely need logs on Entry class

- Arjun Mishra


On Jan. 31, 2019, 4:46 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Jan. 31, 2019, 4:46 p.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/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/1/
> 
> 
> 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-01 Thread Arjun Mishra via Review Board

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




sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 748 (patched)


Adding method name will be helpful here



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 751 (patched)


Adding method name will be helpful here



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 771 (original)


I prefer the old way of logging. Easy to track



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 800 (original)


Prefer the old way. You can add the size information to existing log message


- Arjun Mishra


On Jan. 31, 2019, 4:46 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Jan. 31, 2019, 4:46 p.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/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/1/
> 
> 
> 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-01 Thread Arjun Mishra via Review Board

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




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


Can you add a log message here?



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


We need a DEBUG level log here



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


We need a DEBUG level log here



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


Same here



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


Can you add log here?



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


Can you add log here?



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


Log here?



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


Log here?



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


Same here?



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 454 (patched)


Can we change the convention to method name then other information? It'll 
be easy to track. Thoughts?



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 464 (patched)


Same as above method name + arguments - followed by message



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


Log here?



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 476 (patched)


This should be logged before the method



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


Log here?



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 502 (patched)


Same here?



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


Log here?



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


Log here?



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


I prefer the old way of logging. Logs the messag that is called. Its easier 
to track



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 725 (original)


I prefer the old way of logging


- Arjun Mishra


On Jan. 31, 2019, 4:46 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69875/
> ---
> 
> (Updated Jan. 31, 2019, 4:46 p.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