My application needs to keep an access log of requests to a certain resource and multiple threads will be recording log entries. The only pertinent piece of information is the timestamp of the request and the stats being retrieved will be how many requests occurred in the last X seconds. The method that returns the stats for a given number of seconds also needs to support multiple threads.
I was thinking of approaching the concurrency handling using the Locks framework, with which I am not the most familiar, hence this question. Here is my code:
import java.util.LinkedList;
import java.util.concurrent.locks.ReentrantLock;
public class ConcurrentRecordStats
{
private LinkedList recLog;
private final ReentrantLock lock = new ReentrantLock();
public LinkedConcurrentStats()
{
this.recLog = new LinkedList();
}
//this method will be utilized by multiple clients concurrently
public void addRecord(int wrkrID)
{
long crntTS = System.currentTimeMillis();
this.lock.lock();
this.recLog.addFirst(crntTS);
this.lock.unlock();
}
//this method will be utilized by multiple clients concurrently
public int getTrailingStats(int lastSecs)
{
long endTS = System.currentTimeMillis();
long bgnTS = endTS - (lastSecs * 1000);
int rslt = 0;
//acquire the lock only until we have read
//the first (latest) element in the list
this.lock.lock();
for(long crntRec : this.recLog)
{
//release the lock upon fetching the first element in the list
if(this.lock.isLocked())
{
this.lock.unlock();
}
if(crntRec > bgnTS)
{
rslt++;
}
else
{
break;
}
}
return rslt;
}
}
My questions are:
Will this use of ReentrantLock insure thread safety?
Is it needed to use a lock in getTrailingStats?
Can I do all this using synchronized blocks? The reason I went with locks is because I wanted to have the same lock in both R and W sections so that both writes and reading of the first element in the list (most recently added entry) is done a single thread at a time and I couldn't do that with just synchronized.
Should I use the ReentrantReadWriteLock instead?
解决方案
The locks can present a major performance bottleneck. An alternative is to use a ConcurrentLinkedDeque: use offerFirst to add a new element, and use the (weakly consistent) iterator (that won't throw a ConcurrentModificationException) in place of your for-each loop. The advantage is that this will perform much better than your implementation or than the synchronizedList implementation, but the disadvantage is that the iterator is weakly consistent - thread1 might add elements to the list while thread2 is iterating through it, which means that thread2 won't count those new elements. However, this is functionally equivalent to having thread2 lock the list so that thread1 can't add to it - either way thread2 isn't counting the new elements.