Registering any CacheEntryListener can block on all cache events even if they are unrelated

[Samuel Bussmann]

If a single CacheEntryListener is registered, all cache modification events that can result in a Listener-events will trigger a CacheEvent through CacheEventDispatcherImpl[1] even if there is no existing listener for this specific event.

In case of a synchronized Listener this will block until all other previous events are processed. Therefore increasing response times.

And even in asynchronous mode this will spawn a new thread just to discover it has nothing to do.

In our case we wanted to use a Listener to do some housekeeping onRemoved(), which could add a new entry to the same cache through event.getSource.put(). In a synchronous Listener this will result in a deadlock.

Does anyone have an alternative solution to our requirement or do you think this is an actual issue that should be adressed?

[1] cacheModification -> StoreEventDispatcher.releaseEventSink() -> FireableStoreEventHolder.fireOn() -> CacheEventDispatcherImpl.StoreListener.onEvent() -> CacheEventDispatcherImpl.onEvent()

[Chris Dennis]

It’s not a great idea to mutate a same cache from its own event listener, since the possibility of a deadlock is always looming. Using asynchronous listeners obviously helps… but in principle if there’s a bounded pool anywhere in the system you can still get in to a jam. That said, the queue there is currently unbounded… so you are safe atm (I think).

Moving on… yes that dispatch code is dumb… no reason to not veto the dispatch when nobody is listening for the event, and we have all the information needed to make that call.

 

It’s an issue that should be addressed, and I don’t think the fix is that complex. PR should be incoming in the next hour or so.

 

Chris

--
You received this message because you are subscribed to the Google Groups "ehcache-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ehcache-user...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ehcache-users/8b5e5827-bbb3-4eaa-bb97-740a2cdf5176n%40googlegroups.com.

[Samuel Bussmann]

Hi Chris,

Thank you for the quick response and the corresponding changes!

Is there an schedule for the next Release including this fix?

In our case using an asynchronous listener opens up a race condition even running single threaded, so being able to use synchronous would be preferred.

I've got two additional, somewhat related questions:

- Through the javax.cache interface it's not possible to add a unordered, synchronous which is all that is needed in our case. Is there any way to configure this through the JCache-API?

- Unless I'm mistaken cache evictions are no longer possible to listen to through JCache, but still notified by ehCache internals. Is there a possibility to map the event to remove or expired to catch all cache modifications?

Sam

[Samuel Bussmann]

I just did some really crude perf-Test:

50k add(),get(),get(),remove() with a NoOp Listener on Removes

duration:

no listener: 420ms
async listener: 585ms
synchronous listener: 3000ms

looking forward to the effect of your changes

[Samuel Bussmann]

As this issue is affecting our production I tried some changes on my own and repeated the test:
Testcase: 

  20k add(),get(),get(),remove() with a NoOp Listener on Removes

rev_ehcache=3.10.8

-> Baseline
 - test no_listener ended average duration was 17ms
 - test unordered_asynch ended average duration was 42ms
 - test unordered_synch ended average duration was 1411ms
 - test ordered_asynch ended average duration was 53ms
 - test ordered_synch ended average duration was 1780ms


rev_ehcache=3.10_PR3170 (https://github.com/ehcache/ehcache3/pull/3170)

-> applied the changes of PR 3170
 -   test no_listener average duration was 17ms
 -   test unordered_asynch average duration was 40ms
 -   test unordered_synch average duration was 591ms
 -   test ordered_asynch average duration was 46ms
 -   test ordered_synch average duration was 553ms

rev_ehcache=3.10_RegisterNoEvent-SNAPSHOT
-> Filter irrelevant EventTypes already in InvocationScopedEventSink
 -   test no_listener average duration was 17ms
 -   test unordered_asynch average duration was 35ms
 -   test unordered_synch average duration was 516ms
 -   test ordered_asynch average duration was 37ms
 -   test ordered_synch average duration was 580ms

rev_ehcache =3.10_InlineUnorderedSynchListener-SNAPSHOT
-> run unordered synch listeners directly in current thread of CacheEventDispatcherImpl instead of delegating to executor service.

 -   test no_listener average duration was 17ms
 -   test unordered_asynch average duration was 33ms
 -   test unordered_synch average duration was 19ms
 -   test ordered_asynch average duration was 36ms
 -   test ordered_synch average duration was 498ms

So there's some significant potential here.

And in a test of our production-implementation I saw clear improvements too:

Test real impl (rev_ehcache=3.10.8)
test average duration was 2226ms

Test real impl (rev_ehcache=3.10_PR3170)
test average duration was 1177ms

Test real impl (rev_ehcach=3.10_InlineUnorderedSynchListener-SNAPSHOT)
test average duration was 564ms

[Samuel Bussmann]

Hello

I've created a MergeRequest [1] to implement the described improvements.
I hope you can take a look at it and support the proposed approach.

[1] https://github.com/ehcache/ehcache3/pull/3236

best regards

[Chris Dennis]

Hi Samuel,

 

I’m very interested in looking over this but there are complicated reasons why I can’t review it at the moment, and can’t merge the code either. The short version is that IBM have acquired the Ehcache project (as part of a much larger acquisition from Software AG the previous owners) and that is going to require changes to the CLA, and a lot more work both publically and privately by those of us inside the machine. Once the dust settles, I’ll definitely get back to you, and be working to get things sorted out, but for now my hands are a little tied.

 

Thanks,

 

Chris

 

To view this discussion on the web visit https://groups.google.com/d/msgid/ehcache-users/e7287611-7d56-4607-bffa-c57486f7d14bn%40googlegroups.com.

[Samuel Bussmann]

Hello Chris,

I've seen the changes to CLA-Process. Do I have to redo the whole Merge-Request or is there a way to sign off the MR-Commits retroactively?
Is there an ETA, when you can proceed working on ehCache-tasks?

Thanks,
Samuel