Broken e2e tests in master?

I was looking at test failures from my patch and the failures seem to be quite unrelated. A lot of e2e tests failed because there were too many failures like Unable to finalize edits file /tmp/1477513681071-0/dfs/name2/current/edits_inprogress_001 For example here you can see

Review Request 52916: SENTRY-1505 CommitContext isn't used by anything and should be reoved

sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java 6a2f48f39246efbb5d7e80b4562ea19240dfd8b6 Diff: https://reviews.apache.org/r/52916/diff/ Testing --- Thanks, Alexander Kolbasov

Question about SentryStore/DelegateSentryStore

There is some strangeness in the way SentryStore/DelegateSentryStore is implemented. DelegateSentryStore implements SentryStoreLayer interface, but SentryStore doesn't. It looks like SentryStore should implement this as well. Any thoughts on this?

Re: Review Request 52138: SENTRY-1463: Ensure HMS point-in-time snapshot consistency

ecific word to see whether any of these happened. - Alexander Kolbasov On Oct. 12, 2016, 7:15 a.m., Hao Hao wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 52138: SENTRY-1463: Ensure HMS point-in-time snapshot consistency

> (Updated Oct. 12, 2016, 7:15 a.m.) > > > Review request for sentry, Alexander Kolbasov, Anne Yu, Li Li, and Sravya > Tirukkovalur. > > > Repository: sentry > > > Description > --- > > SENTRY-1463: Ensure HMS point-in-time snapshot consistency >

Re: Review Request 52888: NPE in log4j.properties parsing

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52888/#review152713 --- Ship it! Ship It! - Alexander Kolbasov On Oct. 14, 2016, 4

Re: Review Request 51848: Apply Checkstyle changes to the core

deviation between trunk and sentry-ha branch where there are a lot of changes happening. The style changes are very useful but they cause quite complicated merges. I think that we should consider delaying these changes until we merge sentry HA branch into trunk. - Alexander Kolbasov On Sept. 13

Re: Review Request 54525: SENTRY-1541: toSentryPrivilege() should not copy fields that are not set in the source

- > > (Updated Dec. 8, 2016, 8:47 a.m.) > > > Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar > kalvagadda. > > > Repository: sentry > > > Description > --- > > Only sets the fields in TSentryPrivilege that are set i

Re: Review Request 54526: SENTRY-1540: SentryStore.isMultiActionsSupported() is always true

> On Dec. 8, 2016, 9:32 p.m., Alexander Kolbasov wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 1607 > > <https://reviews.apache.org/r/54526/diff/1/?file=1579704#file1579704

Re: Review Request 54526: SENTRY-1540: SentryStore.isMultiActionsSupported() is always true

/sentry/provider/db/service/persistent/SentryStore.java (line 1607) <https://reviews.apache.org/r/54526/#comment229355> So this is just the rename - right? The point is that getDbName is never NULL. Or did you intend to check for the actual __NULL__ string here? - Alexander Kolbasov

Re: Review Request 54525: SENTRY-1541: toSentryPrivilege() should not copy fields that are not set in the source

this is fixed we don't need fromNULLCol() anymore. - Alexander Kolbasov On Dec. 8, 2016, 8:47 a.m., Vamsee Yarlagadda wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

/sentry/provider/db/service/persistent/SentryStore.java (line 1258) <https://reviews.apache.org/r/54464/#comment229171> It is neat, but does it actually work? - Alexander Kolbasov On Dec. 7, 2016, 2:39 a.m., Vamsee Yarlagadda

Re: Review Request 54697: SENTRY-1513: Changed the way the split works when parsing a path and corrected spelling in PathsUpdate

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54697/#review159178 --- Ship it! Ship It! - Alexander Kolbasov On Dec. 13, 2016, 9

Review Request 54808: SENTRY-1526 Sentry processed stays alive after being killed

://reviews.apache.org/r/54808/diff/ Testing --- Now process successfully dies after hitting ^C. Thanks, Alexander Kolbasov

Re: Review Request 54454: SENTRY-1548 Setting GrantOption to UNSET upsets Sentry

ilege here. I guess the check should be for the original privilege instead. And the message probably should be something like "grant option is not specified" - Alexander Kolbasov On Dec. 6, 2016, 11:05 p.m., kalyan kumar kalvagadda wrote: > > --

Re: Review Request 54454: SENTRY-1548 Setting GrantOption to UNSET upsets Sentry

