CODEWITHSUNDEEP
javamultithreadingthread-safety
arsenal
arsenal

Reputation: 24174

Thread Contention while adding the values in a ConcurrentHashMap

I am tring to count the number of exceptions happening in the multithreaded code. So for that what I did is, I created one method addException in which I am counting all the exceptions using the AtomicInteger.

addException method accepts two parameters, one is the String, and other is the boolean flag which means whether we want to terminate the program or not because of any exceptions. Meaning, if that flag is true, then I need to terminate the program whenever there are any exceptions.

So if you take a look into my below catch block, I have addException method call for counting the exceptions and below that method call I am logging the exceptions as well.

ExecutorService service = Executors.newFixedThreadPool(threads);

    for (int i = 0; i < threads; i++) {
            service.submit(new ReadTask());
    }

class ReadTask implements Runnable {

    public static ConcurrentHashMap<String, AtomicInteger> exceptionMap = new ConcurrentHashMap<String, AtomicInteger>();

public ReadTask() {

}

@Override
public run() {

    try {

        .........

    } catch (ClassNotFoundException e) {
        addException(e.getCause() != null ? e.getCause().toString() : e.toString(), Read.flagTerminate);
        LOG.error("Threw a ClassNotFoundException in " + getClass().getSimpleName(), e);
    } catch (SQLException e) {
        addException(e.getCause() != null ? e.getCause().toString() : e.toString(), Read.flagTerminate);
        LOG.error("Threw a SQLException while making connection to database in " + getClass().getSimpleName(), e);
    }
}

    /**
     * A simple method that will add the count of exceptions and name of
     * exception to a map
     * 
     * @param cause
     * @param flagTerminate 
     */
    private static void addException(String cause, boolean flagTerminate) {
        AtomicInteger count = exceptionMap.get(cause);
        if (count == null) {
            count = new AtomicInteger();
            AtomicInteger curCount = exceptionMap.putIfAbsent(cause, count);
            if (curCount != null) {
                count = curCount;
            }
        }
        count.incrementAndGet();

        if(flagTerminate) {
            System.exit(1);
        }
    }
}

Problem Statement:-

Is there any Thread Contention possible with this code? If yes, how can I write addException method in a better way to avoid Thread Contention?

Is there any more efficient way of writing addException method here?

Upvotes: 1

Views: 428

Answers (2)

Narendra Pathai
Narendra Pathai

Reputation: 41965

   //This check-then-act block can cause two threads can see count as null
    if (count == null) {
        count = new AtomicInteger();
        AtomicInteger curCount = exceptionMap.putIfAbsent(cause, count);
        if (curCount != null) {
            count = curCount;
        }
    }

Suggestion: 1) How about a Map<Class<?>,AtomicInteger> where Class would be class of the exception.

UPDATE: Try using ThreadLocals here each thread will have its own copy of map and will update in its own copy. So zero contention.

Upvotes: 0

Cameron Skinner
Cameron Skinner

Reputation: 54376

Your code looks logically correct, but there is still the possibility of thread contention.

Consider what happens if each thread throws the same exception: they will serialize on updating the AtomicInteger keeping track of the exception count.

There's really no easy way around this: if all threads are updating the same piece of data then they pretty much have to serialize. That's not a problem; that's just reality.

There are ways you could work around this but it will turn a simple, correct piece of code into a complex nightmare.

The question you should be asking is "Do I need to make this more efficient?" Chances are the answer is no since exceptions are, almost by definition, rare. The question of "How do I make it more efficient" should only be asked if the answer to the previous question is yes.

Upvotes: 2

Related Questions