wicket-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thomas Heigl <tho...@umschalt.com>
Subject Re: PerSessionPageStore thread-safety
Date Thu, 27 Feb 2020 10:40:26 GMT
I created https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6751

Feedback would be very welcome.

Best regards,

Thomas

On Thu, Feb 27, 2020 at 10:08 AM Thomas Heigl <thomas@umschalt.com> wrote:

> 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 <thomas@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 <sven@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 <
>>> thomas@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 <thomas@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
>>> >>
>>>
>>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message