4> It is a bit confusing for these two functions to share the same name. Also, given that this can be simply written as return privilege.getGrantOption() != TSentryGrantOption.UNSET do we need a function for that at all? - Alexander Kolbasov On Dec. 14, 2

Re: Review Request 54445: SENTRY-1547 - It is possible to create a privilege with all empty fields

changes in SentryPolicyStoreProcessor but they are not in this diff. - Alexander Kolbasov On Dec. 14, 2016, 10:23 p.m., kalyan kumar kalvagadda wrote: > > --- > This is an automatically generated e-mail. To reply, visit

Re: Review Request 54409: SENTRY-1476: SentryStore is subject to JDQL injection

-- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54409/#review159226 ------- On Dec. 6, 2016, 5:30 a.m., Alexander Kolbasov wrote: > > ---

Re: Review Request 54338: SENTRY-1515: Cleanup exception handling in SentryStore

-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java e8d0e2505c5b178cae3d27e7d85caa652f630a2d Diff: https://reviews.apache.org/r/54338/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 54445: SENTRY-1547 - It is possible to create a privilege with all empty fields

lly generated e-mail. To reply, visit: > https://reviews.apache.org/r/54445/ > --- > > (Updated Dec. 14, 2016, 10:23 p.m.) > > > Review request for sentry, Alexander Kolbasov, Colin Ma, Hao Hao, Vamsee > Yarlagadda, and Vadim Spector. > > > Repository: sentry

Re: [DISCUSS] Sentry interactive shell

> > Thanks, > Lenni Thanks for your input! - Alex. > > On Wed, Dec 14, 2016 at 5:19 PM, Alexander Kolbasov <ak...@cloudera.com> > wrote: > >> Inspired by SentryShell I wrote a prototype of interactive Sentry shell >> where you have an open sessi

Re: Review Request 54808: SENTRY-1526 Sentry processed stays alive after being killed

ps://reviews.apache.org/r/54808/#review159478 ------- On Dec. 16, 2016, 7:23 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 54338: SENTRY-1515: Cleanup exception handling in SentryStore

-- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54338/#review158054 ------- On Dec. 3, 2016, 6:52 a.m., Alexander Kolbasov wrote: > > -

Re: Review Request 54338: SENTRY-1515: Cleanup exception handling in SentryStore

sit: https://reviews.apache.org/r/54338/#review158055 --- On Dec. 3, 2016, 6:52 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically generated e-mail. To re

Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

PermChange.java (line 29) <https://reviews.apache.org/r/54729/#comment232352> Nit: add - Alexander Kolbasov On Jan. 10, 2017, 1:57 a.m., Hao Hao wrote: > > --- > This is an automatically generated e-mail. To re

Re: Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

working Diffs (updated) - sentry-service/sentry-service-server/pom.xml 97bd326b0305303194a965a60b6afd0feefbbe0f sentry-tests/sentry-tests-solr/pom.xml a60b4eed4dde6c182b061681305013b3bd548cc3 Diff: https://reviews.apache.org/r/55094/diff/ Testing --- Thanks, Alexander

Re: Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55094/#review160918 --- On Jan. 5, 2017, 12:06 a.m., Alexander Kolba

Re: Review Request 55246: SENTRY-1536: Refactor SentryStore transaction management to allow for extra transactions for a single permission update

of passing the update map or specific update to SentryStore, pass a TransactionBlock that should be executed as part of a SentrySTore transaction. This is a more generic approach. What do you think? - Alexander Kolbasov On Jan. 10, 2017, 2:31 a.m., Hao Hao wrote

Re: Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

il. To reply, visit: https://reviews.apache.org/r/55094/#review161459 ------- On Jan. 11, 2017, 1:35 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5

Re: Review Request 53038: SENTRY-1507

-server/pom.xml 97bd326b0305303194a965a60b6afd0feefbbe0f Diff: https://reviews.apache.org/r/53038/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 53872: SENTRY-1525: Provide script to run Sentry directly from the repo

directly from the repo Diffs (updated) - bin/run_sentry.sh PRE-CREATION Diff: https://reviews.apache.org/r/53872/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Show groups for a given role

Hi Jan, You may check my new sentry command-line tool at https://github.com/akolb1/sentrytool . You can easily do it with this tool: sentrytool group list -v Caveat - it doesn’t work in Kerberos setup. I am not sure whether there is a more “official”

