You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Colin McCabe <cm...@cloudera.com> on 2016/07/07 01:13:53 UTC

Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

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

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/SentryHDFSServiceProcessorFactory.java db55b5a 
  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/generic/service/persistent/DelegateSentryStore.java e960dcd 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorFactory.java 1cce1fc 
  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/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 691c1fb 
  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/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/ProcessorFactory.java a3bb6ab 
  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


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49738/#review141790
-----------------------------------------------------------


Ship it!




Ship It!

- Hao Hao


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


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49738/#review141940
-----------------------------------------------------------


Ship it!




Ship It!

- Sravya Tirukkovalur


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


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.

> On July 12, 2016, 4:36 p.m., Sravya Tirukkovalur wrote:
> > Thanks Colin! Code mostly looks good to me ( I have one unresolved comment). I think it is ok to add e2e testing in a follow on patch, is there a jira tracking it? Would you be able to add a unit test for Fencer as part of this patch?

Just spoke to Colin offline. Seems like unit tests for Fencer are on the way as part of next patch, and also start up case is unit tested by TestSentryStore. Hence +1 from me.


- Sravya


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


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


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49738/#review141910
-----------------------------------------------------------



Thanks Colin! Code mostly looks good to me ( I have one unresolved comment). I think it is ok to add e2e testing in a follow on patch, is there a jira tracking it? Would you be able to add a unit test for Fencer as part of this patch?

- Sravya Tirukkovalur


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


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Colin McCabe <cm...@cloudera.com>.
-----------------------------------------------------------
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 (updated)
-----

  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


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Colin McCabe <cm...@cloudera.com>.

> On July 11, 2016, 9:17 a.m., Hao Hao wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java, line 161
> > <https://reviews.apache.org/r/49738/diff/4/?file=1439872#file1439872line161>
> >
> >     Exception will be thrown if create table failed?

yes


> On July 11, 2016, 9:17 a.m., Hao Hao wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java, line 222
> > <https://reviews.apache.org/r/49738/diff/4/?file=1439872#file1439872line222>
> >
> >     Should it be expected? It seems it should never happen?

Good question.  It is not a harmful condition, so there is no reason to consider it an error.  It could happen if a daemon loses the leader election but then regains the leadership shortly thereafter.


> On July 11, 2016, 9:17 a.m., Hao Hao 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>
> >
> >     Yeah, not clear about what is the function for? It seems not be called anywhere?

see above comment-- the purpose is to verify that the fencing table still exists as part of a transaction.  There will be callers later


> On July 11, 2016, 9:17 a.m., Hao Hao wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 175
> > <https://reviews.apache.org/r/49738/diff/4/?file=1439874#file1439874line175>
> >
> >     Could you document a bit more why do we need these two properties to disallow operations outside of transcations? Will it ever happen?

This is an existing setting of SentryStore-- nothing is being changed here, just moving around some code.  See the section about "non-transactional reads" here: http://www.datanucleus.org/products/datanucleus/jdo/performance_tuning.html to see why we don't want them


> On July 11, 2016, 9:17 a.m., Hao Hao wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java, line 156
> > <https://reviews.apache.org/r/49738/diff/4/?file=1439877#file1439877line156>
> >
> >     Why do we need to removeth haContext?

HAContext was used by the old HA implementation which is being replaced.  There is no function for it any more.


> On July 11, 2016, 9:17 a.m., Hao Hao wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreIntegrationBase.java, line 36
> > <https://reviews.apache.org/r/49738/diff/4/?file=1439884#file1439884line36>
> >
> >     Should we add a e2e test for fencing between active and standby?

I definitely agree that more tests would be good.  We have that on the roadmap, and shouldn't block this patch on that, though.


- Colin


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


On July 8, 2016, 7:59 p.m., Colin McCabe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49738/
> -----------------------------------------------------------
> 
> (Updated July 8, 2016, 7:59 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
> 
>


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49738/#review141650
-----------------------------------------------------------




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java (line 161)
<https://reviews.apache.org/r/49738/#comment207011>

    Exception will be thrown if create table failed?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java (line 222)
<https://reviews.apache.org/r/49738/#comment207012>

    Should it be expected? It seems it should never happen?



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/#comment207013>

    Yeah, not clear about what is the function for? It seems not be called anywhere?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 172)
