Reloadable cache

Question

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

Answer 1

If the file is actually in properties file format (# lines as comments and key=value lines as key/value pairs), consider using java.util.ResourceBundle. Its default ResourceBundle.Control implementation will reload the file automatically at certain intervals. You can even override it with a custom implementation.

Good luck.

Answer 2

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.

Answer 3

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.

Answer 4

Thanks for all the answers, I'll check out that concurrency book.

I took all your comments into consideration and I decided to move the update functionality into a different method, and then call that method from a separate background process( that runs every minute or something ). So only one thread at a time will be executing the update method. And since updates to object references are atomic there shouldn't be any problems with just changing the reference. But let me know if you see any possible problems.

Lets see if it the code pasting works this time..

public String getValue(String key)
{
return map.get(key);
}

public void update()
{
long fileLastUpdate = sourceFile.lastModified();
if (fileLastUpdate > mapLastModified)
{
    this.map = loadMap();
    this.mapLastModified = fileLastUpdate;
}
}