Review Request 54409: SENTRY-1476: SentryStore is subject to JDQL injection

/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 53905: SENTRY-1518: Add metrics for SentryStore transactions

/sentry/provider/db/service/persistent/TransactionManager.java 3d0a6ab0b514cb9fbead4b8e527f7cb84865ce28 Diff: https://reviews.apache.org/r/53905/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 53520: SENTRY-1517: SentryStore shoud actually use function getMSentryRole to get roles

, Alexander Kolbasov

Re: Review Request 54338: SENTRY-1515: Cleanup exception handling in SentryStore

/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java e8d0e2505c5b178cae3d27e7d85caa652f630a2d Diff: https://reviews.apache.org/r/54338/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

----- On Jan. 5, 2017, 9:04 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53920/ > -

Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

eId as the hashcode? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java (line 29) <https://reviews.apache.org/r/54729/#comment230430> Add -

Re: Review Request 52795: SENTRY-1499: Add feature flag for using NotificationLog

che.org/r/52795/#comment231957> Seems like other constants are prefixed with 'sentry' - it may be better to follow the convention. - Alexander Kolbasov On Dec. 20, 2016, 12:21 a.m., Hao Hao wrote: > > --- > This is an au

Re: Review Request 54454: SENTRY-1548 Setting GrantOption to UNSET upsets Sentry

/54454/#comment231811> Do we allow any action as long as it is non-empty? Other code makes certain assumption about valid set of actions. - Alexander Kolbasov On Jan. 5, 2017, 4:50 p.m., kalyan kumar kalvagadda wrote: > > --

Re: Review Request 55190: SENTRY-1583: Refactor ZK/Curator code

generated e-mail. To reply, visit: https://reviews.apache.org/r/55190/#review160806 ------- On Jan. 6, 2017, 6:06 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically

Re: Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

Diffs (updated) - sentry-service/sentry-service-server/pom.xml 97bd326b0305303194a965a60b6afd0feefbbe0f sentry-tests/sentry-tests-solr/pom.xml a60b4eed4dde6c182b061681305013b3bd548cc3 Diff: https://reviews.apache.org/r/55094/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

------- On Jan. 4, 2017, 12:31 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55094/ > -

Re: Review Request 54409: SENTRY-1476: SentryStore is subject to JDQL injection

/TestSentryStore.java 64df6a5655cf2c121cb44f2274369fbe9d70ec83 Diff: https://reviews.apache.org/r/54409/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 54338: SENTRY-1515: Cleanup exception handling in SentryStore

makes exceptions more explicit. - Alexander Kolbasov On Dec. 14, 2016, 11:57 p.m., Alexander Kolbasov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Review Request 55092: SENTRY-1581: Provide Log4J metrics reporter

--- Thanks, Alexander Kolbasov

Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

-service-server/pom.xml 97bd326b0305303194a965a60b6afd0feefbbe0f Diff: https://reviews.apache.org/r/55094/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 54525: SENTRY-1541/SENTRY-1582: Additional comments to clarify the intent of string manipulation methods in SentryStore.java

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54525/#review160430 --- Ship it! Ship It! - Alexander Kolbasov On Jan. 3, 2017, 8

Re: Review Request 55190: SENTRY-1583: Refactor ZK/Curator code

> > Diff: https://reviews.apache.org/r/55190/diff/ > > > Testing > --- > > Performed manual testing with two instances of Sentry running. Tried the > following actions: > > - Sending SIGUSR2 to processes which changes leadership status (verified via > log messages and emtrics) > - Killing active server - new sserver is elected as a leader > - Restarting ZK service - a leader is elected once ZK service is back. > > > Thanks, > > Alexander Kolbasov > >

Re: Review Request 55190: SENTRY-1583: Refactor ZK/Curator code

SIGUSR2 to processes which changes leadership status (verified via log messages and emtrics) - Killing active server - new sserver is elected as a leader - Restarting ZK service - a leader is elected once ZK service is back. Thanks, Alexander Kolbasov

Re: Review Request 55190: SENTRY-1583: Refactor ZK/Curator code

SIGUSR2 to processes which changes leadership status (verified via log messages and emtrics) - Killing active server - new sserver is elected as a leader - Restarting ZK service - a leader is elected once ZK service is back. Thanks, Alexander Kolbasov

Re: Review Request 52795: SENTRY-1499: Add feature flag for using NotificationLog

