Hi Shreedhar.
I am satisfied with your diff.
Thanks!
--
Bongjae Chang
----- Original Message -----
From: Shreedhar Ganapathy
To: dev_at_shoal.dev.java.net
Sent: Monday, July 21, 2008 2:06 AM
Subject: Re: [Shoal-Dev] About FailureRecoverySignal.acquire()
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 has FailureRecoverySignalImpl for default FailureRecoverySignal.
As you know, FailureRecoverySignalImpl implements Signal.acquire().
The following are 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 of returning no exception 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 current members.
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 be selected, 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 additional documents and javadoc express that FailureRecoverySignalImpl.acquire() can be returned with no exceptions though 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