> On April 19, 2017, 3:06 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Lines 258 (patched)
> > <https://reviews.apache.org/r/58412/diff/3/?file=1693883#file1693883line258>
> >
> >     Would this work correctly if we have two HMSFollowers running 
> > concurrently?

You mean concurrently running from two different server or from the same server?


> On April 19, 2017, 3:06 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Lines 438 (patched)
> > <https://reviews.apache.org/r/58412/diff/3/?file=1693883#file1693883line491>
> >
> >     Here and in other places in this function there isn't any value in 
> > throwing an exception. We got something wrong from Hive, we should just log 
> > it and continue. Stack trace would add much value.
> >     
> >     Also here we know that one of the three is null and yet we try to show 
> > them all as %s so we are likely to get NPE here instead.

Currently if we have SentryInvalidInputException or 
SentryInvalidHMSEventException, it will log the error/exception. I can add to 
print the stacktrace there.


- Hao


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


On April 18, 2017, 9:34 p.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58412/
> -----------------------------------------------------------
> 
> (Updated April 18, 2017, 9:34 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1587: Refactor SentryStore transaction to persist a single path 
> transcation bundled with corresponding delta path change
> 
> Change-Id: Ice77f631e92c0e74b42ae644e1b2f90c6089e62f
> 
> 
> Diffs
> -----
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java
>  90aaaef0e15306627d7108f12a74a29848055c0b 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
>  14e967aa1065f16e8d4c3f61db2f9055959fa9e6 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
>  c22364fa26c80415827313f4d26f6c53b71b6f6c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MPath.java
>  c74384688ca920c79fb2987d225042e135cdfd53 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
>  81a4c6ecb271d8f04fe8caab1b52e8b4a2813ba1 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  08520f30f3529af88d7dab9ae3623f28f0e43558 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  16676fb13b0d5015aefe892a6f7e46812ea75124 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  87e329588990be129197d598dd1db4487f8e0f25 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
>  24ab1a8b392f23bc75759733bef7cecd4bc2ac34 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  7769f24bb4c062016084bcafe7d50a0f0e013824 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  0e97466c9ba6973d8e787819e292c3b826472b38 
> 
> 
> Diff: https://reviews.apache.org/r/58412/diff/3/
> 
> 
> Testing
> -------
> 
> 1. Add new unit test in TestSentryStore
> 2. Enabled hdfs sync integration test
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Reply via email to