You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org> on 2019/01/31 16:46:51 UTC
Review Request 69875: SENTRY-2205: Improve Sentry NN Logging
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69875/
-----------------------------------------------------------
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
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> 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)
> > <https://reviews.apache.org/r/69875/diff/1/?file=2123540#file2123540line472>
> >
> > 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)
> > <https://reviews.apache.org/r/69875/diff/1/?file=2123540#file2123540line487>
> >
> > 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)
> > <https://reviews.apache.org/r/69875/diff/1/?file=2123540#file2123540line518>
> >
> > 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
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69875/#comment298276>
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)
<https://reviews.apache.org/r/69875/#comment298277>
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)
<https://reviews.apache.org/r/69875/#comment298278>
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)
<https://reviews.apache.org/r/69875/#comment298279>
Same here
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 358 (original), 359 (patched)
<https://reviews.apache.org/r/69875/#comment298280>
Can you add log here?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 387 (original), 388 (patched)
<https://reviews.apache.org/r/69875/#comment298281>
Can you add log here?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 412 (original), 413 (patched)
<https://reviews.apache.org/r/69875/#comment298282>
Log here?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 425 (original), 426 (patched)
<https://reviews.apache.org/r/69875/#comment298283>
Log here?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 435 (original), 436 (patched)
<https://reviews.apache.org/r/69875/#comment298284>
Same here?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 454 (patched)
<https://reviews.apache.org/r/69875/#comment298285>
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)
<https://reviews.apache.org/r/69875/#comment298286>
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)
<https://reviews.apache.org/r/69875/#comment298288>
Log here?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 476 (patched)
<https://reviews.apache.org/r/69875/#comment298287>
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)
<https://reviews.apache.org/r/69875/#comment298289>
Log here?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 502 (patched)
<https://reviews.apache.org/r/69875/#comment298290>
Same here?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 511 (original), 518 (patched)
<https://reviews.apache.org/r/69875/#comment298291>
Log here?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 526 (original), 535 (patched)
<https://reviews.apache.org/r/69875/#comment298292>
Log here?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 695 (original), 704 (patched)
<https://reviews.apache.org/r/69875/#comment298293>
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)
<https://reviews.apache.org/r/69875/#comment298294>
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
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> 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
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69875/#comment298398>
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
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
> 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)
> > <https://reviews.apache.org/r/69875/diff/1/?file=2123545#file2123545line124>
> >
> > 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
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> 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)
> > <https://reviews.apache.org/r/69875/diff/1/?file=2123545#file2123545line124>
> >
> > 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
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69875/#comment298310>
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
>
>
Re: Review Request 69875: SENTRY-2205: Improve Sentry NN Logging
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69875/#comment298402>
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)
<https://reviews.apache.org/r/69875/#comment298403>
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
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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.
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/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/
Changes: https://reviews.apache.org/r/69875/diff/2-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
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69875/#review212520
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 202 (patched)
<https://reviews.apache.org/r/69875/#comment298321>
Can we add method name?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 224 (patched)
<https://reviews.apache.org/r/69875/#comment298322>
Can we add method name?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
Line 125 (original), 125 (patched)
<https://reviews.apache.org/r/69875/#comment298337>
This is not right. More paranthesis than arguments
- Arjun Mishra
On Feb. 1, 2019, 10:22 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> 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.
>
>
> 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/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
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69875/#review212526
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
Line 117 (original), 117 (patched)
<https://reviews.apache.org/r/69875/#comment298338>
Log message here to say renaming, just like below where we did for adding and removing
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 149 (original)
<https://reviews.apache.org/r/69875/#comment298339>
We need this lock right?
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 199 (original), 203 (patched)
<https://reviews.apache.org/r/69875/#comment298340>
Better to keep old information. As it is more verbose
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 209 (original), 213 (patched)
<https://reviews.apache.org/r/69875/#comment298341>
Same as above
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 216 (original), 218 (patched)
<https://reviews.apache.org/r/69875/#comment298342>
Same as above
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 236 (original), 235 (patched)
<https://reviews.apache.org/r/69875/#comment298343>
I thought we are changing this to Throwable???
- Arjun Mishra
On Feb. 1, 2019, 10:22 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> 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.
>
>
> 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/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
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69875/#review212527
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
Line 125 (original), 125 (patched)
<https://reviews.apache.org/r/69875/#comment298344>
I prefer the old way of logging
- Arjun Mishra
On Feb. 1, 2019, 10:22 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> 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.
>
>
> 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/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
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
Posted by Haley Reeve via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69875/#comment298304>
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
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69875/#comment298295>
Adding method name will be helpful here
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 751 (patched)
<https://reviews.apache.org/r/69875/#comment298296>
Adding method name will be helpful here
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 771 (original)
<https://reviews.apache.org/r/69875/#comment298297>
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)
<https://reviews.apache.org/r/69875/#comment298298>
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
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> 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)
> > <https://reviews.apache.org/r/69875/diff/1/?file=2123543#file2123543line204>
> >
> > 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)
> > <https://reviews.apache.org/r/69875/diff/1/?file=2123543#file2123543line217>
> >
> > 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
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69875/#comment298299>
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)
<https://reviews.apache.org/r/69875/#comment298300>
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)
<https://reviews.apache.org/r/69875/#comment298301>
Same as above
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 216 (original), 218 (patched)
<https://reviews.apache.org/r/69875/#comment298302>
Same as above
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
Line 124 (original), 124 (patched)
<https://reviews.apache.org/r/69875/#comment298303>
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
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69875/#review212499
-----------------------------------------------------------
General comment - Instead of logging sentances can we simplify it like method name and attributes? Also add that to the start of every method? What do you think?
For example: "putChild({},{})", or "deleteFromParent: Removing path {}"
- 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
>
>