From a20913ab7f67758ee09b0820f7925301c2121e44 Mon Sep 17 00:00:00 2001 From: hns Date: Thu, 24 May 2007 14:10:53 +0000 Subject: [PATCH] * Use incremental serial numbers for DbMapping.lastDataChange and Node.lastSubnode* fields instead of timestamps, because the latter may not work consitently. Fixes bug 518 http://helma.org/bugs/show_bug.cgi?id=518 * Do not fetch named subnodes from relational database. Fixes regression described in comment #4 of bug 516 http://helma.org/bugs/show_bug.cgi?id=516#c4 --- src/helma/objectmodel/db/DbMapping.java | 16 ++--- src/helma/objectmodel/db/Node.java | 77 +++++++++++++---------- src/helma/objectmodel/db/NodeManager.java | 16 ++--- src/helma/objectmodel/db/Transactor.java | 8 +-- 4 files changed, 65 insertions(+), 52 deletions(-) diff --git a/src/helma/objectmodel/db/DbMapping.java b/src/helma/objectmodel/db/DbMapping.java index 0be9a3e0..98013870 100644 --- a/src/helma/objectmodel/db/DbMapping.java +++ b/src/helma/objectmodel/db/DbMapping.java @@ -112,7 +112,7 @@ public final class DbMapping { long lastTypeChange = -1; // timestamp of last modification of an object of this type - long lastDataChange; + long lastDataChange = 0; // evict objects of this type when received via replication private boolean evictOnReplication; @@ -1251,18 +1251,18 @@ public final class DbMapping { * Set the last time something changed in the data, propagating the event * to mappings that depend on us through an additionalTables switch. */ - public void setLastDataChange(long t) { + public void setLastDataChange() { // forward data change timestamp to storage-compatible parent mapping if (inheritsStorage()) { - parentMapping.setLastDataChange(t); + parentMapping.setLastDataChange(); } else { - lastDataChange = t; + lastDataChange += 1; // propagate data change timestamp to mappings that depend on us if (!dependentMappings.isEmpty()) { Iterator it = dependentMappings.iterator(); while(it.hasNext()) { DbMapping dbmap = (DbMapping) it.next(); - dbmap.setIndirectDataChange(t); + dbmap.setIndirectDataChange(); } } } @@ -1273,12 +1273,12 @@ public final class DbMapping { * data change triggered by a mapping we depend on, so we don't propagate it to * mappings that depend on us through an additionalTables switch. */ - protected void setIndirectDataChange(long t) { + protected void setIndirectDataChange() { // forward data change timestamp to storage-compatible parent mapping if (inheritsStorage()) { - parentMapping.setIndirectDataChange(t); + parentMapping.setIndirectDataChange(); } else { - lastDataChange = t; + lastDataChange += 1; } } diff --git a/src/helma/objectmodel/db/Node.java b/src/helma/objectmodel/db/Node.java index 057696fd..1dd9b1cc 100644 --- a/src/helma/objectmodel/db/Node.java +++ b/src/helma/objectmodel/db/Node.java @@ -82,6 +82,15 @@ public final class Node implements INode, Serializable { created = lastmodified = System.currentTimeMillis(); } + /** + * Creates an empty, uninitialized Node with the given create and modify time. + * This is used for null-node references in the node cache. + * @param timestamp + */ + protected Node(long timestamp) { + created = lastmodified = timestamp; + } + /** * Creates a new Node with the given name. Used by NodeManager for creating "root nodes" * outside of a Transaction context, which is why we can immediately mark it as CLEAN. @@ -357,8 +366,8 @@ public final class Node implements INode, Serializable { * Called by the transactor on registered parent nodes to mark the * child index as changed */ - public void setLastSubnodeChange(long t) { - lastSubnodeChange = t; + public void markSubnodesChanged() { + lastSubnodeChange += 1; } /** @@ -1508,9 +1517,9 @@ public final class Node implements INode, Serializable { // If the subnodes are loaded aggressively, we really just // do a count statement, otherwise we just return the size of the id index. // (after loading it, if it's coming from a relational data source). - DbMapping smap = (dbmap == null) ? null : dbmap.getSubnodeMapping(); + DbMapping subMap = (dbmap == null) ? null : dbmap.getSubnodeMapping(); - if ((smap != null) && smap.isRelational()) { + if ((subMap != null) && subMap.isRelational()) { // check if subnodes need to be rechecked Relation subRel = dbmap.getSubnodeRelation(); @@ -1520,20 +1529,16 @@ public final class Node implements INode, Serializable { (((state != TRANSIENT) && (state != NEW)) || (subnodeRelation != null))) { // we don't want to load *all* nodes if we just want to count them - long lastChange = subRel.aggressiveCaching ? lastSubnodeChange - : smap.getLastDataChange(); + long lastChange = getLastSubnodeChange(subRel); - // also reload if the type mapping has changed. - lastChange = Math.max(lastChange, dbmap.getLastTypeChange()); - - if ((lastChange < lastSubnodeFetch) && (subnodes != null)) { + if ((lastChange == lastSubnodeFetch) && (subnodes != null)) { // we can use the nodes vector to determine number of subnodes subnodeCount = subnodes.size(); - lastSubnodeCount = System.currentTimeMillis(); - } else if ((lastChange >= lastSubnodeCount) || (subnodeCount < 0)) { + lastSubnodeCount = lastChange; + } else if ((lastChange != lastSubnodeCount) || (subnodeCount < 0)) { // count nodes in db without fetching anything subnodeCount = nmgr.countNodes(this, subRel); - lastSubnodeCount = System.currentTimeMillis(); + lastSubnodeCount = lastChange; } return subnodeCount; } @@ -1555,21 +1560,17 @@ public final class Node implements INode, Serializable { return; } - DbMapping smap = (dbmap == null) ? null : dbmap.getSubnodeMapping(); + DbMapping subMap = (dbmap == null) ? null : dbmap.getSubnodeMapping(); - if ((smap != null) && smap.isRelational()) { + if ((subMap != null) && subMap.isRelational()) { // check if subnodes need to be reloaded Relation subRel = dbmap.getSubnodeRelation(); synchronized (this) { - long lastChange = subRel.aggressiveCaching ? lastSubnodeChange - : smap.getLastDataChange(); - // also reload if the type mapping has changed. - lastChange = Math.max(lastChange, dbmap.getLastTypeChange()); + long lastChange = getLastSubnodeChange(subRel); - if ((lastChange >= lastSubnodeFetch && !subRel.autoSorted) || - (subnodes == null)) { + if ((lastChange != lastSubnodeFetch && !subRel.autoSorted) || (subnodes == null)) { if (subRel.updateCriteria!=null) { // updateSubnodeList is setting the subnodes directly returning an integer nmgr.updateSubnodeList(this, subRel); @@ -1579,7 +1580,7 @@ public final class Node implements INode, Serializable { subnodes = nmgr.getNodeIDs(this, subRel); } - lastSubnodeFetch = System.currentTimeMillis(); + lastSubnodeFetch = lastChange; } } } @@ -1602,6 +1603,18 @@ public final class Node implements INode, Serializable { return subnodes; } + /** + * Compute a serial number indicating the last change in subnode collection + * @param subRel the subnode relation + * @return a serial number that increases with each subnode change + */ + long getLastSubnodeChange(Relation subRel) { + // include dbmap.getLastTypeChange to also reload if the type mapping has changed. + return subRel.aggressiveCaching ? + lastSubnodeChange + dbmap.getLastTypeChange() : + subRel.otherType.getLastDataChange() + dbmap.getLastTypeChange(); + } + /** * * @@ -1701,8 +1714,8 @@ public final class Node implements INode, Serializable { Relation prel = (dbmap == null) ? null : dbmap.getSubnodeRelation(); - if ((prel != null) && prel.hasAccessName() && (prel.otherType != null) && - prel.otherType.isRelational()) { + if (state != TRANSIENT && prel != null && prel.hasAccessName() && + prel.otherType != null && prel.otherType.isRelational()) { // return names of objects from a relational db table return nmgr.getPropertyNames(this, prel).elements(); } else if (propMap != null) { @@ -2079,9 +2092,9 @@ public final class Node implements INode, Serializable { if (((oldStorage == null) && (newStorage == null)) || ((oldStorage != null) && oldStorage.equals(newStorage))) { - long now = System.currentTimeMillis(); - dbmap.setLastDataChange(now); - newmap.setLastDataChange(now); + // long now = System.currentTimeMillis(); + dbmap.setLastDataChange(); + newmap.setLastDataChange(); this.dbmap = newmap; this.prototype = value; } @@ -2699,14 +2712,14 @@ public final class Node implements INode, Serializable { * @return the number of loaded nodes within this collection update */ public int updateSubnodes () { - // FIXME: what do we do if this.dbmap is null - if (this.dbmap == null) { + // FIXME: what do we do if dbmap is null + if (dbmap == null) { throw new RuntimeException (this + " doesn't have a DbMapping"); } - Relation rel = this.dbmap.getSubnodeRelation(); + Relation subRel = dbmap.getSubnodeRelation(); synchronized (this) { - lastSubnodeFetch = System.currentTimeMillis(); - return this.nmgr.updateSubnodeList(this, rel); + lastSubnodeFetch = getLastSubnodeChange(subRel); + return nmgr.updateSubnodeList(this, subRel); } } diff --git a/src/helma/objectmodel/db/NodeManager.java b/src/helma/objectmodel/db/NodeManager.java index 8fbc540b..d0088c74 100644 --- a/src/helma/objectmodel/db/NodeManager.java +++ b/src/helma/objectmodel/db/NodeManager.java @@ -251,8 +251,7 @@ public final class NodeManager { if ((node != null) && (node.getState() != Node.INVALID)) { // check if node is null node (cached null) if (node.isNullNode()) { - if ((node.created < rel.otherType.getLastDataChange()) || - (node.created < rel.ownType.getLastTypeChange())) { + if (node.created != home.getLastSubnodeChange(rel)) { node = null; // cached null not valid anymore } } else if (!rel.virtual) { @@ -293,7 +292,7 @@ public final class NodeManager { } else { // node fetched from db is null, cache result using nullNode synchronized (cache) { - cache.put(key, new Node()); + cache.put(key, new Node(home.getLastSubnodeChange(rel))); // we ignore the case that onother thread has created the node in the meantime return null; @@ -669,7 +668,7 @@ public final class NodeManager { Node parent = node.getCachedParent(); if (parent != null) { - parent.setLastSubnodeChange(System.currentTimeMillis()); + parent.markSubnodesChanged(); } } @@ -1329,7 +1328,8 @@ public final class NodeManager { Node groupnode = home.getGroupbySubnode(groupname, true); groupnode.setSubnodes((SubnodeList) groupbySubnodes.get(groupname)); - groupnode.lastSubnodeFetch = System.currentTimeMillis(); + groupnode.lastSubnodeFetch = + groupnode.getLastSubnodeChange(groupnode.dbmap.getSubnodeRelation()); } } } catch (Exception x) { @@ -1977,14 +1977,14 @@ public final class NodeManager { } synchronized (cache) { - long now = System.currentTimeMillis(); + // long now = System.currentTimeMillis(); for (Enumeration en = add.elements(); en.hasMoreElements();) { Node n = (Node) en.nextElement(); DbMapping dbm = app.getDbMapping(n.getPrototype()); if (dbm != null) { - dbm.setLastDataChange(now); + dbm.setLastDataChange(); } n.setDbMapping(dbm); @@ -2009,7 +2009,7 @@ public final class NodeManager { DbMapping dbm = app.getDbMapping(n.getPrototype()); if (dbm != null) { - dbm.setLastDataChange(now); + dbm.setLastDataChange(); } n.setDbMapping(dbm); diff --git a/src/helma/objectmodel/db/Transactor.java b/src/helma/objectmodel/db/Transactor.java index 549e8e9c..e8b50cea 100644 --- a/src/helma/objectmodel/db/Transactor.java +++ b/src/helma/objectmodel/db/Transactor.java @@ -319,11 +319,11 @@ public class Transactor extends Thread { } // set last data change times in db-mappings - long now = System.currentTimeMillis(); + // long now = System.currentTimeMillis(); for (Iterator i = dirtyDbMappings.iterator(); i.hasNext(); ) { DbMapping dbm = (DbMapping) i.next(); if (dbm != null) { - dbm.setLastDataChange(now); + dbm.setLastDataChange(); } } } @@ -334,7 +334,7 @@ public class Transactor extends Thread { // set last subnode change times in parent nodes for (Iterator i = parentNodes.iterator(); i.hasNext(); ) { Node node = (Node) i.next(); - node.setLastSubnodeChange(now); + node.markSubnodesChanged(); if (hasListeners) { modifiedParentNodes.add(node); } @@ -385,7 +385,7 @@ public class Transactor extends Thread { // set last subnode change times in parent nodes for (Iterator i = parentNodes.iterator(); i.hasNext(); ) { Node node = (Node) i.next(); - node.setLastSubnodeChange(now); + node.markSubnodesChanged(); } // clear the node collections