* 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
This commit is contained in:
hns 2007-05-24 14:10:53 +00:00
parent 13365e16df
commit a20913ab7f
4 changed files with 65 additions and 52 deletions

View file

@ -112,7 +112,7 @@ public final class DbMapping {
long lastTypeChange = -1; long lastTypeChange = -1;
// timestamp of last modification of an object of this type // timestamp of last modification of an object of this type
long lastDataChange; long lastDataChange = 0;
// evict objects of this type when received via replication // evict objects of this type when received via replication
private boolean evictOnReplication; private boolean evictOnReplication;
@ -1251,18 +1251,18 @@ public final class DbMapping {
* Set the last time something changed in the data, propagating the event * Set the last time something changed in the data, propagating the event
* to mappings that depend on us through an additionalTables switch. * 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 // forward data change timestamp to storage-compatible parent mapping
if (inheritsStorage()) { if (inheritsStorage()) {
parentMapping.setLastDataChange(t); parentMapping.setLastDataChange();
} else { } else {
lastDataChange = t; lastDataChange += 1;
// propagate data change timestamp to mappings that depend on us // propagate data change timestamp to mappings that depend on us
if (!dependentMappings.isEmpty()) { if (!dependentMappings.isEmpty()) {
Iterator it = dependentMappings.iterator(); Iterator it = dependentMappings.iterator();
while(it.hasNext()) { while(it.hasNext()) {
DbMapping dbmap = (DbMapping) it.next(); 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 * 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. * 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 // forward data change timestamp to storage-compatible parent mapping
if (inheritsStorage()) { if (inheritsStorage()) {
parentMapping.setIndirectDataChange(t); parentMapping.setIndirectDataChange();
} else { } else {
lastDataChange = t; lastDataChange += 1;
} }
} }

View file

@ -82,6 +82,15 @@ public final class Node implements INode, Serializable {
created = lastmodified = System.currentTimeMillis(); 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" * 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. * 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 * Called by the transactor on registered parent nodes to mark the
* child index as changed * child index as changed
*/ */
public void setLastSubnodeChange(long t) { public void markSubnodesChanged() {
lastSubnodeChange = t; lastSubnodeChange += 1;
} }
/** /**
@ -1508,9 +1517,9 @@ public final class Node implements INode, Serializable {
// If the subnodes are loaded aggressively, we really just // If the subnodes are loaded aggressively, we really just
// do a count statement, otherwise we just return the size of the id index. // 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). // (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 // check if subnodes need to be rechecked
Relation subRel = dbmap.getSubnodeRelation(); Relation subRel = dbmap.getSubnodeRelation();
@ -1520,20 +1529,16 @@ public final class Node implements INode, Serializable {
(((state != TRANSIENT) && (state != NEW)) || (((state != TRANSIENT) && (state != NEW)) ||
(subnodeRelation != null))) { (subnodeRelation != null))) {
// we don't want to load *all* nodes if we just want to count them // we don't want to load *all* nodes if we just want to count them
long lastChange = subRel.aggressiveCaching ? lastSubnodeChange long lastChange = getLastSubnodeChange(subRel);
: smap.getLastDataChange();
// also reload if the type mapping has changed. if ((lastChange == lastSubnodeFetch) && (subnodes != null)) {
lastChange = Math.max(lastChange, dbmap.getLastTypeChange());
if ((lastChange < lastSubnodeFetch) && (subnodes != null)) {
// we can use the nodes vector to determine number of subnodes // we can use the nodes vector to determine number of subnodes
subnodeCount = subnodes.size(); subnodeCount = subnodes.size();
lastSubnodeCount = System.currentTimeMillis(); lastSubnodeCount = lastChange;
} else if ((lastChange >= lastSubnodeCount) || (subnodeCount < 0)) { } else if ((lastChange != lastSubnodeCount) || (subnodeCount < 0)) {
// count nodes in db without fetching anything // count nodes in db without fetching anything
subnodeCount = nmgr.countNodes(this, subRel); subnodeCount = nmgr.countNodes(this, subRel);
lastSubnodeCount = System.currentTimeMillis(); lastSubnodeCount = lastChange;
} }
return subnodeCount; return subnodeCount;
} }
@ -1555,21 +1560,17 @@ public final class Node implements INode, Serializable {
return; 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 // check if subnodes need to be reloaded
Relation subRel = dbmap.getSubnodeRelation(); Relation subRel = dbmap.getSubnodeRelation();
synchronized (this) { synchronized (this) {
long lastChange = subRel.aggressiveCaching ? lastSubnodeChange
: smap.getLastDataChange();
// also reload if the type mapping has changed. // also reload if the type mapping has changed.
lastChange = Math.max(lastChange, dbmap.getLastTypeChange()); long lastChange = getLastSubnodeChange(subRel);
if ((lastChange >= lastSubnodeFetch && !subRel.autoSorted) || if ((lastChange != lastSubnodeFetch && !subRel.autoSorted) || (subnodes == null)) {
(subnodes == null)) {
if (subRel.updateCriteria!=null) { if (subRel.updateCriteria!=null) {
// updateSubnodeList is setting the subnodes directly returning an integer // updateSubnodeList is setting the subnodes directly returning an integer
nmgr.updateSubnodeList(this, subRel); nmgr.updateSubnodeList(this, subRel);
@ -1579,7 +1580,7 @@ public final class Node implements INode, Serializable {
subnodes = nmgr.getNodeIDs(this, subRel); subnodes = nmgr.getNodeIDs(this, subRel);
} }
lastSubnodeFetch = System.currentTimeMillis(); lastSubnodeFetch = lastChange;
} }
} }
} }
@ -1602,6 +1603,18 @@ public final class Node implements INode, Serializable {
return subnodes; 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(); Relation prel = (dbmap == null) ? null : dbmap.getSubnodeRelation();
if ((prel != null) && prel.hasAccessName() && (prel.otherType != null) && if (state != TRANSIENT && prel != null && prel.hasAccessName() &&
prel.otherType.isRelational()) { prel.otherType != null && prel.otherType.isRelational()) {
// return names of objects from a relational db table // return names of objects from a relational db table
return nmgr.getPropertyNames(this, prel).elements(); return nmgr.getPropertyNames(this, prel).elements();
} else if (propMap != null) { } else if (propMap != null) {
@ -2079,9 +2092,9 @@ public final class Node implements INode, Serializable {
if (((oldStorage == null) && (newStorage == null)) || if (((oldStorage == null) && (newStorage == null)) ||
((oldStorage != null) && oldStorage.equals(newStorage))) { ((oldStorage != null) && oldStorage.equals(newStorage))) {
long now = System.currentTimeMillis(); // long now = System.currentTimeMillis();
dbmap.setLastDataChange(now); dbmap.setLastDataChange();
newmap.setLastDataChange(now); newmap.setLastDataChange();
this.dbmap = newmap; this.dbmap = newmap;
this.prototype = value; this.prototype = value;
} }
@ -2699,14 +2712,14 @@ public final class Node implements INode, Serializable {
* @return the number of loaded nodes within this collection update * @return the number of loaded nodes within this collection update
*/ */
public int updateSubnodes () { public int updateSubnodes () {
// FIXME: what do we do if this.dbmap is null // FIXME: what do we do if dbmap is null
if (this.dbmap == null) { if (dbmap == null) {
throw new RuntimeException (this + " doesn't have a DbMapping"); throw new RuntimeException (this + " doesn't have a DbMapping");
} }
Relation rel = this.dbmap.getSubnodeRelation(); Relation subRel = dbmap.getSubnodeRelation();
synchronized (this) { synchronized (this) {
lastSubnodeFetch = System.currentTimeMillis(); lastSubnodeFetch = getLastSubnodeChange(subRel);
return this.nmgr.updateSubnodeList(this, rel); return nmgr.updateSubnodeList(this, subRel);
} }
} }

View file

@ -251,8 +251,7 @@ public final class NodeManager {
if ((node != null) && (node.getState() != Node.INVALID)) { if ((node != null) && (node.getState() != Node.INVALID)) {
// check if node is null node (cached null) // check if node is null node (cached null)
if (node.isNullNode()) { if (node.isNullNode()) {
if ((node.created < rel.otherType.getLastDataChange()) || if (node.created != home.getLastSubnodeChange(rel)) {
(node.created < rel.ownType.getLastTypeChange())) {
node = null; // cached null not valid anymore node = null; // cached null not valid anymore
} }
} else if (!rel.virtual) { } else if (!rel.virtual) {
@ -293,7 +292,7 @@ public final class NodeManager {
} else { } else {
// node fetched from db is null, cache result using nullNode // node fetched from db is null, cache result using nullNode
synchronized (cache) { 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 // we ignore the case that onother thread has created the node in the meantime
return null; return null;
@ -669,7 +668,7 @@ public final class NodeManager {
Node parent = node.getCachedParent(); Node parent = node.getCachedParent();
if (parent != null) { if (parent != null) {
parent.setLastSubnodeChange(System.currentTimeMillis()); parent.markSubnodesChanged();
} }
} }
@ -1329,7 +1328,8 @@ public final class NodeManager {
Node groupnode = home.getGroupbySubnode(groupname, true); Node groupnode = home.getGroupbySubnode(groupname, true);
groupnode.setSubnodes((SubnodeList) groupbySubnodes.get(groupname)); groupnode.setSubnodes((SubnodeList) groupbySubnodes.get(groupname));
groupnode.lastSubnodeFetch = System.currentTimeMillis(); groupnode.lastSubnodeFetch =
groupnode.getLastSubnodeChange(groupnode.dbmap.getSubnodeRelation());
} }
} }
} catch (Exception x) { } catch (Exception x) {
@ -1977,14 +1977,14 @@ public final class NodeManager {
} }
synchronized (cache) { synchronized (cache) {
long now = System.currentTimeMillis(); // long now = System.currentTimeMillis();
for (Enumeration en = add.elements(); en.hasMoreElements();) { for (Enumeration en = add.elements(); en.hasMoreElements();) {
Node n = (Node) en.nextElement(); Node n = (Node) en.nextElement();
DbMapping dbm = app.getDbMapping(n.getPrototype()); DbMapping dbm = app.getDbMapping(n.getPrototype());
if (dbm != null) { if (dbm != null) {
dbm.setLastDataChange(now); dbm.setLastDataChange();
} }
n.setDbMapping(dbm); n.setDbMapping(dbm);
@ -2009,7 +2009,7 @@ public final class NodeManager {
DbMapping dbm = app.getDbMapping(n.getPrototype()); DbMapping dbm = app.getDbMapping(n.getPrototype());
if (dbm != null) { if (dbm != null) {
dbm.setLastDataChange(now); dbm.setLastDataChange();
} }
n.setDbMapping(dbm); n.setDbMapping(dbm);

View file

@ -319,11 +319,11 @@ public class Transactor extends Thread {
} }
// set last data change times in db-mappings // set last data change times in db-mappings
long now = System.currentTimeMillis(); // long now = System.currentTimeMillis();
for (Iterator i = dirtyDbMappings.iterator(); i.hasNext(); ) { for (Iterator i = dirtyDbMappings.iterator(); i.hasNext(); ) {
DbMapping dbm = (DbMapping) i.next(); DbMapping dbm = (DbMapping) i.next();
if (dbm != null) { 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 // set last subnode change times in parent nodes
for (Iterator i = parentNodes.iterator(); i.hasNext(); ) { for (Iterator i = parentNodes.iterator(); i.hasNext(); ) {
Node node = (Node) i.next(); Node node = (Node) i.next();
node.setLastSubnodeChange(now); node.markSubnodesChanged();
if (hasListeners) { if (hasListeners) {
modifiedParentNodes.add(node); modifiedParentNodes.add(node);
} }
@ -385,7 +385,7 @@ public class Transactor extends Thread {
// set last subnode change times in parent nodes // set last subnode change times in parent nodes
for (Iterator i = parentNodes.iterator(); i.hasNext(); ) { for (Iterator i = parentNodes.iterator(); i.hasNext(); ) {
Node node = (Node) i.next(); Node node = (Node) i.next();
node.setLastSubnodeChange(now); node.markSubnodesChanged();
} }
// clear the node collections // clear the node collections