[ 
https://issues.apache.org/jira/browse/SENTRY-2300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Na Li updated SENTRY-2300:
--------------------------
    Description: 
There was a code in MetastorePlugin that modified Sentry privileges on table 
Create/Drop and database Create/Drop. As part of Sentry HA work we moved all 
this logic from Sentry plugin to be driven by notifications which required the 
extra synchronization between HMS and Sentry.

It should be possible to do permission changes in the post event listener 
itself to avoid blocking for Sentry. This requires some experiments though 
because it may cause strange artifacts since at the time these DDL operations 
are done Sentry may not be aware of the current state - for example you may try 
to change permissions of a table that Sentry doesn’t know about, which seems to 
be OK. 

This update will have the following benefits:
{code}
* HMS waits on Sentry polling HMS update takes 0.5 to 1 second. This update 
will remove this delay
* Sentry knows every DDL update, and therefore can update permission correctly. 
In current approach using notification processing, Sentry could miss updates if 
full snapshot is fetched from HMS, and permission is not updated correctly. In 
the case of table rename, when mission DDL update event because of full 
snapshot, sentry will not move the permissions associated with old table to the 
new table. And the authorization on queries on the renamed table will fail.
{code}

The proposed approach is:
{code}
1) Add the permission update in 
SentryPolicyStoreProcessor.sentry_notify_hms_event() through 
SentrySyncHMSNotificationsPostEventListener for create/drop/alter table, 
create/drop database commands. This is synchronous processing.
2) Remove the permission update from 
NotificationProcessor.processNotificationEvent() for create/drop/alter table, 
create/drop database commands. 
3) Remove the blocking of HMS in SentrySyncHMSNotificationsPostEventListener 
for Sentry fetching notification events asynchronously from HMS for 
create/drop/alter table, create/drop database commands. The reason is that now 
the permission is updated through SentrySyncHMSNotificationsPostEventListener. 
Therefore, there is no need for HMS to wait for Sentry fetching those 
notification events. 
{code}

Without this approach, the DDL operation take at least 500 to 1000 
milliseconds. It is easy for HMS to encounter SyncTimeoutException when it 
takes long time for Sentry to get full snapshot or fetch notification events. 
Many users choose to remove the post event listener in configuration (HMS not 
waiting for Sentry fetching notification events asynchronously) to get rid of 
this exception. Users cannot take this workaround if they want owner privilege 
enabled. 

This approach makes DDL operation in HMS independent from Sentry fetching full 
snapshot or notification events, and reduces the DDL operation delay 
significantly. With this approach, the DDL operation is in the range of few 
milliseconds. This approach fixes the timeout issue once for all. And users can 
get rid of timeout issue and enable owner privilege at the same time

  was:
There was a code in MetastorePlugin that modified Sentry privileges on table 
Create/Drop and database Create/Drop. As part of Sentry HA work we moved all 
this logic from Sentry plugin to be driven by notifications which required the 
extra synchronization between HMS and Sentry.

It should be possible to do permission changes in the post event listener 
itself to avoid blocking for Sentry. This requires some experiments though 
because it may cause strange artifacts since at the time these DDL operations 
are done Sentry may not be aware of the current state - for example you may try 
to change permissions of a table that Sentry doesn’t know about, which seems to 
be OK. 

This update will have the following benefits:
{code}
* HMS waits on Sentry polling HMS update takes 0.5 to 1 second. This update 
will remove this delay
* Sentry knows every DDL update, and therefore can update permission correctly. 
In current approach using notification processing, Sentry could miss updates if 
full snapshot is fetched from HMS, and permission is not updated correctly. In 
the case of table rename, when mission DDL update event because of full 
snapshot, sentry will not move the permissions associated with old table to the 
new table. And the authorization on queries on the renamed table will fail.
{code}

The proposed approach is:
{code}
1) Add the permission update in 
SentryPolicyStoreProcessor.sentry_notify_hms_event() through 
SentrySyncHMSNotificationsPostEventListener for create/drop/alter table, 
create/drop database commands. This is synchronous processing.
2) Remove the permission update from 
NotificationProcessor.processNotificationEvent() for create/drop/alter table, 
create/drop database commands. 
3. Remove the blocking of HMS in SentrySyncHMSNotificationsPostEventListener 
for Sentry fetching notification events asynchronously from HMS for 
create/drop/alter table, create/drop database commands. The reason is that now 
the permission is updated through SentrySyncHMSNotificationsPostEventListener. 
Therefore, there is no need for HMS to wait for Sentry fetching those 
notification events. 
{code}

