jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Berry van Halderen <b.vanhalde...@1hippo.com>
Subject Re: Modified externally exception due to eviction from shared ism cache
Date Tue, 18 May 2010 10:22:19 GMT
I've created an issue in Jira for the modified externally problem I've
stumbled upon as I've now been able to reproduce it with a unit test.  Quite a
large amount of investigation because a lot of factors must be in place.
In the Jira issue the unit test, and patches are attached, so I won't have to
post them here.

  https://issues.apache.org/jira/browse/JCR-2633

The unit test itself is mostly trivial, the problem arrises when you add a
mixin to a node, save it, do this again with another mixin and then remove
both mixins and in the following save the shared item state cache is shrunk,
and the garbage collectors hits at exactly the right time.  In the unit test a
reference to the jcr:mixinTypes property must have been held.

What will happen is that the ItemState of the jcr:mixinTypes property will get
a modification count higher than 0 when addin the mixins.  Because a reference
to the property is kept, it will not be evicted from the primary cache in the
local item state manager.  When removing all mixins, an overlayed state will
be created of this ItemState.  Because the state and overlayed state are
linked, the ItemState won't be dropped from the primary cache of the shared
item state manager.

But is MAY be evicted from the secondary cache implementing the LRU/FIFO
functionality.  If this is the case when while persisting the changes there is
a small window where the overlayed state will be disconnected from the state.
This happens just before collecting the changelog at:
  o.a.j.c.state.ItemState.disconnect():211
  o.a.j.c.state.SessionItemStateManager.disconnectTransientItemState():674
  o.a.j.c.PropertyImpl.makePersistent():143
  o.a.j.c.ItemImpl.persistTransientItems():609
  o.a.j.c.ItemImpl.save():1087
  o.a.j.c.SessionImpl.save():858
Or in the when actually collecting the changelog in one of the methods
Changelog.{modified(),deleted() or both.  I think the latter, but not really
sure.

In any case, this breaks the bondage that prevents the cached state in the
shared item state manager.

Now if the shared item state cache has been shrunk enough and the GC hits
before o.a.j.c.state.SharedItemStateManager.Update.begin():650 when the
disconnected state will be purged from the shared item state cache.  Just
below line 650 the check for stale items will then cause a re-retrieval from
the persisted store of the property.  Because that state will have a
modification count of 0, it will conflict with the modification count of the
mixin property type that is being persisted.

It is true that the GC needs to go off at exactly the right time and the
secondary cache needs to have shrunk to be able to evict the item.  This can
however happen in extreme cases.  The patch that contains the unit test
contains code that will force the purging of the item.

There is still the matter why the modcount comes back at 0 when retrieving the
property from the persistence manager, basically the assumption made in the
code between session, local, and shared item state managers, their caches,
etcetera is that the modification count in the shared item state is always
incremented, and never reset.

There is an apparent contract (partially documented) that the modification
count is to be persisted.  Which is in fact the case for the InMemPersistence-
Manager, but all bundle derived persistence managers do not persist the
jcr:mixinType property at all, but just the mixintypes as part of the
nodedefinition information.  This means that the jcr:mixinTypes property will
always be re-created with modcount of 0, which leads to clashes.

There are multiple approaches to fix this:
- One could prevent the overlayed states being evicted by holding some
  alternative reference to it (patch provided (holdrefs.patch));
- modify the cache implementation at the shared item state manager such that
  it will simply never release states from its secondary cache unless all
  items are also gone from all local item state caches;
- replace the modcount implementation, where instead of counting modifications
  a state is marked dirty.  This is probably too much work, but has the
  advantage of avoiding problems with a modcount which can actually reset
  anyway (it being a short making it not unlikely);
- ensure the modification count of the jcr:mixinTypes is stored by the
  persistence manager (patch provided in jira issue (persistMixinTypes.patch).

I would prefer the latter.  It holds more firm to the JCR model where
meta-data as mixinTypes just properties, and it holds the apparent contract
that modcount is to be persisted.  In my opinion it is unjust that the
mixinTypes property wasn't persisted.  Apart from jcr:mixinTypes, only
jcr:uuid and jcr:primaryType aren't persisted, but those are immutable
therefor they are really in a different league.

The patches also contrain a small modification as suggestion such that the
caching mechanism may be overridden.  Caching strategies are never a one-size
fits all, therefor it is good to make it pluggable.  And a small modification
to make jackrabbit-core compilable with Java5.

\Berry

Mime
View raw message