From e1b4a3ab7162e74ab86465f90ad50c72c1c19f15 Mon Sep 17 00:00:00 2001
From: Faxian Zhao <232775890@qq.com>
Date: Fri, 27 Sep 2019 18:55:02 +0800
Subject: [PATCH] bug fix for lookup leak when we remove the last lookup from
 lookup tier (#8598)

* bug fix for lookup leak when we remove the last lookup from lookup tier

* warnings about lookups that will never be loaded

* fix unit test
---
 .../cache/LookupCoordinatorManager.java       | 21 ++++++++++++++-----
 .../cache/LookupCoordinatorManagerTest.java   |  4 ++++
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/server/src/main/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManager.java b/server/src/main/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManager.java
index b6589aae058..cd3ac2c68bd 100644
--- a/server/src/main/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManager.java
+++ b/server/src/main/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManager.java
@@ -557,16 +557,27 @@ public class LookupCoordinatorManager
 
     try {
       List<ListenableFuture<Map.Entry>> futures = new ArrayList<>();
-      for (Map.Entry<String, Map<String, LookupExtractorFactoryMapContainer>> tierEntry : allLookupTiers.entrySet()) {
 
-        LOG.debug("Starting lookup mgmt for tier [%s].", tierEntry.getKey());
+      Set<String> discoveredLookupTiers = lookupNodeDiscovery.getAllTiers();
 
-        final Map<String, LookupExtractorFactoryMapContainer> tierLookups = tierEntry.getValue();
-        for (final HostAndPortWithScheme node : lookupNodeDiscovery.getNodesInTier(tierEntry.getKey())) {
+      // Check and Log warnings about lookups configured by user in DB but no nodes discovered to load those.
+      for (String tierInDB : allLookupTiers.keySet()) {
+        if (!discoveredLookupTiers.contains(tierInDB) &&
+            !allLookupTiers.getOrDefault(tierInDB, ImmutableMap.of()).isEmpty()) {
+          LOG.warn("Found lookups for tier [%s] in DB, but no nodes discovered for it", tierInDB);
+        }
+      }
+
+      for (String tier : discoveredLookupTiers) {
+
+        LOG.debug("Starting lookup mgmt for tier [%s].", tier);
+
+        final Map<String, LookupExtractorFactoryMapContainer> tierLookups = allLookupTiers.getOrDefault(tier, ImmutableMap.of());
+        for (final HostAndPortWithScheme node : lookupNodeDiscovery.getNodesInTier(tier)) {
 
           LOG.debug(
               "Starting lookup mgmt for tier [%s] and host [%s:%s:%s].",
-              tierEntry.getKey(),
+              tier,
               node.getScheme(),
               node.getHostText(),
               node.getPort()
diff --git a/server/src/test/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManagerTest.java b/server/src/test/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManagerTest.java
index 772b185240c..962c950e7ee 100644
--- a/server/src/test/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManagerTest.java
+++ b/server/src/test/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManagerTest.java
@@ -1138,6 +1138,10 @@ public class LookupCoordinatorManagerTest
     HostAndPortWithScheme host2 = HostAndPortWithScheme.fromParts("http", "host2", 3456);
 
     EasyMock.reset(lookupNodeDiscovery);
+    EasyMock
+        .expect(lookupNodeDiscovery.getAllTiers())
+        .andReturn(ImmutableSet.of("tier1"))
+        .once();
     EasyMock
         .expect(lookupNodeDiscovery.getNodesInTier("tier1"))
         .andReturn(ImmutableList.of(host1, host2))