I have a cache that gets loaded upfront with a large amount of data (by a background thread) and is unusable until full (it will also get reloaded every so often and be unusable during that load). I want the classes that use it to check a flag isLoaded()
before accesses. I use a ReentrantReadWriteLock (I omit this in the code for simplicity) for access control like this:
public class Cache {
private volatile boolean loaded = false; //starts false
private static String[] cache;
private static Lock readLock;
private static Lock writeLock;
public Object get(Object key) {
if (!readLock.t开发者_如何学GoryLock()) throw IllegalStateException(...);
try {
... do some work
} finally {
readLock.unlock();
}
}
// called by background thread
private void loadFull() {
loaded = false;
writeLock.lock()
try {
cache = new String[];
... fill cache
} finally {
writeLock.unlock();
loaded = true;
}
}
....
}
Now in my other class I have a block like this:
if (cache.isLoaded()) {
try {
Object x = cache.get(y);
} catch (IllegalStateException iex) {
// goto database for object
}
} else {
// goto database for object
}
Do I really need the try/catch
? Is it ever possible that the flag will be set to false and the readLock try() will fail? Should I even bother with the flag and jut catch the Exception (since I basically do the same code if the Exception is thrown as if the flag is false). I just feel like I am doing something slightly wrong but I can't put my finger on it. Thanks.
Do I really need the try/catch? Is it ever possible that the flag will be set to false and the readLock try() will fail?
Yes, you need it. Between the time cache.isLoaded()
and cache.get()
are called, a writer can come in and get the write lock - in which case cache.isLoaded()
will return true
, but cache.get()
will throw the exception.
Should I even bother with the flag and jut catch the Exception (since I basically do the same code if the Exception is thrown as if the flag is false).
From the code you have shown, the exception is thrown only in cases where the get
fails to acquire the read lock. Acquisition of the read lock fails only if there is a concurrent writer at the time. isLoaded
also returns false in precisely this scenario. So just relying on the exception would suffice. Also, consider creating a specialized CacheStaleException
.
The tryLock
will fail if some other thread has already acquired that lock. This typically means that an exception would be thrown if a client fails to acquire a lock due to high contention (multiple clients accessing the same cache). Is there any fallback strategy you have implemented in your client layer which deals with such situations?
Also, why static
locks? I think that even though your cache is typically used in the application as a singleton, there is no need to limit its usability by making Locks static.
No, but to be honest your paradigm is confusing. Presumably it is expensive to go to the actual database and that is the purpose of the cache. In the case that the cache is being reloaded, is it not better to just wait until it is?
Assuming you really do want to go to the database if the read lock is not immediately available, I would do this:
public Object get(Object key) {
Object returnValue;
if (readLock.tryLock()) {
try {
... do some work
returnValue = ...
} finally {
readLock.unlock();
}
} else {
//go to database
returnValue = ...
}
return returnValue;
}
精彩评论