Hi all,

I can confirm that my analysis was correct. I implemented a quick version
of ApplicationScopedPageAccessSynchronizer and everything works as expected.

Providing a custom PageAccessSynchronizer is very cumbersome at the moment.
I had to override all concrete methods in the base class and completely
duplicate the PageLock class because waitForRelease and markReleased have
package visibility.

I would suggest to make these two methods in PageLock public so the class
can be reused and extract all the locking logic into an ILockManager with 3
methods: lockPage, unlockPage, and unlockAllPages. The default lock manager
could hold the session-scoped locks collection and custom implementations
could provide their own locking mechanism.

I will create a JIRA issue for this.

Best regards,

Thomas

On Wed, Feb 26, 2020 at 8:17 PM Thomas Heigl <tho...@umschalt.com> 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
> 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 <s...@meiers.net> 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 <
>> tho...@umschalt.com>:
>> >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 <tho...@umschalt.com>
>> >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
>> >>
>>
>

Reply via email to