here a need to undo any work? sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java (line 114) <https://reviews.apache.org/r/52795/#comment230560> What is ment by drop here? The method is about creating a table. - Alexa

Re: Review Request 54819: SENTRY-1526: Sentry processed stays alive after being killed (Fixing Mismerge) (Alexander Kolbasov via Vamsee Yarlagadda)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54819/#review159491 --- Ship it! Ship It! - Alexander Kolbasov On Dec. 16, 2016, 8

Re: Review Request 57232: SENTRY-1613: Add propagating logic for Perm/Path updates in Sentry service

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57232/#review169982 --- Ship it! Ship It! - Alexander Kolbasov On March 24, 2017, 1

Re: Review Request 57570: SENTRY-1644. Partition ACLs disappear after renaming Hive table with partitions

s://reviews.apache.org/r/57570/#comment242853> Why do you need subList() here? - Alexander Kolbasov On March 22, 2017, 9:27 p.m., Lei Xu wrote: > > --- > This is an automatically generated e-ma

Review Request 58087: SENTRY-1676 FullUpdateInitializer#createInitialUpdate should not throw RuntimeException

/FullUpdateInitializer.java 146cea2b9467ce82b69bbf402933b1aa350bcd46 sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 6c14f5e46aad4223347d8d057188d31efbb68ed8 Diff: https://reviews.apache.org/r/58087/diff/1/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 58087: SENTRY-1676 FullUpdateInitializer#createInitialUpdate should not throw RuntimeException

: https://reviews.apache.org/r/58087/diff/2/ Changes: https://reviews.apache.org/r/58087/diff/1-2/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 57901: SENTRY-1593 Implement client failover for all the sentry clients

ather then client_object - Alexander Kolbasov On March 29, 2017, 11:32 p.m., kalyan kumar kalvagadda wrote: > > --- > This is an automatically generated e-mail. To reply, visit: >

[DISCUSS] Merging Sentry HA branch with master

Hello, I would like to start the discussion on merging sentry-ha-redesign branch with master. As of now most of the changes from master are merged into sentry-ha-redesign. The major missing part is SENTRY-1205 (Refactor the code for sentry-provider-db and create sentry-service module) and

Re: Review Request 57232: SENTRY-1613: Add propagating logic for Perm/Path updates in Sentry service

db/service/persistent/SentryStore.java Lines 3435 (patched) <https://reviews.apache.org/r/57232/#comment242489> return Collections.emptyList() - Alexander Kolbasov On March 22, 2017, 11:08 p.m., Hao Hao wrote: > > ---

Review Request 58069: SENTRY-1670 Expose current HMS notification ID as a Sentry gauge (metric)

bbfa713262c5b4d5cecccd95c823a78c9149752c sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java 6ed2c781b4ac8819f6d17a249c33e32f0116e15a Diff: https://reviews.apache.org/r/58069/diff/1/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 56954: SENTRY-1639 Refactor the usage of configuration constants related to transport

SENTRY-1675? - Alexander Kolbasov On March 22, 2017, 11:05 p.m., kalyan kumar kalvagadda wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 57657: SENTRY-1658: Fixed possible null pointer dereference in SentryShellHive

> On March 15, 2017, 7:07 p.m., Alexander Kolbasov wrote: > > Please fix the commit message > > Jan Hentschel wrote: > What is wrong with the commit message? The commit message should be SENTRY-1658: Null pointer derefeence in SentryShell

Re: Review Request 57375: Unable to truncate table .; from "default" databases

/sentry/binding/hive/HiveAuthzBindingHook.java Lines 253 (patched) <https://reviews.apache.org/r/57375/#comment241401> You use ast.getChild(0)getCHild(0) several times - would it make sense to assign it to a variable with a meaningful name? - Alexander Kolbasov On March 7, 2017, 1:

Re: Review Request 57657: SENTRY-1658: Fixed possible null pointer dereference in SentryShellHive

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57657/#review169046 --- Ship it! Ship It! - Alexander Kolbasov On March 15, 2017, 6

Re: Review Request 57654: SENTRY-1663: Moved mutable field in UpdateableAuthzPermissions to immutable

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57654/#review169048 --- Ship it! Please fix the commit message - Alexander Kolbasov

Re: Review Request 58069: SENTRY-1670 Expose current HMS notification ID as a Sentry gauge (metric)