Without this approach, the DDL operation take at least 500 to 1000 
milliseconds. It is easy for HMS to encounter SyncTimeoutException when it 
takes long time for Sentry to get full snapshot or fetch notification events. 
Many users choose to remove the post event listener in configuration (HMS not 
waiting for Sentry fetching notification events asynchronously) to get rid of 
this exception. Users cannot take this workaround if they want owner privilege 
enabled. 

This approach makes DDL operation in HMS independent from Sentry fetching full 
snapshot or notification events, and reduces the DDL operation delay 
significantly. With this approach, the DDL operation is in the range of few 
milliseconds. This approach fixes the timeout issue once for all. And users can 
get rid of timeout issue and enable owner privilege at the same time


> Move Permission Update due to DDL to HMS Post Event Listener
> ------------------------------------------------------------
>
>                 Key: SENTRY-2300
>                 URL: https://issues.apache.org/jira/browse/SENTRY-2300
>             Project: Sentry
>          Issue Type: Bug
>          Components: Sentry
>    Affects Versions: 2.1.0, 2.2.0
>            Reporter: Na Li
>            Assignee: Na Li
>            Priority: Major
>         Attachments: SENTRY-2300.001.patch, SENTRY-2300.002.patch
>
>
> There was a code in MetastorePlugin that modified Sentry privileges on table 
> Create/Drop and database Create/Drop. As part of Sentry HA work we moved all 
> this logic from Sentry plugin to be driven by notifications which required 
> the extra synchronization between HMS and Sentry.
> It should be possible to do permission changes in the post event listener 
> itself to avoid blocking for Sentry. This requires some experiments though 
> because it may cause strange artifacts since at the time these DDL operations 
> are done Sentry may not be aware of the current state - for example you may 
> try to change permissions of a table that Sentry doesn’t know about, which 
> seems to be OK. 
> This update will have the following benefits:
> {code}
> * HMS waits on Sentry polling HMS update takes 0.5 to 1 second. This update 
> will remove this delay
> * Sentry knows every DDL update, and therefore can update permission 
> correctly. In current approach using notification processing, Sentry could 
> miss updates if full snapshot is fetched from HMS, and permission is not 
> updated correctly. In the case of table rename, when mission DDL update event 
> because of full snapshot, sentry will not move the permissions associated 
> with old table to the new table. And the authorization on queries on the 
> renamed table will fail.
> {code}
> The proposed approach is:
> {code}
> 1) Add the permission update in 
> SentryPolicyStoreProcessor.sentry_notify_hms_event() through 
> SentrySyncHMSNotificationsPostEventListener for create/drop/alter table, 
> create/drop database commands. This is synchronous processing.
> 2) Remove the permission update from 
> NotificationProcessor.processNotificationEvent() for create/drop/alter table, 
> create/drop database commands. 
> 3) Remove the blocking of HMS in SentrySyncHMSNotificationsPostEventListener 
> for Sentry fetching notification events asynchronously from HMS for 
> create/drop/alter table, create/drop database commands. The reason is that 
> now the permission is updated through 
> SentrySyncHMSNotificationsPostEventListener. Therefore, there is no need for 
> HMS to wait for Sentry fetching those notification events. 
> {code}
> Without this approach, the DDL operation take at least 500 to 1000 
> milliseconds. It is easy for HMS to encounter SyncTimeoutException when it 
> takes long time for Sentry to get full snapshot or fetch notification events. 
> Many users choose to remove the post event listener in configuration (HMS not 
> waiting for Sentry fetching notification events asynchronously) to get rid of 
> this exception. Users cannot take this workaround if they want owner 
> privilege enabled. 
> This approach makes DDL operation in HMS independent from Sentry fetching 
> full snapshot or notification events, and reduces the DDL operation delay 
> significantly. With this approach, the DDL operation is in the range of few 
> milliseconds. This approach fixes the timeout issue once for all. And users 
> can get rid of timeout issue and enable owner privilege at the same time



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to