Wanted to raise a point about about timestamps cache handling in case
there's any desire to change the UpdateTimestampsCache API in 3.3.
AIUI, a goal of UpdateTimestampsCache is to ensure the cached timestamp
never moves backward in time *except* when a caller that has set the
timestamp to a far-in-the-future value in preInvalidate() later comes
back and calls invalidate(), passing the current time.
There's a race in UpdateTimestampsCache where this could break under
concurrent load. For example, you could see:
(now = 0)
tx1 : preInvalidate(60);
(now = 1)
tx2 : preInvalidate(61);
tx1 : cache queryA w/ timestamp 1
tx1 : invalidate(1)
tx2 : update entity in a way that would query A results
tx2 : read queryA; check timestamp; 1 == 1 so passes. Wrong!
To deal with this, there are some comments in UpdateTimestampsCache
about having preInvalidate() return some sort of Lock object, which
would then be returned as a param to invalidate(). Idea here is to
ensure that only the caller that most recently called preInvalidate is
allowed to call invalidate.
That could work if the backing TimestampsRegion isn't clustered, but it
doesn't address the fact that a clustered TimestampsRegion can be
getting updates not only via the local UpdateTimestampsCache, but also
asynchronously over the network. If a clustered TimestampsRegion gets a
replicated update that moves the timestamp back in time, it has no
simple way to know if this is because 1) a peer that earlier replicated
a high preinvalidate value is now replicating a normal invalidate
value or 2) an earlier change from peer A has arrived *after* a later
change from peer B.
This could be addressed with a change to the TimestampsRegion API.
Basically replace
public void put(Object key, Object value) throws CacheException;
with
public void preInvalidate(Object key, Object value) throws CacheException;
public void invalidate(Object key, Object value, Object
preInvalidateValue) throws CacheException;
Basically the value that is passed to preInvalidate is also passed as a
2nd param to invalidate. This gives the TimestampsRegion the
information it needs to properly track preinvalidations vs invalidations.
The UpdateTimestampsCache API is then changed to provide the caller with
the timestamp in preInvalidate() and take it back in invalidate():
public synchronized Object preinvalidate(Serializable[] spaces) throws
CacheException {
Long ts = new Long( region.nextTimestamp() + region.getTimeout() );
for ( int i=0; i<spaces.length; i++ ) {
region.preInvalidate( spaces[i], ts );
}
return ts;
}
public synchronized void invalidate(Serializable[] spaces, Object
preInvalidateValue) throws CacheException {
Long ts = new Long( region.nextTimestamp() );
for ( int i=0; i<spaces.length; i++ ) {
region.invalidate( spaces[i], ts, preInvalidateValue );
}
}
This is basically similar to the Lock concept in the
UpdateTimestampsCache comments; but the control over the update is
delegated to the TimestampsRegion.
The issue here is the UpdateTimestampsCache caller needs to hold onto
the value returned by preInvalidate() and then pass it back. Likely
requires a change to Executable to provide a holder for it.
A change to the TimestampsRegion API has no benefit without a
corresponding change in UpdateTimestampsCache and its caller.
--
Brian Stansberry
Lead, AS Clustering
JBoss, a division of Red Hat
brian.stansberry(a)redhat.com
Show replies by date