> 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 > >