[jboss-cvs] JBossAS SVN: r71191 - in projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache: spi and 1 other directory.
jboss-cvs-commits at lists.jboss.org
jboss-cvs-commits at lists.jboss.org
Mon Mar 24 01:19:45 EDT 2008
Author: bstansberry at jboss.com
Date: 2008-03-24 01:19:41 -0400 (Mon, 24 Mar 2008)
New Revision: 71191
Modified:
projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/impl/backing/SerializationGroupImpl.java
projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/impl/backing/SerializationGroupMemberContainer.java
projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/impl/backing/SerializationGroupMemberImpl.java
projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/spi/SerializationGroup.java
Log:
[EJBTHREE-1026] Coordination of "is modified" across the group
Modified: projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/impl/backing/SerializationGroupImpl.java
===================================================================
--- projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/impl/backing/SerializationGroupImpl.java 2008-03-24 04:20:07 UTC (rev 71190)
+++ projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/impl/backing/SerializationGroupImpl.java 2008-03-24 05:19:41 UTC (rev 71191)
@@ -24,6 +24,7 @@
import java.io.IOException;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
@@ -91,7 +92,7 @@
/** Is this object used in a clustered cache? */
private boolean clustered;
- private transient boolean invalid;
+ private transient boolean groupModified;
private transient ReentrantLock lock = new ReentrantLock();
@@ -186,11 +187,12 @@
*/
public void prePassivate()
{
- for(SerializationGroupMember<T> member : active.values())
+ for(Iterator<SerializationGroupMember<T>> it = active.values().iterator(); it.hasNext();)
{
+ SerializationGroupMember<T> member = it.next();
member.prePassivate();
+ it.remove();
}
- active.clear();
}
/**
@@ -198,7 +200,6 @@
*/
public void postActivate()
{
- invalid = false;
}
/**
@@ -206,11 +207,12 @@
*/
public void preReplicate()
{
- for(SerializationGroupMember<T> member : active.values())
+ for(Iterator<SerializationGroupMember<T>> it = active.values().iterator(); it.hasNext();)
{
+ SerializationGroupMember<T> member = it.next();
member.preReplicate();
+ it.remove();
}
- active.clear();
}
/**
@@ -219,7 +221,6 @@
*/
public void postReplicate()
{
- invalid = false;
}
/**
@@ -302,28 +303,21 @@
return inUseKeys.size();
}
- /**
- * Always returns <code>true</code>.
- */
public boolean isModified()
{
- return true;
+ boolean result = this.groupModified;
+ setGroupModified(false);
+ return result;
}
- /**
- * Returns true if this object has been passivated (meaning whoever
- * holds a ref to it is holding an out-of-date object)
- *
- * @return
- */
- public boolean isInvalid()
+ public boolean isGroupModified()
{
- return invalid;
+ return this.groupModified;
}
- public void setInvalid(boolean invalid)
+ public void setGroupModified(boolean modified)
{
- this.invalid = invalid;
+ this.groupModified = modified;
}
/**
Modified: projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/impl/backing/SerializationGroupMemberContainer.java
===================================================================
--- projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/impl/backing/SerializationGroupMemberContainer.java 2008-03-24 04:20:07 UTC (rev 71190)
+++ projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/impl/backing/SerializationGroupMemberContainer.java 2008-03-24 05:19:41 UTC (rev 71191)
@@ -98,45 +98,27 @@
{
log.trace("postActivate(): " + entry);
- boolean groupOK = false;
- while (!groupOK)
+ // Restore the entry's ref to the group and object
+ SerializationGroup<C> group = entry.getGroup();
+ if(group == null && entry.getGroupId() != null)
{
- // Restore the entry's ref to the group and object
- SerializationGroup<C> group = entry.getGroup();
- if(group == null && entry.getGroupId() != null)
+ group = groupCache.get(entry.getGroupId());
+ }
+
+ if(group != null)
+ {
+ group.lock();
+ try
{
- // TODO: peek or get?
- // BES 2007/10/06 I think peek is better; no
- // sense marking the group as in-use and then having
- // to release it or something
-// group = groupCache.peek(entry.getGroupId());
- group = groupCache.get(entry.getGroupId());
+ entry.setGroup(group);
+ entry.setUnderlyingItem(group.getMemberObject(entry.getId()));
+ // Notify the group that this entry is active
+ group.addActive(entry);
}
-
- if(group != null)
+ finally
{
- group.lock();
- try
- {
- if (!group.isInvalid())
- {
- entry.setGroup(group);
- entry.setUnderlyingItem(group.getMemberObject(entry.getId()));
- // Notify the group that this entry is active
- group.addActive(entry);
- groupOK = true;
- }
- // else groupOK == false and we loop again
- }
- finally
- {
- group.unlock();
- }
+ group.unlock();
}
- else
- {
- groupOK = true;
- }
}
// Invoke callbacks on the underlying object
@@ -172,42 +154,34 @@
throw new IllegalStateException("Cannot obtain lock on " + group.getId() + " to passivate " + entry);
try
{
- if (!group.isInvalid())
+ // Remove ourself from group's active list so we don't get
+ // called again via entry.prePassivate()
+ group.removeActive(entry.getId());
+
+ // Only tell the group to passivate if no members are in use
+ if (group.getInUseCount() == 0)
{
- // Remove ourself from group's active list so we don't get
- // called again via entry.prePassivate()
- group.removeActive(entry.getId());
-
- // Only tell the group to passivate if no members are in use
- if (group.getInUseCount() == 0)
+ // Go ahead and do the real passivation.
+ groupCache.passivate(group.getId());
+ }
+ else {
+ // this turns into a pretty meaningless exercise of just
+ // passivating an empty entry. TODO consider throwing
+ // ItemInUseException here, thus aborting everything. Need to
+ // be sure that doesn't lead to problems as the exception propagates
+ if (log.isTraceEnabled())
{
- // Go ahead and do the real passivation.
- groupCache.passivate(group.getId());
- // Mark the group as invalid so if another thread is
- // blocking on the above sync lock, when they enter
- // we realize they have a ref to an out-of-date group
- group.setInvalid(true);
- }
- else {
- // this turns into a pretty meaningless exercise of just
- // passivating an empty entry. TODO consider throwing
- // ItemInUseException here, thus aborting everything. Need to
- // be sure that doesn't lead to problems as the exception propagates
- if (log.isTraceEnabled())
- {
- log.trace("Group " + group.getId() + " has " +
- group.getInUseCount() + " in-use members; " +
- "not passivating group for " + entry.getId());
- }
+ log.trace("Group " + group.getId() + " has " +
+ group.getInUseCount() + " in-use members; " +
+ "not passivating group for " + entry.getId());
}
-
- // This call didn't come through entry.prePassivate() (which nulls
- // group and obj) so we have to do it ourselves. Otherwise
- // when this call returns, delegate may serialize the entry
- // with a ref to group and obj.
- entry.setGroup(null);
- entry.setUnderlyingItem(null);
}
+
+ // This call didn't come through entry.prePassivate() (which nulls
+ // group) so we have to do it ourselves. Otherwise
+ // when this call returns, delegate may serialize the entry
+ // with a ref to group and obj.
+ entry.setGroup(null);
}
finally
{
@@ -236,7 +210,7 @@
try
{
// Remove ourself from group's active list so we don't get
- // called again via entry.prePassivate()
+ // called again via entry.preReplicate()
group.removeActive(entry.getId());
try
@@ -244,6 +218,7 @@
if (group.getInUseCount() == 0)
{
group.getGroupCache().release(group.getId());
+ group.setGroupModified(false);
}
}
finally
@@ -254,8 +229,7 @@
group.addActive(entry);
}
-// entry.setGroup(null);
-// entry.setUnderlyingItem(null);
+ entry.setGroup(null);
}
finally
{
@@ -268,46 +242,28 @@
{
log.trace("postReplicate(): " + entry);
- boolean groupOK = false;
- while (!groupOK)
+ // Restore the entry's ref to the group and object
+ SerializationGroup<C> group = entry.getGroup();
+ if(group == null && entry.getGroupId() != null)
{
- // Restore the entry's ref to the group and object
- SerializationGroup<C> group = entry.getGroup();
- if(group == null && entry.getGroupId() != null)
+ group = groupCache.get(entry.getGroupId());
+ }
+
+ if(group != null)
+ {
+ group.lock();
+ try
{
- // TODO: peek or get?
- // BES 2007/10/06 peek is better; you'll get multiple calls to
- // this as different members are accessed, and the cache
- // will throw an ISE if we use get() since we're already in use
-// group = groupCache.peek(entry.getGroupId());
- group = groupCache.get(entry.getGroupId());
+ entry.setGroup(group);
+ entry.setUnderlyingItem(group.getMemberObject(entry.getId()));
+
+ // Notify the group that this entry is active
+ group.addActive(entry);
}
-
- if(group != null)
+ finally
{
- group.lock();
- try
- {
- if (!group.isInvalid())
- {
- entry.setGroup(group);
- entry.setUnderlyingItem(group.getMemberObject(entry.getId()));
-
- // Notify the group that this entry is active
- group.addActive(entry);
- groupOK = true;
- }
- // else groupOK == false and we loop again
- }
- finally
- {
- group.unlock();
- }
+ group.unlock();
}
- else
- {
- groupOK = true;
- }
}
// Invoke callbacks on the underlying object
Modified: projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/impl/backing/SerializationGroupMemberImpl.java
===================================================================
--- projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/impl/backing/SerializationGroupMemberImpl.java 2008-03-24 04:20:07 UTC (rev 71190)
+++ projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/impl/backing/SerializationGroupMemberImpl.java 2008-03-24 05:19:41 UTC (rev 71191)
@@ -61,15 +61,6 @@
private transient MarshalledObject marshalledObj;
- /**
- * Hack. We hold two refs to our object; one we clear in prePassivate,
- * one we keep, but it's transient. getUnderlyingItem() returns
- * whichever is available, making it available for passivation callbacks.
- *
- * FIXME WTF???
- */
- private transient T transientObj;
-
/** The group. Never serialize the group; only the groupCache does that */
private transient SerializationGroup<T> group;
@@ -89,12 +80,14 @@
/** The cache that's handling us */
private transient GroupAwareBackingCache<T, SerializationGroupMember<T>> cache;
+ private transient Object lockObject = new Object();
+
public SerializationGroupMemberImpl(T obj, GroupAwareBackingCache<T, SerializationGroupMember<T>> cache)
{
assert obj != null : "obj is null";
assert cache != null : "cache is null";
- this.obj = transientObj = obj;
+ this.obj = obj;
this.id = obj.getId();
this.cache = cache;
this.clustered = cache.isClustered();
@@ -108,7 +101,10 @@
public boolean isModified()
{
T unmarshalled = getObj();
- return (unmarshalled != null && unmarshalled.isModified());
+ boolean localModified = (unmarshalled != null && unmarshalled.isModified());
+ if (localModified && group != null)
+ group.setGroupModified(true);
+ return (group == null ? localModified : group.isGroupModified());
}
/**
@@ -125,8 +121,7 @@
@SuppressWarnings("unchecked")
public T getUnderlyingItem()
{
- T unmarshalled = getObj();
- return unmarshalled == null ? transientObj : unmarshalled;
+ return getObj();
}
/**
@@ -136,7 +131,9 @@
*/
public void setUnderlyingItem(T obj)
{
- this.obj = transientObj = obj;
+ assert obj != null : "obj is null";
+
+ this.obj = obj;
}
/**
@@ -146,30 +143,7 @@
*/
public SerializationGroup<T> getGroup()
{
- SerializationGroup<T> result = group;
- if (result != null)
- {
- boolean localGroupLock = false;
- if (!groupLockHeld)
- {
- group.lock();
- localGroupLock = groupLockHeld = true;
- }
- try
- {
- if (result.isInvalid())
- result = null;
- }
- finally
- {
- if (localGroupLock)
- {
- group.unlock();
- groupLockHeld = false;
- }
- }
- }
- return result;
+ return group;
}
/**
@@ -228,12 +202,6 @@
// use the setter to clear any group lock
setGroup(null);
- // null out obj so when delegate passivates this entry
- // we don't serialize it. It serializes with the PassivationGroup only
- // We still have a ref to transientObj, so it can be retrieved
- // for passivation callbacks
- obj = null;
-
cache.passivate(this.id);
}
finally
@@ -264,12 +232,6 @@
// use the setter to clear any group lock
setGroup(null);
- // null out obj so when delegate passivates this entry
- // we don't serialize it. It serializes with the PassivationGroup only
- // We still have a ref to transientObj, so it can be retrieved
- // for passivation callbacks
- obj = null;
-
cache.notifyPreReplicate(this);
}
finally
@@ -288,7 +250,6 @@
*/
public void postActivate()
{
- // no-op
}
public boolean isPreReplicated()
@@ -307,7 +268,6 @@
*/
public void postReplicate()
{
- // no-op
}
public void setInUse(boolean inUse)
@@ -457,7 +417,7 @@
@SuppressWarnings("unchecked")
private T getObj()
{
- synchronized (id)
+ synchronized (lockObject)
{
if (obj == null && marshalledObj != null)
{
@@ -480,6 +440,7 @@
{
in.defaultReadObject();
lock = new ReentrantLock();
+ lockObject = new Object();
if (groupId == null)
{
marshalledObj = (MarshalledObject) in.readObject();
@@ -491,7 +452,6 @@
if (groupId != null)
{
setGroup(null);
- obj = null;
}
out.defaultWriteObject();
if (groupId == null)
Modified: projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/spi/SerializationGroup.java
===================================================================
--- projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/spi/SerializationGroup.java 2008-03-24 04:20:07 UTC (rev 71190)
+++ projects/ejb3/branches/cluster-dev/ejb3-cache/src/main/java/org/jboss/ejb3/cache/spi/SerializationGroup.java 2008-03-24 05:19:41 UTC (rev 71191)
@@ -61,20 +61,6 @@
int size();
/**
- * Gets whether the group should be considered invalid. A reference
- * to an invalid group should be replaced with a fresh reference gotten
- * from the group's cache.
- *
- * @return
- */
- boolean isInvalid();
-
- /**
- * Marks the group as invalid (or once again as valid).
- */
- void setInvalid(boolean invalid);
-
- /**
* Gets the {@link BackingCacheEntry#getUnderlyingItem() underlying item}
* whose {@link SerializationGroupMember#getId() id} matches <code>key</code>.
*
@@ -124,8 +110,8 @@
/**
* Gets the cache used to manage the group.
*
- * @return the cache. Will not return <code>null</code> if the group is
- * {@link #isInvalid() valid}.
+ * @return the cache. May return <code>null</code> if the group has
+ * been serialized and not yet activated.
*/
PassivatingBackingCache<T, SerializationGroup<T>> getGroupCache();
@@ -137,6 +123,23 @@
void setGroupCache(PassivatingBackingCache<T, SerializationGroup<T>> groupCache);
/**
+ * Returns whether the group has been modified. Differs from
+ * {@link CacheItem#isModified()} in that invoking this method does not
+ * clear the modified state.
+ *
+ * {@inheritDoc}
+ */
+ boolean isGroupModified();
+
+ /**
+ * Sets the modified state.
+ *
+ * @param modified <code>true</code> if the group should be considered
+ * to have been modified, <code>false</code> if not
+ */
+ void setGroupModified(boolean modified);
+
+ /**
* Callback that must be invoked before the group is replicated.
*/
void preReplicate();
More information about the jboss-cvs-commits
mailing list