CODEWITHSUNDEEP
javacaching
d.bb
d.bb

Reputation: 11

Reloadable cache

I need to store a lookup map in memory on a servlet. The map should be loaded from a file, and whenever the file is updated (which is not very often), the map should be reloaded in the same thread that is doing the lookup.

But I'm not sure how to implement this functionality in a thread safe manner. I want to make sure that the reload does not happen more than once.

public class LookupTable
{

    private File file;
    private long mapLastUpdate;
    private Map<String, String> map;

    public String getValue(String key)
    {
        long fileLastUpdate = file.lastModified();
        if (fileLastUpdate > mapLastUpdate)
        {
         // Only the first thread should run the code in the synchronized block.
         // The other threads will wait until it is finished. Then skip it.

            synchronized (this)
            {
                Map newMap = loadMap();
                this.map = newMap;
                this.mapLastUpdate = fileLastUpdate;
            }
        }

        return map.get(key);
    }

    private Map<String, String> loadMap()
    {
        // Load map from file.
        return null;
    }
}

If anyone has any suggestions on external libraries that has solved this already, that would work too. I took a quick look at some caching libraries, but I couldn't find what I needed.

Thanks

Upvotes: 1

Views: 1029

Answers (3)

yusufaytas
yusufaytas

Reputation: 1211

I would suggest you using imcache. Please a build a concurrent cache with cache loader as follows,

Cache<String,LookupTable> lookupTableCache = CacheBuilder.
   concurrentHeapCache().cacheLoader(new CacheLoader<String, LookupTable>() {
   public LookupTable load(String key) {
   //code to load item from file.
   }
}).build();

Upvotes: 3

overthink
overthink

Reputation: 24443

As suggested by z5h, you need to protect your condition (fileLastUpdate > mapsLastUpdate) by the same lock that is used to keep the file reloading atomic.

The way I think about this stuff is to look at all of the member variables in the class and figure out what thread-safety guarantees they need. In your case, none of the members (File, long, HashMap -- ok, I'm assuming HashMap) are thread safe, and thus they must all be protected by a lock. They're also all involved in an invariant (they all change together) together, so they must be protected by the SAME lock.

Your code, updated, and using the annotations (these are just info, they don't enforce anything!) suggested by Java Concurrency In Practice (an excellent book all Java devs should read :))

/**
* Lookup table that automatically reloads itself from a file
* when the filechanges.
*/
@ThreadSafe
public class LookupTable
{
    @GuardedBy("this")
    private long mapLastUpdate;
    @GuardedBy("this")
    private final File file;
    @GuardedBy("this")
    private Map<String, String> map;

    public LookupTable(File file)
    {
        this.file = file;
        this.map = loadMap()
    }

    public synchronized String getValue(String key)
    {
        long fileLastUpdate = file.lastModified();
        if (fileLastUpdate > this.mapLastUpdate)
        {
            // Only the first thread should run the code in the synchronized block.
            // The other threads will wait until it is finished. Then skip it.
            Map newMap = loadMap();
            this.map = newMap;
            this.mapLastUpdate = fileLastUpdate;
        }
        return map.get(key);
    }

    private synchronized Map<String, String> loadMap()
    {
        // Load map from file.
        return null;
    }
}

This will be safe, but it is fully synchronized: only one thread doing a lookup in the map at once. If you need concurrency on the lookups, you'll need a more sophisticated scheme. Implementation would depend on whether threads are allowed to see the old version of the lookup table while the new one is loading, among other things.

If you made the map member final, and protected it with a ReadWriteLock, you might get some bang. It's hard to predict how much contention you might have on this lock from the limited info here.

Upvotes: 2

Mark Bolusmjak
Mark Bolusmjak

Reputation: 24399

Your check needs to be in the synchronized block. Otherwise several threads could read (fileLastUpdate > mapLastUpdate) as true, then all block on the update code. Worst of both worlds.

Upvotes: 0

Related Questions