You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Alexander Kolbasov <ak...@gmail.com> on 2016/12/29 09:56:23 UTC
Review Request 55080: SENTRY-1428: Only leader should follow HMS
updates
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55080/
-----------------------------------------------------------
Review request for sentry, Colin Ma, Misha Dmitriev, Hao Hao, kalyan kumar kalvagadda, Lenni Kuff, Vamsee Yarlagadda, and Vadim Spector.
Bugs: SENTRY-1428
https://issues.apache.org/jira/browse/SENTRY-1428
Repository: sentry
Description
-------
SENTRY-1428: Only leader should follow HMS updates
Diffs
-----
pom.xml df26edf74c04fffcd9fbc3da07e949362eb94728
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java c094058142706a6b564c54fa69ddff5f1e2e5048
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java f80605c4d68ef266a24e65d14f51066388b48417
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java c3df4d8a6568f236dc5e05ab190cd1f484f7967a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activators.java 2926eeb97ec450f12d0d08086165c091709ead99
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java f3cefd6a232bfb91db28f04bebcc98ab3c1ca658
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java 2467921b3cdc9ebd267d308d5cdcce52a836f207
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusAdaptor.java e87b3d13dcf2d10eb7ef00d21b8fa8846b4d16c0
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java a9d49f86961f1ced1af40afcfe6172f884a23d44
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d80fa1e71c165b7f1faf1c50c451e049d76b770b
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java bc375e32685dcbcbf0b6398856dceced3de789e6
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatus.java c796fabc21005479fa6afc687cf1df7af76538a9
Diff: https://reviews.apache.org/r/55080/diff/
Testing
-------
I manually tested this with 2-server setup using both regular process kill and sending special signal that drops leadership status.
I also tested shutting down ZK service itself and then restarting it.
Thanks,
Alexander Kolbasov
Re: Review Request 55080: SENTRY-1428: Only leader should follow HMS
updates
Posted by Misha Dmitriev <mi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55080/#review160427
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java (line 259)
<https://reviews.apache.org/r/55080/#comment231566>
What does "will try" mean in this message. Maybe "will retry"?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java (line 32)
<https://reviews.apache.org/r/55080/#comment231573>
What is "the activator" that this javadoc talks about? I think the new javadoc should be made more relevant to the LeaderStatus. Or, if it tries to explain a bigger picture of which LeaderStatus is just a part, it should do so in some more detail.
- Misha Dmitriev
On Dec. 29, 2016, 9:56 a.m., Alexander Kolbasov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55080/
> -----------------------------------------------------------
>
> (Updated Dec. 29, 2016, 9:56 a.m.)
>
>
> Review request for sentry, Colin Ma, Misha Dmitriev, Hao Hao, kalyan kumar kalvagadda, Lenni Kuff, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1428
> https://issues.apache.org/jira/browse/SENTRY-1428
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1428: Only leader should follow HMS updates
>
>
> Diffs
> -----
>
> pom.xml df26edf74c04fffcd9fbc3da07e949362eb94728
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java c094058142706a6b564c54fa69ddff5f1e2e5048
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java f80605c4d68ef266a24e65d14f51066388b48417
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java c3df4d8a6568f236dc5e05ab190cd1f484f7967a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activators.java 2926eeb97ec450f12d0d08086165c091709ead99
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java f3cefd6a232bfb91db28f04bebcc98ab3c1ca658
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java 2467921b3cdc9ebd267d308d5cdcce52a836f207
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusAdaptor.java e87b3d13dcf2d10eb7ef00d21b8fa8846b4d16c0
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java a9d49f86961f1ced1af40afcfe6172f884a23d44
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d80fa1e71c165b7f1faf1c50c451e049d76b770b
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java bc375e32685dcbcbf0b6398856dceced3de789e6
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatus.java c796fabc21005479fa6afc687cf1df7af76538a9
>
> Diff: https://reviews.apache.org/r/55080/diff/
>
>
> Testing
> -------
>
> I manually tested this with 2-server setup using both regular process kill and sending special signal that drops leadership status.
>
> I also tested shutting down ZK service itself and then restarting it.
>
>
> Thanks,
>
> Alexander Kolbasov
>
>
Re: Review Request 55080: SENTRY-1428: Only leader should follow HMS
updates
Posted by Vadim Spector <vs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55080/#review160431
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java (line 273)
<https://reviews.apache.org/r/55080/#comment231576>
Am I missing something? authzPaths is private member of HMSFollower class, it is set in this class, yet I can't seem to find the code to access values store in authzPaths. And what about partial updates - how do they happen - or do they happen at all?
Hard to review the code not knowing what it needs to acomplish - javadoc would help.
- Vadim Spector
On Dec. 29, 2016, 9:56 a.m., Alexander Kolbasov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55080/
> -----------------------------------------------------------
>
> (Updated Dec. 29, 2016, 9:56 a.m.)
>
>
> Review request for sentry, Colin Ma, Misha Dmitriev, Hao Hao, kalyan kumar kalvagadda, Lenni Kuff, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1428
> https://issues.apache.org/jira/browse/SENTRY-1428
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1428: Only leader should follow HMS updates
>
>
> Diffs
> -----
>
> pom.xml df26edf74c04fffcd9fbc3da07e949362eb94728
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java c094058142706a6b564c54fa69ddff5f1e2e5048
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java f80605c4d68ef266a24e65d14f51066388b48417
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java c3df4d8a6568f236dc5e05ab190cd1f484f7967a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activators.java 2926eeb97ec450f12d0d08086165c091709ead99
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java f3cefd6a232bfb91db28f04bebcc98ab3c1ca658
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java 2467921b3cdc9ebd267d308d5cdcce52a836f207
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusAdaptor.java e87b3d13dcf2d10eb7ef00d21b8fa8846b4d16c0
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java a9d49f86961f1ced1af40afcfe6172f884a23d44
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d80fa1e71c165b7f1faf1c50c451e049d76b770b
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java bc375e32685dcbcbf0b6398856dceced3de789e6
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatus.java c796fabc21005479fa6afc687cf1df7af76538a9
>
> Diff: https://reviews.apache.org/r/55080/diff/
>
>
> Testing
> -------
>
> I manually tested this with 2-server setup using both regular process kill and sending special signal that drops leadership status.
>
> I also tested shutting down ZK service itself and then restarting it.
>
>
> Thanks,
>
> Alexander Kolbasov
>
>