Hi Bongjae
Here's a diff for the raiseFence() throwing the exception when a fence
already exists while raising the fence.
Added an else block that would throw an exception if the fence exists.
If there is more that needs to be done, please let me know.
Index: src/java/com/sun/enterprise/ee/cms/impl/jxta/GroupHandleImpl.java
===================================================================
RCS file:
/cvs/shoal/gms/src/java/com/sun/enterprise/ee/cms/impl/jxta/GroupHandleImpl.java,v
retrieving revision 1.11
diff -r1.11 GroupHandleImpl.java
245a246,248
> else {
> throw new GMSException ("Could not raise fence. Fence for member "
+failedMemberToken +" and Component "+ componentName+" already exists");
> }
Thanks
Shreedhar
Shreedhar Ganapathy wrote:
>
>
> Bongjae Chang wrote:
>> Hi.
>> I know Shoal hasFailureRecoverySignalImpl for default
>> FailureRecoverySignal.
>> As you know, FailureRecoverySignalImpl implements Signal.acquire().
>> The followingare FailureRecoverySignalImpl.acquire() and
>> GroupHandleImpl.raiseFence()
>> [FailureRecoverySignalImpl.java]
>> public void acquire() throws SignalAcquireException {
>> try {
>> final GroupHandle gh = ctx.getGroupHandle();
>> if(gh.isMemberAlive( failedMember ) ){
>> throw new GMSException( "Cannot raise fence on " + failedMember + "
>> as it is already alive");
>> }
>> gh.raiseFence(componentName, failedMember);
>> logger.log(Level.FINE, "raised fence for component " + componentName
>> + " and member " + failedMember);
>> } catch( GMSException e ) {
>> throw new SignalAcquireException(e);
>> }
>> }
>> [GroupHandleImpl.java]
>> public void raiseFence(final String componentNAme, final String
>> failedMemberToken) throws GMSException {
>> if(!isFenced(componentName, failedMemberToken)){
>> ...
>> }
>> }
>> I am interested in the case ofreturning noexception when
>> FailureRecoverySignalImpl.acquire() is invoked.
>> 1. when failedMember is not alive and isFenced() is false --> normal case
>> 2. when failedMember is not alive and isFenced() is true --> abnormal
>> case
>> Of course, I think that above 2 case is not occurred in current Shoal
>> version because
>> RecoveryTargetSelector.resolveWithSimpleSelectionAlgorithm() will
>> select only same and one recoverer in currentmembers.
>> But in the future, Shoal can provide custom recovery selection
>> algorithm for users and another recovery selection algorithm that
>> multiple recoverers can be selected,
>> I think that above 2 case can be occurred.
>> Of course, if users check GroupHandle.isFenced() before users invoke
>> FailureRecoverySignal.acquire(), there is no problem.
>> But most users will use FailureRecoverySignalImpl and
>> FailureRecoveryActionImpl like me.
>> Then in the above 2 case, if multiple recoverers can beselected,
>> FailureRecoverySignal.acquire() is meaningless because
>> FailureRecoverySignal.acquire() will be returned with no exception
>> quietly.
>> So I think that it is better that FailureRecoverySignalImpl.acquire()
>> should throw an exception or GroupHandleImpl.raiseFence should throw
>> an exception when isFenced() is true.
> I see. Inside of GroupHandle.raiseFence() there is no case where a
> fence already exists and hence an exception should be thrown. That's a
> very good observation. Thanks for the code review. If I am missing
> something more from your above use case, please me know.
>
>> Or I think it is better that additionaldocuments and javadocexpress
>> that FailureRecoverySignalImpl.acquire() can be returnedwith no
>> exceptionsthough isFenced() is true, so users should check isFenced()
>> before invoking acquire()safely.
> Ideally the impl behavior should do the right thing.
>> If recovery selection algorithm's rule allows only unique recoverer,
>> this suggestion is meaningless. :-)
> Not at all. :)
>> This is just my opinion.
> And a good one :)
>> Thanks.
>> --
>> Bongjae Chang