6ed2c781b4ac8819f6d17a249c33e32f0116e15a Diff: https://reviews.apache.org/r/58069/diff/3/ Changes: https://reviews.apache.org/r/58069/diff/2-3/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 58069: SENTRY-1670 Expose current HMS notification ID as a Sentry gauge (metric)

/apache/sentry/provider/db/service/thrift/SentryMetrics.java 6ed2c781b4ac8819f6d17a249c33e32f0116e15a Diff: https://reviews.apache.org/r/58069/diff/2/ Changes: https://reviews.apache.org/r/58069/diff/1-2/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 57570: SENTRY-1644. Partition ACLs disappear after renaming Hive table with partitions

e it reaches here? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java Lines 339 (patched) <https://reviews.apache.org/r/57570/#comment243851> should it be private? The return value is never used - is it intended? - Alexander Kolbasov On March 28, 201

Re: Review Request 58164: SENTRY-1638 Update SQL script of MSentryPathChange table to add a column for notification ID

we just update the initial creation scripts rather then add new scripts to fix the tables? - Alexander Kolbasov On April 4, 2017, 5:05 p.m., kalyan kumar kalvagadda wrote: > > --- > This is an automatically generated e-mail. To rep

Re: Review Request 57570: SENTRY-1644. Partition ACLs disappear after renaming Hive table with partitions

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57570/#review171052 --- Ship it! Ship It! - Alexander Kolbasov On April 4, 2017, 8

Re: [DISCUSS] Merging Sentry HA branch with master

Exclude big refactoring commit (SENTRY-1205) and related commits (SENTRY-1436, SENTRY-1438, SENTRY-1406) Rename master to a dev branch Rename sentry-ha-redesign to master What does community think about such approach? - Alex > On Mar 22, 2017, at 1:43 PM, Alexander Kolbasov <ak...@cloude

Re: Review Request 58166: SENTRY-1692 ZK namespace configuration doesn't work

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58166/#review170957 --- On April 4, 2017, 5:03 a.m., Alexander Kolbasov wrote: > > ---

Re: Review Request 58069: SENTRY-1670 Expose current HMS notification ID as a Sentry gauge (metric)

ps://reviews.apache.org/r/58069/#review170958 ------- On March 30, 2017, 5:25 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 58131: SENTRY-1680: Removed MetastoreCacheInitializer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58131/#review170998 --- Ship it! Ship It! - Alexander Kolbasov On April 4, 2017, 11

Re: Review Request 58281: SENTRY-1643. AutoIncrement ChangeID of MSentryPermChange/MSentryPathChange may be error-prone

se you might miss some updates sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java Lines 2616 (patched) <https://reviews.apache.org/r/58281/#comment244425> it may be better to log it rather then print to stdout -

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58221/#review171256 --- Ship it! Ship It! - Alexander Kolbasov On April 7, 2017, 5

Re: Review Request 58267: SENTRY-1629 sql changed needed for MAuthzPathsMapping.

-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java Lines 69 (patched) <https://reviews.apache.org/r/58267/#comment27> Please provide javadoc for this new function And, again, the name of this function seems wrong - Alexander Kolbasov On Ap

Review Request 58121: SENTRY-1677 Add metrics to measure how much time to get Delta Path/Perm Updates

28bf20edb63b08a92b6ec060bc76934dea82f746 Diff: https://reviews.apache.org/r/58121/diff/1/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 58094: SENTRY-1683 MetastoreCacheInitializer has a race condition in handling results list

/ Testing --- Thanks, Alexander Kolbasov

Review Request 58093: SENTRY-1676: FullUpdateInitializer#createInitialUpdate should not throw RuntimeExceptio

/FullUpdateInitializer.java 146cea2b9467ce82b69bbf402933b1aa350bcd46 sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 6c14f5e46aad4223347d8d057188d31efbb68ed8 Diff: https://reviews.apache.org/r/58093/diff/1/ Testing --- Thanks, Alexander Kolbasov

Review Request 58130: SENTRY-1684 FullUpdateInitializer has a race condition in handling results list

/FullUpdateInitializer.java 146cea2b9467ce82b69bbf402933b1aa350bcd46 Diff: https://reviews.apache.org/r/58130/diff/1/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

actually cancel given that this is periodic executor? U think you should just use shutdown on the executor instead of this. - Alexander Kolbasov On April 12, 2017, 3:47 a.m., Na Li wrote: > > --- > This is an automatica

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

