From 964095ec0a5a726e0413e55b8f3341fe74c782d3 Mon Sep 17 00:00:00 2001 From: dominictootell Date: Thu, 17 May 2018 16:40:52 +0100 Subject: [PATCH] Fix reference issue in background dns --- .../factory/BackgroundDnsResolver.java | 64 ++++++++++--------- .../factory/BackgroundDnsResolverTest.java | 26 ++++++-- 2 files changed, 55 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/greencheek/caching/herdcache/memcached/factory/BackgroundDnsResolver.java b/src/main/java/org/greencheek/caching/herdcache/memcached/factory/BackgroundDnsResolver.java index 83f68d0..74c09a5 100644 --- a/src/main/java/org/greencheek/caching/herdcache/memcached/factory/BackgroundDnsResolver.java +++ b/src/main/java/org/greencheek/caching/herdcache/memcached/factory/BackgroundDnsResolver.java @@ -14,10 +14,7 @@ import java.util.Arrays; import java.util.Comparator; import java.util.List; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; +import java.util.concurrent.*; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; @@ -56,45 +53,52 @@ public BackgroundDnsResolver(Host host,long backgroundPollingTimeInMillis, ReferencedClientFactory connnectionFactory, AddressResolver resolver) { this.scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(DEFAULT_THREAD_FACTORY); this.host = host; + this.connnectionFactory = connnectionFactory; + this.resolver = resolver; - - scheduledExecutorService.scheduleWithFixedDelay( - backgroundResolver(), - 0,backgroundPollingTimeInMillis, + ScheduledFuture f= scheduledExecutorService.scheduleWithFixedDelay( + backgroundResolver(resolver,connnectionFactory,host), + 0, backgroundPollingTimeInMillis, TimeUnit.MILLISECONDS); - - this.connnectionFactory = connnectionFactory; - this.resolver = resolver; } - private Runnable backgroundResolver() { + private Runnable backgroundResolver(final AddressResolver resolver, final ReferencedClientFactory connnectionFactory, + final Host host ) { return () -> { - String hostName = host.getHost(); - Holder currentResolvedAddresses = client.get(); - InetAddress[] addresses = resolver.resolve(hostName); + try { + String hostName = host.getHost(); + Holder currentResolvedAddresses = client.get(); + InetAddress[] addresses = resolver.resolve(hostName); - addresses = checkForCollisionResponses(addresses, hostName); - InetAddress[] existingAddresses = currentResolvedAddresses.addresses; + addresses = checkForCollisionResponses(addresses, hostName); - if(addresses.length == 0) { - if(existingAddresses.length==0) { - LOG.error("Failed to resolve address for '{}', no pre-cached addresses to re-use", host); - } else { - LOG.error("Failed to resolve address for '{}', old pre-cached addresses will be kept",host); - } - } else { - if (haveAddressesChanged(addresses,existingAddresses)) { - ReferencedClient referencedClient = connnectionFactory.createClient(toSocketAddresses(addresses)); - if (referencedClient != SpyReferencedClient.UNAVAILABLE_REFERENCE_CLIENT) { - client.set(new Holder(referencedClient,addresses)); + InetAddress[] existingAddresses = currentResolvedAddresses.addresses; + + if (addresses.length == 0) { + if (existingAddresses.length == 0) { + LOG.error("Failed to resolve address for '{}', no pre-cached addresses to re-use", host.getHost()); + } else { + LOG.error("Failed to resolve address for '{}', old pre-cached addresses will be kept", host.getHost()); } - LOG.debug("[{}] Addresses available: {}", host, toCommaSeparated(addresses)); } else { - LOG.debug("[{}] Has Same Addresses: {}", host, toCommaSeparated(addresses)); + if (haveAddressesChanged(addresses, existingAddresses)) { + ReferencedClient referencedClient = connnectionFactory.createClient(toSocketAddresses(addresses)); + if (referencedClient != SpyReferencedClient.UNAVAILABLE_REFERENCE_CLIENT) { + client.set(new Holder(referencedClient, addresses)); + } + LOG.debug("[{}] Addresses available: {}", host, toCommaSeparated(addresses)); + } else { + LOG.debug("[{}] Has Same Addresses: {}", host, toCommaSeparated(addresses)); + } } + } catch(Exception e) { + LOG.error("Exception Encountered During Background DNS Resolver. Caught Error so as to not stop further executions. Investigation in cause suggested.",e); } + + + }; } diff --git a/src/test/java/org/greencheek/caching/herdcache/memcached/factory/BackgroundDnsResolverTest.java b/src/test/java/org/greencheek/caching/herdcache/memcached/factory/BackgroundDnsResolverTest.java index 562a397..750f1e7 100644 --- a/src/test/java/org/greencheek/caching/herdcache/memcached/factory/BackgroundDnsResolverTest.java +++ b/src/test/java/org/greencheek/caching/herdcache/memcached/factory/BackgroundDnsResolverTest.java @@ -13,6 +13,8 @@ import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import static org.junit.Assert.assertEquals; @@ -24,6 +26,7 @@ public class BackgroundDnsResolverTest { class SimpleCircularAddressResolver implements AddressResolver{ + private final CountDownLatch startSignal = new CountDownLatch(1); private AtomicInteger called = new AtomicInteger(0); private final InetAddress[][] addresses; @@ -33,12 +36,17 @@ public SimpleCircularAddressResolver(InetAddress[][] addressesToReturn) { @Override public InetAddress[] resolve(String host) { + startSignal.countDown(); if(called.get()==addresses.length) { called.set(0); } return addresses[called.getAndIncrement()]; } + + public CountDownLatch getStartSignal() { + return startSignal; + } } ReferencedClientFactory reffactory; @@ -74,8 +82,11 @@ public void updateOfDnsIsCaptured() throws UnknownHostException, InterruptedExce addresses3[1] = InetAddress.getByName("127.0.0.2"); addresses3[2] = InetAddress.getByName("127.0.0.3"); - BackgroundDnsResolver resolver = new BackgroundDnsResolver(new Host("bob.com",11211),10000,reffactory,new SimpleCircularAddressResolver(new InetAddress[][]{addresses1,addresses2,addresses3})); + SimpleCircularAddressResolver addressResolver = new SimpleCircularAddressResolver(new InetAddress[][]{addresses1,addresses2,addresses3}); + CountDownLatch latch = addressResolver.getStartSignal(); + BackgroundDnsResolver resolver = new BackgroundDnsResolver(new Host("bob.com",11211),10000,reffactory,addressResolver); try { + latch.await(10000, TimeUnit.SECONDS); Thread.sleep(2000); ReferencedClient client = resolver.getClient(); assertNotSame(client, SpyReferencedClient.UNAVAILABLE_REFERENCE_CLIENT); @@ -112,8 +123,11 @@ public void updateOfDnsIsNotChangedWhenInvalidAddressesReturned() throws Unknown addresses3[1] = InetAddress.getByName("127.0.53.53"); addresses3[2] = InetAddress.getByName("127.0.53.53"); - BackgroundDnsResolver resolver = new BackgroundDnsResolver(new Host("bob.com",11211),10000,reffactory,new SimpleCircularAddressResolver(new InetAddress[][]{addresses1,addresses2,addresses3})); + SimpleCircularAddressResolver addressResolver = new SimpleCircularAddressResolver(new InetAddress[][]{addresses1,addresses2,addresses3}); + CountDownLatch latch = addressResolver.getStartSignal(); + BackgroundDnsResolver resolver = new BackgroundDnsResolver(new Host("bob.com",11211),10000,reffactory,addressResolver); try { + latch.await(10000, TimeUnit.SECONDS); Thread.sleep(2000); ReferencedClient client = resolver.getClient(); assertNotSame(client, SpyReferencedClient.UNAVAILABLE_REFERENCE_CLIENT); @@ -147,10 +161,12 @@ public void updateOfDnsIsChangedAndFaultyAddressRemoved() throws UnknownHostExce addresses2[2] = InetAddress.getByName("127.0.0.5"); - - BackgroundDnsResolver resolver = new BackgroundDnsResolver(new Host("bob.com",11211),10000,reffactory,new SimpleCircularAddressResolver(new InetAddress[][]{addresses1,addresses2})); + SimpleCircularAddressResolver addressResolver = new SimpleCircularAddressResolver(new InetAddress[][]{addresses1,addresses2}); + CountDownLatch latch = addressResolver.getStartSignal(); + BackgroundDnsResolver resolver = new BackgroundDnsResolver(new Host("bob.com",11211),10000,reffactory,addressResolver); try { - Thread.sleep(5000); + latch.await(10000, TimeUnit.SECONDS); + Thread.sleep(2000); ReferencedClient client = resolver.getClient(); assertNotSame(client, SpyReferencedClient.UNAVAILABLE_REFERENCE_CLIENT); Thread.sleep(13000);