date:20180220

Re: Review Request 65533: SENTRY-2115: Incorrect behavior of HMsFollower when HDFSSync feature is disabled.

2018-02-20 Thread kalyan kumar kalvagadda via Review Board



> On Feb. 20, 2018, 9:13 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Line 261 (original), 279 (patched)
> > 
> >
> > isAuthzPathsSnapshotEmpty() checks if MAuthzPathsSnapshotId is empty, 
> > why did you change it to MAuthzPathsMapping?

MAuthzPathsMapping is nothing but snapshot. It just that naming of 
MAuthzPathsMapping class is confusing.


> On Feb. 20, 2018, 9:13 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 587-589 (patched)
> > 
> >
> > What about MAuthzPathsSnapshotId.class?

Data in MAuthzPathsSnapshotId.class is not cleared intentionally. This data 
will help sentry retain the history of snapshots taken before
and help in picking appropriate ID even when hdfs sync is enabled/disabled. I 
have added the same in the code.


> On Feb. 20, 2018, 9:13 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 3115 (original), 3131 (patched)
> > 
> >
> > The name of the method and what is checks does not match. Why is it 
> > needed to check if the MAuthzPathsMapping is empty instead of the 
> > MAuthzPathsSnapshotId?

MAuthzPathsMapping is nothing but snapshot. It just that naming of 
MAuthzPathsMapping class is confusing.


> On Feb. 20, 2018, 9:13 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
> > Lines 1148-1152 (patched)
> > 
> >
> > What's wrong with /* */?

There is nothing wrong. I have updated the test case and have updated the java 
doc for the same. I just changed the way it is commented.


- kalyan kumar


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


