On Thu, Feb 27, 2020 at 3:58 PM Thomas Heigl <[email protected]> wrote:
> Hi Martin, > > Could you please explain how exactly Spring Session works ? This will help > > us both understand where the problem comes from. > > > > Sure. From the docs > <https://docs.spring.io/spring-session/docs/current/reference/html5/>: > > Spring Session provides an API and implementations for managing a user’s > > session information while also making it trivial to support clustered > > sessions without being tied to an application container-specific > solution. > > It allows replacing the HttpSession in an application container-neutral > > way, with support for providing session IDs in headers to work with > RESTful > > APIs. > > > Spring Session basically is a servlet filter > < > https://docs.spring.io/spring-session/docs/current/reference/html5/#httpsession-how > > > that looks up and persists sessions in an external store. > It does so, by wrapping the servlet request and providing a custom > implementation of HttpSession that keeps > track of changes and persists them at the end of the request. It is similar > to what container-based replication > does but is *independent* of the container. That's why I prefer to use it > instead of adding a custom session manager to Tomcat. > It is much easier to customize Spring Session behavior (e.g. how cookies > are stored, etc) because it is part > of your application and not your container. > > Also how exactly do you integrate it with Wicket ? Which extension > > points do you use ? > > > There are *no* integration points with Wicket. Replacing the HttpSession > with Spring Session is completely transparent. > Wicket does not know anything about Spring session. The only difference is > that the session is not held in the containers memory, > but serialized to and from an external source. I have done extensive > testing and except for access synchronization everything > else works as expected. > > Since it is backed by Redis it means that it is distributed. > > > But at the same time you try to use PerSessionPageStore which is in-memory > > store, i.e. it lives only in the memory of one of the web container > nodes. > > Another node has its own instance of this class. > > > I want to use PerSessionPageStore as a local cache on each node together > with sticky sessions to avoid > having to go through the whole process of Redis + Deserialization for every > single ajax request. > In a non-clustered environment or an environment where clustering is done > by the servlet container, Wicket > can make use of the session cache that stores the last touched pages in the > session. In case of > Spring Session, the session cache cannot be used. Sessions are > (de)serialized on every request and the session > cache in Wicket is transient. > > So strictly speaking, I don't need the PerSessionPageStore but since I > cannot use the session cache, ajax requests > become quite expensive. I'm using it as an application-scoped alternative > to the session cache. > > Here is how it works in normal Servler environment: > > ..... > > > I completely agree on all your descriptions of session management in a > (clustered) container environment. We have > a shared understanding on container-based session management. > > I don't see how application-scope would help at all. As I explained > > earlier PageAccessSynchronizer is per o.a.w.Session instance. > > Maing it application-scoped will only add more concurrency issues and you > > will have to add locks on session level. At the moment it is lock-free. > > > There will be more concurrency, but that is not really an issue: > > The quick replacement for the PageAccessSynchronizer I wrote this morning > uses the following data structure: > > private static final ConcurrentMap<PageKey, PageLock> LOCKS = new > > ConcurrentHashMap<>(); > > > I made it static instead of moving it to the application, because it was > faster to implement. The only difference to > the default synchronizer is the key used for the map. Instead of an integer > for the pageId, I'm using a composite > key of sessionId and pageId. > > @Value > > static final class PageKey { > > String sessionId; > > Integer pageId; > > } > > > All locking/synchronization is done on the PageLock object that is scoped > to a single session by the composite key. > There will be more reads/writes to the concurrent map, but no additional > locking is necessary and unless there are > thousands of requests per second this should hardly be an issue. > > Let us first hear back how this will solve the issue and then we will > > discuss such change. > > > To summarize: > > I'm using Spring Session that is a simple, container-neutral, transparent > solution for clustering sessions. Wicket > works fine with Spring Session. Except for one minor issue: > PageAccessSynchronizer does not work because > sessions are not held in memory but (de)serialized on every request. Thus > the internal lock map is not shared > between requests. Moving the lock map into global or application scope > solves the issue with the downside of > increased concurrency to the, now shared, map but should not require any > additional locking or synchronization. > It is quite trivial to write such a global synchronizer, but it could be > much easier if some additional methods > in the default access manager were public and lock handling would be > encapsulated in an interface. > > I hope I managed to explain myself. The topic is quite complex ;) > Thank you for this comprehensive explanation, Thomas! :-) Please create a Pull Request with your suggested changes! Martin > > Best, > > Thomas > > > > On Thu, Feb 27, 2020 at 1:47 PM Martin Grigorov <[email protected]> > wrote: > > > Hi Thomas, > > > > Could you please explain how exactly Spring Session works ? This will > help > > us both understand where the problem comes from. > > Also how exactly do you integrate it with Wicket ? Which extension points > > do you use ? > > > > Since it is backed by Redis it means that it is distributed. > > But at the same time you try to use PerSessionPageStore which is > in-memory > > store, i.e. it lives only in the memory of one of the web container > nodes. > > Another node has its own instance of this class. > > > > Here is how it works in normal Servler environment: > > > > The HttpSession instances are managed by the web container, e.g. Tomcat. > > The container keeps them in memory! The container may persist them for > > failover/restart but usually the HttpSessions are kept in memory. > > > > 1) if there is just one web container node then Wicket asks the container > > for the javax.servler.HttpSession: httpServletRequest.getSession() > > Then Wicket extracts the org.apache.wicket.Session from the HttpSession > > attributes. > > The Wicket Session class has a member instance of PageAccessSynchronizer > > that is specific for the current instance of the Session. > > PageAccessSynchronizer has a Map<Integer, PageLock>, i.e. pageId -> > > PageLock. > > Whenever a request needs to use a specific Wicket Page then the current > > session's PageAccessSynchronizer is used to acquire a lock on it. This > > makes sure that a specific page instance is used by at most one request > in > > the *current* server node. > > > > 2) if there is session replication in place, i.e. more than one server > > nodes, then: > > The web container fetches the HttpSessions for its backend. In case of > > Tomcat - it keeps the HttpSessions in memory but there is a > ClusterManager > > that replicates the HttpSessions' which have been modified, i.e. their > > #setAttibute() has been called. > > In cluster mode there will be one HttpSession per server node, > respectfully > > one Wicket Session instance per node, and one PageAccessSynchronizer per > > session per node. > > Using Wicket or not if you don't use sticky sessions you may face timing > > related issues in such environment. Every distributed software has this > > problem. > > > > > > On Wed, Feb 26, 2020 at 9:17 PM Thomas Heigl <[email protected]> > wrote: > > > > > Hi Sven, > > > > > > Thanks for the link but I'm not using asynchronous serialization. > > > > > > I thought some more about the issue and I think I figured it out. My > > setup > > > looks like this: > > > > > > 1. Spring Session Redis > > > 2. [Session Cache] (Not used because it is transient and stored with > > > writeObject/readObject and does not get serialized into Redis as we do > > not > > > use Java serialization) > > > 3. PerSessionPageStore (Application-level cache held in memory) > > > 4. RedisDataStore (Synchronous) > > > > > > Observations: > > > > > > 1. If i disable second-level cache or use the serializing second-level > > > cache provided by the DefaultPageManager, there are no issues > > > 2. As soon as I enable the PerSessionPageStore we run into concurrency > > > issues > > > > > > Analysis: > > > > > > I first thought that there were some thread-safety issues > > > with PerSessionPageStore but that is not the case because even a fully > > > synchronized version shows these problems. > > > > > > The reason why disabling the 2nd-level cache or using a serializing > > variant > > > works, is because they do not operate on the same *instance* of the > page. > > > Each thread gets their > > > own instance because the page is deserialized before being accessed. > > > > > > PerSessionPageStore stores the page in memory without serializing it, > > thus > > > all threads share the same instance. This is also the case when using > the > > > session cache or > > > the session-based stores, but the PageAccessSynchronizer bound to the > > > session takes care of ensuring that only single request can manipulate > > the > > > page at any given time. > > > > > > So how does the synchronizer work? It keeps a Map<Integer, PageLock> in > > the > > > session and checks whether the page is locked on every request. In a > > > non-replicated > > > environment this works as expected as the session object lives in the > > > servlet container and is the same for each concurrent request. In my > > case, > > > the session > > > is not provided by the servlet container, but fetched from Redis by > > Spring > > > Session on every request. So each concurrent thread has *their own > > version* > > > of the session and > > > the locks are *not shared between threads* because the session is only > > > saved back to Redis after the request has finished. > > > > > > So the problematic flow looks like this > > > > > > 1. A request comes in, we fetch the session from Redis, the request > > > acquires the session-scoped lock and starts processing > > > 2. Before the request is finished, another request comes in, fetches > the > > > session from Redis, the lock map is empty because the state of request > #1 > > > has not been persisted to Redis > > > 3. Now both requests can modify the page and we run into concurrency > > issues > > > > > > Summary: > > > > > > PageAccessSynchronizer does not work with Spring Session or other > > solutions > > > that replace the servlet container session. > > > > > > Possible solutions: > > > > > > 1. We could ensure that session locks are updated in Redis immediately > > but > > > that still leaves a couple of milliseconds for race conditions and > adds a > > > lot of overhead > > > > > > > This is called a distributed lock. > > It will reduce the problems you face but will make things slower too. > > > > > > > 2. We could use an application-scoped PageAccessSynchronizer. This > solves > > > the problem as long as sessions are sticky and all concurrent requests > > are > > > sent to the same server. > > > > > > > I don't see how application-scope would help at all. As I explained > > earlier PageAccessSynchronizer is per o.a.w.Session instance. > > Maing it application-scoped will only add more concurrency issues and you > > will have to add locks on session level. At the moment it is lock-free. > > > > > > > 3. If we want to use non-sticky session we could use Redis locks for > > > implementing a global PageAccessSynchronizer > > > > > > > Sticky sessions is usually good enough for most of the cases. > > Distributed locks will make your application much slower. > > If you use single instance of Redis you will have single point of > failure. > > If you use Redis replication then the distributed locking will become > even > > more complex and slow. > > > > > > > > > > I would like to go with solution #2 for now. The problem is > > > that PageAccessSynchronizer is not an interface. > > > > > > Would it be possible to extract an interface so I can easily implement > > > access synchronizers with different scopes? > > > > > > > Let us first hear back how this will solve the issue and then we will > > discuss such change. > > > > > > > > > > Best regards, > > > > > > Thomas > > > > > > > > > > > > > > > > > > On Wed, Feb 26, 2020 at 12:24 PM Sven Meier <[email protected]> wrote: > > > > > > > Hi Thomas, > > > > > > > > Im wondering whether you're running into > > > > https://issues.apache.org/jira/browse/WICKET-6702 > > > > > > > > I've been working on a solution to that problem, which is caused by > > pages > > > > being asynchronously serialized while another request is already > coming > > > in. > > > > > > > > Or maybe it is something different. > > > > Could you create a quickstart? > > > > > > > > Sven > > > > > > > > Am 25. Februar 2020 22:12:46 MEZ schrieb Thomas Heigl < > > > [email protected] > > > > >: > > > > >Hi again, > > > > > > > > > >I investigated a bit and it does not seem to have anything to do > with > > > > >the > > > > >PerSessionPageStore. I implemented a completely synchronized version > > of > > > > >it > > > > >and the problems still exist. > > > > > > > > > >If I switch to the default second-level cache that stores serialized > > > > >pages > > > > >in application scope, everything works as expected. Only the > > > > >non-serialized > > > > >pages in PerSessionPageStore seem to be affected by concurrent ajax > > > > >modifications. > > > > > > > > > >What is the difference between keeping pages in the session and > > keeping > > > > >the > > > > >same pages in the PerSessionPageStore? Is there some additional > > locking > > > > >done for pages in the session? > > > > > > > > > >Best, > > > > > > > > > >Thomas > > > > > > > > > >On Tue, Feb 25, 2020 at 8:25 PM Thomas Heigl <[email protected]> > > > > >wrote: > > > > > > > > > >> Hi all, > > > > >> > > > > >> I'm currently experimenting with PerSessionPageStore as a > > > > >second-level > > > > >> cache. We are moving our page store from memory (i.e. session) to > > > > >Redis and > > > > >> keeping 1-2 pages per session in memory speeds up ajax requests > > quite > > > > >a bit > > > > >> because network roundtrips and (de)serialization can be skipped > for > > > > >cached > > > > >> pages. > > > > >> > > > > >> Our application is very ajax heavy (it is basically a single page > > > > >> application with lots of lazy-loading). While rapidly clicking > > around > > > > >and > > > > >> firing as many parallel ajax requests as possible, I noticed that > it > > > > >is > > > > >> quite easy to trigger exceptions that I have never seen before. > > > > >> ConcurrentModificationExceptions during serialization, > > > > >> MarkupNotFoundExceptions, exceptions about components already > > > > >dequeuing etc. > > > > >> > > > > >> So I had a look at the implementation of PerSessionPageStore and > > > > >noticed > > > > >> that is does not do any kind of synchronization and does not use > > > > >atomic > > > > >> operations when updating the cache. It seems to me that the > > > > >second-level > > > > >> cache is not really usable in a concurrent ajax environment. > > > > >> > > > > >> I think that writing pages to the second level cache store should > > > > >either > > > > >> synchronize on sessionId+pageId or attempt to use atomic > operations > > > > >> provided by ConcurrentHashMap. > > > > >> > > > > >> Did anyone else ever run into these issues? The code > > > > >> of PerSessionPageStore is quite complex because of soft > references, > > > > >> skip-list maps etc. so I'm not sure what the right approach to > > > > >address > > > > >> these problems would be. > > > > >> > > > > >> Best regards, > > > > >> > > > > >> Thomas > > > > >> > > > > > > > > > >
