Re: PerSessionPageStore thread-safety

2020-02-26 Thread Thomas Heigl
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 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
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.
3. If we want to use non-sticky session we could use Redis locks for
implementing a global PageAccessSynchronizer

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?

Best regards,

Thomas





On Wed, Feb 26, 2020 at 12:24 PM Sven Meier  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  >:
> >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 
> >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 

Re: PerSessionPageStore thread-safety

2020-02-26 Thread Sven Meier
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 :
>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 
>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
>>