Потокобезопасная реализация кэша - PullRequest
0 голосов
/ 24 февраля 2020

Я обнаружил следующий код в одном из наших приложений, и он, кажется, имеет много потоков, касающихся многопоточного доступа.

public class DCache {

    private enum MAP_TYPE {
        GREEN, RED, //some more 
    }

    private static final int MAP_LIFE_TIME = 10 * 60 * 1000;

    private Map<MAP_TYPE, Long> accesstimes = new HashMap<>();
    private Map<MAP_TYPE, Map<String, String>> dics = new HashMap<>();

    private final static Object greenLock = new Object();
    private final static Object redLock   = new Object();
    // some more

    private static DCache instance = null;

    //should be corrected according to double check idiom for lazy initialization
    public static DCache getInstance() {
        if(instance == null){
              instance = new DCache();
         }
            return instance;
        }
    }

    private DCache() {
        long createtime = Long.valueOf(System.currentTimeMillis());
        accesstimes.put(MAP_TYPE.GREEN, createtime );
        dics.put(MAP_TYPE.GREEN, DB.getGreenMap());     
        accesstimes.put(MAP_TYPE.RED, createtime );
        dics.put(MAP_TYPE.RED, DB.getRedMap());
        // some more
    }    

    private Map<String, String> getCachedMap(Object lock, MAP_TYPE mapType, Supplier<Map<String, String>> supplier) {
        long accessTime = System.currentTimeMillis();
        long lastUpdateTime = accesstimes.get(mapType);

        if (accessTime - lastUpdateTime > MAP_LIFE_TIME) {
            synchronized (lock) {
                dics.put(mapType, supplier.get());
                accesstimes.put(mapType, Long.valueOf(accessTime));
            }
        }
        return maps.get(mapType);
    }

    public Map<String, String> getGreenDic() {
        return getCachedMap(mvpLock, MAP_TYPE.GREEN, () -> DB.getGreenMap());
    }    
    public Map<String, String> getRedDic() {
        return getCachedMap(fpLock, MAP_TYPE.RED, () -> DB.getRedMap());
    }
    // some more
}

, поэтому цель кэша состоит в том, что, если кто-то получил доступ к кешу, прошло более 10 минут Значение должно быть обновлено из базы данных. Мои основные проблемы касаются метода getCachedMap. Таким образом, если многие потоки обращаются к нему, они могут не увидеть обновления карты доступа, даже если есть синхронизированный блок. Он не позволяет нескольким потокам одновременно обновлять один и тот же ключ на карте, но не решает проблему видимости, так как чтение из accesstimes выполняется без какой-либо синхронизации, я прав?

Изменение обеих карт (accesstimes и dics) к ConcurrentHasmMap должен решить проблему?

И если я вообще хочу избавиться от карты accesstimes, я придумаю следующее решение:

public class DCache2 {

private enum MAP_TYPE {
    GREEN, RED, //some more 
}

static class Holder{
    private final long lastUpdateTime;
    private final Map<String, String> map;

    public Holder(long lastUpdateTime, Map<String, String> map) {
        this.lastUpdateTime = lastUpdateTime;
        this.cache = map;
    }

    public long getLastUpdateTime() {
        return lastUpdateTime;
    }

    public Map<String, String> getMap() {
        return map;
    }
}

private static final int MAP_LIFE_TIME = 10 * 60 * 1000;

private Map<MAP_TYPE, Holder> cache = new ConcurrentHashMap<>();

private static volatile DCache2 instance = null;

public static DCache2 getInstance() {
  // DCL
}

private DCache2() {
    Long createTime = Long.valueOf(System.currentTimeMillis());
    maps.put(MAP_TYPE.GREEN,  new Holder(createTime, DB.getGreenMap()));
    maps.put(MAP_TYPE.RED,    new Holder(createTime, DB.getRedMap()));
    // some more
}

private Map<String, String> getCachedMap(MAP_TYPE key, Supplier<Map<String, String>> supplier) {
    Holder h = cache.get(key);
    if(!h.isExpired()){
        return h.getCache();
    } else{
       return cache.compute(key, (k, holder) ->{
      // here I check one more time in case other thread 
      //   has done update to avoid one more roundtrip to db
           if(holder.isExpired()){ /
               return new Holder(System.currentTimeMillis(), supplier.get()); 
            // supplier retrievs value from db
           }
           return holder;
       }).getCache();
    }
}

    public Map<String, String> getGreenDic() {
        return getCachedMap(MAP_TYPE.GREEN, () -> DB.getGreenMap());
    }    
    public Map<String, String> getRedDic() {
        return getCachedMap(MAP_TYPE.RED, () -> DB.getRedMap());
    }
    // some more

}

Безопасно ли это решение?

...