1 2 Previous Next 28 Replies Latest reply on Jul 2, 2009 12:30 PM by brian.stansberry Go to original post
      • 15. Re: ConcurrentModificationException during session serializa
        brian.stansberry

        Scott, this gets trickier in AS 5. In AS 5 the serialization of the attributes is deferred until JBoss Cache needs to serialize for replication. Which happens in ClusteredSessionValve.handleRequest:

        [code
        ]finally
        {
        if (startedBatch)
        {
        tm.endBatch();
        }
        }

        • 16. Re: ConcurrentModificationException during session serializa
          smarlow

          Brian,

          We will have to try to get access to the StandardSessionFacade from there as well (perhaps through the ClusteredSession).



          The concern I have is StandardSession.facade is not thread safe, and what's returned from getSession() needs to be if we are going to use it as a mutex.

          We will only synchronize on the StandardSession.facade object and not modify it. Since we are not modifying the StandardSession.facade object, we do not need it to be thread safe. At least that is my thinking, let me know if I'm missing something :-)


          There's no ref to facade outside StandardSession.getSession(), so ClusteredSession can override the getSession() method and replace the field with a volatile field and synchronize the construction if null.

          I don't really understand the case when getSession would return null. I wonder if the null session case signify something special that needs to be preserved? For what its worth, the javadoc for Requests.getsession() is:

          /**
           * Return the session associated with this Request, creating one if necessary.
           */
          


          • 17. Re: ConcurrentModificationException during session serializa
            brian.stansberry

             

            "smarlow@redhat.com" wrote:
            Brian,

            We will have to try to get access to the StandardSessionFacade from there as well (perhaps through the ClusteredSession).



            Yes. The tricky bit is if there are multiple sessions being replicated (as part of a cross-context request.) See below for more.

            "smarlow@redhat.com" wrote:


            The concern I have is StandardSession.facade is not thread safe, and what's returned from getSession() needs to be if we are going to use it as a mutex.

            We will only synchronize on the StandardSession.facade object and not modify it. Since we are not modifying the StandardSession.facade object, we do not need it to be thread safe. At least that is my thinking, let me know if I'm missing something :-)



            Sorry, I wasn't clear. The concern I have is concurrent requests to getSession() resulting in 2 facades being created. The classic singleton "getInstance() method" concurrency problem, except here it's not a static field. If you're going to lock on the facade you need to make sure there is only one facade. The current getSession() method doesn't do that.


            There's no ref to facade outside StandardSession.getSession(), so ClusteredSession can override the getSession() method and replace the field with a volatile field and synchronize the construction if null.


            What I described above is probably way overkill. Perhaps just calling getSession() in the constructor to ensure the field is initialized will work.


            Seems my last post was misformatted and got cut off. Further to the AS 5 discussion, in the future it's possible the serialization will happen on a different thread. (Infinispan was looking at putting replication messages in a queue and then having seperate threads process them, including serialization.) If that were the case, using a synchronized block around the ClusteredSessionValve's tm.endBatch() call won't work.

            But, the thing that actually gets serialized is an org.jboss.ha.framework.server.SimpleCachableMarshalledValue. A possible solution is to change that class to optionally pass in a "MutexSource" to its constructor:

            public interface MutexSource {
            
             public abstract Object getMutex();
            
            }


            If a MutexSource is available, SimpleCachableMarshalledValue.serialize() would call that method and synchronize on the return value. ClusteredSession would implement MutexSource by returning the result of a getSession() call.

            This would also deal with the cross-context request issue noted above.

            • 18. Re: ConcurrentModificationException during session serializa
              smarlow

              This isn't perfect but we might be able to convert the HTTP session attribute value to byte array before multiplexing and the reverse on the receiving end. I'm not thrilled about double buffering the attribute value (keeping a copy in serialized form and original object form), but maybe we are doing that already on the multiplexing side (in which case there isn't extra memory overhead).

              The advantage of the above approach is that we could easily synchronize the relevant session facade during serialization/de-serialization. I'm not sure how that impacts the current design. What do you think?

              • 19. Re: ConcurrentModificationException during session serializa
                smarlow

                After sleeping on it, I don't like the memory impact of my suggestion (serializing attribute values earlier). Your idea sounds better

                If a MutexSource is available, SimpleCachableMarshalledValue.serialize() would call that method and synchronize on the return value. ClusteredSession would implement MutexSource by returning the result of a getSession() call.

                This would also deal with the cross-context request issue noted above.


                +1

                • 20. Re: ConcurrentModificationException during session serializa
                  brian.stansberry

                  Good. :-) I very deliberately eliminated the double buffering because of the memory cost.

                  FYI, before this thread got going I'd been thinking (vaguely) in terms of using read/write locks in AS 6 to better handle coordination. E.g., in ClusteredSessionValve acquire a RL before passing the request down the pipeline, release when it returns, acquire a WL before handling replication. Basically, allow concurrent access to a session but lock out requests while a thread is replicating.

                  The same thing could be done by having ClusteredSessionValve synchronize on the facade while passing the request down the pipeline, but then only one request can proceed at a time.

                  • 21. Re: ConcurrentModificationException during session serializa
                    smarlow

                     

                    FYI, before this thread got going I'd been thinking (vaguely) in terms of using read/write locks in AS 6 to better handle coordination. E.g., in ClusteredSessionValve acquire a RL before passing the request down the pipeline, release when it returns, acquire a WL before handling replication. Basically, allow concurrent access to a session but lock out requests while a thread is replicating.


                    In the current case, the application code is synchronizing on the session facade object, so they would be left out if we synchronize on an internal r/w lock.

                    I wonder if we could somehow expose our (HTTP session level) r/w lock object to the application (in a reasonably portable way), so that they could also get a RL when getting the attribute values. Applications will also want to get a WL when setting attribute values.



                    • 22. Re: ConcurrentModificationException during session serializa
                      brian.stansberry

                      If we use a R/W lock, there shouldn't be any application activity during the periods where we are serializing etc. The WL we acquire prevents it.

                      • 23. Re: ConcurrentModificationException during session serializa
                        smarlow

                        If we are holding the WL lock on some thread, couldn't another thread (processing an application request against the same HTTP session) access the locked attribute (reading/writing attribute values).

                        Maybe I missed how we would force application code to use our RW lock. Does that come for free with your approach? :-)

                        • 24. Re: ConcurrentModificationException during session serializa
                          brian.stansberry

                          The key is acquiring a RL in ClusteredSessionValve:

                           // let the servlet invocation go through
                           Lock lock = manager.getReadLock(requestedSessionId);
                           lock.lock();
                           try
                           {
                           if (isEvent)
                           {
                           getNext().event(request, response, event);
                           }
                           else
                           {
                           getNext().invoke(request, response);
                           }
                           }
                           finally
                           {
                           lock.unlock();
                           }
                          


                          The stuff inside that try/finally block encapsulates the application code's interaction with the session. The lock returned by manager.getReadlLock(...) is the RL from a java.util.concurrent.lock.ReadWriteLock. When any replication activity happens, it happens outside the above block and the corresponding WL is acquired.

                          Result is, application code will not be manipulating the session when the WL is held. Application code doesn't need access to the RL as it will never be conflicting with the replication code.


                          Unspoken and unproven assumption here: there is a sufficient benefit to having *guaranteed* lockout of application activity. One benefit is it removes the need for the application to synchronize.

                          • 25. Re: ConcurrentModificationException during session serializa
                            smarlow

                            I see, so we are obtaining the RL for the application (held while the application request is running). This avoids the cooperative locking scheme that would of required the application to lock on the sessionFacade object.

                            Some potential performance issues that could challenge this approach.

                            1. During http session replication, further http requests (from users that are involved with the replication) will block (at least while the session level write lock is held for the replication).

                            2. When a http request is completing that has modified http session attributes, the replication will be delayed until the write lock can be obtained. This could introduce unexpected performance delays.

                            3. Are there additional performance implications for synchronous replication? If the user waits in the browser for the replication to complete, they could be waiting a long time, depending on when the write lock can be obtained.

                            • 26. Re: ConcurrentModificationException during session serializa
                              brian.stansberry

                              To answer your #1 and #3, the key thing is to hold the WL for the minimum required time. You only have to hold it long enough to:

                              a) Accurately determine if anything needs to be replicated
                              b) Copy that into the DTO that's passed to the replication layer.
                              c) Serialize attributes.

                              Ideally a) and b) can be combined into one very quick step, although that would take some rework of logic. Then the WL could be acquired separately for c), which again should be quick. So you don't want to hold the WL for the duration of the replication.

                              Hmm, although that somewhat defeats the point, which is to get a consistent snapshot of the session. Separately locking for the serialization allows a request thread to sneak in and change the state of an attribute value.

                              #2 is more of an issue. By relying on synchronizing on the facade, we're basically giving priority to getting the replication out over consistency, and then letting the app enforce consistency by synchronizing on the facade.

                              • 27. Re: ConcurrentModificationException during session serializa
                                smarlow

                                Currently, deployed applications do not hold a RL lock while processing requests. If we introduce a RL lock to be held while processing requests, will that be slower?

                                There is some potential additional overhead to the ReadWriteLock (http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/locks/ReadWriteLock.html).

                                Perhaps it would be better (performance wise) to synchronize on the session level facade object as I think the performance should be close to what applications currently see (whether or not they also synchronize on the session object when accessing mutable attributes).

                                What do you think?

                                • 28. Re: ConcurrentModificationException during session serializa
                                  brian.stansberry

                                  I (or you or nbelaevski if either is up for it) need to review the session replication code to see how much of an issue this is. I haven't had cycles to do that, which is why my comments here are vague.

                                  If the issue is only protecting the attributes, then synching on the facade in the places where we serialize it is fine.

                                  If something broader is needed, the best way to decide what to do is the perf test the two approaches.

                                  1 2 Previous Next