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
[EMAIL PROTECTED]
_______________________________________________
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Reply via email to