Skip to content

Commit

Permalink
MastersListener: Fix race conditions around static 'blacklist' map
Browse files Browse the repository at this point in the history
This fixes two primary issues around this static map:
* The first being that it was a simple 'HashMap' despite being modified and read from multiple threads, possibly concurrently
* The second is to fix a condition where an entry in the blacklist map may be updated, but still removed from 'resetOldsBlackListHosts'
  - This fix is to use the ConcurrentMap functionality to verify the value on the removal, so that if the value has been updated, the removal wont proceed.

Additional there is a slight performance improvement in 'resetOldsBlackListHosts' by both not needing to copy the key set, as well as iterating over the entry set to avoid the additional .get() call to get the stored time value.
  • Loading branch information
Mike Jensen committed Dec 11, 2015
1 parent 2da1999 commit 06494a7
Showing 1 changed file with 9 additions and 10 deletions.
Expand Up @@ -61,10 +61,9 @@ WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWIS
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.sql.SQLException;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
Expand All @@ -79,7 +78,7 @@ public abstract class AbstractMastersListener implements Listener {
/**
* List the recent failedConnection.
*/
protected static Map<HostAddress, Long> blacklist = new HashMap<>();
protected static ConcurrentMap<HostAddress, Long> blacklist = new ConcurrentHashMap<>();
/* =========================== Failover variables ========================================= */
public final UrlParser urlParser;
protected AtomicInteger currentConnectionAttempts = new AtomicInteger();
Expand Down Expand Up @@ -151,12 +150,12 @@ public void addToBlacklist(HostAddress hostAddress) {
*/
public void resetOldsBlackListHosts() {
long currentTime = System.currentTimeMillis();
Set<HostAddress> currentBlackListkeys = new HashSet<HostAddress>(blacklist.keySet());
for (HostAddress blackListHost : currentBlackListkeys) {
if (blacklist.get(blackListHost) < currentTime - urlParser.getOptions().loadBalanceBlacklistTimeout * 1000) {
// if (log.isTraceEnabled()) log.trace("host " + blackListHost+" remove of blacklist");
blacklist.remove(blackListHost);
}
for (Map.Entry<HostAddress, Long> blEntry : blacklist.entrySet()) {
long entryTime = blEntry.getValue();
if (entryTime < currentTime - urlParser.getOptions().loadBalanceBlacklistTimeout * 1000) {
// if (log.isTraceEnabled()) log.trace("host " + blackListHost+" remove of blacklist");
blacklist.remove(blEntry.getKey(), entryTime);
}
}
}

Expand Down

0 comments on commit 06494a7

Please sign in to comment.