On 20 Jan 2010, at 11:20, Galder Zamarreno wrote:
Hi,
Re:
https://jira.jboss.org/jira/browse/JBCACHE-1555
I've got a fix for this issue that fixes both the writeSkew=true and
writeSkew=false situations:
The problem as stated in 1555 is that
MVCCNodeHelper.wrapNodeForWriting() peeks for data, via
DC.peekInternalNodeAndDirectParent before acquiring any locks. So, if a
thread ends up waiting for the lock and someone else comes and writes
the data, when the waiting thread wakes up, it uses stale peeked data.
The solution I propose is the following:
if (lockForWriting && acquireLock(context, fqn)) {
needToCopy = true;
// re-peek in case we waited for a lock and some other thread
modified the data while we're waiting
nodes = dataContainer.peekInternalNodeAndDirectParent(fqn,
includeInvalidNodes);
}
n = nodeFactory.createWrappedNode(nodes[0], nodes[1]);
We only need to worry about the case where we wanna write cos on reads,
we never have to acquire locks. So, on writes, after acquiring the lock,
we re-peek to get the most up to date data, which we then use to create
the wrapped node.
This seems to work fine for the test. We'd have to check hudson.
One doubt I have is whether the 1st peak here could be optimised to be
an exist() call:
InternalNode[] nodes =
dataContainer.peekInternalNodeAndDirectParent(fqn, includeInvalidNodes);
InternalNode in = nodes[0];
if (in != null) {
...
However, includeInvalidNodes might be key here and I don't see any
exists in the DataContainer API that allows to pass that parameter. So,
I think this needs to stay as a peek.
Yeah it can stay as a peek. Compared to the cost of writing, an additional peek is
negligible.
Thoughts?
_______________________________________________
jbosscache-dev mailing list
jbosscache-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/jbosscache-dev
--
Manik Surtani
manik(a)jboss.org
Lead, Infinispan
Lead, JBoss Cache
http://www.infinispan.org
http://www.jbosscache.org