Threadsafe HashMap with snapshot support

The problem

ConcurrentHashMap provides very weak consistency guarantees w.r.t iteration:


guaranteed to traverse elements as they existed upon construction
exactly once, and may (but are not guaranteed to) reflect any
modifications subsequent to construction

(note the "may" part).

In my case, I have a concurrent map that I need to periodically back-up. It's very important that what I back-up be a consistent point-in-time representation of the map. Back-ups are few and far between, triggered on a schedule and never run at the same time. I ended up implementing my own "map" (only the methods I use, not a full-blown map impl) based on 2 underlying concurrent maps and a RW lock:

notes

• the map is expected to be big. VERY big. so probably no copying it. also - there's absolutely no guarantee that a copy-constructor call will land you a consistent copy - it uses iteration under the hood.

Holder.java (used to return values out of closures/inner classes)

public class Holder {
    private T value;
    private boolean isEmpty = true;

    public T getValue() {
        return value;
    }

    public void setValue(T value) {
        this.value = value;
        isEmpty = false;
    }

    public boolean isEmpty() {
        return isEmpty;
    }
}

AutoCloseableLock.java (abuses AutoCloseable for locking, which I think makes for cleaner code)

import java.util.concurrent.locks.Lock;

public class AutoCloseableLock implements AutoCloseable{
    private final Lock delegate;

    public AutoCloseableLock(Lock delegate) {
        this.delegate = delegate;
    }

    public AutoCloseableLock lock() {
        delegate.lock();
        return this;
    }

    @Override
    public void close() {
        delegate.unlock();
    }
}

SingleSnapshotMap.java - a (partial) map implementation to allows a single consistent snapshot

```
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.locks.ReadWriteLock;
import java.

There are a number of things that I think you have completely overengineered.
Holder

This can be replaced with Optional
SingleSnapshotMap

This class exposes 4 methods:

• get

• put

• remove

• snapshot

Which is a 'light' implementation of a Map. The amount of locking though is overkill. I would go with short, sweet, and simple....

The following code takes a 'defensive' copy of the HashSet (which is a relatively fast operation - in the order of milliseconds). If you only occasionally do this operation I cannot see any reason why it would be a problem. Once you have the snapshot, it is independent of the store, and the process for 'backing it up' can be as slow as necessary.

Unless you have significant evidence to the contrary, I cannot imagine how trying to maintain a read/write list while allowing the concurrent backup, will help you. Do you have evidence that the functionality may be needed?

public class SingleSnapshotMap {

    private final HashMap store;
    private final Lock lock = new ReentrantLock();

    public SinglSnapshotMap() {
        store = new HashMap<>();
    }

    public V get(K key) {
        lock.lock();
        try {
            return store.get(key);
        } finally {
            lock.unlock();
        }
    }

    public V put(K key, V val) {
        lock.lock();
        try {
            return store.put(key, val);
        } finally {
            lock.unlock();
        }
    }

    public V remove(K key) {
        lock.lock();
        try {
            return store.remove(key, val);
        } finally {
            lock.unlock();
        }
    }

    public Map snapshot() {
        lock.lock();
        try {
            return new HashMap<>(store);
        } finally {
            lock.unlock();
        }
    }
}

StackExchange Code Review Q#64890, answer score: 7