Re: Review Request 64955: SENTRY-2109: Fix the logic of identifying HMS out of Sync

2018-01-25 Thread Vadim Spector via Review Board

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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
Line 235 (original), 270 (patched)


It seems better to produce two dufferent logs, e.g.

if(isFoundInCache(notificationEvent.getEventId(), hash) == true) {
  LOGGER.debug("Ignoring HMS notification already processed: ID = {}", 
notificationEvent.getEventId());
  return false;
} else if (sentryStore.isNotificationProcessed(hash)
LOGGER.debug("Ignoring HMS notification already processed: ID = {} - cache 
miss", notificationEvent.getEventId());
  return false;
}

Note "cache miss" in the second log.

Cache miss is what we try to avoid for better performance. It may happen 
sometimes, but if there are too many "cache miss" messages, it's useful to know.


- Vadim Spector


On Jan. 25, 2018, 5 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64955/
> ---
> 
> (Updated Jan. 25, 2018, 5 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Na Li, Sergio 
> Pena, and Vadim Spector.
> 
> 
> Bugs: SENTRY-2109
> https://issues.apache.org/jira/browse/SENTRY-2109
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This patch does couple of things
> 1. Avoid triggering full snapshots when gaps are observed while fetching new 
> notifications. While fetching new notifications HMSFollower would would fetch 
> notifications with last event-id as well. When it gets the notifications and 
> if it doesn't get the notifications with event-id, full snpshot is triggered.
> 2. Functinalty to address gaps and out-of-sequence notificaitons by 
> re-fetching addtional notifications that were already fetched. This solution 
> is not fool proof. It does a best effort to reduse the chance of loosing 
> notifications by re-fetching the notifications.This approach will introduce 
> an over head of fetching addtional notifications that were already fetched. 
> Overhead of DB look-up is addressed by using a cache. This reduces additional 
> DB lookups needed to check if the notification was already processed.
> 2. Added looging to report duplicate events and out of order events for debug 
> purpose.
> 3. Added new e2e tests to verfy this behavior.
> 
> 
> Diffs
> -
> 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryOutOfSyncException.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
>  2f2b98412e7dfdcc847ffe7975a70f452554e747 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  edea5b64d8f98c93aafc1fe43fa97e00c2ce2948 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
>  93cc34f34c044dceb11d27e41ecbd1a14f64bed9 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  96c6810baa4d554db2b7d3739a28e3ff7e8b33a0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  7e02874b4be6a7109108809b1f404fe971b7b8e2 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
>  79030780c35e5bda432e3ec3f01328e627cb50a6 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
>  91c90f9d302f4feb3a8b3d06541f43541c87bf0f 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  b4100278392986c161625a366212c6fef66ec0a9 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHiveNotificationFetcher.java
>  83a1becd46ac2d69c7d6dd05ed6253d1cdd8800d 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestSnapshotCreation.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestSnapshotCreationWithShorterHMSEventTtl.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestSnapshotWithLongerHMSFollowerLongerInterval.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  

Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Alexander Kolbasov
Stephen - can you formulate these in JIRAs so we can add these to the
roadmap?

On Thu, Jan 25, 2018 at 12:31 PM, Stephen Moist  wrote:

> A few things come to mind.
>
> Improving and expanding on the capabilities of the Sentry CLI.  It would
> be good to see all the other services integrate with Sentry in a consistent
> way.  Along with be able to administer grants/roles/etc through a common
> framework rather than say beeline.
>
> Improving documentation of Sentry’s integration, preferably with more
> examples of how to configure services.
>
> Adding access control on database operations such as drop table, insert,
> delete from, update, etc.
>
> I know for sure a feature we need is going to be tag based attribute
> control for Hive.
>
> These last two ideas would need some reworking to make Sentry more
> flexible to support these, and I’m willing to lead up the latter for tags.
>
> > On Jan 25, 2018, at 2:19 PM, Na Li  wrote:
> >
> > https://issues.apache.org/jira/browse/SENTRY-2129 is create to track the
> > development activities for user-based privilege. I will add more
> sub-tasks
> > to it
> >
> > On Thu, Jan 25, 2018 at 1:42 PM, Alexander Kolbasov 
> > wrote:
> >
> >> Agreed, making 2.1 with just user-level privileges improvements (plus
> set
> >> of accumulated bug fixes) sounds reasonable.
> >>
> >> On Thu, Jan 25, 2018 at 11:41 AM, Alexander Kolbasov <
> ak...@cloudera.com>
> >> wrote:
> >>
> >>> Looks like we have a consensus of doing user-level privileges
> >> improvements
> >>> for 2.1. Let's see whether anyone wants to add more content.
> >>>
> >>> On Thu, Jan 25, 2018 at 11:38 AM, Na Li  wrote:
> >>>
>  Sasha,
> 
>  I have looked into how to complete the user-based privilege for a
> while,
>  and can commit to implement it. I can work with Kalyan to create a
> >> design
>  doc for user-based privilege.
> 
>  Thanks,
> 
>  Lina
> 
>  On Thu, Jan 25, 2018 at 1:35 PM, Na Li  wrote:
> 
> > Sasha,
> >
> > The current user-based privilege missed some items:
> >
> >
> >   - Sentry policy has two service API: SentryPolicyService and
>  SentryGenericPolicyService.
> >   The current implementation does not support user-based privilege
> >> for
> >   SentryGenericPolicyService
> >   - SENTRY-2091: User-based Privilege is broken by SENTRY-769. The
>  patch
> >   is available for review.
> >   - Name Node need change to generate ACL using user privilege.
> >  - The full snapshot update only contains authorization to roles
> >  mapping and role to group mapping. *Need to add role to user
> >  mapping in* SentryStore.retrieveFullRoleImageCore
> >  - The delta updates are taken from table SENTRY_PERM_CHANGE,
> >> which
> >  does not distinguish group based permission or user based
>  permission. No
> >  change is needed
> >  - The user changes to a role is not included when sending delta
> >  update from Sentry to NN. *Need to add AddUsers and DropUsers
> >  in TRoleChanges*.
> >  - Sentry only create ACL for group with ACL type
> >  as AclEntryType.GROUP. *Need to add code to create ACL with type
> >  as *AclEntryType.USER
> >  - SentryINodeAttributesProvider.checkPermission
> > -> FSPermissionChecker.checkPermission ->
> > SentryINodeAttributesProvider.getAclFeature
> > -> SentryAuthorizationInfo.getAclEntries ->
> >> SentryPermissions.
> > constructAclEntry
> >  - SentryStore.grantOptionCheck() has to be changed to find user
> >   level privilege.
> >
> > Thanks,
> >
> > Lina
> >
> > On Thu, Jan 25, 2018 at 1:13 PM, Sergio Pena <
> >> sergio.p...@cloudera.com>
> > wrote:
> >
> >> There is a section on the Wiki about roadmap ideas and JIRAs already
> >> created:
> >> https://cwiki.apache.org/confluence/display/SENTRY/Sentry+
> >> Roadmap+and+ideas
> >>
> >> I'm interested in having user-level privileges and special user
>  privileges
> >> for objects owners.
> >>
> >> I got this from the linked above:
> >>  SENTRY-1073 User who creates a table should be granted all
>  privileges on
> >> it by default
> >>  SENTRY-1068 Allow user who created a table to have "with grant"
> >> over
> >> that
> >> table by default
> >>  Creator of a table should have ownership of it (all privileges)
> >>  Allow privileges to be granted to users directly
> >>
> >> We should start planning the next Sentry 2.1 release based on the
>  desired
> >> features. What about
> >> having 2 or 3 features on Sentry 2.1?
> >>
> >> I vote for:
> >> - user-level privileges (currently grant user to role is only
>  supported)
> >> - 

Re: Review Request 65268: SENTRY-1904: TransactionManager should limit the max time spent by transaction retry

2018-01-25 Thread Vadim Spector via Review Board


> On Jan. 25, 2018, 4:16 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
> > Line 231 (original), 246 (patched)
> > 
> >
> > If you don't think that you can use time because we do not know how 
> > much time it may take to execute transaction, then the whole point of this 
> > JIRA is moot and we shouldn't fix it in the first place.
> 
> Vadim Spector wrote:
> I am not sure if it's the real issue. The two limita are not redundant or 
> mutually exclusive. The old approach limits the number of re-tries; this 
> change also adds limits the total re-try time but keeps the old limitation as 
> well. Are two controls necessarily worse than one? If someone re-configures 
> by mistake retry interval too small (much smaller than the default), so there 
> may be lots of retries (and possibly very unresponsive system) before time 
> limit reached - perhaps we want to prevent it? Can system clock reset at run 
> time messing with the time-only logic? Unlikely, but not impossible.
> 
> kalyan kumar kalvagadda wrote:
> Vadim, Having both limits on retry-count and the retry-time would address 
> the scenario you are taking about.

Kalyan, that was exactly my point, sorry if it was not clear. I support keeping 
both mechanisms in place, as in your patch.


- Vadim


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


On Jan. 23, 2018, 11:40 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65268/
> ---
> 
> (Updated Jan. 23, 2018, 11:40 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, Sergio Pena, and Vadim 
> Spector.
> 
> 
> Bugs: SENTRY-1904
> https://issues.apache.org/jira/browse/SENTRY-1904
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The TransactionManager uses exponential backoff strategy for transaction 
> retries. This may cause some transactions to be delayed by a very long time. 
> We should also have a constraint on the max time for a transaction so that we 
> do not retry for too long. 
> 
> New patch that is attached adds upper bounds on below
> 
> 1.Interval between the retry attempts which increases exponentially.
> 2.Total time a transaction could spend in retries.
>  
> 
> With out these limits we would not have a control on how long a transaction 
> could be be active.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
>  2f2b98412e7dfdcc847ffe7975a70f452554e747 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  edea5b64d8f98c93aafc1fe43fa97e00c2ce2948 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
>  f4ff962a67f8a5c23cc5c8daa7bcb861d2e6b6a5 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  7e02874b4be6a7109108809b1f404fe971b7b8e2 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  b4100278392986c161625a366212c6fef66ec0a9 
> 
> 
> Diff: https://reviews.apache.org/r/65268/diff/3/
> 
> 
> Testing
> ---
> 
> Made sure all the tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>



Re: Review Request 64955: SENTRY-2109: Fix the logic of identifying HMS out of Sync

2018-01-25 Thread kalyan kumar kalvagadda via Review Board

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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
Lines 119 (patched)


Stale log



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
Lines 125 (patched)


Stale log



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
Lines 140 (patched)


Stale log


- kalyan kumar kalvagadda


On Jan. 25, 2018, 5 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64955/
> ---
> 
> (Updated Jan. 25, 2018, 5 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Na Li, Sergio 
> Pena, and Vadim Spector.
> 
> 
> Bugs: SENTRY-2109
> https://issues.apache.org/jira/browse/SENTRY-2109
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This patch does couple of things
> 1. Avoid triggering full snapshots when gaps are observed while fetching new 
> notifications. While fetching new notifications HMSFollower would would fetch 
> notifications with last event-id as well. When it gets the notifications and 
> if it doesn't get the notifications with event-id, full snpshot is triggered.
> 2. Functinalty to address gaps and out-of-sequence notificaitons by 
> re-fetching addtional notifications that were already fetched. This solution 
> is not fool proof. It does a best effort to reduse the chance of loosing 
> notifications by re-fetching the notifications.This approach will introduce 
> an over head of fetching addtional notifications that were already fetched. 
> Overhead of DB look-up is addressed by using a cache. This reduces additional 
> DB lookups needed to check if the notification was already processed.
> 2. Added looging to report duplicate events and out of order events for debug 
> purpose.
> 3. Added new e2e tests to verfy this behavior.
> 
> 
> Diffs
> -
> 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryOutOfSyncException.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
>  2f2b98412e7dfdcc847ffe7975a70f452554e747 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  edea5b64d8f98c93aafc1fe43fa97e00c2ce2948 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
>  93cc34f34c044dceb11d27e41ecbd1a14f64bed9 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  96c6810baa4d554db2b7d3739a28e3ff7e8b33a0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  7e02874b4be6a7109108809b1f404fe971b7b8e2 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
>  79030780c35e5bda432e3ec3f01328e627cb50a6 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
>  91c90f9d302f4feb3a8b3d06541f43541c87bf0f 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  b4100278392986c161625a366212c6fef66ec0a9 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHiveNotificationFetcher.java
>  83a1becd46ac2d69c7d6dd05ed6253d1cdd8800d 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestSnapshotCreation.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestSnapshotCreationWithShorterHMSEventTtl.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestSnapshotWithLongerHMSFollowerLongerInterval.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  4cd00e6672730773c74b9840247d1f4d5f7bdfe4 
> 
> 
> Diff: https://reviews.apache.org/r/64955/diff/5/
> 
> 
> Testing
> ---
> 
> Made sure that tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>



Re: Review Request 65268: SENTRY-1904: TransactionManager should limit the max time spent by transaction retry

2018-01-25 Thread kalyan kumar kalvagadda via Review Board


> On Jan. 25, 2018, 4:16 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
> > Line 231 (original), 246 (patched)
> > 
> >
> > If you don't think that you can use time because we do not know how 
> > much time it may take to execute transaction, then the whole point of this 
> > JIRA is moot and we shouldn't fix it in the first place.
> 
> Vadim Spector wrote:
> I am not sure if it's the real issue. The two limita are not redundant or 
> mutually exclusive. The old approach limits the number of re-tries; this 
> change also adds limits the total re-try time but keeps the old limitation as 
> well. Are two controls necessarily worse than one? If someone re-configures 
> by mistake retry interval too small (much smaller than the default), so there 
> may be lots of retries (and possibly very unresponsive system) before time 
> limit reached - perhaps we want to prevent it? Can system clock reset at run 
> time messing with the time-only logic? Unlikely, but not impossible.

Vadim, Having both limits on retry-count and the retry-time would address the 
scenario you are taking about.


- kalyan kumar


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


On Jan. 23, 2018, 11:40 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65268/
> ---
> 
> (Updated Jan. 23, 2018, 11:40 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, Sergio Pena, and Vadim 
> Spector.
> 
> 
> Bugs: SENTRY-1904
> https://issues.apache.org/jira/browse/SENTRY-1904
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The TransactionManager uses exponential backoff strategy for transaction 
> retries. This may cause some transactions to be delayed by a very long time. 
> We should also have a constraint on the max time for a transaction so that we 
> do not retry for too long. 
> 
> New patch that is attached adds upper bounds on below
> 
> 1.Interval between the retry attempts which increases exponentially.
> 2.Total time a transaction could spend in retries.
>  
> 
> With out these limits we would not have a control on how long a transaction 
> could be be active.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
>  2f2b98412e7dfdcc847ffe7975a70f452554e747 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  edea5b64d8f98c93aafc1fe43fa97e00c2ce2948 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
>  f4ff962a67f8a5c23cc5c8daa7bcb861d2e6b6a5 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  7e02874b4be6a7109108809b1f404fe971b7b8e2 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  b4100278392986c161625a366212c6fef66ec0a9 
> 
> 
> Diff: https://reviews.apache.org/r/65268/diff/3/
> 
> 
> Testing
> ---
> 
> Made sure all the tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>



Re: Review Request 65268: SENTRY-1904: TransactionManager should limit the max time spent by transaction retry

2018-01-25 Thread Vadim Spector via Review Board


> On Jan. 25, 2018, 4:16 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
> > Line 231 (original), 246 (patched)
> > 
> >
> > If you don't think that you can use time because we do not know how 
> > much time it may take to execute transaction, then the whole point of this 
> > JIRA is moot and we shouldn't fix it in the first place.

I am not sure if it's the real issue. The two limita are not redundant or 
mutually exclusive. The old approach limits the number of re-tries; this change 
also adds limits the total re-try time but keeps the old limitation as well. 
Are two controls necessarily worse than one? If someone re-configures by 
mistake retry interval too small (much smaller than the default), so there may 
be lots of retries (and possibly very unresponsive system) before time limit 
reached - perhaps we want to prevent it? Can system clock reset at run time 
messing with the time-only logic? Unlikely, but not impossible.


- Vadim


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


On Jan. 23, 2018, 11:40 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65268/
> ---
> 
> (Updated Jan. 23, 2018, 11:40 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, Sergio Pena, and Vadim 
> Spector.
> 
> 
> Bugs: SENTRY-1904
> https://issues.apache.org/jira/browse/SENTRY-1904
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The TransactionManager uses exponential backoff strategy for transaction 
> retries. This may cause some transactions to be delayed by a very long time. 
> We should also have a constraint on the max time for a transaction so that we 
> do not retry for too long. 
> 
> New patch that is attached adds upper bounds on below
> 
> 1.Interval between the retry attempts which increases exponentially.
> 2.Total time a transaction could spend in retries.
>  
> 
> With out these limits we would not have a control on how long a transaction 
> could be be active.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
>  2f2b98412e7dfdcc847ffe7975a70f452554e747 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  edea5b64d8f98c93aafc1fe43fa97e00c2ce2948 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
>  f4ff962a67f8a5c23cc5c8daa7bcb861d2e6b6a5 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  7e02874b4be6a7109108809b1f404fe971b7b8e2 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  b4100278392986c161625a366212c6fef66ec0a9 
> 
> 
> Diff: https://reviews.apache.org/r/65268/diff/3/
> 
> 
> Testing
> ---
> 
> Made sure all the tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>



Re: Review Request 64955: SENTRY-2109: Fix the logic of identifying HMS out of Sync

2018-01-25 Thread kalyan kumar kalvagadda via Review Board


> On Jan. 22, 2018, 8:49 p.m., Vadim Spector wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
> > Lines 231 (patched)
> > 
> >
> > optimization - no need for this DB call if the ID > max(storedId)
> 
> kalyan kumar kalvagadda wrote:
> will consider it in the new patch.

I have added simple cache logic to avoid Db lookups.


- kalyan kumar


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


On Jan. 25, 2018, 5 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64955/
> ---
> 
> (Updated Jan. 25, 2018, 5 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Na Li, Sergio 
> Pena, and Vadim Spector.
> 
> 
> Bugs: SENTRY-2109
> https://issues.apache.org/jira/browse/SENTRY-2109
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This patch does couple of things
> 1. Avoid triggering full snapshots when gaps are observed while fetching new 
> notifications. While fetching new notifications HMSFollower would would fetch 
> notifications with last event-id as well. When it gets the notifications and 
> if it doesn't get the notifications with event-id, full snpshot is triggered.
> 2. Functinalty to address gaps and out-of-sequence notificaitons by 
> re-fetching addtional notifications that were already fetched. This solution 
> is not fool proof. It does a best effort to reduse the chance of loosing 
> notifications by re-fetching the notifications.This approach will introduce 
> an over head of fetching addtional notifications that were already fetched. 
> Overhead of DB look-up is addressed by using a cache. This reduces additional 
> DB lookups needed to check if the notification was already processed.
> 2. Added looging to report duplicate events and out of order events for debug 
> purpose.
> 3. Added new e2e tests to verfy this behavior.
> 
> 
> Diffs
> -
> 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryOutOfSyncException.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
>  2f2b98412e7dfdcc847ffe7975a70f452554e747 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  edea5b64d8f98c93aafc1fe43fa97e00c2ce2948 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
>  93cc34f34c044dceb11d27e41ecbd1a14f64bed9 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  96c6810baa4d554db2b7d3739a28e3ff7e8b33a0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  7e02874b4be6a7109108809b1f404fe971b7b8e2 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
>  79030780c35e5bda432e3ec3f01328e627cb50a6 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
>  91c90f9d302f4feb3a8b3d06541f43541c87bf0f 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  b4100278392986c161625a366212c6fef66ec0a9 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHiveNotificationFetcher.java
>  83a1becd46ac2d69c7d6dd05ed6253d1cdd8800d 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestSnapshotCreation.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestSnapshotCreationWithShorterHMSEventTtl.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestSnapshotWithLongerHMSFollowerLongerInterval.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  4cd00e6672730773c74b9840247d1f4d5f7bdfe4 
> 
> 
> Diff: https://reviews.apache.org/r/64955/diff/5/
> 
> 
> Testing
> ---
> 
> Made sure that tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>



Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Na Li
https://issues.apache.org/jira/browse/SENTRY-2129 is create to track the
development activities for user-based privilege. I will add more sub-tasks
to it

On Thu, Jan 25, 2018 at 1:42 PM, Alexander Kolbasov 
wrote:

> Agreed, making 2.1 with just user-level privileges improvements (plus set
> of accumulated bug fixes) sounds reasonable.
>
> On Thu, Jan 25, 2018 at 11:41 AM, Alexander Kolbasov 
> wrote:
>
> > Looks like we have a consensus of doing user-level privileges
> improvements
> > for 2.1. Let's see whether anyone wants to add more content.
> >
> > On Thu, Jan 25, 2018 at 11:38 AM, Na Li  wrote:
> >
> >> Sasha,
> >>
> >> I have looked into how to complete the user-based privilege for a while,
> >> and can commit to implement it. I can work with Kalyan to create a
> design
> >> doc for user-based privilege.
> >>
> >> Thanks,
> >>
> >> Lina
> >>
> >> On Thu, Jan 25, 2018 at 1:35 PM, Na Li  wrote:
> >>
> >> > Sasha,
> >> >
> >> > The current user-based privilege missed some items:
> >> >
> >> >
> >> >- Sentry policy has two service API: SentryPolicyService and
> >> SentryGenericPolicyService.
> >> >The current implementation does not support user-based privilege
> for
> >> >SentryGenericPolicyService
> >> >- SENTRY-2091: User-based Privilege is broken by SENTRY-769. The
> >> patch
> >> >is available for review.
> >> >- Name Node need change to generate ACL using user privilege.
> >> >   - The full snapshot update only contains authorization to roles
> >> >   mapping and role to group mapping. *Need to add role to user
> >> >   mapping in* SentryStore.retrieveFullRoleImageCore
> >> >   - The delta updates are taken from table SENTRY_PERM_CHANGE,
> which
> >> >   does not distinguish group based permission or user based
> >> permission. No
> >> >   change is needed
> >> >   - The user changes to a role is not included when sending delta
> >> >   update from Sentry to NN. *Need to add AddUsers and DropUsers
> >> >   in TRoleChanges*.
> >> >   - Sentry only create ACL for group with ACL type
> >> >   as AclEntryType.GROUP. *Need to add code to create ACL with type
> >> >   as *AclEntryType.USER
> >> >   - SentryINodeAttributesProvider.checkPermission
> >> >  -> FSPermissionChecker.checkPermission ->
> >> >  SentryINodeAttributesProvider.getAclFeature
> >> >  -> SentryAuthorizationInfo.getAclEntries ->
> SentryPermissions.
> >> >  constructAclEntry
> >> >   - SentryStore.grantOptionCheck() has to be changed to find user
> >> >level privilege.
> >> >
> >> > Thanks,
> >> >
> >> > Lina
> >> >
> >> > On Thu, Jan 25, 2018 at 1:13 PM, Sergio Pena <
> sergio.p...@cloudera.com>
> >> > wrote:
> >> >
> >> >> There is a section on the Wiki about roadmap ideas and JIRAs already
> >> >> created:
> >> >> https://cwiki.apache.org/confluence/display/SENTRY/Sentry+
> >> >> Roadmap+and+ideas
> >> >>
> >> >> I'm interested in having user-level privileges and special user
> >> privileges
> >> >> for objects owners.
> >> >>
> >> >> I got this from the linked above:
> >> >>   SENTRY-1073 User who creates a table should be granted all
> >> privileges on
> >> >> it by default
> >> >>   SENTRY-1068 Allow user who created a table to have "with grant"
> over
> >> >> that
> >> >> table by default
> >> >>   Creator of a table should have ownership of it (all privileges)
> >> >>   Allow privileges to be granted to users directly
> >> >>
> >> >> We should start planning the next Sentry 2.1 release based on the
> >> desired
> >> >> features. What about
> >> >> having 2 or 3 features on Sentry 2.1?
> >> >>
> >> >> I vote for:
> >> >> - user-level privileges (currently grant user to role is only
> >> supported)
> >> >> - default user privileges for objects owners
> >> >>
> >> >> Should we start a vote for new features for 2.1?
> >> >>
> >> >> - Sergio
> >> >>
> >> >> On Thu, Jan 25, 2018 at 12:46 PM, Kalyan Kumar Kalvagadda <
> >> >> kkal...@cloudera.com> wrote:
> >> >>
> >> >> > I would like to add something here.
> >> >> >
> >> >> >
> >> >> >1. Current support for user-based-privileges allows admin to
> >> grant a
> >> >> >role to user. Ideally, user-based-privileges feature should be
> >> >> allowing
> >> >> >administrator to grant privileges to individual users directly.
> >> >> >   -  I'm working on this to come up with a scope doc.
> >> >> >   2. Currently sentry stores only grant privileges. This is not
> >> >> >flexible. Let's say an administrator wants to grant role with
> >> select
> >> >> on
> >> >> > the
> >> >> >all tables in a database except for couple to them, he needs to
> >> >> > individual
> >> >> >select privileges for each table.
> >> >> >   1. Implementation should let you add a grant privilege on
> >> database
> >> >> >   and revokes privileges on the tables with in that database,

Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Alexander Kolbasov
Agreed, making 2.1 with just user-level privileges improvements (plus set
of accumulated bug fixes) sounds reasonable.

On Thu, Jan 25, 2018 at 11:41 AM, Alexander Kolbasov 
wrote:

> Looks like we have a consensus of doing user-level privileges improvements
> for 2.1. Let's see whether anyone wants to add more content.
>
> On Thu, Jan 25, 2018 at 11:38 AM, Na Li  wrote:
>
>> Sasha,
>>
>> I have looked into how to complete the user-based privilege for a while,
>> and can commit to implement it. I can work with Kalyan to create a design
>> doc for user-based privilege.
>>
>> Thanks,
>>
>> Lina
>>
>> On Thu, Jan 25, 2018 at 1:35 PM, Na Li  wrote:
>>
>> > Sasha,
>> >
>> > The current user-based privilege missed some items:
>> >
>> >
>> >- Sentry policy has two service API: SentryPolicyService and
>> SentryGenericPolicyService.
>> >The current implementation does not support user-based privilege for
>> >SentryGenericPolicyService
>> >- SENTRY-2091: User-based Privilege is broken by SENTRY-769. The
>> patch
>> >is available for review.
>> >- Name Node need change to generate ACL using user privilege.
>> >   - The full snapshot update only contains authorization to roles
>> >   mapping and role to group mapping. *Need to add role to user
>> >   mapping in* SentryStore.retrieveFullRoleImageCore
>> >   - The delta updates are taken from table SENTRY_PERM_CHANGE, which
>> >   does not distinguish group based permission or user based
>> permission. No
>> >   change is needed
>> >   - The user changes to a role is not included when sending delta
>> >   update from Sentry to NN. *Need to add AddUsers and DropUsers
>> >   in TRoleChanges*.
>> >   - Sentry only create ACL for group with ACL type
>> >   as AclEntryType.GROUP. *Need to add code to create ACL with type
>> >   as *AclEntryType.USER
>> >   - SentryINodeAttributesProvider.checkPermission
>> >  -> FSPermissionChecker.checkPermission ->
>> >  SentryINodeAttributesProvider.getAclFeature
>> >  -> SentryAuthorizationInfo.getAclEntries -> SentryPermissions.
>> >  constructAclEntry
>> >   - SentryStore.grantOptionCheck() has to be changed to find user
>> >level privilege.
>> >
>> > Thanks,
>> >
>> > Lina
>> >
>> > On Thu, Jan 25, 2018 at 1:13 PM, Sergio Pena 
>> > wrote:
>> >
>> >> There is a section on the Wiki about roadmap ideas and JIRAs already
>> >> created:
>> >> https://cwiki.apache.org/confluence/display/SENTRY/Sentry+
>> >> Roadmap+and+ideas
>> >>
>> >> I'm interested in having user-level privileges and special user
>> privileges
>> >> for objects owners.
>> >>
>> >> I got this from the linked above:
>> >>   SENTRY-1073 User who creates a table should be granted all
>> privileges on
>> >> it by default
>> >>   SENTRY-1068 Allow user who created a table to have "with grant" over
>> >> that
>> >> table by default
>> >>   Creator of a table should have ownership of it (all privileges)
>> >>   Allow privileges to be granted to users directly
>> >>
>> >> We should start planning the next Sentry 2.1 release based on the
>> desired
>> >> features. What about
>> >> having 2 or 3 features on Sentry 2.1?
>> >>
>> >> I vote for:
>> >> - user-level privileges (currently grant user to role is only
>> supported)
>> >> - default user privileges for objects owners
>> >>
>> >> Should we start a vote for new features for 2.1?
>> >>
>> >> - Sergio
>> >>
>> >> On Thu, Jan 25, 2018 at 12:46 PM, Kalyan Kumar Kalvagadda <
>> >> kkal...@cloudera.com> wrote:
>> >>
>> >> > I would like to add something here.
>> >> >
>> >> >
>> >> >1. Current support for user-based-privileges allows admin to
>> grant a
>> >> >role to user. Ideally, user-based-privileges feature should be
>> >> allowing
>> >> >administrator to grant privileges to individual users directly.
>> >> >   -  I'm working on this to come up with a scope doc.
>> >> >   2. Currently sentry stores only grant privileges. This is not
>> >> >flexible. Let's say an administrator wants to grant role with
>> select
>> >> on
>> >> > the
>> >> >all tables in a database except for couple to them, he needs to
>> >> > individual
>> >> >select privileges for each table.
>> >> >   1. Implementation should let you add a grant privilege on
>> database
>> >> >   and revokes privileges on the tables with in that database,
>> >> >   2. This needs new look into privilege model that sentry
>> currently
>> >> > has.
>> >> >
>> >> >
>> >> > -Kalyan
>> >> >
>> >> >
>> >> > -Kalyan
>> >> >
>> >> > On Thu, Jan 25, 2018 at 12:16 PM, Alexander Kolbasov <
>> >> ak...@cloudera.com>
>> >> > wrote:
>> >> >
>> >> > > Good point. There is some support for user-level privileges in 2.0
>> >> > already
>> >> > > - do you think that it is not sufficient and is missing some parts?
>> >> > >
>> >> > > Is there anyone 

Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Sergio Pena
I don't think there exists a user-level privileges doc yet.
I can commit to finish the default owner privileges and submit a spec doc
for the changes we can do to finish it soon.

Anyone else likes to commit to user-level?

Btw, I'd like to lower Sentry 2.1 to 1 or 2 features so that we have a
release sooner? since Sentry 2.0 was released back in November 2017, it
should be good to have another release soon with fewer features
ideas?

- Sergio

On Thu, Jan 25, 2018 at 1:19 PM, Alexander Kolbasov 
wrote:

> Thanks for the link - it is nice to integrate this discussion with JIRA
> keywords. Looks like we need to go through the list and add categorize it
> into short-term and long-term buckets.
>
> I think Sergio's idea of doing smaller releases with small number of
> features included makes sense.  We can vote for individual features, of
> course but it only makes sense if someone actually commits to implementing
> it.
>
> Looks like so far the discussion is about improving user-level privileges -
> it would be a good content for 2.1 release.
>
> Is there some kind of design doc for user-level privileges in general? If
> not, would it make sense to create one?
>
> - Alex
>
> On Thu, Jan 25, 2018 at 11:13 AM, Sergio Pena 
> wrote:
>
> > There is a section on the Wiki about roadmap ideas and JIRAs already
> > created:
> > https://cwiki.apache.org/confluence/display/SENTRY/
> > Sentry+Roadmap+and+ideas
> >
> > I'm interested in having user-level privileges and special user
> privileges
> > for objects owners.
> >
> > I got this from the linked above:
> >   SENTRY-1073 User who creates a table should be granted all privileges
> on
> > it by default
> >   SENTRY-1068 Allow user who created a table to have "with grant" over
> that
> > table by default
> >   Creator of a table should have ownership of it (all privileges)
> >   Allow privileges to be granted to users directly
> >
> > We should start planning the next Sentry 2.1 release based on the desired
> > features. What about
> > having 2 or 3 features on Sentry 2.1?
> >
> > I vote for:
> > - user-level privileges (currently grant user to role is only supported)
> > - default user privileges for objects owners
> >
> > Should we start a vote for new features for 2.1?
> >
> > - Sergio
> >
> > On Thu, Jan 25, 2018 at 12:46 PM, Kalyan Kumar Kalvagadda <
> > kkal...@cloudera.com> wrote:
> >
> > > I would like to add something here.
> > >
> > >
> > >1. Current support for user-based-privileges allows admin to grant a
> > >role to user. Ideally, user-based-privileges feature should be
> > allowing
> > >administrator to grant privileges to individual users directly.
> > >   -  I'm working on this to come up with a scope doc.
> > >   2. Currently sentry stores only grant privileges. This is not
> > >flexible. Let's say an administrator wants to grant role with select
> > on
> > > the
> > >all tables in a database except for couple to them, he needs to
> > > individual
> > >select privileges for each table.
> > >   1. Implementation should let you add a grant privilege on
> database
> > >   and revokes privileges on the tables with in that database,
> > >   2. This needs new look into privilege model that sentry currently
> > > has.
> > >
> > >
> > > -Kalyan
> > >
> > >
> > > -Kalyan
> > >
> > > On Thu, Jan 25, 2018 at 12:16 PM, Alexander Kolbasov <
> ak...@cloudera.com
> > >
> > > wrote:
> > >
> > > > Good point. There is some support for user-level privileges in 2.0
> > > already
> > > > - do you think that it is not sufficient and is missing some parts?
> > > >
> > > > Is there anyone reading this who participated in the user-level
> > > privileges
> > > > in Sentry work done earlier? Is there any design doc for this?
> > > >
> > > > - Alex
> > > >
> > > > On Thu, Jan 25, 2018 at 10:11 AM, Na Li 
> wrote:
> > > >
> > > > > Sasha,
> > > > >
> > > > > It would be nice to have more features for sentry.
> > > > >
> > > > > For example, make user-based privileges working. So user can assign
> > > user
> > > > > directly to a role instead of through group.
> > > > >
> > > > > Lina
> > > > >
> > > > > On Thu, Jan 25, 2018 at 11:58 AM, Alexander Kolbasov <
> > > ak...@cloudera.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Now that we have Sentry 2.0 release, I think it is a good time to
> > > step
> > > > > back
> > > > > > from fixing bugs and immediate problems and start discussions on
> > > > roadmap
> > > > > > for Sentry going forward. Do we want to just keep it as is and
> > > improve
> > > > > > things here and there or we want to add new features?
> > > > > >
> > > > > > What do people think?
> > > > > >
> > > > > > - Alex
> > > > > >
> > > > >
> > > >
> > >
> >
>


Re: Review Request 65321: SENTRY-2127: Fix unstable unit test TestColumnEndToEnd.testCrossDbTableOperations

2018-01-25 Thread Sergio Pena via Review Board

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


Ship it!




Ship It!

- Sergio Pena


On Jan. 24, 2018, 9:33 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65321/
> ---
> 
> (Updated Jan. 24, 2018, 9:33 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Make the statement that could fail to a statement that is more stable to 
> excute
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java
>  8c15257 
> 
> 
> Diff: https://reviews.apache.org/r/65321/diff/1/
> 
> 
> Testing
> ---
> 
> unit tests on different enviornment and the update makes the unit test more 
> stable.
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Alexander Kolbasov
Looks like we have a consensus of doing user-level privileges improvements
for 2.1. Let's see whether anyone wants to add more content.

On Thu, Jan 25, 2018 at 11:38 AM, Na Li  wrote:

> Sasha,
>
> I have looked into how to complete the user-based privilege for a while,
> and can commit to implement it. I can work with Kalyan to create a design
> doc for user-based privilege.
>
> Thanks,
>
> Lina
>
> On Thu, Jan 25, 2018 at 1:35 PM, Na Li  wrote:
>
> > Sasha,
> >
> > The current user-based privilege missed some items:
> >
> >
> >- Sentry policy has two service API: SentryPolicyService and
> SentryGenericPolicyService.
> >The current implementation does not support user-based privilege for
> >SentryGenericPolicyService
> >- SENTRY-2091: User-based Privilege is broken by SENTRY-769. The patch
> >is available for review.
> >- Name Node need change to generate ACL using user privilege.
> >   - The full snapshot update only contains authorization to roles
> >   mapping and role to group mapping. *Need to add role to user
> >   mapping in* SentryStore.retrieveFullRoleImageCore
> >   - The delta updates are taken from table SENTRY_PERM_CHANGE, which
> >   does not distinguish group based permission or user based
> permission. No
> >   change is needed
> >   - The user changes to a role is not included when sending delta
> >   update from Sentry to NN. *Need to add AddUsers and DropUsers
> >   in TRoleChanges*.
> >   - Sentry only create ACL for group with ACL type
> >   as AclEntryType.GROUP. *Need to add code to create ACL with type
> >   as *AclEntryType.USER
> >   - SentryINodeAttributesProvider.checkPermission
> >  -> FSPermissionChecker.checkPermission ->
> >  SentryINodeAttributesProvider.getAclFeature
> >  -> SentryAuthorizationInfo.getAclEntries -> SentryPermissions.
> >  constructAclEntry
> >   - SentryStore.grantOptionCheck() has to be changed to find user
> >level privilege.
> >
> > Thanks,
> >
> > Lina
> >
> > On Thu, Jan 25, 2018 at 1:13 PM, Sergio Pena 
> > wrote:
> >
> >> There is a section on the Wiki about roadmap ideas and JIRAs already
> >> created:
> >> https://cwiki.apache.org/confluence/display/SENTRY/Sentry+
> >> Roadmap+and+ideas
> >>
> >> I'm interested in having user-level privileges and special user
> privileges
> >> for objects owners.
> >>
> >> I got this from the linked above:
> >>   SENTRY-1073 User who creates a table should be granted all privileges
> on
> >> it by default
> >>   SENTRY-1068 Allow user who created a table to have "with grant" over
> >> that
> >> table by default
> >>   Creator of a table should have ownership of it (all privileges)
> >>   Allow privileges to be granted to users directly
> >>
> >> We should start planning the next Sentry 2.1 release based on the
> desired
> >> features. What about
> >> having 2 or 3 features on Sentry 2.1?
> >>
> >> I vote for:
> >> - user-level privileges (currently grant user to role is only supported)
> >> - default user privileges for objects owners
> >>
> >> Should we start a vote for new features for 2.1?
> >>
> >> - Sergio
> >>
> >> On Thu, Jan 25, 2018 at 12:46 PM, Kalyan Kumar Kalvagadda <
> >> kkal...@cloudera.com> wrote:
> >>
> >> > I would like to add something here.
> >> >
> >> >
> >> >1. Current support for user-based-privileges allows admin to grant
> a
> >> >role to user. Ideally, user-based-privileges feature should be
> >> allowing
> >> >administrator to grant privileges to individual users directly.
> >> >   -  I'm working on this to come up with a scope doc.
> >> >   2. Currently sentry stores only grant privileges. This is not
> >> >flexible. Let's say an administrator wants to grant role with
> select
> >> on
> >> > the
> >> >all tables in a database except for couple to them, he needs to
> >> > individual
> >> >select privileges for each table.
> >> >   1. Implementation should let you add a grant privilege on
> database
> >> >   and revokes privileges on the tables with in that database,
> >> >   2. This needs new look into privilege model that sentry
> currently
> >> > has.
> >> >
> >> >
> >> > -Kalyan
> >> >
> >> >
> >> > -Kalyan
> >> >
> >> > On Thu, Jan 25, 2018 at 12:16 PM, Alexander Kolbasov <
> >> ak...@cloudera.com>
> >> > wrote:
> >> >
> >> > > Good point. There is some support for user-level privileges in 2.0
> >> > already
> >> > > - do you think that it is not sufficient and is missing some parts?
> >> > >
> >> > > Is there anyone reading this who participated in the user-level
> >> > privileges
> >> > > in Sentry work done earlier? Is there any design doc for this?
> >> > >
> >> > > - Alex
> >> > >
> >> > > On Thu, Jan 25, 2018 at 10:11 AM, Na Li 
> wrote:
> >> > >
> >> > > > Sasha,
> >> > > >
> >> > > > It would be nice to have more features for 

Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Alexander Kolbasov
Lina - would it make sense to create uber-JIRA for ULP, mark it with
"roadmap" keyword and start putting some of these as subtasks?

On Thu, Jan 25, 2018 at 11:35 AM, Na Li  wrote:

> Sasha,
>
> The current user-based privilege missed some items:
>
>
>- Sentry policy has two service API: SentryPolicyService
>and SentryGenericPolicyService. The current implementation does not
> support
>user-based privilege for SentryGenericPolicyService
>- SENTRY-2091: User-based Privilege is broken by SENTRY-769. The patch
>is available for review.
>- Name Node need change to generate ACL using user privilege.
>   - The full snapshot update only contains authorization to roles
>   mapping and role to group mapping. *Need to add role to user mapping
>   in* SentryStore.retrieveFullRoleImageCore
>   - The delta updates are taken from table SENTRY_PERM_CHANGE, which
>   does not distinguish group based permission or user based
> permission. No
>   change is needed
>   - The user changes to a role is not included when sending delta
>   update from Sentry to NN. *Need to add AddUsers and DropUsers
>   in TRoleChanges*.
>   - Sentry only create ACL for group with ACL type
>   as AclEntryType.GROUP. *Need to add code to create ACL with type as *
>   AclEntryType.USER
>   - SentryINodeAttributesProvider.checkPermission
>  -> FSPermissionChecker.checkPermission
>  -> SentryINodeAttributesProvider.getAclFeature
>  -> SentryAuthorizationInfo.getAclEntries
>  -> SentryPermissions.constructAclEntry
>   - SentryStore.grantOptionCheck() has to be changed to find user level
>privilege.
>
> Thanks,
>
> Lina
>
> On Thu, Jan 25, 2018 at 1:13 PM, Sergio Pena 
> wrote:
>
> > There is a section on the Wiki about roadmap ideas and JIRAs already
> > created:
> > https://cwiki.apache.org/confluence/display/SENTRY/
> > Sentry+Roadmap+and+ideas
> >
> > I'm interested in having user-level privileges and special user
> privileges
> > for objects owners.
> >
> > I got this from the linked above:
> >   SENTRY-1073 User who creates a table should be granted all privileges
> on
> > it by default
> >   SENTRY-1068 Allow user who created a table to have "with grant" over
> that
> > table by default
> >   Creator of a table should have ownership of it (all privileges)
> >   Allow privileges to be granted to users directly
> >
> > We should start planning the next Sentry 2.1 release based on the desired
> > features. What about
> > having 2 or 3 features on Sentry 2.1?
> >
> > I vote for:
> > - user-level privileges (currently grant user to role is only supported)
> > - default user privileges for objects owners
> >
> > Should we start a vote for new features for 2.1?
> >
> > - Sergio
> >
> > On Thu, Jan 25, 2018 at 12:46 PM, Kalyan Kumar Kalvagadda <
> > kkal...@cloudera.com> wrote:
> >
> > > I would like to add something here.
> > >
> > >
> > >1. Current support for user-based-privileges allows admin to grant a
> > >role to user. Ideally, user-based-privileges feature should be
> > allowing
> > >administrator to grant privileges to individual users directly.
> > >   -  I'm working on this to come up with a scope doc.
> > >   2. Currently sentry stores only grant privileges. This is not
> > >flexible. Let's say an administrator wants to grant role with select
> > on
> > > the
> > >all tables in a database except for couple to them, he needs to
> > > individual
> > >select privileges for each table.
> > >   1. Implementation should let you add a grant privilege on
> database
> > >   and revokes privileges on the tables with in that database,
> > >   2. This needs new look into privilege model that sentry currently
> > > has.
> > >
> > >
> > > -Kalyan
> > >
> > >
> > > -Kalyan
> > >
> > > On Thu, Jan 25, 2018 at 12:16 PM, Alexander Kolbasov <
> ak...@cloudera.com
> > >
> > > wrote:
> > >
> > > > Good point. There is some support for user-level privileges in 2.0
> > > already
> > > > - do you think that it is not sufficient and is missing some parts?
> > > >
> > > > Is there anyone reading this who participated in the user-level
> > > privileges
> > > > in Sentry work done earlier? Is there any design doc for this?
> > > >
> > > > - Alex
> > > >
> > > > On Thu, Jan 25, 2018 at 10:11 AM, Na Li 
> wrote:
> > > >
> > > > > Sasha,
> > > > >
> > > > > It would be nice to have more features for sentry.
> > > > >
> > > > > For example, make user-based privileges working. So user can assign
> > > user
> > > > > directly to a role instead of through group.
> > > > >
> > > > > Lina
> > > > >
> > > > > On Thu, Jan 25, 2018 at 11:58 AM, Alexander Kolbasov <
> > > ak...@cloudera.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Now that we have Sentry 2.0 release, I think it is a good time to
> > > step
> > > > > back
> > > > > > from fixing 

Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Na Li
Sasha,

I have looked into how to complete the user-based privilege for a while,
and can commit to implement it. I can work with Kalyan to create a design
doc for user-based privilege.

Thanks,

Lina

On Thu, Jan 25, 2018 at 1:35 PM, Na Li  wrote:

> Sasha,
>
> The current user-based privilege missed some items:
>
>
>- Sentry policy has two service API: SentryPolicyService and 
> SentryGenericPolicyService.
>The current implementation does not support user-based privilege for
>SentryGenericPolicyService
>- SENTRY-2091: User-based Privilege is broken by SENTRY-769. The patch
>is available for review.
>- Name Node need change to generate ACL using user privilege.
>   - The full snapshot update only contains authorization to roles
>   mapping and role to group mapping. *Need to add role to user
>   mapping in* SentryStore.retrieveFullRoleImageCore
>   - The delta updates are taken from table SENTRY_PERM_CHANGE, which
>   does not distinguish group based permission or user based permission. No
>   change is needed
>   - The user changes to a role is not included when sending delta
>   update from Sentry to NN. *Need to add AddUsers and DropUsers
>   in TRoleChanges*.
>   - Sentry only create ACL for group with ACL type
>   as AclEntryType.GROUP. *Need to add code to create ACL with type
>   as *AclEntryType.USER
>   - SentryINodeAttributesProvider.checkPermission
>  -> FSPermissionChecker.checkPermission ->
>  SentryINodeAttributesProvider.getAclFeature
>  -> SentryAuthorizationInfo.getAclEntries -> SentryPermissions.
>  constructAclEntry
>   - SentryStore.grantOptionCheck() has to be changed to find user
>level privilege.
>
> Thanks,
>
> Lina
>
> On Thu, Jan 25, 2018 at 1:13 PM, Sergio Pena 
> wrote:
>
>> There is a section on the Wiki about roadmap ideas and JIRAs already
>> created:
>> https://cwiki.apache.org/confluence/display/SENTRY/Sentry+
>> Roadmap+and+ideas
>>
>> I'm interested in having user-level privileges and special user privileges
>> for objects owners.
>>
>> I got this from the linked above:
>>   SENTRY-1073 User who creates a table should be granted all privileges on
>> it by default
>>   SENTRY-1068 Allow user who created a table to have "with grant" over
>> that
>> table by default
>>   Creator of a table should have ownership of it (all privileges)
>>   Allow privileges to be granted to users directly
>>
>> We should start planning the next Sentry 2.1 release based on the desired
>> features. What about
>> having 2 or 3 features on Sentry 2.1?
>>
>> I vote for:
>> - user-level privileges (currently grant user to role is only supported)
>> - default user privileges for objects owners
>>
>> Should we start a vote for new features for 2.1?
>>
>> - Sergio
>>
>> On Thu, Jan 25, 2018 at 12:46 PM, Kalyan Kumar Kalvagadda <
>> kkal...@cloudera.com> wrote:
>>
>> > I would like to add something here.
>> >
>> >
>> >1. Current support for user-based-privileges allows admin to grant a
>> >role to user. Ideally, user-based-privileges feature should be
>> allowing
>> >administrator to grant privileges to individual users directly.
>> >   -  I'm working on this to come up with a scope doc.
>> >   2. Currently sentry stores only grant privileges. This is not
>> >flexible. Let's say an administrator wants to grant role with select
>> on
>> > the
>> >all tables in a database except for couple to them, he needs to
>> > individual
>> >select privileges for each table.
>> >   1. Implementation should let you add a grant privilege on database
>> >   and revokes privileges on the tables with in that database,
>> >   2. This needs new look into privilege model that sentry currently
>> > has.
>> >
>> >
>> > -Kalyan
>> >
>> >
>> > -Kalyan
>> >
>> > On Thu, Jan 25, 2018 at 12:16 PM, Alexander Kolbasov <
>> ak...@cloudera.com>
>> > wrote:
>> >
>> > > Good point. There is some support for user-level privileges in 2.0
>> > already
>> > > - do you think that it is not sufficient and is missing some parts?
>> > >
>> > > Is there anyone reading this who participated in the user-level
>> > privileges
>> > > in Sentry work done earlier? Is there any design doc for this?
>> > >
>> > > - Alex
>> > >
>> > > On Thu, Jan 25, 2018 at 10:11 AM, Na Li  wrote:
>> > >
>> > > > Sasha,
>> > > >
>> > > > It would be nice to have more features for sentry.
>> > > >
>> > > > For example, make user-based privileges working. So user can assign
>> > user
>> > > > directly to a role instead of through group.
>> > > >
>> > > > Lina
>> > > >
>> > > > On Thu, Jan 25, 2018 at 11:58 AM, Alexander Kolbasov <
>> > ak...@cloudera.com
>> > > >
>> > > > wrote:
>> > > >
>> > > > > Now that we have Sentry 2.0 release, I think it is a good time to
>> > step
>> > > > back
>> > > > > from fixing bugs and immediate problems and start 

Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Na Li
Sasha,

The current user-based privilege missed some items:


   - Sentry policy has two service API: SentryPolicyService
   and SentryGenericPolicyService. The current implementation does not support
   user-based privilege for SentryGenericPolicyService
   - SENTRY-2091: User-based Privilege is broken by SENTRY-769. The patch
   is available for review.
   - Name Node need change to generate ACL using user privilege.
  - The full snapshot update only contains authorization to roles
  mapping and role to group mapping. *Need to add role to user mapping
  in* SentryStore.retrieveFullRoleImageCore
  - The delta updates are taken from table SENTRY_PERM_CHANGE, which
  does not distinguish group based permission or user based permission. No
  change is needed
  - The user changes to a role is not included when sending delta
  update from Sentry to NN. *Need to add AddUsers and DropUsers
  in TRoleChanges*.
  - Sentry only create ACL for group with ACL type
  as AclEntryType.GROUP. *Need to add code to create ACL with type as *
  AclEntryType.USER
  - SentryINodeAttributesProvider.checkPermission
 -> FSPermissionChecker.checkPermission
 -> SentryINodeAttributesProvider.getAclFeature
 -> SentryAuthorizationInfo.getAclEntries
 -> SentryPermissions.constructAclEntry
  - SentryStore.grantOptionCheck() has to be changed to find user level
   privilege.

Thanks,

Lina

On Thu, Jan 25, 2018 at 1:13 PM, Sergio Pena 
wrote:

> There is a section on the Wiki about roadmap ideas and JIRAs already
> created:
> https://cwiki.apache.org/confluence/display/SENTRY/
> Sentry+Roadmap+and+ideas
>
> I'm interested in having user-level privileges and special user privileges
> for objects owners.
>
> I got this from the linked above:
>   SENTRY-1073 User who creates a table should be granted all privileges on
> it by default
>   SENTRY-1068 Allow user who created a table to have "with grant" over that
> table by default
>   Creator of a table should have ownership of it (all privileges)
>   Allow privileges to be granted to users directly
>
> We should start planning the next Sentry 2.1 release based on the desired
> features. What about
> having 2 or 3 features on Sentry 2.1?
>
> I vote for:
> - user-level privileges (currently grant user to role is only supported)
> - default user privileges for objects owners
>
> Should we start a vote for new features for 2.1?
>
> - Sergio
>
> On Thu, Jan 25, 2018 at 12:46 PM, Kalyan Kumar Kalvagadda <
> kkal...@cloudera.com> wrote:
>
> > I would like to add something here.
> >
> >
> >1. Current support for user-based-privileges allows admin to grant a
> >role to user. Ideally, user-based-privileges feature should be
> allowing
> >administrator to grant privileges to individual users directly.
> >   -  I'm working on this to come up with a scope doc.
> >   2. Currently sentry stores only grant privileges. This is not
> >flexible. Let's say an administrator wants to grant role with select
> on
> > the
> >all tables in a database except for couple to them, he needs to
> > individual
> >select privileges for each table.
> >   1. Implementation should let you add a grant privilege on database
> >   and revokes privileges on the tables with in that database,
> >   2. This needs new look into privilege model that sentry currently
> > has.
> >
> >
> > -Kalyan
> >
> >
> > -Kalyan
> >
> > On Thu, Jan 25, 2018 at 12:16 PM, Alexander Kolbasov  >
> > wrote:
> >
> > > Good point. There is some support for user-level privileges in 2.0
> > already
> > > - do you think that it is not sufficient and is missing some parts?
> > >
> > > Is there anyone reading this who participated in the user-level
> > privileges
> > > in Sentry work done earlier? Is there any design doc for this?
> > >
> > > - Alex
> > >
> > > On Thu, Jan 25, 2018 at 10:11 AM, Na Li  wrote:
> > >
> > > > Sasha,
> > > >
> > > > It would be nice to have more features for sentry.
> > > >
> > > > For example, make user-based privileges working. So user can assign
> > user
> > > > directly to a role instead of through group.
> > > >
> > > > Lina
> > > >
> > > > On Thu, Jan 25, 2018 at 11:58 AM, Alexander Kolbasov <
> > ak...@cloudera.com
> > > >
> > > > wrote:
> > > >
> > > > > Now that we have Sentry 2.0 release, I think it is a good time to
> > step
> > > > back
> > > > > from fixing bugs and immediate problems and start discussions on
> > > roadmap
> > > > > for Sentry going forward. Do we want to just keep it as is and
> > improve
> > > > > things here and there or we want to add new features?
> > > > >
> > > > > What do people think?
> > > > >
> > > > > - Alex
> > > > >
> > > >
> > >
> >
>


Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Alexander Kolbasov
Thanks for the link - it is nice to integrate this discussion with JIRA
keywords. Looks like we need to go through the list and add categorize it
into short-term and long-term buckets.

I think Sergio's idea of doing smaller releases with small number of
features included makes sense.  We can vote for individual features, of
course but it only makes sense if someone actually commits to implementing
it.

Looks like so far the discussion is about improving user-level privileges -
it would be a good content for 2.1 release.

Is there some kind of design doc for user-level privileges in general? If
not, would it make sense to create one?

- Alex

On Thu, Jan 25, 2018 at 11:13 AM, Sergio Pena 
wrote:

> There is a section on the Wiki about roadmap ideas and JIRAs already
> created:
> https://cwiki.apache.org/confluence/display/SENTRY/
> Sentry+Roadmap+and+ideas
>
> I'm interested in having user-level privileges and special user privileges
> for objects owners.
>
> I got this from the linked above:
>   SENTRY-1073 User who creates a table should be granted all privileges on
> it by default
>   SENTRY-1068 Allow user who created a table to have "with grant" over that
> table by default
>   Creator of a table should have ownership of it (all privileges)
>   Allow privileges to be granted to users directly
>
> We should start planning the next Sentry 2.1 release based on the desired
> features. What about
> having 2 or 3 features on Sentry 2.1?
>
> I vote for:
> - user-level privileges (currently grant user to role is only supported)
> - default user privileges for objects owners
>
> Should we start a vote for new features for 2.1?
>
> - Sergio
>
> On Thu, Jan 25, 2018 at 12:46 PM, Kalyan Kumar Kalvagadda <
> kkal...@cloudera.com> wrote:
>
> > I would like to add something here.
> >
> >
> >1. Current support for user-based-privileges allows admin to grant a
> >role to user. Ideally, user-based-privileges feature should be
> allowing
> >administrator to grant privileges to individual users directly.
> >   -  I'm working on this to come up with a scope doc.
> >   2. Currently sentry stores only grant privileges. This is not
> >flexible. Let's say an administrator wants to grant role with select
> on
> > the
> >all tables in a database except for couple to them, he needs to
> > individual
> >select privileges for each table.
> >   1. Implementation should let you add a grant privilege on database
> >   and revokes privileges on the tables with in that database,
> >   2. This needs new look into privilege model that sentry currently
> > has.
> >
> >
> > -Kalyan
> >
> >
> > -Kalyan
> >
> > On Thu, Jan 25, 2018 at 12:16 PM, Alexander Kolbasov  >
> > wrote:
> >
> > > Good point. There is some support for user-level privileges in 2.0
> > already
> > > - do you think that it is not sufficient and is missing some parts?
> > >
> > > Is there anyone reading this who participated in the user-level
> > privileges
> > > in Sentry work done earlier? Is there any design doc for this?
> > >
> > > - Alex
> > >
> > > On Thu, Jan 25, 2018 at 10:11 AM, Na Li  wrote:
> > >
> > > > Sasha,
> > > >
> > > > It would be nice to have more features for sentry.
> > > >
> > > > For example, make user-based privileges working. So user can assign
> > user
> > > > directly to a role instead of through group.
> > > >
> > > > Lina
> > > >
> > > > On Thu, Jan 25, 2018 at 11:58 AM, Alexander Kolbasov <
> > ak...@cloudera.com
> > > >
> > > > wrote:
> > > >
> > > > > Now that we have Sentry 2.0 release, I think it is a good time to
> > step
> > > > back
> > > > > from fixing bugs and immediate problems and start discussions on
> > > roadmap
> > > > > for Sentry going forward. Do we want to just keep it as is and
> > improve
> > > > > things here and there or we want to add new features?
> > > > >
> > > > > What do people think?
> > > > >
> > > > > - Alex
> > > > >
> > > >
> > >
> >
>


Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Sergio Pena
There is a section on the Wiki about roadmap ideas and JIRAs already
created:
https://cwiki.apache.org/confluence/display/SENTRY/Sentry+Roadmap+and+ideas

I'm interested in having user-level privileges and special user privileges
for objects owners.

I got this from the linked above:
  SENTRY-1073 User who creates a table should be granted all privileges on
it by default
  SENTRY-1068 Allow user who created a table to have "with grant" over that
table by default
  Creator of a table should have ownership of it (all privileges)
  Allow privileges to be granted to users directly

We should start planning the next Sentry 2.1 release based on the desired
features. What about
having 2 or 3 features on Sentry 2.1?

I vote for:
- user-level privileges (currently grant user to role is only supported)
- default user privileges for objects owners

Should we start a vote for new features for 2.1?

- Sergio

On Thu, Jan 25, 2018 at 12:46 PM, Kalyan Kumar Kalvagadda <
kkal...@cloudera.com> wrote:

> I would like to add something here.
>
>
>1. Current support for user-based-privileges allows admin to grant a
>role to user. Ideally, user-based-privileges feature should be allowing
>administrator to grant privileges to individual users directly.
>   -  I'm working on this to come up with a scope doc.
>   2. Currently sentry stores only grant privileges. This is not
>flexible. Let's say an administrator wants to grant role with select on
> the
>all tables in a database except for couple to them, he needs to
> individual
>select privileges for each table.
>   1. Implementation should let you add a grant privilege on database
>   and revokes privileges on the tables with in that database,
>   2. This needs new look into privilege model that sentry currently
> has.
>
>
> -Kalyan
>
>
> -Kalyan
>
> On Thu, Jan 25, 2018 at 12:16 PM, Alexander Kolbasov 
> wrote:
>
> > Good point. There is some support for user-level privileges in 2.0
> already
> > - do you think that it is not sufficient and is missing some parts?
> >
> > Is there anyone reading this who participated in the user-level
> privileges
> > in Sentry work done earlier? Is there any design doc for this?
> >
> > - Alex
> >
> > On Thu, Jan 25, 2018 at 10:11 AM, Na Li  wrote:
> >
> > > Sasha,
> > >
> > > It would be nice to have more features for sentry.
> > >
> > > For example, make user-based privileges working. So user can assign
> user
> > > directly to a role instead of through group.
> > >
> > > Lina
> > >
> > > On Thu, Jan 25, 2018 at 11:58 AM, Alexander Kolbasov <
> ak...@cloudera.com
> > >
> > > wrote:
> > >
> > > > Now that we have Sentry 2.0 release, I think it is a good time to
> step
> > > back
> > > > from fixing bugs and immediate problems and start discussions on
> > roadmap
> > > > for Sentry going forward. Do we want to just keep it as is and
> improve
> > > > things here and there or we want to add new features?
> > > >
> > > > What do people think?
> > > >
> > > > - Alex
> > > >
> > >
> >
>


Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Kalyan Kumar Kalvagadda
I would like to add something here.


   1. Current support for user-based-privileges allows admin to grant a
   role to user. Ideally, user-based-privileges feature should be allowing
   administrator to grant privileges to individual users directly.
  -  I'm working on this to come up with a scope doc.
  2. Currently sentry stores only grant privileges. This is not
   flexible. Let's say an administrator wants to grant role with select on the
   all tables in a database except for couple to them, he needs to individual
   select privileges for each table.
  1. Implementation should let you add a grant privilege on database
  and revokes privileges on the tables with in that database,
  2. This needs new look into privilege model that sentry currently has.


-Kalyan


-Kalyan

On Thu, Jan 25, 2018 at 12:16 PM, Alexander Kolbasov 
wrote:

> Good point. There is some support for user-level privileges in 2.0 already
> - do you think that it is not sufficient and is missing some parts?
>
> Is there anyone reading this who participated in the user-level privileges
> in Sentry work done earlier? Is there any design doc for this?
>
> - Alex
>
> On Thu, Jan 25, 2018 at 10:11 AM, Na Li  wrote:
>
> > Sasha,
> >
> > It would be nice to have more features for sentry.
> >
> > For example, make user-based privileges working. So user can assign user
> > directly to a role instead of through group.
> >
> > Lina
> >
> > On Thu, Jan 25, 2018 at 11:58 AM, Alexander Kolbasov  >
> > wrote:
> >
> > > Now that we have Sentry 2.0 release, I think it is a good time to step
> > back
> > > from fixing bugs and immediate problems and start discussions on
> roadmap
> > > for Sentry going forward. Do we want to just keep it as is and improve
> > > things here and there or we want to add new features?
> > >
> > > What do people think?
> > >
> > > - Alex
> > >
> >
>


Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Alexander Kolbasov
Good point. There is some support for user-level privileges in 2.0 already
- do you think that it is not sufficient and is missing some parts?

Is there anyone reading this who participated in the user-level privileges
in Sentry work done earlier? Is there any design doc for this?

- Alex

On Thu, Jan 25, 2018 at 10:11 AM, Na Li  wrote:

> Sasha,
>
> It would be nice to have more features for sentry.
>
> For example, make user-based privileges working. So user can assign user
> directly to a role instead of through group.
>
> Lina
>
> On Thu, Jan 25, 2018 at 11:58 AM, Alexander Kolbasov 
> wrote:
>
> > Now that we have Sentry 2.0 release, I think it is a good time to step
> back
> > from fixing bugs and immediate problems and start discussions on roadmap
> > for Sentry going forward. Do we want to just keep it as is and improve
> > things here and there or we want to add new features?
> >
> > What do people think?
> >
> > - Alex
> >
>


Re: [DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Na Li
Sasha,

It would be nice to have more features for sentry.

For example, make user-based privileges working. So user can assign user
directly to a role instead of through group.

Lina

On Thu, Jan 25, 2018 at 11:58 AM, Alexander Kolbasov 
wrote:

> Now that we have Sentry 2.0 release, I think it is a good time to step back
> from fixing bugs and immediate problems and start discussions on roadmap
> for Sentry going forward. Do we want to just keep it as is and improve
> things here and there or we want to add new features?
>
> What do people think?
>
> - Alex
>


[DISCUSS] Sentry roadmap after 2.0

2018-01-25 Thread Alexander Kolbasov
Now that we have Sentry 2.0 release, I think it is a good time to step back
from fixing bugs and immediate problems and start discussions on roadmap
for Sentry going forward. Do we want to just keep it as is and improve
things here and there or we want to add new features?

What do people think?

- Alex


Re: Review Request 64955: SENTRY-2109: Fix the logic of identifying HMS out of Sync

2018-01-25 Thread kalyan kumar kalvagadda via Review Board

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

(Updated Jan. 25, 2018, 5 p.m.)


Review request for sentry, Alexander Kolbasov, Arjun Mishra, Na Li, Sergio 
Pena, and Vadim Spector.


Changes
---

Details of the code changes
1. Addressed review comments from vadim and lina
2. Changed the cache logic
3. rebased the patch


Bugs: SENTRY-2109
https://issues.apache.org/jira/browse/SENTRY-2109


Repository: sentry


Description
---

This patch does couple of things
1. Avoid triggering full snapshots when gaps are observed while fetching new 
notifications. While fetching new notifications HMSFollower would would fetch 
notifications with last event-id as well. When it gets the notifications and if 
it doesn't get the notifications with event-id, full snpshot is triggered.
2. Functinalty to address gaps and out-of-sequence notificaitons by re-fetching 
addtional notifications that were already fetched. This solution is not fool 
proof. It does a best effort to reduse the chance of loosing notifications by 
re-fetching the notifications.This approach will introduce an over head of 
fetching addtional notifications that were already fetched. Overhead of DB 
look-up is addressed by using a cache. This reduces additional DB lookups 
needed to check if the notification was already processed.
2. Added looging to report duplicate events and out of order events for debug 
purpose.
3. Added new e2e tests to verfy this behavior.


Diffs (updated)
-

  
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryOutOfSyncException.java
 PRE-CREATION 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
 2f2b98412e7dfdcc847ffe7975a70f452554e747 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 edea5b64d8f98c93aafc1fe43fa97e00c2ce2948 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
 93cc34f34c044dceb11d27e41ecbd1a14f64bed9 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
 7e02874b4be6a7109108809b1f404fe971b7b8e2 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
 79030780c35e5bda432e3ec3f01328e627cb50a6 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
 91c90f9d302f4feb3a8b3d06541f43541c87bf0f 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
 b4100278392986c161625a366212c6fef66ec0a9 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHiveNotificationFetcher.java
 83a1becd46ac2d69c7d6dd05ed6253d1cdd8800d 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestSnapshotCreation.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestSnapshotCreationWithShorterHMSEventTtl.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestSnapshotWithLongerHMSFollowerLongerInterval.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
 4cd00e6672730773c74b9840247d1f4d5f7bdfe4 


Diff: https://reviews.apache.org/r/64955/diff/5/

Changes: https://reviews.apache.org/r/64955/diff/4-5/


Testing
---

Made sure that tests pass.


Thanks,

kalyan kumar kalvagadda



Re: Review Request 65268: SENTRY-1904: TransactionManager should limit the max time spent by transaction retry

2018-01-25 Thread Alexander Kolbasov

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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
Line 231 (original), 246 (patched)


If you don't think that you can use time because we do not know how much 
time it may take to execute transaction, then the whole point of this JIRA is 
moot and we shouldn't fix it in the first place.


- Alexander Kolbasov


On Jan. 23, 2018, 11:40 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65268/
> ---
> 
> (Updated Jan. 23, 2018, 11:40 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, Sergio Pena, and Vadim 
> Spector.
> 
> 
> Bugs: SENTRY-1904
> https://issues.apache.org/jira/browse/SENTRY-1904
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The TransactionManager uses exponential backoff strategy for transaction 
> retries. This may cause some transactions to be delayed by a very long time. 
> We should also have a constraint on the max time for a transaction so that we 
> do not retry for too long. 
> 
> New patch that is attached adds upper bounds on below
> 
> 1.Interval between the retry attempts which increases exponentially.
> 2.Total time a transaction could spend in retries.
>  
> 
> With out these limits we would not have a control on how long a transaction 
> could be be active.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
>  2f2b98412e7dfdcc847ffe7975a70f452554e747 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  edea5b64d8f98c93aafc1fe43fa97e00c2ce2948 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
>  f4ff962a67f8a5c23cc5c8daa7bcb861d2e6b6a5 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  7e02874b4be6a7109108809b1f404fe971b7b8e2 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  b4100278392986c161625a366212c6fef66ec0a9 
> 
> 
> Diff: https://reviews.apache.org/r/65268/diff/3/
> 
> 
> Testing
> ---
> 
> Made sure all the tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>



Re: Review Request 65268: SENTRY-1904: TransactionManager should limit the max time spent by transaction retry

2018-01-25 Thread kalyan kumar kalvagadda via Review Board


> On Jan. 25, 2018, 12:35 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
> > Line 231 (original), 246 (patched)
> > 
> >
> > Since you are switching to time-based max it becomes the hard line. You 
> > want to either limit number of retries or total number but not both at the 
> > same time.

Configuration to limit the retry count is already avaialable in the released 
versions of sentry. It's not a good idea to remove it.

We do not know what is the time that would take for a transaction to be 
succsfull in the event of JDO failure that is why time based limit is set to 
some higher number so that it will not be hit in normal cases unless some 
configured ridiculously high retry count or has configured high retry time.


> On Jan. 25, 2018, 12:35 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
> > Line 247 (original), 268 (patched)
> > 
> >
> > This may make total sleep time greater then the allowed sleep time. You 
> > need to make sure that you never sleep longer then the remaining time left.

In each iteration sleep time is calculated for next iteration. Logic added at 
line 255 will make sure that the total sleep time never exceeds max allowed 
sleep time.


- kalyan kumar


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


On Jan. 23, 2018, 11:40 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65268/
> ---
> 
> (Updated Jan. 23, 2018, 11:40 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, Sergio Pena, and Vadim 
> Spector.
> 
> 
> Bugs: SENTRY-1904
> https://issues.apache.org/jira/browse/SENTRY-1904
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The TransactionManager uses exponential backoff strategy for transaction 
> retries. This may cause some transactions to be delayed by a very long time. 
> We should also have a constraint on the max time for a transaction so that we 
> do not retry for too long. 
> 
> New patch that is attached adds upper bounds on below
> 
> 1.Interval between the retry attempts which increases exponentially.
> 2.Total time a transaction could spend in retries.
>  
> 
> With out these limits we would not have a control on how long a transaction 
> could be be active.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
>  2f2b98412e7dfdcc847ffe7975a70f452554e747 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  edea5b64d8f98c93aafc1fe43fa97e00c2ce2948 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
>  f4ff962a67f8a5c23cc5c8daa7bcb861d2e6b6a5 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  7e02874b4be6a7109108809b1f404fe971b7b8e2 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  b4100278392986c161625a366212c6fef66ec0a9 
> 
> 
> Diff: https://reviews.apache.org/r/65268/diff/3/
> 
> 
> Testing
> ---
> 
> Made sure all the tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>