From 4ad8a2ec4ad6f40b3c4da876127a89760c2b6a91 Mon Sep 17 00:00:00 2001 From: zhaofaxian Date: Thu, 26 Sep 2019 17:24:23 +0800 Subject: [PATCH 1/3] bug fix for lookup leak when we remove the last lookup from lookup tier --- .../server/lookup/cache/LookupCoordinatorManager.java | 10 +++++----- 1 file changed, 5 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 b6589aae0587..44da5cebb378 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,16 @@ void lookupManagementLoop() try { List> futures = new ArrayList<>(); - for (Map.Entry> tierEntry : allLookupTiers.entrySet()) { + for (String tier : lookupNodeDiscovery.getAllTiers()) { - LOG.debug("Starting lookup mgmt for tier [%s].", tierEntry.getKey()); + LOG.debug("Starting lookup mgmt for tier [%s].", tier); - final Map tierLookups = tierEntry.getValue(); - for (final HostAndPortWithScheme node : lookupNodeDiscovery.getNodesInTier(tierEntry.getKey())) { + final Map 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() From 1324b31e27fd038d2c5db014f8d52700ca8fd624 Mon Sep 17 00:00:00 2001 From: Himanshu Gupta Date: Thu, 26 Sep 2019 08:31:25 -0700 Subject: [PATCH 2/3] warnings about lookups that will never be loaded --- .../lookup/cache/LookupCoordinatorManager.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) 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 44da5cebb378..cd3ac2c68bdd 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,7 +557,18 @@ void lookupManagementLoop() try { List> futures = new ArrayList<>(); - for (String tier : lookupNodeDiscovery.getAllTiers()) { + + Set discoveredLookupTiers = lookupNodeDiscovery.getAllTiers(); + + // 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); From 1ef4e44965753255ea146017093635602a89dcb2 Mon Sep 17 00:00:00 2001 From: zhaofaxian Date: Fri, 27 Sep 2019 10:40:07 +0800 Subject: [PATCH 3/3] fix unit test --- .../server/lookup/cache/LookupCoordinatorManagerTest.java | 4 ++++ 1 file changed, 4 insertions(+) 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 772b185240c3..962c950e7ee8 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 void testLookupManagementLoop() throws Exception 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))