[jbosscache-dev] Re: Fixing MVCC(RC) + UnversionedNode for PCACHE-82

Hi all, Re: https://jira.jboss.org/jira/browse/PCACHE-82 Even though this is not urgent, it's a good exercise for me to get a better understanding of MVCC and hence why I decided to finally tackle it. I've been trying to figure out what exactly is the bug in this JIRA. I've created a test case that reproduces the bug (attached). The issue is clearly related to MVCC(ReadCommitted) and UnversionedNode instances but I'm failing to understand what the fix exactly is. Let me sum up what happens internally (note that a lot of the information below is from logging statements I added): 1. Create a Person, Galder, and attach it under /person/ testDoubleAttachAndDetach 2. Create a Person, Asier, and attach it in the same fqn. 3. In the 2nd attachment, ReadCommittedNode.markForUpdate() backs up the NodeReference for /person/testDoubleAttachAndDetach creates a copy of the UnversionedNode representing underneath. 4. More importantly, because calling attach the 2nd time means detach needs to happen before to remove the old value, so MVCCNodeHelper.wrapNodesRecursivelyForRemoval() executes the addChild() below: // update child ref on parent to point to child as this is now a copy. if (parentLockNeeded && (needToCopyNode || needToCopyParent)) { if (parent == null) throw new NodeNotExistsException("Parent node " + parentFqn + " does not exist!"); parent.getDelegationTarget().addChild(node.getDelegationTarget()); } This leads to adding the UnversionedNode version of testDoubleAttachAndDetach to /person node's children. 5. At the end 2nd attachment, ReadCommittedNode.updateNode() method takes the backup (NodeReference) and the node (UnversionedNode) and swaps them again but the parent's children, /person, is still pointing to an UnversionedNode which leads to the ClassCastException that you see when you execute the test. I'm not sure what really should be happening here since I don't understand the rationaly of that addChild() call. Should it be happening at all in this case? Or maybe a corrective action of some sort needs to be done somewhere? Thanks for the help in advance! -- Galder Zamarreño Sr. Software Engineer Infinispan, JBoss Cache /* * JBoss, Home of Professional Open Source. * Copyright 2009, Red Hat Middleware LLC, and individual contributors * as indicated by the @author tags. See the copyright.txt file in the * distribution for a full listing of individual contributors. * * This is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as * published by the Free Software Foundation; either version 2.1 of * the License, or (at your option) any later version. * * This software is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public * License along with this software; if not, write to the Free * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA * 02110-1301 USA, or see the FSF site: http://www.fsf.org. */ package org.jboss.cache.pojo; import static org.testng.AssertJUnit.assertNull; import static org.testng.AssertJUnit.assertEquals; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.jboss.cache.CacheSPI; import org.jboss.cache.Fqn; import org.jboss.cache.pojo.PojoCache; import org.jboss.cache.pojo.PojoCacheFactory; import org.jboss.cache.pojo.test.Address; import org.jboss.cache.pojo.test.Person; import org.testng.annotations.Test; /** * ClassCastExceptionTest. * * @author Galder Zamarreño */ @Test(groups = {"functional"}) public class AttachDetachTest { private static final Log log = LogFactory.getLog(AttachDetachTest.class); private PojoCache cache; public void testDoubleAttachAndDetach() throws Exception { log.info("testDoubleAttachAndDetach() ...."); String configFile = "META-INF/local-readcommitted.xml"; cache = PojoCacheFactory.createCache(configFile); try { // String id = "person/testLoadAttachDetach"; // Person galder = createPerson("Galder", 20); // log.info("Attaching Galder..."); // cache.attach(id, galder); // cache.stop(); // // cache = PojoCacheFactory.createCache(configFile); // Person p = (Person) cache.find(id); // log.info("Person found: " + p); // // assertEquals("age ", p.getAge(), galder.getAge()); // // Person asier = createPerson("Asier", 34); // log.info("Attaching Asier..."); // cache.attach(id, asier); // // cache.detach(id); String id = "person/testDoubleAttachAndDetach"; Person galder = createPerson("Galder", 20); log.info("Attaching Galder..."); cache.attach(id, galder); // cache.stop(); // // cache = PojoCacheFactory.createCache(configFile); // Person p = (Person) cache.find(id); // log.info("Person found: " + p); // assertEquals("age ", p.getAge(), galder.getAge()); Person asier = createPerson("Asier", 34); log.info("Attaching Asier..."); cache.attach(id, asier); cache.detach(id); } catch (Exception e) { log.error("Unexpected exception", e); } finally { cache.getCache().removeNode(Fqn.ROOT); // ((CacheSPI<Object, Object>) cache .getCache ()).getCacheLoaderManager().getCacheLoader().remove(Fqn.ROOT); cache.stop(); } // Thread.sleep(9100);// default is 3 seconds so joe should have been passivated. // // assertNull("Node should be evicted ", ((CacheSPI<Object, Object>) cache.getCache()).peek(new Fqn<String>(id), false)); // Person p = (Person) cache.find(id); // // log.info("Person found: " + p); // // Person galder = createPerson("Galder Zamarreno", 29); // cache.attach(id, galder); // assertEquals("age ", 20, joe.getAge()); // joe.setAge(30); // assertEquals("age ", 30, joe.getAge()); // assertEquals("age ", p.getAge(), joe.getAge()); // assertFalse("Instance not equal (this is known side effect) ", joe == p); // cache.detach(id); } private Person createPerson(String name, int age) { Person p = new Person(); p.setName(name); p.setAge(age); Address add = new Address(); add.setZip(2000); add.setCity("Neuchatel"); p.setAddress(add); return p; } }