On Feb. 15, 2018, 9 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> ---
> 
> (Updated Feb. 15, 2018, 9 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the 
> first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is 
> less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the 
> subsequent pull is not greater than the last event-id processed by sentry by 
> 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset 
> SENTRY_HMS_NOTIFICATION_ID table and start processing the event in 
> HMS_NOTIFICATION_LOG from beginning.
> 
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while 
> and then HDFS sync is enabled and sentry service is restarted to get it to 
> effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset 
> SENTRY_HMS_NOTIFICATION_ID table and start processing the event in 
> HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping 
> information when ever HDFS sync is disabled. That way it can learn from 
> scratch when the feature is enabled back. There is no value is holding stale 
> data even when we know it will have issues when the feature is enabled back.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
>  eae7861728f2bc11b4c1b44aa3b61b881a87740b 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  cf764eda1a006ce96f301e3ecb87749e05ba4a09 
>   
> 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/NotificationProcessor.java
>  e7558370025c6acd83492615be093f2bd16a202b 
>   
>

Re: Review Request 58727: SENTRY-1708 Extend the current test classes to handle multiple sentry servers

2018-02-20 Thread kalyan kumar kalvagadda via Review Board


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

(Updated Feb. 20, 2018, 11:27 p.m.)


Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, 
Vamsee Yarlagadda, and Vadim Spector.


Changes
---

Addressed review comments.


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


Repository: sentry


Description
---

Here is the abstract of the code changes done. There are basically changes to 
the base classes used tests to be able to handle multiple servers
* List of sentry sentry servers
* List of the address used for these servers
* This is learnt so re-use the same port when the servers are re-started
* API's to start/stop all the servers
* API's to start/stop specific servers
* API to learn the server addresses
* API to update the sentry server config list for the clients to use
* API to for service failover

There are some HA test classes added to demonstrate the how these abstract base 
classes can be used to perform HA tests.
Model HA Tests using Abstract class providing Ha capability

* TestColumnEndToEndWithHa
* TestGrantUserToRoleWithHa
* TestPrivilegeWithHAGrantOption


Diffs (updated)
-

  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
 eae7861728f2bc11b4c1b44aa3b61b881a87740b 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
 cf764eda1a006ce96f301e3ecb87749e05ba4a09 
  
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/NotificationProcessor.java
 e7558370025c6acd83492615be093f2bd16a202b 
  
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/SentryService.java
 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
 79030780c35e5bda432e3ec3f01328e627cb50a6 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
 4cd00e6672730773c74b9840247d1f4d5f7bdfe4 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
 e64f5cd687bf59133d6475c912ebdd7930601151 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java
 91397061e78a6524410d35643dd3a33be8353ecc 


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

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


Testing
---

Made sure that all the unit tests passed.


Thanks,

kalyan kumar kalvagadda

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

2018-02-20 Thread Xinran Tinney


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


Ship it!




Ship It!

- Xinran Tinney


On Feb. 20, 2018, 8:04 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65642/
> ---
> 
> (Updated Feb. 20, 2018, 8:04 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
> Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> sentry CreateTime is the difference, measured in milliseconds, between the 
> current time and midnight, January 1, 1970 UTC. hive granttime is in seconds. 
> So need to convert the time.
> 
> The original code just cost the timestamp from long to int without converting 
> milliseconds to seconds. Therefore, the timestamp value is wrong when 
> retrieving the privilege from hive.
> 
> The solution is to convert the time correctly.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
>  b536283 
> 
> 
> Diff: https://reviews.apache.org/r/65642/diff/3/
> 
> 
> Testing
> ---
> 
> tested manually by stepping into code. I cannot test it in unit test because 
> Hive returns timestamp as -1L for test mode
> 
>   DDLTask.writeGrantInfo
> static String writeGrantInfo(List privileges, boolean 
> testMode) {
> if (privileges != null && !privileges.isEmpty()) {
>   StringBuilder builder = new StringBuilder();
>   Collections.sort(privileges, new Comparator() {
> public int compare(HivePrivilegeInfo o1, HivePrivilegeInfo o2) {
>   int compare = o1.getObject().compareTo(o2.getObject());
>   if (compare == 0) {
> compare = o1.getPrincipal().compareTo(o2.getPrincipal());
>   }
> 
>   if (compare == 0) {
> compare = o1.getPrivilege().compareTo(o2.getPrivilege());
>   }
> 
>   return compare;
> }
>   });
>   Iterator var3 = privileges.iterator();
> 
>   while(var3.hasNext()) {
> HivePrivilegeInfo privilege = (HivePrivilegeInfo)var3.next();
> HivePrincipal principal = privilege.getPrincipal();
> HivePrivilegeObject resource = privilege.getObject();
> HivePrincipal grantor = privilege.getGrantorPrincipal();
> appendNonNull(builder, resource.getDbname(), true);
> appendNonNull(builder, resource.getObjectName());
> appendNonNull(builder, resource.getPartKeys());
> appendNonNull(builder, resource.getColumns());
> appendNonNull(builder, principal.getName());
> appendNonNull(builder, principal.getType());
> appendNonNull(builder, privilege.getPrivilege().getName());
> appendNonNull(builder, privilege.isGrantOption());
> appendNonNull(builder, testMode ? -1L : 
> (long)privilege.getGrantTime() * 1000L); <-- in test mode, does not 
> return real timestamp
> appendNonNull(builder, grantor.getName());
>   }
> 
>   return builder.toString();
> } else {
>   return "";
> }
>   }
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 65533: SENTRY-2115: Incorrect behavior of HMsFollower when HDFSSync feature is disabled.

2018-02-20 Thread Na Li via Review Board


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




sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Lines 306 (patched)


should be "fullsnapshot should not have been fetched"



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Line 336 (original), 354 (patched)


why do you comment out this line?



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Line 999 (original), 1161 (patched)


if you don't need those lines, can you just remove them?


- Na Li


On Feb. 15, 2018, 9 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> ---
> 
> (Updated Feb. 15, 2018, 9 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the 
> first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is 
> less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the 
> subsequent pull is not greater than the last event-id processed by sentry by 
> 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset 
> SENTRY_HMS_NOTIFICATION_ID table and start processing the event in 
> HMS_NOTIFICATION_LOG from beginning.
> 
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while 
> and then HDFS sync is enabled and sentry service is restarted to get it to 
> effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset 
> SENTRY_HMS_NOTIFICATION_ID table and start processing the event in 
> HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping 
> information when ever HDFS sync is disabled. That way it can learn from 
> scratch when the feature is enabled back. There is no value is holding stale 
> data even when we know it will have issues when the feature is enabled back.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
>  eae7861728f2bc11b4c1b44aa3b61b881a87740b 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  cf764eda1a006ce96f301e3ecb87749e05ba4a09 
>   
> 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/NotificationProcessor.java
>  e7558370025c6acd83492615be093f2bd16a202b 
>   
> 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/SentryService.java
>  96c6810baa4d554db2b7d3739a28e3ff7e8b33a0 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
>  79030780c35e5bda432e3ec3f01328e627cb50a6 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  4cd00e6672730773c74b9840247d1f4d5f7bdfe4 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
>  e64f5cd687bf59133d6475c912ebdd7930601151 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java
>  91397061e78a6524410d35643dd3a33be8353ecc 
> 
> 
> Diff: https://reviews.apache.org/r/65533/diff/3/
> 
> 
> Testing
> ---
> 
> Made sure that all the tests passed.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

2018-02-20 Thread kalyan kumar kalvagadda via Review Board


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


Ship it!




Ship It!

- kalyan kumar kalvagadda


On Feb. 20, 2018, 8:04 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65642/
> ---
> 
> (Updated Feb. 20, 2018, 8:04 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
> Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> sentry CreateTime is the difference, measured in milliseconds, between the 
> current time and midnight, January 1, 1970 UTC. hive granttime is in seconds. 
> So need to convert the time.
> 
> The original code just cost the timestamp from long to int without converting 
> milliseconds to seconds. Therefore, the timestamp value is wrong when 
> retrieving the privilege from hive.
> 
> The solution is to convert the time correctly.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
>  b536283 
> 
> 
> Diff: https://reviews.apache.org/r/65642/diff/3/
> 
> 
> Testing
> ---
> 
> tested manually by stepping into code. I cannot test it in unit test because 
> Hive returns timestamp as -1L for test mode
> 
>   DDLTask.writeGrantInfo
> static String writeGrantInfo(List privileges, boolean 
> testMode) {
> if (privileges != null && !privileges.isEmpty()) {
>   StringBuilder builder = new StringBuilder();
>   Collections.sort(privileges, new Comparator() {
> public int compare(HivePrivilegeInfo o1, HivePrivilegeInfo o2) {
>   int compare = o1.getObject().compareTo(o2.getObject());
>   if (compare == 0) {
> compare = o1.getPrincipal().compareTo(o2.getPrincipal());
>   }
> 
>   if (compare == 0) {
> compare = o1.getPrivilege().compareTo(o2.getPrivilege());
>   }
> 
>   return compare;
> }
>   });
>   Iterator var3 = privileges.iterator();
> 
>   while(var3.hasNext()) {
> HivePrivilegeInfo privilege = (HivePrivilegeInfo)var3.next();
> HivePrincipal principal = privilege.getPrincipal();
> HivePrivilegeObject resource = privilege.getObject();
> HivePrincipal grantor = privilege.getGrantorPrincipal();
> appendNonNull(builder, resource.getDbname(), true);
> appendNonNull(builder, resource.getObjectName());
> appendNonNull(builder, resource.getPartKeys());
> appendNonNull(builder, resource.getColumns());
> appendNonNull(builder, principal.getName());
> appendNonNull(builder, principal.getType());
> appendNonNull(builder, privilege.getPrivilege().getName());
> appendNonNull(builder, privilege.isGrantOption());
> appendNonNull(builder, testMode ? -1L : 
> (long)privilege.getGrantTime() * 1000L); <-- in test mode, does not 
> return real timestamp
> appendNonNull(builder, grantor.getName());
>   }
> 
>   return builder.toString();
> } else {
>   return "";
> }
>   }
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

2018-02-20 Thread Na Li via Review Board


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

(Updated Feb. 20, 2018, 8:04 p.m.)


Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
Pena.


Repository: sentry


Description
---

sentry CreateTime is the difference, measured in milliseconds, between the 
current time and midnight, January 1, 1970 UTC. hive granttime is in seconds. 
So need to convert the time.

The original code just cost the timestamp from long to int without converting 
milliseconds to seconds. Therefore, the timestamp value is wrong when 
retrieving the privilege from hive.

The solution is to convert the time correctly.


Diffs (updated)
-

  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
 b536283 


Diff: https://reviews.apache.org/r/65642/diff/3/

Changes: https://reviews.apache.org/r/65642/diff/2-3/


Testing
---

tested manually by stepping into code. I cannot test it in unit test because 
Hive returns timestamp as -1L for test mode

  DDLTask.writeGrantInfo
static String writeGrantInfo(List privileges, boolean 
testMode) {
if (privileges != null && !privileges.isEmpty()) {
  StringBuilder builder = new StringBuilder();
  Collections.sort(privileges, new Comparator() {
public int compare(HivePrivilegeInfo o1, HivePrivilegeInfo o2) {
  int compare = o1.getObject().compareTo(o2.getObject());
  if (compare == 0) {
compare = o1.getPrincipal().compareTo(o2.getPrincipal());
  }

  if (compare == 0) {
compare = o1.getPrivilege().compareTo(o2.getPrivilege());
  }

  return compare;
}
  });
  Iterator var3 = privileges.iterator();

  while(var3.hasNext()) {
HivePrivilegeInfo privilege = (HivePrivilegeInfo)var3.next();
HivePrincipal principal = privilege.getPrincipal();
HivePrivilegeObject resource = privilege.getObject();
HivePrincipal grantor = privilege.getGrantorPrincipal();
appendNonNull(builder, resource.getDbname(), true);
appendNonNull(builder, resource.getObjectName());
appendNonNull(builder, resource.getPartKeys());
appendNonNull(builder, resource.getColumns());
appendNonNull(builder, principal.getName());
appendNonNull(builder, principal.getType());
appendNonNull(builder, privilege.getPrivilege().getName());
appendNonNull(builder, privilege.isGrantOption());
appendNonNull(builder, testMode ? -1L : (long)privilege.getGrantTime() 
* 1000L); <-- in test mode, does not return real timestamp
appendNonNull(builder, grantor.getName());
  }

  return builder.toString();
} else {
  return "";
}
  }


Thanks,

Na Li

Review Request 65715: SENTRY-1720: Re-enable or remove TestHDFSIntegrationWithHA

2018-02-20 Thread Xinran Tinney


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

Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar 
kalvagadda, Na Li, and Sergio Pena.


Repository: sentry


Description
---

The TestHDFSIntegrationWithHA test is disabled now. We need to either fix it or 
remove if it is no longer applicable.
The TestHDFSIntegration test is disabled as well.


Diffs
-

  
sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 8852cbc0 
  
sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
 92c0693e 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
 96a2f901 


Diff: https://reviews.apache.org/r/65715/diff/1/


Testing
---

mvn clean install all success


Thanks,

Xinran Tinney

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

2018-02-20 Thread Na Li via Review Board



> On Feb. 20, 2018, 5:34 p.m., Xinran Tinney wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
> > Lines 228-229 (patched)
> > 
> >
> > This is converting from long to int, will there be overflow?

The timestamp is first converted from milliseconds to seconds, and then cast 
from long to int. It is OK


- Na


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


On Feb. 17, 2018, 12:30 a.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65642/
> ---
> 
> (Updated Feb. 17, 2018, 12:30 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
> Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> sentry CreateTime is the difference, measured in milliseconds, between the 
> current time and midnight, January 1, 1970 UTC. hive granttime is in seconds. 
> So need to convert the time.
> 
> The original code just cost the timestamp from long to int without converting 
> milliseconds to seconds. Therefore, the timestamp value is wrong when 
> retrieving the privilege from hive.
> 
> The solution is to convert the time correctly.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
>  b536283 
> 
> 
> Diff: https://reviews.apache.org/r/65642/diff/2/
> 
> 
> Testing
> ---
> 
> tested manually by stepping into code. I cannot test it in unit test because 
> Hive returns timestamp as -1L for test mode
> 
>   DDLTask.writeGrantInfo
> static String writeGrantInfo(List privileges, boolean 
> testMode) {
> if (privileges != null && !privileges.isEmpty()) {
>   StringBuilder builder = new StringBuilder();
>   Collections.sort(privileges, new Comparator() {
> public int compare(HivePrivilegeInfo o1, HivePrivilegeInfo o2) {
>   int compare = o1.getObject().compareTo(o2.getObject());
>   if (compare == 0) {
> compare = o1.getPrincipal().compareTo(o2.getPrincipal());
>   }
> 
>   if (compare == 0) {
> compare = o1.getPrivilege().compareTo(o2.getPrivilege());
>   }
> 
>   return compare;
> }
>   });
>   Iterator var3 = privileges.iterator();
> 
>   while(var3.hasNext()) {
> HivePrivilegeInfo privilege = (HivePrivilegeInfo)var3.next();
> HivePrincipal principal = privilege.getPrincipal();
> HivePrivilegeObject resource = privilege.getObject();
> HivePrincipal grantor = privilege.getGrantorPrincipal();
> appendNonNull(builder, resource.getDbname(), true);
> appendNonNull(builder, resource.getObjectName());
> appendNonNull(builder, resource.getPartKeys());
> appendNonNull(builder, resource.getColumns());
> appendNonNull(builder, principal.getName());
> appendNonNull(builder, principal.getType());
> appendNonNull(builder, privilege.getPrivilege().getName());
> appendNonNull(builder, privilege.isGrantOption());
> appendNonNull(builder, testMode ? -1L : 
> (long)privilege.getGrantTime() * 1000L); <-- in test mode, does not 
> return real timestamp
> appendNonNull(builder, grantor.getName());
>   }
> 
>   return builder.toString();
> } else {
>   return "";
> }
>   }
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

2018-02-20 Thread Xinran Tinney


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




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
Lines 228-229 (patched)


This is converting from long to int, will there be overflow?


- Xinran Tinney


On Feb. 17, 2018, 12:30 a.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65642/
> ---
> 
> (Updated Feb. 17, 2018, 12:30 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
> Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> sentry CreateTime is the difference, measured in milliseconds, between the 
> current time and midnight, January 1, 1970 UTC. hive granttime is in seconds. 
> So need to convert the time.
> 
> The original code just cost the timestamp from long to int without converting 
> milliseconds to seconds. Therefore, the timestamp value is wrong when 
> retrieving the privilege from hive.
> 
> The solution is to convert the time correctly.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
>  b536283 
> 
> 
> Diff: https://reviews.apache.org/r/65642/diff/2/
> 
> 
> Testing
> ---
> 
> tested manually by stepping into code. I cannot test it in unit test because 
> Hive returns timestamp as -1L for test mode
> 
>   DDLTask.writeGrantInfo
> static String writeGrantInfo(List privileges, boolean 
> testMode) {
> if (privileges != null && !privileges.isEmpty()) {
>   StringBuilder builder = new StringBuilder();
>   Collections.sort(privileges, new Comparator() {
> public int compare(HivePrivilegeInfo o1, HivePrivilegeInfo o2) {
>   int compare = o1.getObject().compareTo(o2.getObject());
>   if (compare == 0) {
> compare = o1.getPrincipal().compareTo(o2.getPrincipal());
>   }
> 
>   if (compare == 0) {
> compare = o1.getPrivilege().compareTo(o2.getPrivilege());
>   }
> 
>   return compare;
> }
>   });
>   Iterator var3 = privileges.iterator();
> 
>   while(var3.hasNext()) {
> HivePrivilegeInfo privilege = (HivePrivilegeInfo)var3.next();
> HivePrincipal principal = privilege.getPrincipal();
> HivePrivilegeObject resource = privilege.getObject();
> HivePrincipal grantor = privilege.getGrantorPrincipal();
> appendNonNull(builder, resource.getDbname(), true);
> appendNonNull(builder, resource.getObjectName());
> appendNonNull(builder, resource.getPartKeys());
> appendNonNull(builder, resource.getColumns());
> appendNonNull(builder, principal.getName());
> appendNonNull(builder, principal.getType());
> appendNonNull(builder, privilege.getPrivilege().getName());
> appendNonNull(builder, privilege.isGrantOption());
> appendNonNull(builder, testMode ? -1L : 
> (long)privilege.getGrantTime() * 1000L); <-- in test mode, does not 
> return real timestamp
> appendNonNull(builder, grantor.getName());
>   }
> 
>   return builder.toString();
> } else {
>   return "";
> }
>   }
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

2018-02-20 Thread Sergio Pena via Review Board


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




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
Lines 225-227 (patched)


I understand the cast from Millis to Secs is not correct, but can you 
rewrite the comment a little bit? It starts with 'sentry createTime is the 
difference' which looks like an answer from a question that is not posted here.


- Sergio Pena


On Feb. 17, 2018, 12:30 a.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65642/
> ---
> 
> (Updated Feb. 17, 2018, 12:30 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
> Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> sentry CreateTime is the difference, measured in milliseconds, between the 
> current time and midnight, January 1, 1970 UTC. hive granttime is in seconds. 
> So need to convert the time.
> 
> The original code just cost the timestamp from long to int without converting 
> milliseconds to seconds. Therefore, the timestamp value is wrong when 
> retrieving the privilege from hive.
> 
> The solution is to convert the time correctly.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
>  b536283 
> 
> 
> Diff: https://reviews.apache.org/r/65642/diff/2/
> 
> 
> Testing
> ---
> 
> tested manually by stepping into code. I cannot test it in unit test because 
> Hive returns timestamp as -1L for test mode
> 
>   DDLTask.writeGrantInfo
> static String writeGrantInfo(List privileges, boolean 
> testMode) {
> if (privileges != null && !privileges.isEmpty()) {
>   StringBuilder builder = new StringBuilder();
>   Collections.sort(privileges, new Comparator() {
> public int compare(HivePrivilegeInfo o1, HivePrivilegeInfo o2) {
>   int compare = o1.getObject().compareTo(o2.getObject());
>   if (compare == 0) {
> compare = o1.getPrincipal().compareTo(o2.getPrincipal());
>   }
> 
>   if (compare == 0) {
> compare = o1.getPrivilege().compareTo(o2.getPrivilege());
>   }
> 
>   return compare;
> }
>   });
>   Iterator var3 = privileges.iterator();
> 
>   while(var3.hasNext()) {
> HivePrivilegeInfo privilege = (HivePrivilegeInfo)var3.next();
> HivePrincipal principal = privilege.getPrincipal();
> HivePrivilegeObject resource = privilege.getObject();
> HivePrincipal grantor = privilege.getGrantorPrincipal();
> appendNonNull(builder, resource.getDbname(), true);
> appendNonNull(builder, resource.getObjectName());
> appendNonNull(builder, resource.getPartKeys());
> appendNonNull(builder, resource.getColumns());
> appendNonNull(builder, principal.getName());
> appendNonNull(builder, principal.getType());
> appendNonNull(builder, privilege.getPrivilege().getName());
> appendNonNull(builder, privilege.isGrantOption());
> appendNonNull(builder, testMode ? -1L : 
> (long)privilege.getGrantTime() * 1000L); <-- in test mode, does not 
> return real timestamp
> appendNonNull(builder, grantor.getName());
>   }
> 
>   return builder.toString();
> } else {
>   return "";
> }
>   }
> 
> 
> Thanks,
> 
> Na Li
> 
>

Save the date: ApacheCon North America, September 24-27 in Montréal

2018-02-20 Thread Rich Bowen


Dear Apache Enthusiast,

(You’re receiving this message because you’re subscribed to a user@ or 
dev@ list of one or more Apache Software Foundation projects.)


We’re pleased to announce the upcoming ApacheCon [1] in Montréal, 
September 24-27. This event is all about you — the Apache project community.


We’ll have four tracks of technical content this time, as well as lots 
of opportunities to connect with your project community, hack on the 
code, and learn about other related (and unrelated!) projects across the 
foundation.


The Call For Papers (CFP) [2] and registration are now open. Register 
early to take advantage of the early bird prices and secure your place 
at the event hotel.


Important dates
March 30: CFP closes
April 20: CFP notifications sent
	August 24: Hotel room block closes (please do not wait until the last 
minute)


Follow @ApacheCon on Twitter to be the first to hear announcements about 
keynotes, the schedule, evening events, and everything you can expect to 
see at the event.


See you in Montréal!

Sincerely, Rich Bowen, V.P. Events,
on behalf of the entire ApacheCon team

[1] http://www.apachecon.com/acna18
[2] https://cfp.apachecon.com/conference.html?apachecon-north-america-2018

Re: Review Request 65533: SENTRY-2115: Incorrect behavior of HMsFollower when HDFSSync feature is disabled.

Re: Review Request 58727: SENTRY-1708 Extend the current test classes to handle multiple sentry servers

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

Re: Review Request 65533: SENTRY-2115: Incorrect behavior of HMsFollower when HDFSSync feature is disabled.

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

Review Request 65715: SENTRY-1720: Re-enable or remove TestHDFSIntegrationWithHA

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

Save the date: ApacheCon North America, September 24-27 in Montréal

11 matches

Site Navigation

Mail list logo

Footer information