From 0f7cfa9f79257f8f9b9f271d604d13acb3baae2e Mon Sep 17 00:00:00 2001 From: Himanshu Gupta Date: Tue, 23 Jan 2018 16:28:31 -0600 Subject: [PATCH 1/2] add LookupExtractorFactory.destroy() method --- .../query/lookup/LookupExtractorFactory.java | 17 ++++++++++++++++- .../query/lookup/LookupReferencesManager.java | 8 ++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/processing/src/main/java/io/druid/query/lookup/LookupExtractorFactory.java b/processing/src/main/java/io/druid/query/lookup/LookupExtractorFactory.java index 78bea9e1095e..4b4ff7083075 100644 --- a/processing/src/main/java/io/druid/query/lookup/LookupExtractorFactory.java +++ b/processing/src/main/java/io/druid/query/lookup/LookupExtractorFactory.java @@ -44,13 +44,28 @@ public interface LookupExtractorFactory extends Supplier /** *

- * This method will be called to stop the LookupExtractor upon deletion. + * This method will be called to stop the LookupExtractor upon Druid process stop. This would be used, for + * example, to stop any thread pools it might have. * Calling this method multiple times should always return true if successfully closed. *

* @return Returns false if not successfully closed the {@link LookupExtractor} otherwise returns true */ boolean close(); + /** + *

+ * This method will be called to drop the LookupExtractor upon explicit user request to coordinator to drop + * this lookup. In this method user can do additional cleanup (e.g. deleting disk persisted cache) not done simply + * when Druid process was being stopped to be restarted. + * Calling this method multiple times should always return true if successfully destroyed. + *

+ * @return Returns false if not successfully destroyed the {@link LookupExtractor} otherwise returns true + */ + default boolean destroy() + { + return close(); + } + /** * This method is deprecated and is not removed only to allow 0.10.0 to 0.10.1 transition. It is not used * on a cluster that is running 0.10.1. It will be removed in a later release. diff --git a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java index df2229f9229b..96aaa3052642 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -626,8 +626,8 @@ public void handle(Map lookupMap) throw LOG.debug("Loaded lookup [%s] with spec [%s].", lookupName, lookupExtractorFactoryContainer); if (old != null) { - if (!old.getLookupExtractorFactory().close()) { - throw new ISE("close method returned false for lookup [%s]:[%s]", lookupName, old); + if (!old.getLookupExtractorFactory().destroy()) { + throw new ISE("destroy method returned false for lookup [%s]:[%s]", lookupName, old); } } } @@ -659,9 +659,9 @@ public void handle(Map lookupMap) if (lookupExtractorFactoryContainer != null) { LOG.debug("Removed lookup [%s] with spec [%s].", lookupName, lookupExtractorFactoryContainer); - if (!lookupExtractorFactoryContainer.getLookupExtractorFactory().close()) { + if (!lookupExtractorFactoryContainer.getLookupExtractorFactory().destroy()) { throw new ISE( - "close method returned false for lookup [%s]:[%s]", + "destroy method returned false for lookup [%s]:[%s]", lookupName, lookupExtractorFactoryContainer ); From 7f4121a228d4d9dcd002d21f36403c9334b31fc5 Mon Sep 17 00:00:00 2001 From: Himanshu Gupta Date: Thu, 25 Jan 2018 09:49:06 -0600 Subject: [PATCH 2/2] fix LookupReferencesManagerTest --- .../query/lookup/LookupReferencesManagerTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java b/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java index 62860d4d4d4f..3d136a80921e 100644 --- a/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java +++ b/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java @@ -156,7 +156,7 @@ public void testAddGetRemove() throws Exception { LookupExtractorFactory lookupExtractorFactory = EasyMock.createMock(LookupExtractorFactory.class); EasyMock.expect(lookupExtractorFactory.start()).andReturn(true).once(); - EasyMock.expect(lookupExtractorFactory.close()).andReturn(true).once(); + EasyMock.expect(lookupExtractorFactory.destroy()).andReturn(true).once(); EasyMock.replay(lookupExtractorFactory); Map lookupMap = new HashMap<>(); @@ -221,15 +221,15 @@ public void testCloseIsCalledAfterStopping() throws Exception } @Test - public void testCloseIsCalledAfterRemove() throws Exception + public void testDestroyIsCalledAfterRemove() throws Exception { LookupExtractorFactory lookupExtractorFactory = EasyMock.createStrictMock(LookupExtractorFactory.class); EasyMock.expect(lookupExtractorFactory.start()).andReturn(true).once(); - EasyMock.expect(lookupExtractorFactory.close()).andReturn(true).once(); + EasyMock.expect(lookupExtractorFactory.destroy()).andReturn(true).once(); EasyMock.replay(lookupExtractorFactory); Map lookupMap = new HashMap<>(); - lookupMap.put("testMockForCloseIsCalledAfterRemove", container); + lookupMap.put("testMockForDestroyIsCalledAfterRemove", container); String strResult = mapper.writeValueAsString(lookupMap); Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); expect(config.getLookupTier()).andReturn(LOOKUP_TIER); @@ -280,7 +280,7 @@ public void testUpdateWithHigherVersion() throws Exception { LookupExtractorFactory lookupExtractorFactory1 = EasyMock.createNiceMock(LookupExtractorFactory.class); EasyMock.expect(lookupExtractorFactory1.start()).andReturn(true).once(); - EasyMock.expect(lookupExtractorFactory1.close()).andReturn(true).once(); + EasyMock.expect(lookupExtractorFactory1.destroy()).andReturn(true).once(); LookupExtractorFactory lookupExtractorFactory2 = EasyMock.createNiceMock(LookupExtractorFactory.class); EasyMock.expect(lookupExtractorFactory2.start()).andReturn(true).once(); @@ -461,7 +461,7 @@ public void testRealModeWithMainThread() throws Exception LookupExtractorFactory lookupExtractorFactory = EasyMock.createMock(LookupExtractorFactory.class); EasyMock.expect(lookupExtractorFactory.start()).andReturn(true).once(); - EasyMock.expect(lookupExtractorFactory.close()).andReturn(true).once(); + EasyMock.expect(lookupExtractorFactory.destroy()).andReturn(true).once(); EasyMock.replay(lookupExtractorFactory); Assert.assertNull(lookupReferencesManager.get("test"));