2014-12-19 21:40 GMT+09:00 Mark Thomas <ma...@apache.org>: > On 17/12/2014 16:57, Mark Thomas wrote: > > On 17/12/2014 09:58, Keiichi Fujino wrote: > > >> [A] I think that the process of updating SSO entry needs to be notify > >> cluster. > > > > I'll take a look at this. I didn't see any issues in my simple testing > > but I may well have missed this scenario. > > My testing did indeed miss this. I have partially fixed this with one > more commit to come. > > >> [B] The mapSendOptions and terminateOnStartFailure attributes do not > need > >> to be configurable? > > > > Fair point. I'll add that. > > Fixed. > > >> [C] Is deregister(String ssoId, Session session) method required? > >> I do not know where it will be called. > > > > I'll look into it. > > Removed. > > >> [D] If my understanding is correct, > >> Session objects are associated with SingleSignOnEntry of primary node > >> and SingleSignOn is registered as a SessionListener. > >> I think this is no problem. > >> However, when the primary node goes down, because SingleSignOnEntry of > >> non-primary node is > >> not associated with the session object, SingleSignOn can not receive a > >> SessionEvent. > >> As a result, SingleSignOnEntry does not delete because the deregister > >> method is no longer called. > >> This might be a potential memory leak. > > > > My testing definitely didn't cover this. I'll add some logging and > > extend my tests. > > Yes this was an issue. I have a fix for this on the way. See below. > > >> At least, the new node that was promoted to the primary node needｓ to > >> associate the session object again. > >> For example, the association of session might be able to in > >> objectMadePrimary method. > >> However, because there is a concern of [E], is not called > objectMadePrimary > >> method. > >> > >> [E] Although I think this is another potential problem, > >> ReplicatedMap sends a COPY message to non-primary node. > >> Node that received the COPY message does not save the entry as a > >> backup(MapEntry.backup=false). > >> As a result, MapEntry.isPrimary() always returns true. > >> I do not know yet whether this is a problem, > >> but, at least, processing of entry relocate in memberDisappeared seem > to > >> not work properly. > > > > Since every MapEntry should be replicated in full to every node (I went > > with ReplicatedMap rather than LazyReplicatedMap) I don't think this is > > an issue but I'll look in to this as well. > > It was an issue. It gets more complicated when you consider that this > needs to work with both the Delta and Backup Managers. > > I looked into making stickiness based on the SSO cookie but that had a > number of issues: > - The SSO cookie doesn't exist until there is an authentication > - The SSO cookie is only a cookie - there is no support for URL encoding > - Current load-balancer configs would need to change to track the SSO > cookie > All of these are solvable but the solution was starting to look messy so > I looked elsewhere. > > I next considered switching to LazyReplicatedMap. This solved that lack > of objectMadePrimary() calls but created a whole other set of problems > mostly because different contexts in the SSO would be stuck to different > nodes so there was no node that made sense as the SSO primary. > Stickiness based on the SSO cookie would solve this but as described > above, that gets messy. > > The solution I opted for was to make the SSO SessionListener > Serializable and use that to perform the necessary deregistration. It > effectively pushes the elements of the reverse Map into the session. For > the clustered case, I added support for replicating listeners (if marked > with the right interface) as part of session replication. > > The end result is something that isn't too complicated and works with > any of the current Cluster Managers and should work with custom ones as > well. The only downside is that there are nearly always more attempts to > deregister sessions than are required as the listener will fire for each > node in the cluster. The up side of this is that it is pretty much > impossible for an edge case to slip through where the session isn't > removed from SSO. > > ClusterSingleSignOn#associate and ClusterSingleSignOn#update invokes the AbstractReplicatedMap#replicate(ssoId, true). In fact, AbstractReplicatedMap#replicate(ssoId, true) sends not a MSG_COPY message but a MSG_BACKUP message. Is this a problem?
The current implementation of AbstractReplicatedMap seems to not handle COPY nodes explicitly. In some methods (get, replicate, messageReceived, memberDisappeared, etc.), it may be better that a COPY node is handled explicitly. Or override these methods (some or all) in LazyReplicatedMap and ReplicatedMap. > I want to review my patches for typos etc and plan to commit this to > trunk later today. > > Based on feedback so far, I also plan to back-port this to 8.0.x, see > how it goes in the next release and then back-port to 7.0.x if all goes > well. > > Mark > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > > -- > Keiichi.Fujino > <dev-h...@tomcat.apache.org> > <dev-h...@tomcat.apache.org> >
