将值放入 ConcurrentHashMap 是否是原子的？

Question

我正在做一个与数据库建立连接的项目。如果有的话，我需要查看exception is happening 的次数。我正在使用Multithreaded code，这意味着多个线程将连接到数据库并插入数据库。因此，在某些时候连接可能会丢失，因此我们需要查看这些异常发生了多少次。

所以我编写了下面的代码，在 catch 块中，我正在捕获异常并在每次出现异常时增加一个计数器并将其放入 ConcurrentHashMap。

class Task implements Runnable {

     public static final AtomicInteger counter_sql_exception = new AtomicInteger(0);
     public static final AtomicInteger counter_exception = new AtomicInteger(0);
     public static ConcurrentHashMap<String, Integer> exceptionMap = new ConcurrentHashMap<String, Integer>();

     @Override
     public void run() {

     try {

         //Make a db connection and then executing the SQL-

         } catch (SQLException e) {
              synchronized(this) {
                   exceptionMap.put(e.getCause().toString(), counter_sql_exception.incrementAndGet());
              }
              LOG.Error("Log Exception")
          } catch (Exception e) {
              synchronized(this) {
                   exceptionMap.put(e.getCause().toString(), counter_exception.incrementAndGet());
              }
              LOG.Error("Log Exception")
        }
      }
  }

我的问题是- 今天我进行了一次代码审查，我的一位高级团队成员说，catch block 中的exceptionMap 上的synchronized(this) 是不需要的。我说是的，我们将需要，因为递增计数器是原子的。在映射中添加新值是原子的。但是在没有同步的情况下两者都不是原子的。他说ConurrentHashMap会为你做这件事。

那么我是否需要synchronized(this) 阻止exceptionMap。如果不是那为什么？如果是，那么我应该向他引用什么理由。

Answer 1

如果你想计算每个异常发生的次数，那么你需要这样的东西：

private static final ConcurrentMap<String, AtomicInteger> exceptionMap = new ConcurrentHashMap<String, AtomicInteger>();

private static void addException(String cause) {
  AtomicInteger count = exceptionMap.get(cause);
  if(count == null) {
    count = new AtomicInteger();
    AtomicInteger curCount = exception.putIfAbsent(cause, count);
    if(curCount != null) {
      count = curCount;
    }
  }
  count.incrementAndGet();
}

请注意，除非您定期清理它，否则拥有异常的静态映射是一种资源泄漏。

正如@dnault 所说，您也可以使用guava's AtomicLongMap。

更新：您原件上的一些 cmets：

• 您是对的，您确实需要另一个包装同步块来确保最新值实际进入地图。但是，正如 @Perception 在 cmets 中已经指出的那样，您正在同步 错误的对象实例（因为您正在更新静态地图，所以您需要一个静态实例，例如 Task.class）
• 但是，您使用的是静态计数器，但字符串键可能因不同的异常而不同，因此您实际上并没有计算每个异常原因，而是将随机数作为各种映射值插入
• 最后，正如我在示例中所展示的，您可以通过适当使用 ConcurrentMap 来解决上述问题并完全丢弃同步块。

Answer 2

而且这种方式也应该有效。

private static final ConcurrentMap<String, Integer> exceptionMap = new ConcurrentHashMap<String, Integer>();

private static void addException(String cause) {
    Integer oldVal, newVal;
    do {
      oldVal = exceptionMap .get(cause);
      newVal = (oldVal == null) ? 1 : (oldVal + 1);
    } while (!queryCounts.replace(q, oldVal, newVal)); 
}

Answer 3

ConcurrentHashMap 不允许空值，所以如果用oldValue == null 调用replace 将抛出异常。我使用此代码将计数器增加delta，并返回oldValue。

private final ConcurrentMap<Integer,Integer> counters = new ConcurrentHashMap<Integer,Integer>();

private Integer counterAddDeltaAndGet(Integer key, Integer delta) {
    Integer oldValue = counters.putIfAbsent(key, delta);
    if(oldValue == null) return null;

    while(!counters.replace(key, oldValue, oldValue + delta)) {
        oldValue = counters.get(key);
    }

    return oldValue;
}

Answer 4

您不必使用synchronized 块和AtomicInteger。你可以使用ConcurrentHashMap.compute 方法来做到这一点，这是一个线程安全的原子操作。所以你的代码看起来像这样

public class Task implements Runnable {

    public static final Map<String, Integer> EXCEPTION_MAP = new ConcurrentHashMap<String, Integer>();

    @Override
    public void run() {
        try {

            // Make a db connection and then executing the SQL-

        } catch (Exception e) {
            EXCEPTION_MAP.compute(e.getCause().toString(), (key, value) -> {
                if (value == null) {
                    return 1;
                }
                return ++value;
            });
        }

    }

}