> On July 8, 2016, 8:17 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java,
> >  line 114
> > <https://reviews.apache.org/r/49738/diff/4/?file=1439872#file1439872line114>
> >
> >     Signature does not throw an exception

These are not checked exceptions and don't need to be declared.


> On July 8, 2016, 8:17 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java,
> >  line 154
> > <https://reviews.apache.org/r/49738/diff/4/?file=1439872#file1439872line154>
> >
> >     Signature does not throw an exception

Same as above


> On July 8, 2016, 8:17 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java,
> >  line 201
> > <https://reviews.apache.org/r/49738/diff/4/?file=1439872#file1439872line201>
> >
> >     Do we want to instead rethrow the exception, so that the caller is 
> > aware that operation did not succeed?
> 
> Sravya Tirukkovalur wrote:
>     I see that caller would never know that rename has failed. Is this a case 
> where we cannot recover from or cannot handle? May be caller can retry as 
> this means that fencing has failed?

We haven't caught the exception.  This is a finally block.  Any exception will 
continue to be thrown after executing this block.


> On July 8, 2016, 8:17 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java,
> >  line 234
> > <https://reviews.apache.org/r/49738/diff/4/?file=1439872#file1439872line234>
> >
> >     Seems like this function is basically executing select* . What is the 
> > purpose of it?

The purpose is to verify that the fencing table still exists


- Colin


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


On July 11, 2016, 6:55 p.m., Colin McCabe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49738/
> -----------------------------------------------------------
> 
> (Updated July 11, 2016, 6:55 p.m.)
> 
> 
> Review request for sentry, Hao Hao, Rahul Sharma, and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1317
>     https://issues.apache.org/jira/browse/SENTRY-1317
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1317: Implement fencing required for active/passive
> 
> 
> Diffs
> -----
> 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryStandbyException.java
>  PRE-CREATION 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java
>  3da4906 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PluginCacheSyncUtil.java
>  4ce16c7 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
>  5bf2f6e 
>   
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestHAUpdateForwarder.java
>  5246e05 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java
>  cacc29f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  7dad496 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceRegister.java
>  79dfe48 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SqlAccessor.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
>  3de1f65 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activators.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e846766 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusAdaptor.java
>  80a6571 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  809af06 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  0ab8192 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreIntegrationBase.java
>  f14b586 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java
>  799d5ef 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  3ef1cf7 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreImportExport.java
>  3ff97df 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java
>  a8e8a03 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
>  cb2d9c9 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatus.java
>  434ac41 
> 
> Diff: https://reviews.apache.org/r/49738/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin McCabe
> 
>

Reply via email to