Caching large object in multithreading environment I'm having to dabble with caching and multithreading (thread per request), and I am an absolute beginner in that area, so any help would be appreciated.

My project requirements are:


Cache one single large object that has ether interval refresh or refresh from user
Because retrieving object data is very time consuming make it thread-safe
When retrieving object data return "old data" until new data is available
Optimize it




public enum DbCachedObject {

INSTANCE;

private final CountDownLatch initLock = new CountDownLatch(1);
private final Object refreshLock = new Object();
private final AtomicReference&lt;CachedObject&gt; cachedInstance = new AtomicReference&lt;CachedObject&gt;();
private final AtomicLong lastUpdate = new AtomicLong();
private volatile boolean refreshing;
private long cachePeriodMs = 1000L;

public CachedObject get() {

    CachedObject o = cachedInstance.get();
    if (o == null || isCacheOutdated()) {
        updateCache();
        if (o == null) {
            awaitInit();
            o = cachedInstance.get();
        }
    }
    return o;
}

public void refresh() {
    updateCache();
}

private boolean isCacheOutdated() {
    return (System.currentTimeMillis() - lastUpdate.get() &gt; cachePeriodMs);
}

private void updateCache() {

    synchronized (refreshLock) {
        // prevent users from refreshing while an update is already in progress
        if (refreshing) {
            return;
        }
        refreshing = true;
        // prevent other threads from calling this method again
        lastUpdate.set(System.currentTimeMillis());
    }
    new Thread() {
        @Override 
        public void run() {
            try {
                cachedInstance.set(getFromDb());
                // set the 'real' last update time
                lastUpdate.set(System.currentTimeMillis());
                initLock.countDown();
            } finally {
                // make sure refreshing is set to false, even in case of error
                refreshing = false;
            }
        }
    }.start();
}

private boolean awaitInit() {

    boolean initialized = false;
    try {
        // assume cache-period is longer as the time it takes to create the cached object 
        initialized = initLock.await(cachePeriodMs, TimeUnit.MILLISECONDS);
    } catch (Exception e) {
        e.printStackTrace();
    }
    return initialized;
}

private CachedObject getFromDb() {
    // dummy call, no db is involved
    return new CachedObject();
}

public long getCachePeriodMs() {
    return cachePeriodMs;
}
}
I surely dislike your mixing of various mean synchronization. Using synchronized and Atomic* and also volatile together looks very strange. However, it looks like you got it right (a deeper analysis would be needed).

private volatile boolean refreshing;


You need no volatile if you access it inside the synchronized block only.

private long cachePeriodMs = 1000L;


I'd suggest cachePeriodMillis. It's only slightly longer and there's a precedent.



I guess, I'd drop the latch, define a non-anonymous Runnable for getting from the DB and let the first interested thread execute it directly. The others would wait on the corresponding synchronized block's entry (and then leave immediately).

I'd probably also aggregate CachedObject and lastUpdate into one object as they belong together. This might be a problem w.r.t "prevent other threads from calling this method again", but it should be possible to eliminate the need for this.

This way I might get a simpler design which I could trust more.