<https://reviews.apache.org/r/49738/#comment207014>

    Could you document a bit more why do we need these two properties to disallow operations outside of transcations? Will it ever happen?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 
<https://reviews.apache.org/r/49738/#comment207016>

    Why do we need to removeth haContext?



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreIntegrationBase.java (line 36)
<https://reviews.apache.org/r/49738/#comment207009>

    Should we add a e2e test for fencing between active and standby?


- Hao Hao


On July 8, 2016, 7:59 p.m., Colin McCabe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49738/
> -----------------------------------------------------------
> 
> (Updated July 8, 2016, 7:59 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
> 
>


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.

> 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?
> 
> Colin McCabe wrote:
>     We haven't caught the exception.  This is a finally block.  Any exception will continue to be thrown after executing this block.

My bad, missed that this is part of finally.


- Sravya


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


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Colin McCabe <cm...@cloudera.com>.

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


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.

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

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?


- Sravya


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


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49738/#review141326
-----------------------------------------------------------




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/#comment206831>

    Signature does not throw an exception



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/#comment206832>

    Signature does not throw an exception



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/#comment206833>

    Do we want to instead rethrow the exception, so that the caller is aware that operation did not succeed?



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/#comment206834>

    Seems like this function is basically executing select* . What is the purpose of it?


- Sravya Tirukkovalur


On July 8, 2016, 7:59 p.m., Colin McCabe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49738/
> -----------------------------------------------------------
> 
> (Updated July 8, 2016, 7:59 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
> 
>


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Colin McCabe <cm...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49738/
-----------------------------------------------------------

(Updated July 8, 2016, 7:59 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 (updated)
-----

  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


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.

> On July 8, 2016, 6:04 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java, line 135
> > <https://reviews.apache.org/r/49738/diff/3/?file=1439071#file1439071line135>
> >
> >     Why rollback? What does roll back mean for read only queries? Is it still recommended to commit the transaction?
> 
> Colin McCabe wrote:
>     While it is true that this is a read-only transaction, rollback is still required to terminate the transaction.  Otherwise, later we will receive the error "javax.jdo.JDOUserException: Transaction is still active.  You should always close your transactions correctly using commit() or rollback()."  See this link for more information:  http://stackoverflow.com/questions/4418317/jdo-exception-in-google-app-engine-transaction

Thanks for the info Colin! Yes, I agree that we should commit for read only transactions as well, and hence I was curious why there is no commit. But rather we just have 

    if (tx.isActive()) {
        tx.rollback();
      }
      

Where it will always be active, as we have not made an attempt to commit.


- Sravya


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


On July 8, 2016, 7:59 p.m., Colin McCabe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49738/
> -----------------------------------------------------------
> 
> (Updated July 8, 2016, 7:59 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
> 
>


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Colin McCabe <cm...@cloudera.com>.

> On July 8, 2016, 6:04 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java, line 135
> > <https://reviews.apache.org/r/49738/diff/3/?file=1439071#file1439071line135>
> >
> >     Why rollback? What does roll back mean for read only queries? Is it still recommended to commit the transaction?
> 
> Colin McCabe wrote:
>     While it is true that this is a read-only transaction, rollback is still required to terminate the transaction.  Otherwise, later we will receive the error "javax.jdo.JDOUserException: Transaction is still active.  You should always close your transactions correctly using commit() or rollback()."  See this link for more information:  http://stackoverflow.com/questions/4418317/jdo-exception-in-google-app-engine-transaction
> 
> Sravya Tirukkovalur wrote:
>     Thanks for the info Colin! Yes, I agree that we should commit for read only transactions as well, and hence I was curious why there is no commit. But rather we just have 
>     
>         if (tx.isActive()) {
>             tx.rollback();
>           }
>           
>     
>     Where it will always be active, as we have not made an attempt to commit.

Good point.  Let me add the tx#commit


- Colin


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


On July 8, 2016, 7:59 p.m., Colin McCabe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49738/
> -----------------------------------------------------------
> 
> (Updated July 8, 2016, 7:59 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
> 
>


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Colin McCabe <cm...@cloudera.com>.

> On July 8, 2016, 6:04 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java, line 135
> > <https://reviews.apache.org/r/49738/diff/3/?file=1439071#file1439071line135>
> >
> >     Why rollback? What does roll back mean for read only queries? Is it still recommended to commit the transaction?

While it is true that this is a read-only transaction, rollback is still required to terminate the transaction.  Otherwise, later we will receive the error "javax.jdo.JDOUserException: Transaction is still active.  You should always close your transactions correctly using commit() or rollback()."  See this link for more information:  http://stackoverflow.com/questions/4418317/jdo-exception-in-google-app-engine-transaction


> On July 8, 2016, 6:04 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java, line 174
> > <https://reviews.apache.org/r/49738/diff/3/?file=1439071#file1439071line174>
> >
> >     Rollback if it has'nt been successful?

Good point.  I made this a transaction.


> On July 8, 2016, 6:04 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java, line 206
> > <https://reviews.apache.org/r/49738/diff/3/?file=1439071#file1439071line206>
> >
> >     The purpose of this function does not look accurate?

renamed


> On July 8, 2016, 6:04 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SqlAccessor.java, line 70
> > <https://reviews.apache.org/r/49738/diff/3/?file=1439075#file1439075line70>
> >
> >     We do not have support for sql server in Sentry. So best to leave it out for now?

ok


> On July 8, 2016, 6:04 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java, line 171
> > <https://reviews.apache.org/r/49738/diff/3/?file=1439071#file1439071line171>
> >
> >     Excecute within a transaction?

It is fine to execute queries within a transaction.


> On July 8, 2016, 6:04 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java, line 992
> > <https://reviews.apache.org/r/49738/diff/3/?file=1439084#file1439084line992>
> >
> >     It is unclear to me what new validations are we adding to this test?

It's not adding new validations, just supplying the Activator object which the SentryStore now needs.


- Colin


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


On July 7, 2016, 9:33 p.m., Colin McCabe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49738/
> -----------------------------------------------------------
> 
> (Updated July 7, 2016, 9:33 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
> 
>


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49738/#review141285
-----------------------------------------------------------



Nice work Colin! Left some initial feedback. Seems like we need to add testing to make sure fencing is being done?


sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java (line 105)
<https://reviews.apache.org/r/49738/#comment206771>

    Document which exceptions are thrown in what conditions?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java (line 120)
<https://reviews.apache.org/r/49738/#comment206770>

    SentryAccessDeniedException does not seem like the right exception we want to throw here?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java (line 126)
<https://reviews.apache.org/r/49738/#comment206772>

    Same as above, is this the right exception type?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java (line 135)
<https://reviews.apache.org/r/49738/#comment206773>

    Why rollback? What does roll back mean for read only queries? Is it still recommended to commit the transaction?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java (line 171)
<https://reviews.apache.org/r/49738/#comment206774>

    Excecute within a transaction?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java (line 174)
<https://reviews.apache.org/r/49738/#comment206775>

    Rollback if it has'nt been successful?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java (line 206)
<https://reviews.apache.org/r/49738/#comment206776>

    The purpose of this function does not look accurate?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SqlAccessor.java (line 70)
<https://reviews.apache.org/r/49738/#comment206768>

    We do not have support for sql server in Sentry. So best to leave it out for now?



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java (line 992)
<https://reviews.apache.org/r/49738/#comment206778>

    It is unclear to me what new validations are we adding to this test?


- Sravya Tirukkovalur


On July 7, 2016, 9:33 p.m., Colin McCabe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49738/
> -----------------------------------------------------------
> 
> (Updated July 7, 2016, 9:33 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
> 
>


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Colin McCabe <cm...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49738/
-----------------------------------------------------------

(Updated July 7, 2016, 9:33 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 (updated)
-----

  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


Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

Posted by Colin McCabe <cm...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49738/
-----------------------------------------------------------

(Updated July 7, 2016, 6:31 p.m.)


Review request for sentry, Hao Hao, Rahul Sharma, and Sravya Tirukkovalur.


Changes
-------

Fix PMD warning


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


Repository: sentry


Description
-------

SENTRY-1317: Implement fencing required for active/passive


Diffs (updated)
-----

  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/SentryHDFSServiceProcessorFactory.java db55b5a 
  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/generic/service/persistent/DelegateSentryStore.java e960dcd 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorFactory.java 1cce1fc 
  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/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 691c1fb 
  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/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/ProcessorFactory.java a3bb6ab 
  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