> On April 12, 2017, 4:50 a.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > > Lines 78 (patched) > > <https://reviews.apache.org/r/58221/diff/5/?file=1689530#file1689530line78> >

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

> On April 12, 2017, 4:50 a.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > > Lines 350 (patched) > > <https://reviews.apache.org/r/58221/diff/5/?file=1689530#file1689530line380&g

Re: Review Request 58093: SENTRY-1676: FullUpdateInitializer#createInitialUpdate should not throw RuntimeExceptio

the > > duration in the log message? I think we should provide this as a metric. Logs are time stamped - this could be sufficient, but having this as a metric is useful - then we can get it afterwords. I'll file a JIRA on this. - Alexander ------

Re: Review Request 58284: SENTRY-1687 FullUpdateInitializer can be more efficient

his is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58284/#review171443 --- On April 10, 2017, 5:42 a.m., Alexander Kolbasov wrote: > > --- > This is a

Re: Review Request 58284: SENTRY-1687 FullUpdateInitializer can be more efficient

gt; > > > > can we have the test case that the first time, client returns > > exception, but the second time, it returns correct result. This is to test > > the retry behavior. > > Alexander Kolbasov wrote: > There was no original test for this as w

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

to it doesn't seem right. - Alexander Kolbasov On April 7, 2017, 5:23 p.m., Na Li wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

> On April 12, 2017, 4:50 a.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > > Lines 78 (patched) > > <https://reviews.apache.org/r/58221/diff/5/?file=1689530#file1689530line78> >

Re: Review Request 57901: SENTRY-1593 Implement client failover for all the sentry clients

gt; It would be better to process all these constants during construction - they never change so there is no need to do it on every connect - Alexander Kolbasov On April 13, 2017, 7:34 p.m., kalyan kumar kalvagadda wrote: > > ---

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

ovider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java Lines 343 (patched) <https://reviews.apache.org/r/58221/#comment245184> You can move this outside the try block and return immediately if storeCleanPeriodSecs <= 0. This will simplify the code a bit. - Alexa

Re: Review Request 58481: SENTRY-1674 HMSFollower shouldn't print the same value of notification ID multiple times

che.org/r/58481/#comment245655> There is no point in calling this if there are no events - Alexander Kolbasov On April 20, 2017, 6:14 p.m., Na Li wrote: > > --- > This is an automatically generated e-mail. To reply, visit: >

Re: Review Request 58412: SENTRY-1587: Refactor SentryStore transaction to persist a single path transcation bundled with corresponding delta path change

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58412/#review172585 --- Ship it! Ship It! - Alexander Kolbasov On April 21, 2017, 2

Re: Review Request 58284: SENTRY-1687 FullUpdateInitializer can be more efficient

line121> > > > > the method javadoc indicates that null is returned if path is > > null/empty." but here we're throwing an exception. Which one is true? Will update comment. - Alexander --- This is an aut

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

onized() on both, so the comment isn't quite right - it would handle concurrent calls to start()/stop() - not that it is needed. - Alexander Kolbasov On April 13, 2017, 7:56 p.m., Na Li wrote: > > --

Re: Review Request 58481: SENTRY-1674 HMSFollower shouldn't print the same value of notification ID multiple times

-empty. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java Line 261 (original), 258 (patched) <https://reviews.apache.org/r/58481/#comment245482> This comment no longer matches the code. - Alexander Kolbasov On April 19, 2017, 2:43 a.m.,

Re: [DISCUSS] Merging Sentry HA branch with master

to me then. Let's try and do development work on >> master from now on though... >> >> Colm. >> >> On Tue, Apr 18, 2017 at 2:14 AM, Alexander Kolbasov <ak...@cloudera.com> >> wrote: >> >>> FYI, I contacted Colin Ma who was working on

Clarification for MPath changes for SENTRY-1587

Hao, Can you clarify the changes you propose for MPath class and related package.jdo changes for SENTRY-1587? You suggest changing the identity type from "database" to "application", but the pathId is not initialized in constructor and not assigned anywhere. What is your intention here? Also

Re: [ATTENTION] PMC members

I am wondering how many people except me are committers but not PMC members? - Alex > On Apr 13, 2017, at 3:17 AM, Colm O hEigeartaigh wrote: > > Hi PMC members of Apache Sentry, > > There is a discussion on the priv...@sentry.apache.org mailing list that > requires the

<    1   2   3   4   5   6   7   8   >