2014-12-17 7:52 GMT+09:00 Mark Thomas <ma...@apache.org>: > > The fix for BZ 57338 (SSO + cluster) ended up making some fairly > invasive changes so that the implementation was sensible. I'd like to > discuss which - if any - of these changes we'd be happy to back-port to > Tomcat 8 and Tomcat 7. > > [1] Make GenericPrincipal Serializable > I don't see any harm in back-porting this. > > [2] Remove SerializablePrincipal from the cluster implementation > This isn't necessary but is it a nice clean-up that is possible because > of [1]. I'm thinking back-port to 8.0.x only. It depends if > SerializablePrincipal is considered part of the API or an implementation > detail. > > [3] Switch to ConcurrentHashMap and remove syncs > I don't see any harm in back-porting this. > > [4] Remove lookup() method > This changes the API and is not necessary so I think we leave this as is. > > [5] Move sync to method > I don't see any harm in back-porting this. > > [6] Make SSO Maps non-final > This is essential to facilitate the fix so it has to be back-ported. > > [7] Switch to ReplicatedMap > This changes the ClusterSSO API is essential to facilitate the fix so it > has to be back-ported. > > [8] Remove Session from SSO Maps > This changes the SSO API but it is essential to facilitate the fix so it > has to be back-ported. > > [9] Make SingleSignOnEntry Serializable > I don't see any harm in back-porting this. > > [A] Logging TODOs > I don't see any harm in back-porting this. > > In summary, patches [7] and [8] look to be the ones that need the most > careful consideration followed by [2]. > > Thoughts? > > [1][2][3][4][5][6][9][A] +1 back-port. There is no objection here.
[7][8] I have few comment. [A] I think that the process of updating SSO entry needs to be notify cluster. [B] The mapSendOptions and terminateOnStartFailure attributes do not need to be configurable? [C] Is deregister(String ssoId, Session session) method required? I do not know where it will be called. [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. 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. Node that received the COPY message may have to save the entry as a backup or copy(newly added). > Mark > > > [1] http://svn.apache.org/r1645953 > [2] http://svn.apache.org/r1645955 > [3] http://svn.apache.org/r1646099 > [4] http://svn.apache.org/r1646100 > [5] http://svn.apache.org/r1646101 > [6] http://svn.apache.org/r1646102 > [7] http://svn.apache.org/r1646103 > [8] http://svn.apache.org/r1646104 > [9] http://svn.apache.org/r1646105 > [A] http://svn.apache.org/r1646106 > > --------------------------------------------------------------------- > 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>
