Re: PerSessionPageStore thread-safety

[Thomas Heigl]

Hi Martin,

I created https://github.com/apache/wicket/pull/411 with my suggested
changes. Feedback would be greatly appreciated ;)

Best regards,

Thomas

On Fri, Feb 28, 2020 at 10:14 AM Martin Grigorov 
wrote:

> On Thu, Feb 27, 2020 at 3:58 PM Thomas Heigl  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
> > :
> >
> > 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 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 so

Re: PerSessionPageStore thread-safety

[Martin Grigorov]

On Thu, Feb 27, 2020 at 3:58 PM Thomas Heigl  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
> :
>
> 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 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 int