From 45fadf3bc1ac454a01afa8890c1a6a4901161110 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 6 Sep 2017 15:12:08 -0500 Subject: [PATCH 01/20] Changes for lookup synchronization --- extensions-core/histogram/pom.xml | 7 + .../io/druid/indexing/common/TestUtils.java | 4 +- .../druid/query/dimension/DimensionSpec.java | 3 +- .../druid/query/extraction/ExtractionFn.java | 2 - .../io/druid/query/lookup/LookupConfig.java | 26 +- .../query/lookup/LookupReferencesManager.java | 325 +++++++++++-- .../druid/query/expression/ExprMacroTest.java | 2 +- .../query/expression/TestExprMacroTable.java | 59 --- .../druid/query/lookup/LookupConfigTest.java | 5 +- .../io/druid/query/lookup/LookupModule.java | 8 +- .../http/LookupCoordinatorResource.java | 10 +- .../dimension/LookupDimensionSpecTest.java | 2 +- .../expression/TestExpressionMacroTable.java | 103 ++++ .../lookup/LookupReferencesManagerTest.java | 455 ++++++++++++++++-- .../http/LookupCoordinatorResourceTest.java | 34 +- sql/pom.xml | 7 + .../druid/sql/calcite/util/CalciteTests.java | 4 +- 17 files changed, 893 insertions(+), 163 deletions(-) create mode 100644 server/src/test/java/io/druid/query/expression/TestExpressionMacroTable.java diff --git a/extensions-core/histogram/pom.xml b/extensions-core/histogram/pom.xml index 8eb5ae08db29..d6df9405bd5b 100644 --- a/extensions-core/histogram/pom.xml +++ b/extensions-core/histogram/pom.xml @@ -59,6 +59,13 @@ test-jar test + + io.druid + druid-server + ${project.parent.version} + test + test-jar + junit junit diff --git a/indexing-service/src/test/java/io/druid/indexing/common/TestUtils.java b/indexing-service/src/test/java/io/druid/indexing/common/TestUtils.java index 013e91b479f9..e34ea20fbc28 100644 --- a/indexing-service/src/test/java/io/druid/indexing/common/TestUtils.java +++ b/indexing-service/src/test/java/io/druid/indexing/common/TestUtils.java @@ -29,7 +29,7 @@ import io.druid.java.util.common.ISE; import io.druid.java.util.common.logger.Logger; import io.druid.math.expr.ExprMacroTable; -import io.druid.query.expression.TestExprMacroTable; +import io.druid.query.expression.TestExpressionMacroTable; import io.druid.segment.IndexIO; import io.druid.segment.IndexMergerV9; import io.druid.segment.column.ColumnConfig; @@ -74,7 +74,7 @@ public int columnCacheSizeBytes() jsonMapper.setInjectableValues( new InjectableValues.Std() - .addValue(ExprMacroTable.class.getName(), TestExprMacroTable.INSTANCE) + .addValue(ExprMacroTable.class.getName(), TestExpressionMacroTable.INSTANCE) .addValue(IndexIO.class, indexIO) .addValue(ObjectMapper.class, jsonMapper) .addValue(ChatHandlerProvider.class, new NoopChatHandlerProvider()) diff --git a/processing/src/main/java/io/druid/query/dimension/DimensionSpec.java b/processing/src/main/java/io/druid/query/dimension/DimensionSpec.java index 7a98ffd5edb8..33f4de0fdbf5 100644 --- a/processing/src/main/java/io/druid/query/dimension/DimensionSpec.java +++ b/processing/src/main/java/io/druid/query/dimension/DimensionSpec.java @@ -35,8 +35,7 @@ @JsonSubTypes.Type(name = "default", value = DefaultDimensionSpec.class), @JsonSubTypes.Type(name = "extraction", value = ExtractionDimensionSpec.class), @JsonSubTypes.Type(name = "regexFiltered", value = RegexFilteredDimensionSpec.class), - @JsonSubTypes.Type(name = "listFiltered", value = ListFilteredDimensionSpec.class), - @JsonSubTypes.Type(name = "lookup", value = LookupDimensionSpec.class) + @JsonSubTypes.Type(name = "listFiltered", value = ListFilteredDimensionSpec.class) }) public interface DimensionSpec extends Cacheable { diff --git a/processing/src/main/java/io/druid/query/extraction/ExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/ExtractionFn.java index df6c86c4867f..5e6a3b95c0ac 100644 --- a/processing/src/main/java/io/druid/query/extraction/ExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/ExtractionFn.java @@ -24,7 +24,6 @@ import io.druid.guice.annotations.ExtensionPoint; import io.druid.java.util.common.Cacheable; import io.druid.query.lookup.LookupExtractionFn; -import io.druid.query.lookup.RegisteredLookupExtractionFn; import javax.annotation.Nullable; @@ -41,7 +40,6 @@ @JsonSubTypes.Type(name = "timeFormat", value = TimeFormatExtractionFn.class), @JsonSubTypes.Type(name = "identity", value = IdentityExtractionFn.class), @JsonSubTypes.Type(name = "lookup", value = LookupExtractionFn.class), - @JsonSubTypes.Type(name = "registeredLookup", value = RegisteredLookupExtractionFn.class), @JsonSubTypes.Type(name = "substring", value = SubstringDimExtractionFn.class), @JsonSubTypes.Type(name = "cascade", value = CascadeExtractionFn.class), @JsonSubTypes.Type(name = "stringFormat", value = StringFormatExtractionFn.class), diff --git a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java index fea346b745fd..3f1c6acc748a 100644 --- a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java +++ b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java @@ -29,15 +29,27 @@ public class LookupConfig @JsonProperty private final String snapshotWorkingDir; + @JsonProperty + private int numLookupLoadingThreads = 10; + + @JsonProperty + private boolean disableLookupSync = false; + /** * @param snapshotWorkingDir working directory to store lookups snapshot file, passing null or empty string will disable the snapshot utility + * @param numLookupLoadingThreads number of threads for loading the lookups as part of the synchronization process + * @param disableLookupSync decides whether the lookup synchronization process should be disabled at startup */ @JsonCreator public LookupConfig( - @JsonProperty("snapshotWorkingDir") String snapshotWorkingDir + @JsonProperty("snapshotWorkingDir") String snapshotWorkingDir, + @JsonProperty("numLookupLoadingThreads") int numLookupLoadingThreads, + @JsonProperty("disableLookupSync") boolean disableLookupSync ) { this.snapshotWorkingDir = Strings.nullToEmpty(snapshotWorkingDir); + this.numLookupLoadingThreads = numLookupLoadingThreads; + this.disableLookupSync = disableLookupSync; } public String getSnapshotWorkingDir() @@ -45,6 +57,16 @@ public String getSnapshotWorkingDir() return snapshotWorkingDir; } + public int getNumLookupLoadingThreads() + { + return numLookupLoadingThreads; + } + + public boolean getDisableLookupSync() + { + return disableLookupSync; + } + @Override public boolean equals(Object o) @@ -67,6 +89,8 @@ public String toString() { return "LookupConfig{" + "snapshotWorkingDir='" + getSnapshotWorkingDir() + '\'' + + " numLookupLoadingThreads='" + getNumLookupLoadingThreads() + '\'' + + " disableLookupSync='" + getDisableLookupSync() + '\'' + '}'; } } diff --git a/processing/src/main/java/io/druid/query/lookup/LookupReferencesManager.java b/processing/src/main/java/io/druid/query/lookup/LookupReferencesManager.java index 1485fe725cae..71012ee98388 100644 --- a/processing/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/processing/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -20,29 +20,53 @@ package io.druid.query.lookup; +import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Charsets; import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningScheduledExecutorService; +import com.google.common.util.concurrent.MoreExecutors; import com.google.inject.Inject; import com.metamx.emitter.EmittingLogger; +import com.metamx.http.client.HttpClient; +import com.metamx.http.client.Request; +import com.metamx.http.client.response.StatusResponseHandler; +import com.metamx.http.client.response.StatusResponseHolder; +import io.druid.client.coordinator.Coordinator; +import io.druid.client.selector.Server; import io.druid.concurrent.Execs; import io.druid.concurrent.LifecycleLock; +import io.druid.curator.discovery.ServerDiscoverySelector; import io.druid.guice.ManageLifecycle; +import io.druid.guice.annotations.Global; import io.druid.guice.annotations.Json; import io.druid.java.util.common.ISE; +import io.druid.java.util.common.StringUtils; import io.druid.java.util.common.lifecycle.LifecycleStart; import io.druid.java.util.common.lifecycle.LifecycleStop; +import org.jboss.netty.handler.codec.http.HttpMethod; +import org.jboss.netty.handler.codec.http.HttpResponseStatus; import javax.annotation.Nullable; +import java.net.MalformedURLException; +import java.net.URI; +import java.net.URL; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.LockSupport; @@ -79,21 +103,49 @@ public class LookupReferencesManager //for unit testing only private final boolean testMode; + private final ServerDiscoverySelector selector; + + private final HttpClient httpClient; + + private final ObjectMapper jsonMapper; + + private static final StatusResponseHandler RESPONSE_HANDLER = new StatusResponseHandler(Charsets.UTF_8); + + private static final TypeReference> LOOKUPS_ALL_REFERENCE = + new TypeReference>() + { + }; + + private LookupListeningAnnouncerConfig lookupListeningAnnouncerConfig; + + private LookupConfig lookupConfig; + @Inject - public LookupReferencesManager(LookupConfig lookupConfig, @Json ObjectMapper objectMapper) + public LookupReferencesManager( + LookupConfig lookupConfig, @Json ObjectMapper objectMapper, + @Coordinator ServerDiscoverySelector selector, @Global HttpClient client, + LookupListeningAnnouncerConfig lookupListeningAnnouncerConfig + ) { - this(lookupConfig, objectMapper, false); + this(lookupConfig, objectMapper, selector, client, lookupListeningAnnouncerConfig, false); } @VisibleForTesting - LookupReferencesManager(LookupConfig lookupConfig, ObjectMapper objectMapper, boolean testMode) + LookupReferencesManager( + LookupConfig lookupConfig, ObjectMapper objectMapper, ServerDiscoverySelector selector, HttpClient client, + LookupListeningAnnouncerConfig lookupListeningAnnouncerConfig, boolean testMode + ) { if (Strings.isNullOrEmpty(lookupConfig.getSnapshotWorkingDir())) { this.lookupSnapshotTaker = null; } else { this.lookupSnapshotTaker = new LookupSnapshotTaker(objectMapper, lookupConfig.getSnapshotWorkingDir()); } - + this.selector = selector; + this.httpClient = client; + this.jsonMapper = objectMapper; + this.lookupListeningAnnouncerConfig = lookupListeningAnnouncerConfig; + this.lookupConfig = lookupConfig; this.testMode = testMode; } @@ -103,44 +155,35 @@ public void start() if (!lifecycleLock.canStart()) { throw new ISE("can't start."); } - try { LOG.info("LookupReferencesManager is starting."); - - loadSnapshotAndInitStateRef(); - + loadAllLookupsAndInitStateRef(); if (!testMode) { mainThread = Execs.makeThread( "LookupReferencesManager-MainThread", - new Runnable() - { - @Override - public void run() - { - try { - - if (!lifecycleLock.awaitStarted()) { - LOG.error("WTF! lifecycle not started, lookup update notices will not be handled."); - return; - } + () -> { + try { + if (!lifecycleLock.awaitStarted()) { + LOG.error("WTF! lifecycle not started, lookup update notices will not be handled."); + return; + } - while (!Thread.interrupted() && lifecycleLock.awaitStarted(1, TimeUnit.MILLISECONDS)) { - try { - handlePendingNotices(); - LockSupport.parkNanos(LookupReferencesManager.this, TimeUnit.MINUTES.toNanos(1)); - } - catch (Throwable t) { - LOG.makeAlert(t, "Error occured while lookup notice handling.").emit(); - } + while (!Thread.interrupted() && lifecycleLock.awaitStarted(1, TimeUnit.MILLISECONDS)) { + try { + handlePendingNotices(); + LockSupport.parkNanos(LookupReferencesManager.this, TimeUnit.MINUTES.toNanos(1)); + } + catch (Throwable t) { + LOG.makeAlert(t, "Error occured while lookup notice handling.").emit(); } - } - catch (Throwable t) { - LOG.error(t, "Error while waiting for lifecycle start. lookup updates notices will not be handled"); - } - finally { - LOG.info("Lookup Management loop exited, Lookup notices are not handled anymore."); } } + catch (Throwable t) { + LOG.error(t, "Error while waiting for lifecycle start. lookup updates notices will not be handled"); + } + finally { + LOG.info("Lookup Management loop exited, Lookup notices are not handled anymore."); + } }, true ); @@ -279,7 +322,11 @@ public LookupsState getAllLookupsState() return new LookupsState<>(lookupUpdateState.lookupMap, lookupsToLoad, lookupsToDrop); } - private void updateToLoadAndDrop(List notices, Map lookupsToLoad, Set lookupsToDrop) + private void updateToLoadAndDrop( + List notices, + Map lookupsToLoad, + Set lookupsToDrop + ) { for (Notice notice : notices) { if (notice instanceof LoadNotice) { @@ -308,26 +355,154 @@ private void takeSnapshot(Map lookupMap } } - private void loadSnapshotAndInitStateRef() + private void loadAllLookupsAndInitStateRef() { - if (lookupSnapshotTaker != null) { - ImmutableMap.Builder builder = ImmutableMap.builder(); - - final List lookupBeanList = lookupSnapshotTaker.pullExistingSnapshot(); - for (LookupBean lookupBean : lookupBeanList) { - LookupExtractorFactoryContainer container = lookupBean.getContainer(); + if (!lookupConfig.getDisableLookupSync()) { + String tier = lookupListeningAnnouncerConfig.getLookupTier(); + List lookupBeanList = new ArrayList<>(); + // Check if the coordinator is accessible + if (getCoordinatorUrl().isEmpty()) { + if (lookupSnapshotTaker != null) { + LOG.info("Coordinator is unavailable. Loading saved snapshot instead"); + lookupBeanList = getLookupListFromSnapshot(); + } + } else { + lookupBeanList = getLookupListFromCoordinator(tier); + if (lookupBeanList == null) { + LOG.info("Lookups not accessible from Coordinator. Loading saved snapshot instead"); + lookupBeanList = getLookupListFromSnapshot(); + } + } + if (!lookupBeanList.isEmpty()) { + ImmutableMap.Builder builder = ImmutableMap.builder(); + ListeningScheduledExecutorService executorService = MoreExecutors.listeningDecorator( + Executors.newScheduledThreadPool( + lookupConfig.getNumLookupLoadingThreads(), + Execs.makeThreadFactory("LookupReferencesManager--%s") + ) + ); + try { + List> futures = new ArrayList<>(); + LOG.info("Starting lookup loading process"); + for (LookupBean lookupBean : lookupBeanList) { + futures.add( + executorService.submit( + () -> { + + LookupExtractorFactoryContainer container = lookupBean.getContainer(); + if (container.getLookupExtractorFactory().start()) { + LOG.info( + "Started lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); + } else { + LOG.error( + "Failed to start lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + return null; + } + } + ) + ); + } + final ListenableFuture> futureList = Futures.allAsList(futures); + try { + futureList.get() + .stream() + .filter(Objects::nonNull) + .forEach(builder::put); + } + catch (Exception ex) { + futureList.cancel(true); + throw ex; + } + stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); + } + catch (Exception e) { + LOG.error(e, "Failed to finish lookup load process."); + } + executorService.shutdownNow(); + } else { + LOG.info("No lookups to be loaded at this point"); + stateRef.set(new LookupUpdateState(ImmutableMap.of(), ImmutableList.of(), ImmutableList.of())); + } + } else { + LOG.info("Lookup synchronization is disabled"); + stateRef.set(new LookupUpdateState(ImmutableMap.of(), ImmutableList.of(), ImmutableList.of())); + } + } - if (container.getLookupExtractorFactory().start()) { - builder.put(lookupBean.getName(), container); + private List getLookupListFromCoordinator(String tier) + { + try { + final StatusResponseHolder response = fetchLookupsForTier(tier); + List lookupBeanList = new ArrayList<>(); + if (!response.getStatus().equals(HttpResponseStatus.OK)) { + LOG.error( + "Error while fetching lookup code from Coordinator with status[%s] and content[%s]", + response.getStatus(), + response.getContent() + ); + if (lookupSnapshotTaker != null) { + LOG.info("Attempting to load saved snapshot"); + return null; + } + } else { + // Older version of getSpecificTier returns a list of lookup names. + // Lookup loading is performed via snapshot if older version is present. + if (response.getContent().startsWith("[")) { + return null; } else { - throw new ISE("Failed to start lookup [%s]:[%s]", lookupBean.getName(), container); + Map lookupMap = jsonMapper.readValue( + response.getContent(), + LOOKUPS_ALL_REFERENCE + ); + if (!lookupMap.isEmpty()) { + for (Map.Entry e : lookupMap.entrySet()) { + lookupBeanList.add(new LookupBean(e.getKey(), null, e.getValue())); + } + } } } + return lookupBeanList; + } + catch (Exception e) { + LOG.error(e, "Error while parsing lookup code for tier [%s] from response", tier); + return null; + } + } - stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); - } else { - stateRef.set(new LookupUpdateState(ImmutableMap.of(), ImmutableList.of(), ImmutableList.of())); + private List getLookupListFromSnapshot() + { + return lookupSnapshotTaker.pullExistingSnapshot(); + } + + private String getCoordinatorUrl() + { + try { + final Server instance = selector.pick(); + if (instance == null) { + LOG.info("Coordinator instance unavailable."); + return ""; + } + return new URI( + instance.getScheme(), + null, + instance.getAddress(), + instance.getPort(), + "/druid/coordinator/v1", + null, + null + ).toString(); } + catch (Exception e) { + LOG.error("Error encountered while connecting to Coordinator"); + } + return ""; } private LookupUpdateState atomicallyUpdateStateRef(Function fn) @@ -341,6 +516,62 @@ private LookupUpdateState atomicallyUpdateStateRef(Function theLookup) - { - final LookupReferencesManager lookupReferencesManager = EasyMock.createMock(LookupReferencesManager.class); - EasyMock.expect(lookupReferencesManager.get(EasyMock.eq("lookyloo"))).andReturn( - new LookupExtractorFactoryContainer( - "v0", - new LookupExtractorFactory() - { - @Override - public boolean start() - { - throw new UnsupportedOperationException(); - } - - @Override - public boolean close() - { - throw new UnsupportedOperationException(); - } - - @Override - public boolean replaces(@Nullable final LookupExtractorFactory other) - { - throw new UnsupportedOperationException(); - } - - @Override - public LookupIntrospectHandler getIntrospectHandler() - { - throw new UnsupportedOperationException(); - } - - @Override - public LookupExtractor get() - { - return new MapLookupExtractor(theLookup, false); - } - } - ) - ).anyTimes(); - EasyMock.expect(lookupReferencesManager.get(EasyMock.not(EasyMock.eq("lookyloo")))).andReturn(null).anyTimes(); - EasyMock.replay(lookupReferencesManager); - return lookupReferencesManager; - } } diff --git a/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java b/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java index 836176e59135..214ab9ba772a 100644 --- a/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java +++ b/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java @@ -32,13 +32,14 @@ public class LookupConfigTest { ObjectMapper mapper = TestHelper.getJsonMapper(); - + private final int NUM_THREADS = 10; + private final boolean LOOKUP_DISABLE = false; @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @Test public void TestSerDesr() throws IOException { - LookupConfig lookupConfig = new LookupConfig(temporaryFolder.newFile().getAbsolutePath()); + LookupConfig lookupConfig = new LookupConfig(temporaryFolder.newFile().getAbsolutePath(), NUM_THREADS, LOOKUP_DISABLE); Assert.assertEquals(lookupConfig, mapper.reader(LookupConfig.class).readValue(mapper.writeValueAsString(lookupConfig))); } } diff --git a/server/src/main/java/io/druid/query/lookup/LookupModule.java b/server/src/main/java/io/druid/query/lookup/LookupModule.java index c8fdeaf68c0d..59c0d7f96c3d 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupModule.java +++ b/server/src/main/java/io/druid/query/lookup/LookupModule.java @@ -25,6 +25,7 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.Module; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.jsontype.NamedType; import com.fasterxml.jackson.databind.module.SimpleModule; import com.google.common.base.Preconditions; import com.google.common.base.Strings; @@ -48,6 +49,7 @@ import io.druid.guice.annotations.Smile; import io.druid.initialization.DruidModule; import io.druid.java.util.common.logger.Logger; +import io.druid.query.dimension.LookupDimensionSpec; import io.druid.query.expression.LookupExprMacro; import io.druid.server.DruidNode; import io.druid.server.http.HostAndPortWithScheme; @@ -84,7 +86,11 @@ public static String getTierListenerPath(String tier) public List getJacksonModules() { return ImmutableList.of( - new SimpleModule("DruidLookupModule").registerSubtypes(MapLookupExtractorFactory.class) + new SimpleModule("DruidLookupModule").registerSubtypes(MapLookupExtractorFactory.class), + new SimpleModule().registerSubtypes( + new NamedType(LookupDimensionSpec.class, "lookup"), + new NamedType(RegisteredLookupExtractionFn.class, "registeredLookup") + ) ); } diff --git a/server/src/main/java/io/druid/server/http/LookupCoordinatorResource.java b/server/src/main/java/io/druid/server/http/LookupCoordinatorResource.java index 3f5ea82c9e91..93a9ce4fc9ea 100644 --- a/server/src/main/java/io/druid/server/http/LookupCoordinatorResource.java +++ b/server/src/main/java/io/druid/server/http/LookupCoordinatorResource.java @@ -269,7 +269,9 @@ public Response getSpecificLookup( @Produces({MediaType.APPLICATION_JSON, SmileMediaTypes.APPLICATION_JACKSON_SMILE}) @Path("/{tier}") public Response getSpecificTier( - @PathParam("tier") String tier + @PathParam("tier") String tier, + @DefaultValue("false") @QueryParam("detailed") boolean detailed + ) { try { @@ -290,7 +292,11 @@ public Response getSpecificTier( .entity(ServletResourceUtils.sanitizeException(new RE("Tier [%s] not found", tier))) .build(); } - return Response.ok().entity(tierLookups.keySet()).build(); + if (detailed) { + return Response.ok().entity(tierLookups).build(); + } else { + return Response.ok().entity(tierLookups.keySet()).build(); + } } catch (Exception e) { LOG.error(e, "Error getting tier [%s]", tier); diff --git a/server/src/test/java/io/druid/query/dimension/LookupDimensionSpecTest.java b/server/src/test/java/io/druid/query/dimension/LookupDimensionSpecTest.java index 6c94da7f8967..bf9e2e2166a3 100644 --- a/server/src/test/java/io/druid/query/dimension/LookupDimensionSpecTest.java +++ b/server/src/test/java/io/druid/query/dimension/LookupDimensionSpecTest.java @@ -75,7 +75,7 @@ public void testSerDesr(DimensionSpec lookupDimSpec) throws IOException LOOKUP_REF_MANAGER ); String serLookup = mapper.writeValueAsString(lookupDimSpec); - Assert.assertEquals(lookupDimSpec, mapper.reader(DimensionSpec.class).with(injectableValues).readValue(serLookup)); + Assert.assertEquals(lookupDimSpec, mapper.reader(LookupDimensionSpec.class).with(injectableValues).readValue(serLookup)); } private Object[] parametersForTestSerDesr() diff --git a/server/src/test/java/io/druid/query/expression/TestExpressionMacroTable.java b/server/src/test/java/io/druid/query/expression/TestExpressionMacroTable.java new file mode 100644 index 000000000000..bef480a33845 --- /dev/null +++ b/server/src/test/java/io/druid/query/expression/TestExpressionMacroTable.java @@ -0,0 +1,103 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.druid.query.expression; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import io.druid.math.expr.ExprMacroTable; +import io.druid.query.extraction.MapLookupExtractor; +import io.druid.query.lookup.LookupExtractor; +import io.druid.query.lookup.LookupExtractorFactory; +import io.druid.query.lookup.LookupExtractorFactoryContainer; +import io.druid.query.lookup.LookupIntrospectHandler; +import io.druid.query.lookup.LookupReferencesManager; +import org.easymock.EasyMock; + +import javax.annotation.Nullable; + +public class TestExpressionMacroTable extends ExprMacroTable +{ + public static final ExprMacroTable INSTANCE = new TestExpressionMacroTable(); + + private TestExpressionMacroTable() + { + super( + ImmutableList.of( + new LikeExprMacro(), + new LookupExprMacro(createTestLookupReferencesManager(ImmutableMap.of("foo", "xfoo"))), + new RegexpExtractExprMacro(), + new TimestampCeilExprMacro(), + new TimestampExtractExprMacro(), + new TimestampFloorExprMacro(), + new TimestampFormatExprMacro(), + new TimestampParseExprMacro(), + new TimestampShiftExprMacro() + ) + ); + } + + /** + * Returns a mock {@link LookupReferencesManager} that has one lookup, "lookyloo". + */ + public static LookupReferencesManager createTestLookupReferencesManager(final ImmutableMap theLookup) + { + final LookupReferencesManager lookupReferencesManager = EasyMock.createMock(LookupReferencesManager.class); + EasyMock.expect(lookupReferencesManager.get(EasyMock.eq("lookyloo"))).andReturn( + new LookupExtractorFactoryContainer( + "v0", + new LookupExtractorFactory() + { + @Override + public boolean start() + { + throw new UnsupportedOperationException(); + } + + @Override + public boolean close() + { + throw new UnsupportedOperationException(); + } + + @Override + public boolean replaces(@Nullable final LookupExtractorFactory other) + { + throw new UnsupportedOperationException(); + } + + @Override + public LookupIntrospectHandler getIntrospectHandler() + { + throw new UnsupportedOperationException(); + } + + @Override + public LookupExtractor get() + { + return new MapLookupExtractor(theLookup, false); + } + } + ) + ).anyTimes(); + EasyMock.expect(lookupReferencesManager.get(EasyMock.not(EasyMock.eq("lookyloo")))).andReturn(null).anyTimes(); + EasyMock.replay(lookupReferencesManager); + return lookupReferencesManager; + } +} 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 8aaf8b5c86ed..68f6a2b770dd 100644 --- a/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java +++ b/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java @@ -19,12 +19,21 @@ package io.druid.query.lookup; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.Futures; import com.metamx.emitter.EmittingLogger; +import com.metamx.http.client.HttpClient; +import com.metamx.http.client.Request; +import com.metamx.http.client.response.StatusResponseHandler; +import com.metamx.http.client.response.StatusResponseHolder; +import io.druid.client.selector.Server; +import io.druid.curator.discovery.ServerDiscoverySelector; import io.druid.jackson.DefaultObjectMapper; import io.druid.server.metrics.NoopServiceEmitter; import org.easymock.EasyMock; +import org.jboss.netty.handler.codec.http.HttpResponseStatus; import org.junit.Assert; import org.junit.Before; import org.junit.Rule; @@ -32,11 +41,38 @@ import org.junit.rules.TemporaryFolder; import java.io.IOException; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.TimeUnit; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.reset; + public class LookupReferencesManagerTest { LookupReferencesManager lookupReferencesManager; + + private ServerDiscoverySelector selector; + + private HttpClient httpClient; + + private Server server; + + private LookupListeningAnnouncerConfig config; + + private static final String propertyBase = "some.property"; + + private static final String LOOKUP_TIER = "lookupTier"; + + private static final int LOOKUP_THREADS = 1; + + private static final boolean LOOKUP_DISABLE = false; + + LookupExtractorFactory lookupExtractorFactory; + + LookupExtractorFactoryContainer container; @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); ObjectMapper mapper = new DefaultObjectMapper(); @@ -46,22 +82,81 @@ public void setUp() throws IOException { EmittingLogger.registerEmitter(new NoopServiceEmitter()); + selector = createMock(ServerDiscoverySelector.class); + + httpClient = createMock(HttpClient.class); + + config = createMock(LookupListeningAnnouncerConfig.class); + + server = new Server() + { + + @Override + public String getScheme() + { + return "http"; + } + + @Override + public int getPort() + { + return 8080; + } + + @Override + public String getHost() + { + return "localhost"; + } + + @Override + public String getAddress() + { + return "localhost"; + } + }; + lookupExtractorFactory = new MapLookupExtractorFactory( + ImmutableMap.of( + "key", + "value" + ), true + ); + container = new LookupExtractorFactoryContainer("v0", lookupExtractorFactory); mapper.registerSubtypes(MapLookupExtractorFactory.class); lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(temporaryFolder.newFolder().getAbsolutePath()), - mapper, + new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), LOOKUP_THREADS, LOOKUP_DISABLE), + mapper, selector, httpClient, config, true ); } @Test - public void testStartStop() + public void testStartStop() throws JsonProcessingException { lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(null), - mapper + new LookupConfig(null, LOOKUP_THREADS, LOOKUP_DISABLE), + mapper, selector, httpClient, config + ); + + Map lookupMap = new HashMap<>(); + lookupMap.put("testMockForStartStop", container); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.OK, + new StringBuilder().append(strResult) ); + expect(selector.pick()).andReturn(server).times(2); + replay(selector); + + EasyMock.expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)) + .times(1); + EasyMock.replay(httpClient); Assert.assertFalse(lookupReferencesManager.lifecycleLock.awaitStarted(1, TimeUnit.MICROSECONDS)); Assert.assertNull(lookupReferencesManager.mainThread); Assert.assertNull(lookupReferencesManager.stateRef.get()); @@ -107,6 +202,26 @@ public void testAddGetRemove() throws Exception EasyMock.expect(lookupExtractorFactory.start()).andReturn(true).once(); EasyMock.expect(lookupExtractorFactory.close()).andReturn(true).once(); EasyMock.replay(lookupExtractorFactory); + + Map lookupMap = new HashMap<>(); + lookupMap.put("testMockForAddGetRemove", container); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.OK, + new StringBuilder().append(strResult) + ); + expect(selector.pick()).andReturn(server).times(2); + replay(selector); + + EasyMock.expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)) + .times(1); + + EasyMock.replay(httpClient); lookupReferencesManager.start(); Assert.assertNull(lookupReferencesManager.get("test")); @@ -130,6 +245,25 @@ public void testCloseIsCalledAfterStopping() throws Exception EasyMock.expect(lookupExtractorFactory.start()).andReturn(true).once(); EasyMock.expect(lookupExtractorFactory.close()).andReturn(true).once(); EasyMock.replay(lookupExtractorFactory); + Map lookupMap = new HashMap<>(); + lookupMap.put("testMockForCloseIsCalledAfterStopping", container); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.OK, + new StringBuilder().append(strResult) + ); + expect(selector.pick()).andReturn(server).times(2); + replay(selector); + + EasyMock.expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)) + .times(1); + + EasyMock.replay(httpClient); lookupReferencesManager.start(); lookupReferencesManager.add("testMock", new LookupExtractorFactoryContainer("0", lookupExtractorFactory)); lookupReferencesManager.handlePendingNotices(); @@ -146,6 +280,25 @@ public void testCloseIsCalledAfterRemove() throws Exception EasyMock.expect(lookupExtractorFactory.close()).andReturn(true).once(); EasyMock.replay(lookupExtractorFactory); + Map lookupMap = new HashMap<>(); + lookupMap.put("testMockForCloseIsCalledAfterRemove", container); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.OK, + new StringBuilder().append(strResult) + ); + expect(selector.pick()).andReturn(server).times(2); + replay(selector); + + EasyMock.expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)) + .times(1); + + EasyMock.replay(httpClient); lookupReferencesManager.start(); lookupReferencesManager.add("testMock", new LookupExtractorFactoryContainer("0", lookupExtractorFactory)); lookupReferencesManager.handlePendingNotices(); @@ -157,8 +310,27 @@ public void testCloseIsCalledAfterRemove() throws Exception } @Test - public void testGetNotThere() + public void testGetNotThere() throws Exception { + Map lookupMap = new HashMap<>(); + lookupMap.put("testMockForGetNotThere", container); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.OK, + new StringBuilder().append(strResult) + ); + expect(selector.pick()).andReturn(server).times(2); + replay(selector); + + EasyMock.expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)) + .times(1); + + EasyMock.replay(httpClient); lookupReferencesManager.start(); Assert.assertNull(lookupReferencesManager.get("notThere")); } @@ -174,7 +346,25 @@ public void testUpdateWithHigherVersion() throws Exception EasyMock.expect(lookupExtractorFactory2.start()).andReturn(true).once(); EasyMock.replay(lookupExtractorFactory1, lookupExtractorFactory2); + Map lookupMap = new HashMap<>(); + lookupMap.put("testMockForUpdateWithHigherVersion", container); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.OK, + new StringBuilder().append(strResult) + ); + expect(selector.pick()).andReturn(server).times(2); + replay(selector); + EasyMock.expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)) + .times(1); + + EasyMock.replay(httpClient); lookupReferencesManager.start(); lookupReferencesManager.add("testName", new LookupExtractorFactoryContainer("1", lookupExtractorFactory1)); lookupReferencesManager.handlePendingNotices(); @@ -194,7 +384,25 @@ public void testUpdateWithLowerVersion() throws Exception LookupExtractorFactory lookupExtractorFactory2 = EasyMock.createNiceMock(LookupExtractorFactory.class); EasyMock.replay(lookupExtractorFactory1, lookupExtractorFactory2); + Map lookupMap = new HashMap<>(); + lookupMap.put("testMockForUpdateWithLowerVersion", container); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.OK, + new StringBuilder().append(strResult) + ); + expect(selector.pick()).andReturn(server).times(2); + replay(selector); + EasyMock.expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)) + .times(1); + + EasyMock.replay(httpClient); lookupReferencesManager.start(); lookupReferencesManager.add("testName", new LookupExtractorFactoryContainer("1", lookupExtractorFactory1)); lookupReferencesManager.handlePendingNotices(); @@ -208,33 +416,28 @@ public void testUpdateWithLowerVersion() throws Exception @Test public void testRemoveNonExisting() throws Exception { - lookupReferencesManager.start(); - lookupReferencesManager.remove("test"); - lookupReferencesManager.handlePendingNotices(); - } - - @Test - public void testBootstrapFromFile() throws Exception - { - LookupExtractorFactory lookupExtractorFactory = new MapLookupExtractorFactory( - ImmutableMap.of( - "key", - "value" - ), true + Map lookupMap = new HashMap<>(); + lookupMap.put("testMockForRemoveNonExisting", container); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.OK, + new StringBuilder().append(strResult) ); - LookupExtractorFactoryContainer container = new LookupExtractorFactoryContainer("v0", lookupExtractorFactory); - lookupReferencesManager.start(); - lookupReferencesManager.add("testMockForBootstrap", container); - lookupReferencesManager.handlePendingNotices(); - lookupReferencesManager.stop(); + expect(selector.pick()).andReturn(server).times(2); + replay(selector); - lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(lookupReferencesManager.lookupSnapshotTaker.getPersistFile().getParent()), - mapper, - true - ); + EasyMock.expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)) + .times(1); + + EasyMock.replay(httpClient); lookupReferencesManager.start(); - Assert.assertEquals(container, lookupReferencesManager.get("testMockForBootstrap")); + lookupReferencesManager.remove("test"); + lookupReferencesManager.handlePendingNotices(); } @Test @@ -269,7 +472,24 @@ public void testGetAllLookupsState() throws Exception ), true ) ); + Map lookupMap = new HashMap<>(); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.OK, + new StringBuilder().append(strResult) + ); + expect(selector.pick()).andReturn(server).times(2); + replay(selector); + + EasyMock.expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)) + .times(1); + EasyMock.replay(httpClient); lookupReferencesManager.start(); lookupReferencesManager.add("one", container1); lookupReferencesManager.add("two", container2); @@ -290,14 +510,32 @@ public void testGetAllLookupsState() throws Exception Assert.assertTrue(state.getToDrop().contains("one")); } - @Test (timeout = 20000) + @Test(timeout = 20000) public void testRealModeWithMainThread() throws Exception { LookupReferencesManager lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(temporaryFolder.newFolder().getAbsolutePath()), - mapper + new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), LOOKUP_THREADS, LOOKUP_DISABLE), + mapper, selector, httpClient, config + ); + Map lookupMap = new HashMap<>(); + lookupMap.put("testMockForRealModeWithMainThread", container); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.OK, + new StringBuilder().append(strResult) ); + expect(selector.pick()).andReturn(server).times(2); + replay(selector); + EasyMock.expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)) + .times(1); + + EasyMock.replay(httpClient); lookupReferencesManager.start(); Assert.assertTrue(lookupReferencesManager.mainThread.isAlive()); @@ -324,4 +562,155 @@ public void testRealModeWithMainThread() throws Exception Assert.assertFalse(lookupReferencesManager.mainThread.isAlive()); } + + @Test + public void testCoordinatorLookupSync() throws Exception + { + LookupExtractorFactoryContainer container1 = new LookupExtractorFactoryContainer( + "0", + new MapLookupExtractorFactory( + ImmutableMap.of( + "key1", + "value1" + ), true + ) + ); + + LookupExtractorFactoryContainer container2 = new LookupExtractorFactoryContainer( + "0", + new MapLookupExtractorFactory( + ImmutableMap.of( + "key2", + "value2" + ), true + ) + ); + + LookupExtractorFactoryContainer container3 = new LookupExtractorFactoryContainer( + "0", + new MapLookupExtractorFactory( + ImmutableMap.of( + "key3", + "value3" + ), true + ) + ); + Map lookupMap = new HashMap<>(); + lookupMap.put("testLookup1", container1); + lookupMap.put("testLookup2", container2); + lookupMap.put("testLookup3", container3); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.OK, + new StringBuilder().append(strResult) + ); + expect(selector.pick()).andReturn(server).times(2); + replay(selector); + + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + + expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)).times(1); + + + replay(httpClient); + + lookupReferencesManager.start(); + Assert.assertEquals(container1, lookupReferencesManager.get("testLookup1")); + Assert.assertEquals(container2, lookupReferencesManager.get("testLookup2")); + Assert.assertEquals(container3, lookupReferencesManager.get("testLookup3")); + + } + + @Test + public void testLoadLookupOnCoordinatorFailure() throws Exception + { + Map lookupMap = new HashMap<>(); + lookupMap.put("testMockForLoadLookupOnCoordinatorFailure", container); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.NOT_FOUND, + new StringBuilder().append(strResult) + ); + expect(selector.pick()).andReturn(null); + replay(selector); + + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + + expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)).times(1); + + + replay(httpClient); + + lookupReferencesManager.start(); + lookupReferencesManager.add("testMockForLoadLookupOnCoordinatorFailure", container); + lookupReferencesManager.handlePendingNotices(); + lookupReferencesManager.stop(); + lookupReferencesManager = new LookupReferencesManager( + new LookupConfig(lookupReferencesManager.lookupSnapshotTaker.getPersistFile().getParent(), + LOOKUP_THREADS, + LOOKUP_DISABLE), + mapper, selector, httpClient, config, + true + ); + reset(config); + reset(selector); + reset(httpClient); + + expect(selector.pick()).andReturn(null); + replay(selector); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)).times(1); + replay(httpClient); + lookupReferencesManager.start(); + Assert.assertEquals(container, lookupReferencesManager.get("testMockForLoadLookupOnCoordinatorFailure")); + } + + @Test + public void testDisableLookupSync() throws Exception + { + LookupReferencesManager lookupReferencesManager = new LookupReferencesManager( + new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), LOOKUP_THREADS, true), + mapper, selector, httpClient, config + ); + Map lookupMap = new HashMap<>(); + lookupMap.put("testMockForDisableLookupSync", container); + String strResult = mapper.writeValueAsString(lookupMap); + StatusResponseHolder responseHolder = new StatusResponseHolder( + HttpResponseStatus.OK, + new StringBuilder().append(strResult) + ); + expect(selector.pick()).andReturn(server).times(2); + replay(selector); + + EasyMock.expect( + httpClient.go( + EasyMock.anyObject(Request.class), + EasyMock.anyObject(StatusResponseHandler.class) + ) + ).andReturn(Futures.immediateFuture(responseHolder)) + .times(1); + + EasyMock.replay(httpClient); + lookupReferencesManager.start(); + Assert.assertNull(lookupReferencesManager.get("testMockForDisableLookupSync")); + } + } diff --git a/server/src/test/java/io/druid/server/http/LookupCoordinatorResourceTest.java b/server/src/test/java/io/druid/server/http/LookupCoordinatorResourceTest.java index 499c236303b2..563542e86ad7 100644 --- a/server/src/test/java/io/druid/server/http/LookupCoordinatorResourceTest.java +++ b/server/src/test/java/io/druid/server/http/LookupCoordinatorResourceTest.java @@ -86,9 +86,10 @@ public InputStream openStream() throws IOException ImmutableMap.of(LOOKUP_NAME, SINGLE_LOOKUP), null, null ); - private static final Map> NODES_LOOKUP_STATE = ImmutableMap.of( - LOOKUP_NODE, LOOKUP_STATE - ); + private static final Map> NODES_LOOKUP_STATE = ImmutableMap + .of( + LOOKUP_NODE, LOOKUP_STATE + ); @Test public void testSimpleGet() @@ -208,6 +209,23 @@ public void testSimpleGetLookup() EasyMock.verify(lookupCoordinatorManager); } + @Test + public void testDetailedGetLookup() + { + final LookupCoordinatorManager lookupCoordinatorManager = EasyMock.createStrictMock(LookupCoordinatorManager.class); + EasyMock.expect(lookupCoordinatorManager.getKnownLookups()).andReturn(SINGLE_TIER_MAP).once(); + EasyMock.replay(lookupCoordinatorManager); + final LookupCoordinatorResource lookupCoordinatorResource = new LookupCoordinatorResource( + lookupCoordinatorManager, + mapper, + mapper + ); + final Response response = lookupCoordinatorResource.getSpecificTier(LOOKUP_TIER, true); + Assert.assertEquals(200, response.getStatus()); + Assert.assertEquals(SINGLE_TIER_MAP.get(LOOKUP_TIER), response.getEntity()); + EasyMock.verify(lookupCoordinatorManager); + } + @Test public void testMissingGetLookup() { @@ -765,7 +783,7 @@ public void testSimpleGetTier() mapper, mapper ); - final Response response = lookupCoordinatorResource.getSpecificTier(LOOKUP_TIER); + final Response response = lookupCoordinatorResource.getSpecificTier(LOOKUP_TIER, false); Assert.assertEquals(200, response.getStatus()); Assert.assertEquals(SINGLE_TIER_MAP.get(LOOKUP_TIER).keySet(), response.getEntity()); EasyMock.verify(lookupCoordinatorManager); @@ -785,7 +803,7 @@ public void testMissingGetTier() mapper, mapper ); - final Response response = lookupCoordinatorResource.getSpecificTier(tier); + final Response response = lookupCoordinatorResource.getSpecificTier(tier, false); Assert.assertEquals(404, response.getStatus()); EasyMock.verify(lookupCoordinatorManager); } @@ -801,7 +819,7 @@ public void testNullGetTier() mapper, mapper ); - final Response response = lookupCoordinatorResource.getSpecificTier(tier); + final Response response = lookupCoordinatorResource.getSpecificTier(tier, false); Assert.assertEquals(400, response.getStatus()); Assert.assertEquals(ImmutableMap.of("error", "`tier` required"), response.getEntity()); EasyMock.verify(lookupCoordinatorManager); @@ -819,7 +837,7 @@ public void testNullLookupsGetTier() mapper, mapper ); - final Response response = lookupCoordinatorResource.getSpecificTier(tier); + final Response response = lookupCoordinatorResource.getSpecificTier(tier, false); Assert.assertEquals(404, response.getStatus()); Assert.assertEquals(ImmutableMap.of("error", "No lookups found"), response.getEntity()); EasyMock.verify(lookupCoordinatorManager); @@ -838,7 +856,7 @@ public void testExceptionalGetTier() mapper, mapper ); - final Response response = lookupCoordinatorResource.getSpecificTier(tier); + final Response response = lookupCoordinatorResource.getSpecificTier(tier, false); Assert.assertEquals(500, response.getStatus()); Assert.assertEquals(ImmutableMap.of("error", errMsg), response.getEntity()); EasyMock.verify(lookupCoordinatorManager); diff --git a/sql/pom.xml b/sql/pom.xml index 5311c33c700a..1bbdf067d473 100644 --- a/sql/pom.xml +++ b/sql/pom.xml @@ -83,6 +83,13 @@ test-jar test + + io.druid + druid-server + ${project.parent.version} + test-jar + test + diff --git a/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java b/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java index e9d12f65ccad..27f4b69bc71f 100644 --- a/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java +++ b/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java @@ -57,7 +57,7 @@ import io.druid.query.aggregation.FloatSumAggregatorFactory; import io.druid.query.aggregation.hyperloglog.HyperUniquesAggregatorFactory; import io.druid.query.expression.LookupExprMacro; -import io.druid.query.expression.TestExprMacroTable; +import io.druid.query.expression.TestExpressionMacroTable; import io.druid.query.groupby.GroupByQuery; import io.druid.query.groupby.GroupByQueryConfig; import io.druid.query.groupby.GroupByQueryRunnerTest; @@ -140,7 +140,7 @@ public void configure(final Binder binder) binder.bind(LookupReferencesManager.class) .toInstance( - TestExprMacroTable.createTestLookupReferencesManager( + TestExpressionMacroTable.createTestLookupReferencesManager( ImmutableMap.of( "a", "xa", "abc", "xabc" From ed107036d021634cf1bf79d10a6bfb9a313fc52a Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 6 Sep 2017 15:58:35 -0500 Subject: [PATCH 02/20] Refactor of Lookup classes --- .../main/java/io/druid/query/dimension/LookupDimensionSpec.java | 0 .../src/main/java/io/druid/query/expression/LookupExprMacro.java | 0 .../main/java/io/druid/query/lookup/LookupReferencesManager.java | 0 .../java/io/druid/query/lookup/RegisteredLookupExtractionFn.java | 0 .../src/test/java/io/druid/query/expression/ExprMacroTest.java | 0 .../io/druid/query/lookup/RegisteredLookupExtractionFnTest.java | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename {processing => server}/src/main/java/io/druid/query/dimension/LookupDimensionSpec.java (100%) rename {processing => server}/src/main/java/io/druid/query/expression/LookupExprMacro.java (100%) rename {processing => server}/src/main/java/io/druid/query/lookup/LookupReferencesManager.java (100%) rename {processing => server}/src/main/java/io/druid/query/lookup/RegisteredLookupExtractionFn.java (100%) rename {processing => server}/src/test/java/io/druid/query/expression/ExprMacroTest.java (100%) rename {processing => server}/src/test/java/io/druid/query/lookup/RegisteredLookupExtractionFnTest.java (100%) diff --git a/processing/src/main/java/io/druid/query/dimension/LookupDimensionSpec.java b/server/src/main/java/io/druid/query/dimension/LookupDimensionSpec.java similarity index 100% rename from processing/src/main/java/io/druid/query/dimension/LookupDimensionSpec.java rename to server/src/main/java/io/druid/query/dimension/LookupDimensionSpec.java diff --git a/processing/src/main/java/io/druid/query/expression/LookupExprMacro.java b/server/src/main/java/io/druid/query/expression/LookupExprMacro.java similarity index 100% rename from processing/src/main/java/io/druid/query/expression/LookupExprMacro.java rename to server/src/main/java/io/druid/query/expression/LookupExprMacro.java diff --git a/processing/src/main/java/io/druid/query/lookup/LookupReferencesManager.java b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java similarity index 100% rename from processing/src/main/java/io/druid/query/lookup/LookupReferencesManager.java rename to server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java diff --git a/processing/src/main/java/io/druid/query/lookup/RegisteredLookupExtractionFn.java b/server/src/main/java/io/druid/query/lookup/RegisteredLookupExtractionFn.java similarity index 100% rename from processing/src/main/java/io/druid/query/lookup/RegisteredLookupExtractionFn.java rename to server/src/main/java/io/druid/query/lookup/RegisteredLookupExtractionFn.java diff --git a/processing/src/test/java/io/druid/query/expression/ExprMacroTest.java b/server/src/test/java/io/druid/query/expression/ExprMacroTest.java similarity index 100% rename from processing/src/test/java/io/druid/query/expression/ExprMacroTest.java rename to server/src/test/java/io/druid/query/expression/ExprMacroTest.java diff --git a/processing/src/test/java/io/druid/query/lookup/RegisteredLookupExtractionFnTest.java b/server/src/test/java/io/druid/query/lookup/RegisteredLookupExtractionFnTest.java similarity index 100% rename from processing/src/test/java/io/druid/query/lookup/RegisteredLookupExtractionFnTest.java rename to server/src/test/java/io/druid/query/lookup/RegisteredLookupExtractionFnTest.java From f61b5e27e4394ec79ac858fe17327ba0754d0a0a Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 6 Sep 2017 20:56:53 -0500 Subject: [PATCH 03/20] Minor refactors and doc update --- docs/content/querying/lookups.md | 1 + .../io/druid/query/lookup/LookupConfig.java | 14 +- .../query/lookup/LookupReferencesManager.java | 141 +++++++----------- 3 files changed, 58 insertions(+), 98 deletions(-) diff --git a/docs/content/querying/lookups.md b/docs/content/querying/lookups.md index 23ef47f6549e..8fdc0718d215 100644 --- a/docs/content/querying/lookups.md +++ b/docs/content/querying/lookups.md @@ -323,6 +323,7 @@ It is possible to save the configuration across restarts such that a node will n |Property|Description|Default| |--------|-----------|-------| |`druid.lookup.snapshotWorkingDir`| Working path used to store snapshot of current lookup configuration, leaving this property null will disable snapshot/bootstrap utility|null| +|`druid.lookup.numLookupLoadingThreads`| Number of threads for loading the lookups in parallel.This threadpool is created only at the time of start and destroyed once startup is done. It is not kept during the lifetime of the JVM.|10| ## Introspect a Lookup diff --git a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java index 3f1c6acc748a..4fb887607ef4 100644 --- a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java +++ b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java @@ -33,23 +33,23 @@ public class LookupConfig private int numLookupLoadingThreads = 10; @JsonProperty - private boolean disableLookupSync = false; + private boolean disableLookupSyncOnStartup = false; /** * @param snapshotWorkingDir working directory to store lookups snapshot file, passing null or empty string will disable the snapshot utility * @param numLookupLoadingThreads number of threads for loading the lookups as part of the synchronization process - * @param disableLookupSync decides whether the lookup synchronization process should be disabled at startup + * @param disableLookupSyncOnStartup decides whether the lookup synchronization process should be disabled at startup */ @JsonCreator public LookupConfig( @JsonProperty("snapshotWorkingDir") String snapshotWorkingDir, @JsonProperty("numLookupLoadingThreads") int numLookupLoadingThreads, - @JsonProperty("disableLookupSync") boolean disableLookupSync + @JsonProperty("disableLookupSyncOnStartup") boolean disableLookupSyncOnStartup ) { this.snapshotWorkingDir = Strings.nullToEmpty(snapshotWorkingDir); this.numLookupLoadingThreads = numLookupLoadingThreads; - this.disableLookupSync = disableLookupSync; + this.disableLookupSyncOnStartup = disableLookupSyncOnStartup; } public String getSnapshotWorkingDir() @@ -62,9 +62,9 @@ public int getNumLookupLoadingThreads() return numLookupLoadingThreads; } - public boolean getDisableLookupSync() + public boolean getDisableLookupSyncOnStartup() { - return disableLookupSync; + return disableLookupSyncOnStartup; } @@ -90,7 +90,7 @@ public String toString() return "LookupConfig{" + "snapshotWorkingDir='" + getSnapshotWorkingDir() + '\'' + " numLookupLoadingThreads='" + getNumLookupLoadingThreads() + '\'' + - " disableLookupSync='" + getDisableLookupSync() + '\'' + + " disableLookupSync='" + getDisableLookupSyncOnStartup() + '\'' + '}'; } } 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 71012ee98388..6d6a5b65585a 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -357,28 +357,19 @@ private void takeSnapshot(Map lookupMap private void loadAllLookupsAndInitStateRef() { - if (!lookupConfig.getDisableLookupSync()) { + if (!lookupConfig.getDisableLookupSyncOnStartup()) { String tier = lookupListeningAnnouncerConfig.getLookupTier(); - List lookupBeanList = new ArrayList<>(); - // Check if the coordinator is accessible - if (getCoordinatorUrl().isEmpty()) { - if (lookupSnapshotTaker != null) { - LOG.info("Coordinator is unavailable. Loading saved snapshot instead"); - lookupBeanList = getLookupListFromSnapshot(); - } - } else { - lookupBeanList = getLookupListFromCoordinator(tier); - if (lookupBeanList == null) { - LOG.info("Lookups not accessible from Coordinator. Loading saved snapshot instead"); - lookupBeanList = getLookupListFromSnapshot(); - } + List lookupBeanList = getLookupListFromCoordinator(tier); + if (lookupBeanList == null) { + LOG.info("Coordinator is unavailable. Loading saved snapshot instead"); + lookupBeanList = getLookupListFromSnapshot(); } - if (!lookupBeanList.isEmpty()) { + if (lookupBeanList != null) { ImmutableMap.Builder builder = ImmutableMap.builder(); ListeningScheduledExecutorService executorService = MoreExecutors.listeningDecorator( Executors.newScheduledThreadPool( lookupConfig.getNumLookupLoadingThreads(), - Execs.makeThreadFactory("LookupReferencesManager--%s") + Execs.makeThreadFactory("LookupReferencesManager-Startup-%s") ) ); try { @@ -390,12 +381,12 @@ private void loadAllLookupsAndInitStateRef() () -> { LookupExtractorFactoryContainer container = lookupBean.getContainer(); + LOG.info( + "Started lookup [%s]:[%s]", + lookupBean.getName(), + container + ); if (container.getLookupExtractorFactory().start()) { - LOG.info( - "Started lookup [%s]:[%s]", - lookupBean.getName(), - container - ); return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); } else { LOG.error( @@ -431,54 +422,60 @@ private void loadAllLookupsAndInitStateRef() stateRef.set(new LookupUpdateState(ImmutableMap.of(), ImmutableList.of(), ImmutableList.of())); } } else { - LOG.info("Lookup synchronization is disabled"); + LOG.info("Lookup loading on startup is disabled"); stateRef.set(new LookupUpdateState(ImmutableMap.of(), ImmutableList.of(), ImmutableList.of())); } } private List getLookupListFromCoordinator(String tier) { - try { - final StatusResponseHolder response = fetchLookupsForTier(tier); - List lookupBeanList = new ArrayList<>(); - if (!response.getStatus().equals(HttpResponseStatus.OK)) { - LOG.error( - "Error while fetching lookup code from Coordinator with status[%s] and content[%s]", - response.getStatus(), - response.getContent() - ); - if (lookupSnapshotTaker != null) { - LOG.info("Attempting to load saved snapshot"); - return null; - } - } else { - // Older version of getSpecificTier returns a list of lookup names. - // Lookup loading is performed via snapshot if older version is present. - if (response.getContent().startsWith("[")) { + // Check if the coordinator is accessible + if (!(getCoordinatorUrl() == null)) { + try { + final StatusResponseHolder response = fetchLookupsForTier(tier); + List lookupBeanList = new ArrayList<>(); + if (!response.getStatus().equals(HttpResponseStatus.OK)) { + LOG.error( + "Error while fetching lookup code from Coordinator with status[%s] and content[%s]", + response.getStatus(), + response.getContent() + ); return null; } else { - Map lookupMap = jsonMapper.readValue( - response.getContent(), - LOOKUPS_ALL_REFERENCE - ); - if (!lookupMap.isEmpty()) { - for (Map.Entry e : lookupMap.entrySet()) { - lookupBeanList.add(new LookupBean(e.getKey(), null, e.getValue())); + // Older version of getSpecificTier returns a list of lookup names. + // Lookup loading is performed via snapshot if older version is present. + // This check is only for backward compatibility and should be removed in a future release. + if (response.getContent().startsWith("[")) { + return null; + } else { + Map lookupMap = jsonMapper.readValue( + response.getContent(), + LOOKUPS_ALL_REFERENCE + ); + if (!lookupMap.isEmpty()) { + for (Map.Entry e : lookupMap.entrySet()) { + lookupBeanList.add(new LookupBean(e.getKey(), null, e.getValue())); + } } } } + return lookupBeanList; } - return lookupBeanList; - } - catch (Exception e) { - LOG.error(e, "Error while parsing lookup code for tier [%s] from response", tier); + catch (Exception e) { + LOG.error(e, "Error while parsing lookup code for tier [%s] from response", tier); + return null; + } + } else { return null; } } private List getLookupListFromSnapshot() { - return lookupSnapshotTaker.pullExistingSnapshot(); + if (lookupSnapshotTaker != null) { + return lookupSnapshotTaker.pullExistingSnapshot(); + } + return null; } private String getCoordinatorUrl() @@ -487,7 +484,7 @@ private String getCoordinatorUrl() final Server instance = selector.pick(); if (instance == null) { LOG.info("Coordinator instance unavailable."); - return ""; + return null; } return new URI( instance.getScheme(), @@ -501,8 +498,8 @@ private String getCoordinatorUrl() } catch (Exception e) { LOG.error("Error encountered while connecting to Coordinator"); + return null; } - return ""; } private LookupUpdateState atomicallyUpdateStateRef(Function fn) @@ -534,44 +531,6 @@ private StatusResponseHolder fetchLookupsForTier(String tier) ).get(); } - private LookupExtractorFactoryContainer getLookupEntryFromCode(String tier, String lookupCode) - { - final StatusResponseHolder lookupTierResponse; - try { - lookupTierResponse = httpClient.go( - new Request( - HttpMethod.GET, - new URL( - StringUtils.format( - "%s/lookups/%s/%s", - getCoordinatorUrl(), - tier, - lookupCode - ) - ) - ), - RESPONSE_HANDLER - ).get(); - if (!lookupTierResponse.getStatus().equals(HttpResponseStatus.OK)) { - throw new ISE( - "Error while fetching lookup entries from Coordinator with status[%s] and content[%s]", - lookupTierResponse.getStatus(), - lookupTierResponse.getContent() - ); - } else { - final LookupExtractorFactoryContainer lookupData = jsonMapper.readValue( - lookupTierResponse.getContent(), - LookupExtractorFactoryContainer.class - ); - return lookupData; - } - } - catch (Exception ioe) { - throw new ISE("Error while parsing lookup entries from response " - ); - } - } - @VisibleForTesting interface Notice { From cbc46ba0fb5d3ae305b8f4b1972bf93c045d86f8 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 13 Sep 2017 11:15:01 -0500 Subject: [PATCH 04/20] Change coordinator instance to be retrieved by DruidLeaderClient --- .../query/lookup/LookupReferencesManager.java | 131 ++---- .../lookup/LookupReferencesManagerTest.java | 376 +++++++----------- 2 files changed, 184 insertions(+), 323 deletions(-) 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 6d6a5b65585a..7f4153397906 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -23,7 +23,6 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Charsets; import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; @@ -34,17 +33,12 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.inject.Inject; import com.metamx.emitter.EmittingLogger; -import com.metamx.http.client.HttpClient; -import com.metamx.http.client.Request; -import com.metamx.http.client.response.StatusResponseHandler; -import com.metamx.http.client.response.StatusResponseHolder; +import com.metamx.http.client.response.FullResponseHolder; import io.druid.client.coordinator.Coordinator; -import io.druid.client.selector.Server; import io.druid.concurrent.Execs; import io.druid.concurrent.LifecycleLock; -import io.druid.curator.discovery.ServerDiscoverySelector; +import io.druid.discovery.DruidLeaderClient; import io.druid.guice.ManageLifecycle; -import io.druid.guice.annotations.Global; import io.druid.guice.annotations.Json; import io.druid.java.util.common.ISE; import io.druid.java.util.common.StringUtils; @@ -55,8 +49,6 @@ import javax.annotation.Nullable; import java.net.MalformedURLException; -import java.net.URI; -import java.net.URL; import java.util.AbstractMap; import java.util.ArrayList; import java.util.HashMap; @@ -103,14 +95,10 @@ public class LookupReferencesManager //for unit testing only private final boolean testMode; - private final ServerDiscoverySelector selector; - - private final HttpClient httpClient; + private final DruidLeaderClient druidLeaderClient; private final ObjectMapper jsonMapper; - private static final StatusResponseHandler RESPONSE_HANDLER = new StatusResponseHandler(Charsets.UTF_8); - private static final TypeReference> LOOKUPS_ALL_REFERENCE = new TypeReference>() { @@ -123,16 +111,16 @@ public class LookupReferencesManager @Inject public LookupReferencesManager( LookupConfig lookupConfig, @Json ObjectMapper objectMapper, - @Coordinator ServerDiscoverySelector selector, @Global HttpClient client, + @Coordinator DruidLeaderClient druidLeaderClient, LookupListeningAnnouncerConfig lookupListeningAnnouncerConfig ) { - this(lookupConfig, objectMapper, selector, client, lookupListeningAnnouncerConfig, false); + this(lookupConfig, objectMapper, druidLeaderClient, lookupListeningAnnouncerConfig, false); } @VisibleForTesting LookupReferencesManager( - LookupConfig lookupConfig, ObjectMapper objectMapper, ServerDiscoverySelector selector, HttpClient client, + LookupConfig lookupConfig, ObjectMapper objectMapper, DruidLeaderClient druidLeaderClient, LookupListeningAnnouncerConfig lookupListeningAnnouncerConfig, boolean testMode ) { @@ -141,8 +129,7 @@ public LookupReferencesManager( } else { this.lookupSnapshotTaker = new LookupSnapshotTaker(objectMapper, lookupConfig.getSnapshotWorkingDir()); } - this.selector = selector; - this.httpClient = client; + this.druidLeaderClient = druidLeaderClient; this.jsonMapper = objectMapper; this.lookupListeningAnnouncerConfig = lookupListeningAnnouncerConfig; this.lookupConfig = lookupConfig; @@ -429,43 +416,42 @@ private void loadAllLookupsAndInitStateRef() private List getLookupListFromCoordinator(String tier) { - // Check if the coordinator is accessible - if (!(getCoordinatorUrl() == null)) { - try { - final StatusResponseHolder response = fetchLookupsForTier(tier); - List lookupBeanList = new ArrayList<>(); - if (!response.getStatus().equals(HttpResponseStatus.OK)) { - LOG.error( - "Error while fetching lookup code from Coordinator with status[%s] and content[%s]", - response.getStatus(), - response.getContent() - ); + try { + final FullResponseHolder response = fetchLookupsForTier(tier); + List lookupBeanList = new ArrayList<>(); + if (!response.getStatus().equals(HttpResponseStatus.OK)) { + LOG.error( + "Error while fetching lookup code from Coordinator with status[%s] and content[%s]", + response.getStatus(), + response.getContent() + ); + return null; + } else { + // Older version of getSpecificTier returns a list of lookup names. + // Lookup loading is performed via snapshot if older version is present. + // This check is only for backward compatibility and should be removed in a future release + if (response.getContent().startsWith("[")) { return null; } else { - // Older version of getSpecificTier returns a list of lookup names. - // Lookup loading is performed via snapshot if older version is present. - // This check is only for backward compatibility and should be removed in a future release. - if (response.getContent().startsWith("[")) { - return null; - } else { - Map lookupMap = jsonMapper.readValue( - response.getContent(), - LOOKUPS_ALL_REFERENCE - ); - if (!lookupMap.isEmpty()) { - for (Map.Entry e : lookupMap.entrySet()) { - lookupBeanList.add(new LookupBean(e.getKey(), null, e.getValue())); - } + Map lookupMap = jsonMapper.readValue( + response.getContent(), + LOOKUPS_ALL_REFERENCE + ); + if (!lookupMap.isEmpty()) { + for (Map.Entry e : lookupMap.entrySet()) { + lookupBeanList.add(new LookupBean(e.getKey(), null, e.getValue())); } } } - return lookupBeanList; } - catch (Exception e) { - LOG.error(e, "Error while parsing lookup code for tier [%s] from response", tier); - return null; - } - } else { + return lookupBeanList; + } + catch (ISE ise) { + LOG.error(ise, "Coordinator is unavailable"); + return null; + } + catch (Exception e) { + LOG.error(e, "Error while parsing lookup code for tier [%s] from response", tier); return null; } } @@ -478,30 +464,6 @@ private List getLookupListFromSnapshot() return null; } - private String getCoordinatorUrl() - { - try { - final Server instance = selector.pick(); - if (instance == null) { - LOG.info("Coordinator instance unavailable."); - return null; - } - return new URI( - instance.getScheme(), - null, - instance.getAddress(), - instance.getPort(), - "/druid/coordinator/v1", - null, - null - ).toString(); - } - catch (Exception e) { - LOG.error("Error encountered while connecting to Coordinator"); - return null; - } - } - private LookupUpdateState atomicallyUpdateStateRef(Function fn) { while (true) { @@ -513,22 +475,17 @@ private LookupUpdateState atomicallyUpdateStateRef(Functionof( "key", @@ -125,38 +92,34 @@ public String getAddress() mapper.registerSubtypes(MapLookupExtractorFactory.class); lookupReferencesManager = new LookupReferencesManager( new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), LOOKUP_THREADS, LOOKUP_DISABLE), - mapper, selector, httpClient, config, + mapper, druidLeaderClient, config, true ); } @Test - public void testStartStop() throws JsonProcessingException + public void testStartStop() throws JsonProcessingException, InterruptedException, MalformedURLException { lookupReferencesManager = new LookupReferencesManager( new LookupConfig(null, LOOKUP_THREADS, LOOKUP_DISABLE), - mapper, selector, httpClient, config + mapper, druidLeaderClient, config ); Map lookupMap = new HashMap<>(); lookupMap.put("testMockForStartStop", container); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.OK, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(server).times(2); - replay(selector); - - EasyMock.expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)) - .times(1); - - EasyMock.replay(httpClient); + expect(druidLeaderClient.go(request)).andReturn(responseHolder); + replay(druidLeaderClient); Assert.assertFalse(lookupReferencesManager.lifecycleLock.awaitStarted(1, TimeUnit.MICROSECONDS)); Assert.assertNull(lookupReferencesManager.mainThread); Assert.assertNull(lookupReferencesManager.stateRef.get()); @@ -206,22 +169,18 @@ public void testAddGetRemove() throws Exception Map lookupMap = new HashMap<>(); lookupMap.put("testMockForAddGetRemove", container); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.OK, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(server).times(2); - replay(selector); - - EasyMock.expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)) - .times(1); - - EasyMock.replay(httpClient); + expect(druidLeaderClient.go(request)).andReturn(responseHolder); + replay(druidLeaderClient); lookupReferencesManager.start(); Assert.assertNull(lookupReferencesManager.get("test")); @@ -248,22 +207,18 @@ public void testCloseIsCalledAfterStopping() throws Exception Map lookupMap = new HashMap<>(); lookupMap.put("testMockForCloseIsCalledAfterStopping", container); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.OK, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(server).times(2); - replay(selector); - - EasyMock.expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)) - .times(1); - - EasyMock.replay(httpClient); + expect(druidLeaderClient.go(request)).andReturn(responseHolder); + replay(druidLeaderClient); lookupReferencesManager.start(); lookupReferencesManager.add("testMock", new LookupExtractorFactoryContainer("0", lookupExtractorFactory)); lookupReferencesManager.handlePendingNotices(); @@ -283,22 +238,18 @@ public void testCloseIsCalledAfterRemove() throws Exception Map lookupMap = new HashMap<>(); lookupMap.put("testMockForCloseIsCalledAfterRemove", container); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.OK, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(server).times(2); - replay(selector); - - EasyMock.expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)) - .times(1); - - EasyMock.replay(httpClient); + expect(druidLeaderClient.go(request)).andReturn(responseHolder); + replay(druidLeaderClient); lookupReferencesManager.start(); lookupReferencesManager.add("testMock", new LookupExtractorFactoryContainer("0", lookupExtractorFactory)); lookupReferencesManager.handlePendingNotices(); @@ -315,22 +266,18 @@ public void testGetNotThere() throws Exception Map lookupMap = new HashMap<>(); lookupMap.put("testMockForGetNotThere", container); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.OK, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(server).times(2); - replay(selector); - - EasyMock.expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)) - .times(1); - - EasyMock.replay(httpClient); + expect(druidLeaderClient.go(request)).andReturn(responseHolder); + replay(druidLeaderClient); lookupReferencesManager.start(); Assert.assertNull(lookupReferencesManager.get("notThere")); } @@ -349,22 +296,18 @@ public void testUpdateWithHigherVersion() throws Exception Map lookupMap = new HashMap<>(); lookupMap.put("testMockForUpdateWithHigherVersion", container); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.OK, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(server).times(2); - replay(selector); - - EasyMock.expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)) - .times(1); - - EasyMock.replay(httpClient); + expect(druidLeaderClient.go(request)).andReturn(responseHolder); + replay(druidLeaderClient); lookupReferencesManager.start(); lookupReferencesManager.add("testName", new LookupExtractorFactoryContainer("1", lookupExtractorFactory1)); lookupReferencesManager.handlePendingNotices(); @@ -387,22 +330,18 @@ public void testUpdateWithLowerVersion() throws Exception Map lookupMap = new HashMap<>(); lookupMap.put("testMockForUpdateWithLowerVersion", container); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.OK, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(server).times(2); - replay(selector); - - EasyMock.expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)) - .times(1); - - EasyMock.replay(httpClient); + expect(druidLeaderClient.go(request)).andReturn(responseHolder); + replay(druidLeaderClient); lookupReferencesManager.start(); lookupReferencesManager.add("testName", new LookupExtractorFactoryContainer("1", lookupExtractorFactory1)); lookupReferencesManager.handlePendingNotices(); @@ -419,22 +358,18 @@ public void testRemoveNonExisting() throws Exception Map lookupMap = new HashMap<>(); lookupMap.put("testMockForRemoveNonExisting", container); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.OK, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(server).times(2); - replay(selector); - - EasyMock.expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)) - .times(1); - - EasyMock.replay(httpClient); + expect(druidLeaderClient.go(request)).andReturn(responseHolder); + replay(druidLeaderClient); lookupReferencesManager.start(); lookupReferencesManager.remove("test"); lookupReferencesManager.handlePendingNotices(); @@ -474,22 +409,18 @@ public void testGetAllLookupsState() throws Exception ); Map lookupMap = new HashMap<>(); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.OK, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(server).times(2); - replay(selector); - - EasyMock.expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)) - .times(1); - - EasyMock.replay(httpClient); + expect(druidLeaderClient.go(request)).andReturn(responseHolder); + replay(druidLeaderClient); lookupReferencesManager.start(); lookupReferencesManager.add("one", container1); lookupReferencesManager.add("two", container2); @@ -515,27 +446,23 @@ public void testRealModeWithMainThread() throws Exception { LookupReferencesManager lookupReferencesManager = new LookupReferencesManager( new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), LOOKUP_THREADS, LOOKUP_DISABLE), - mapper, selector, httpClient, config + mapper, druidLeaderClient, config ); Map lookupMap = new HashMap<>(); lookupMap.put("testMockForRealModeWithMainThread", container); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.OK, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(server).times(2); - replay(selector); - - EasyMock.expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)) - .times(1); - - EasyMock.replay(httpClient); + expect(druidLeaderClient.go(request)).andReturn(responseHolder); + replay(druidLeaderClient); lookupReferencesManager.start(); Assert.assertTrue(lookupReferencesManager.mainThread.isAlive()); @@ -600,25 +527,18 @@ public void testCoordinatorLookupSync() throws Exception lookupMap.put("testLookup2", container2); lookupMap.put("testLookup3", container3); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.OK, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(server).times(2); - replay(selector); - - expect(config.getLookupTier()).andReturn(LOOKUP_TIER); - replay(config); - - expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)).times(1); - - - replay(httpClient); + expect(druidLeaderClient.go(request)).andReturn(responseHolder); + replay(druidLeaderClient); lookupReferencesManager.start(); Assert.assertEquals(container1, lookupReferencesManager.get("testLookup1")); @@ -633,52 +553,40 @@ public void testLoadLookupOnCoordinatorFailure() throws Exception Map lookupMap = new HashMap<>(); lookupMap.put("testMockForLoadLookupOnCoordinatorFailure", container); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.NOT_FOUND, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(null); - replay(selector); - - expect(config.getLookupTier()).andReturn(LOOKUP_TIER); - replay(config); - - expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)).times(1); - - - replay(httpClient); + expect(druidLeaderClient.go(request)).andThrow(new IllegalStateException()); + replay(druidLeaderClient); lookupReferencesManager.start(); lookupReferencesManager.add("testMockForLoadLookupOnCoordinatorFailure", container); lookupReferencesManager.handlePendingNotices(); lookupReferencesManager.stop(); lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(lookupReferencesManager.lookupSnapshotTaker.getPersistFile().getParent(), - LOOKUP_THREADS, - LOOKUP_DISABLE), - mapper, selector, httpClient, config, + new LookupConfig( + lookupReferencesManager.lookupSnapshotTaker.getPersistFile().getParent(), + LOOKUP_THREADS, + LOOKUP_DISABLE + ), + mapper, druidLeaderClient, config, true ); reset(config); - reset(selector); - reset(httpClient); - - expect(selector.pick()).andReturn(null); - replay(selector); + reset(druidLeaderClient); expect(config.getLookupTier()).andReturn(LOOKUP_TIER); replay(config); - expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)).times(1); - replay(httpClient); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + expect(druidLeaderClient.go(request)).andThrow(new IllegalStateException()); + replay(druidLeaderClient); lookupReferencesManager.start(); Assert.assertEquals(container, lookupReferencesManager.get("testMockForLoadLookupOnCoordinatorFailure")); } @@ -688,27 +596,23 @@ public void testDisableLookupSync() throws Exception { LookupReferencesManager lookupReferencesManager = new LookupReferencesManager( new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), LOOKUP_THREADS, true), - mapper, selector, httpClient, config + mapper, druidLeaderClient, config ); Map lookupMap = new HashMap<>(); lookupMap.put("testMockForDisableLookupSync", container); String strResult = mapper.writeValueAsString(lookupMap); - StatusResponseHolder responseHolder = new StatusResponseHolder( + Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx")); + expect(config.getLookupTier()).andReturn(LOOKUP_TIER); + replay(config); + expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true")) + .andReturn(request); + FullResponseHolder responseHolder = new FullResponseHolder( HttpResponseStatus.OK, + EasyMock.createNiceMock(HttpResponse.class), new StringBuilder().append(strResult) ); - expect(selector.pick()).andReturn(server).times(2); - replay(selector); - - EasyMock.expect( - httpClient.go( - EasyMock.anyObject(Request.class), - EasyMock.anyObject(StatusResponseHandler.class) - ) - ).andReturn(Futures.immediateFuture(responseHolder)) - .times(1); + expect(druidLeaderClient.go(request)).andReturn(responseHolder); - EasyMock.replay(httpClient); lookupReferencesManager.start(); Assert.assertNull(lookupReferencesManager.get("testMockForDisableLookupSync")); } From 8131326b3e8e317f201985a8e1bbdb5ff38c7829 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 13 Sep 2017 16:01:00 -0500 Subject: [PATCH 05/20] Wait before thread shutdown --- docs/content/querying/lookups.md | 3 ++- .../query/lookup/LookupReferencesManager.java | 24 +++++++++++++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/docs/content/querying/lookups.md b/docs/content/querying/lookups.md index 8fdc0718d215..1a221f605800 100644 --- a/docs/content/querying/lookups.md +++ b/docs/content/querying/lookups.md @@ -323,7 +323,8 @@ It is possible to save the configuration across restarts such that a node will n |Property|Description|Default| |--------|-----------|-------| |`druid.lookup.snapshotWorkingDir`| Working path used to store snapshot of current lookup configuration, leaving this property null will disable snapshot/bootstrap utility|null| -|`druid.lookup.numLookupLoadingThreads`| Number of threads for loading the lookups in parallel.This threadpool is created only at the time of start and destroyed once startup is done. It is not kept during the lifetime of the JVM.|10| +|`druid.lookup.numLookupLoadingThreads`| Number of threads for loading the lookups in parallel on startup.This threadpool is created only at the time of start and destroyed once startup is done. It is not kept during the lifetime of the JVM.|10| +|`druid.lookup.disableLookupSyncOnStartup`|Disable the lookup synchronization process on startup.|false| ## Introspect a Lookup 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 7f4153397906..44fe8b8c6eef 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -25,6 +25,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.Futures; @@ -369,11 +370,16 @@ private void loadAllLookupsAndInitStateRef() LookupExtractorFactoryContainer container = lookupBean.getContainer(); LOG.info( - "Started lookup [%s]:[%s]", + "Starting lookup [%s]:[%s]", lookupBean.getName(), container ); if (container.getLookupExtractorFactory().start()) { + LOG.info( + "Started lookup [%s]:[%s]", + lookupBean.getName(), + container + ); return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); } else { LOG.error( @@ -393,17 +399,27 @@ private void loadAllLookupsAndInitStateRef() .stream() .filter(Objects::nonNull) .forEach(builder::put); + + stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); + + executorService.shutdownNow(); + if (!executorService.awaitTermination(60, TimeUnit.SECONDS)){ + LOG.warn("Lookup loading didn't complete in 60 seconds!"); + } + } + catch (InterruptedException e) { + LOG.error("Lookup loading interrupted. "); + Thread.currentThread().interrupt(); + } catch (Exception ex) { futureList.cancel(true); throw ex; } - stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); } catch (Exception e) { LOG.error(e, "Failed to finish lookup load process."); } - executorService.shutdownNow(); } else { LOG.info("No lookups to be loaded at this point"); stateRef.set(new LookupUpdateState(ImmutableMap.of(), ImmutableList.of(), ImmutableList.of())); @@ -451,7 +467,7 @@ private List getLookupListFromCoordinator(String tier) return null; } catch (Exception e) { - LOG.error(e, "Error while parsing lookup code for tier [%s] from response", tier); + LOG.error(e, "Error while trying to get lookup list from coordinator for tier[%s]", tier); return null; } } From 848fd7393331dc8fb71b5b9ea95d918ff4f4a269 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Thu, 14 Sep 2017 22:45:57 -0500 Subject: [PATCH 06/20] Make disablelookups flag true by default --- .../io/druid/query/lookup/LookupConfig.java | 6 +++--- .../query/lookup/LookupReferencesManager.java | 17 ++++++----------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java index 4fb887607ef4..58f136e71d2b 100644 --- a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java +++ b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java @@ -33,7 +33,7 @@ public class LookupConfig private int numLookupLoadingThreads = 10; @JsonProperty - private boolean disableLookupSyncOnStartup = false; + private boolean disableLookupSyncOnStartup; /** * @param snapshotWorkingDir working directory to store lookups snapshot file, passing null or empty string will disable the snapshot utility @@ -44,12 +44,12 @@ public class LookupConfig public LookupConfig( @JsonProperty("snapshotWorkingDir") String snapshotWorkingDir, @JsonProperty("numLookupLoadingThreads") int numLookupLoadingThreads, - @JsonProperty("disableLookupSyncOnStartup") boolean disableLookupSyncOnStartup + @JsonProperty("disableLookupSyncOnStartup") Boolean disableLookupSyncOnStartup ) { this.snapshotWorkingDir = Strings.nullToEmpty(snapshotWorkingDir); this.numLookupLoadingThreads = numLookupLoadingThreads; - this.disableLookupSyncOnStartup = disableLookupSyncOnStartup; + this.disableLookupSyncOnStartup = disableLookupSyncOnStartup == null ? true : disableLookupSyncOnStartup; } public String getSnapshotWorkingDir() 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 44fe8b8c6eef..8ee3e2facd8e 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -25,7 +25,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Strings; -import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.Futures; @@ -345,13 +344,17 @@ private void takeSnapshot(Map lookupMap private void loadAllLookupsAndInitStateRef() { + List lookupBeanList = new ArrayList<>(); if (!lookupConfig.getDisableLookupSyncOnStartup()) { String tier = lookupListeningAnnouncerConfig.getLookupTier(); - List lookupBeanList = getLookupListFromCoordinator(tier); + lookupBeanList = getLookupListFromCoordinator(tier); if (lookupBeanList == null) { LOG.info("Coordinator is unavailable. Loading saved snapshot instead"); lookupBeanList = getLookupListFromSnapshot(); } + } else { + lookupBeanList = getLookupListFromSnapshot(); + } if (lookupBeanList != null) { ImmutableMap.Builder builder = ImmutableMap.builder(); ListeningScheduledExecutorService executorService = MoreExecutors.listeningDecorator( @@ -403,7 +406,7 @@ private void loadAllLookupsAndInitStateRef() stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); executorService.shutdownNow(); - if (!executorService.awaitTermination(60, TimeUnit.SECONDS)){ + if (!executorService.awaitTermination(60, TimeUnit.SECONDS)) { LOG.warn("Lookup loading didn't complete in 60 seconds!"); } } @@ -424,10 +427,6 @@ private void loadAllLookupsAndInitStateRef() LOG.info("No lookups to be loaded at this point"); stateRef.set(new LookupUpdateState(ImmutableMap.of(), ImmutableList.of(), ImmutableList.of())); } - } else { - LOG.info("Lookup loading on startup is disabled"); - stateRef.set(new LookupUpdateState(ImmutableMap.of(), ImmutableList.of(), ImmutableList.of())); - } } private List getLookupListFromCoordinator(String tier) @@ -462,10 +461,6 @@ private List getLookupListFromCoordinator(String tier) } return lookupBeanList; } - catch (ISE ise) { - LOG.error(ise, "Coordinator is unavailable"); - return null; - } catch (Exception e) { LOG.error(e, "Error while trying to get lookup list from coordinator for tier[%s]", tier); return null; From d0d466757a4e68fd2f3ef28a4ca1188633fa0bb3 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Fri, 15 Sep 2017 08:37:33 -0500 Subject: [PATCH 07/20] Update docs --- docs/content/querying/lookups.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/querying/lookups.md b/docs/content/querying/lookups.md index 1a221f605800..09b9e3963cea 100644 --- a/docs/content/querying/lookups.md +++ b/docs/content/querying/lookups.md @@ -324,7 +324,7 @@ It is possible to save the configuration across restarts such that a node will n |--------|-----------|-------| |`druid.lookup.snapshotWorkingDir`| Working path used to store snapshot of current lookup configuration, leaving this property null will disable snapshot/bootstrap utility|null| |`druid.lookup.numLookupLoadingThreads`| Number of threads for loading the lookups in parallel on startup.This threadpool is created only at the time of start and destroyed once startup is done. It is not kept during the lifetime of the JVM.|10| -|`druid.lookup.disableLookupSyncOnStartup`|Disable the lookup synchronization process on startup.|false| +|`druid.lookup.disableLookupSyncOnStartup`|Disable the lookup synchronization process on startup.|true| ## Introspect a Lookup From 6f6b155777d6cd9b4197fbfeb59662883101033f Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Fri, 15 Sep 2017 08:59:12 -0500 Subject: [PATCH 08/20] Rename flag --- docs/content/querying/lookups.md | 2 +- .../java/io/druid/query/lookup/LookupConfig.java | 14 +++++++------- .../query/lookup/LookupReferencesManager.java | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/content/querying/lookups.md b/docs/content/querying/lookups.md index 09b9e3963cea..bd3c8c926107 100644 --- a/docs/content/querying/lookups.md +++ b/docs/content/querying/lookups.md @@ -324,7 +324,7 @@ It is possible to save the configuration across restarts such that a node will n |--------|-----------|-------| |`druid.lookup.snapshotWorkingDir`| Working path used to store snapshot of current lookup configuration, leaving this property null will disable snapshot/bootstrap utility|null| |`druid.lookup.numLookupLoadingThreads`| Number of threads for loading the lookups in parallel on startup.This threadpool is created only at the time of start and destroyed once startup is done. It is not kept during the lifetime of the JVM.|10| -|`druid.lookup.disableLookupSyncOnStartup`|Disable the lookup synchronization process on startup.|true| +|`druid.lookup.enableLookupSyncOnStartup`|Enable the lookup synchronization process on startup.|false| ## Introspect a Lookup diff --git a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java index 58f136e71d2b..c417d12aaa06 100644 --- a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java +++ b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java @@ -33,23 +33,23 @@ public class LookupConfig private int numLookupLoadingThreads = 10; @JsonProperty - private boolean disableLookupSyncOnStartup; + private boolean enableLookupSyncOnStartup; /** * @param snapshotWorkingDir working directory to store lookups snapshot file, passing null or empty string will disable the snapshot utility * @param numLookupLoadingThreads number of threads for loading the lookups as part of the synchronization process - * @param disableLookupSyncOnStartup decides whether the lookup synchronization process should be disabled at startup + * @param enableLookupSyncOnStartup decides whether the lookup synchronization process should be enabled at startup */ @JsonCreator public LookupConfig( @JsonProperty("snapshotWorkingDir") String snapshotWorkingDir, @JsonProperty("numLookupLoadingThreads") int numLookupLoadingThreads, - @JsonProperty("disableLookupSyncOnStartup") Boolean disableLookupSyncOnStartup + @JsonProperty("enableLookupSyncOnStartup") Boolean enableLookupSyncOnStartup ) { this.snapshotWorkingDir = Strings.nullToEmpty(snapshotWorkingDir); this.numLookupLoadingThreads = numLookupLoadingThreads; - this.disableLookupSyncOnStartup = disableLookupSyncOnStartup == null ? true : disableLookupSyncOnStartup; + this.enableLookupSyncOnStartup = enableLookupSyncOnStartup == null ? false : enableLookupSyncOnStartup; } public String getSnapshotWorkingDir() @@ -62,9 +62,9 @@ public int getNumLookupLoadingThreads() return numLookupLoadingThreads; } - public boolean getDisableLookupSyncOnStartup() + public boolean getEnableLookupSyncOnStartup() { - return disableLookupSyncOnStartup; + return enableLookupSyncOnStartup; } @@ -90,7 +90,7 @@ public String toString() return "LookupConfig{" + "snapshotWorkingDir='" + getSnapshotWorkingDir() + '\'' + " numLookupLoadingThreads='" + getNumLookupLoadingThreads() + '\'' + - " disableLookupSync='" + getDisableLookupSyncOnStartup() + '\'' + + " disableLookupSync='" + getEnableLookupSyncOnStartup() + '\'' + '}'; } } 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 8ee3e2facd8e..5bbe4fac8bc7 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -345,7 +345,7 @@ private void takeSnapshot(Map lookupMap private void loadAllLookupsAndInitStateRef() { List lookupBeanList = new ArrayList<>(); - if (!lookupConfig.getDisableLookupSyncOnStartup()) { + if (lookupConfig.getEnableLookupSyncOnStartup()) { String tier = lookupListeningAnnouncerConfig.getLookupTier(); lookupBeanList = getLookupListFromCoordinator(tier); if (lookupBeanList == null) { From d13e916dce5ef1e0e91640a7757a7b6cd3a96d39 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 20 Sep 2017 21:33:31 -0500 Subject: [PATCH 09/20] Move executorservice shutdown to finally block --- docs/content/querying/lookups.md | 2 +- .../query/lookup/LookupReferencesManager.java | 159 +++++++++--------- 2 files changed, 82 insertions(+), 79 deletions(-) diff --git a/docs/content/querying/lookups.md b/docs/content/querying/lookups.md index bd3c8c926107..473c3615d859 100644 --- a/docs/content/querying/lookups.md +++ b/docs/content/querying/lookups.md @@ -324,7 +324,7 @@ It is possible to save the configuration across restarts such that a node will n |--------|-----------|-------| |`druid.lookup.snapshotWorkingDir`| Working path used to store snapshot of current lookup configuration, leaving this property null will disable snapshot/bootstrap utility|null| |`druid.lookup.numLookupLoadingThreads`| Number of threads for loading the lookups in parallel on startup.This threadpool is created only at the time of start and destroyed once startup is done. It is not kept during the lifetime of the JVM.|10| -|`druid.lookup.enableLookupSyncOnStartup`|Enable the lookup synchronization process on startup.|false| +|`druid.lookup.enableLookupSyncOnStartup`|Enable the lookup synchronization process with coordinator on startup.|false| ## Introspect a Lookup 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 5bbe4fac8bc7..c909f1944cb4 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -104,9 +104,9 @@ public class LookupReferencesManager { }; - private LookupListeningAnnouncerConfig lookupListeningAnnouncerConfig; + private final LookupListeningAnnouncerConfig lookupListeningAnnouncerConfig; - private LookupConfig lookupConfig; + private final LookupConfig lookupConfig; @Inject public LookupReferencesManager( @@ -355,78 +355,81 @@ private void loadAllLookupsAndInitStateRef() } else { lookupBeanList = getLookupListFromSnapshot(); } - if (lookupBeanList != null) { - ImmutableMap.Builder builder = ImmutableMap.builder(); - ListeningScheduledExecutorService executorService = MoreExecutors.listeningDecorator( - Executors.newScheduledThreadPool( - lookupConfig.getNumLookupLoadingThreads(), - Execs.makeThreadFactory("LookupReferencesManager-Startup-%s") - ) - ); - try { - List> futures = new ArrayList<>(); - LOG.info("Starting lookup loading process"); - for (LookupBean lookupBean : lookupBeanList) { - futures.add( - executorService.submit( - () -> { - - LookupExtractorFactoryContainer container = lookupBean.getContainer(); + if (lookupBeanList != null) { + ImmutableMap.Builder builder = ImmutableMap.builder(); + ListeningScheduledExecutorService executorService = MoreExecutors.listeningDecorator( + Executors.newScheduledThreadPool( + lookupConfig.getNumLookupLoadingThreads(), + Execs.makeThreadFactory("LookupReferencesManager-Startup-%s") + ) + ); + try { + List> futures = new ArrayList<>(); + LOG.info("Starting lookup loading process"); + for (LookupBean lookupBean : lookupBeanList) { + futures.add( + executorService.submit( + () -> { + + LookupExtractorFactoryContainer container = lookupBean.getContainer(); + LOG.info( + "Starting lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + if (container.getLookupExtractorFactory().start()) { LOG.info( - "Starting lookup [%s]:[%s]", + "Started lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); + } else { + LOG.error( + "Failed to start lookup [%s]:[%s]", lookupBean.getName(), container ); - if (container.getLookupExtractorFactory().start()) { - LOG.info( - "Started lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); - } else { - LOG.error( - "Failed to start lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - return null; - } + return null; } - ) - ); - } - final ListenableFuture> futureList = Futures.allAsList(futures); - try { - futureList.get() - .stream() - .filter(Objects::nonNull) - .forEach(builder::put); - - stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); - - executorService.shutdownNow(); - if (!executorService.awaitTermination(60, TimeUnit.SECONDS)) { - LOG.warn("Lookup loading didn't complete in 60 seconds!"); - } - } - catch (InterruptedException e) { - LOG.error("Lookup loading interrupted. "); - Thread.currentThread().interrupt(); + } + ) + ); + } + final ListenableFuture> futureList = Futures.allAsList(futures); + try { + futureList.get() + .stream() + .filter(Objects::nonNull) + .forEach(builder::put); - } - catch (Exception ex) { - futureList.cancel(true); - throw ex; + stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); + } + catch (Exception ex) { + futureList.cancel(true); + throw ex; + } + } + catch (Exception e) { + LOG.error(e, "Failed to finish lookup load process."); + } + finally { + try { + executorService.shutdownNow(); + if (!executorService.awaitTermination(60, TimeUnit.SECONDS)) { + LOG.warn("Lookup loading didn't complete in 60 seconds!"); } } - catch (Exception e) { - LOG.error(e, "Failed to finish lookup load process."); + catch (InterruptedException e) { + LOG.error("Lookup loading interrupted. "); + Thread.currentThread().interrupt(); + } - } else { - LOG.info("No lookups to be loaded at this point"); - stateRef.set(new LookupUpdateState(ImmutableMap.of(), ImmutableList.of(), ImmutableList.of())); } + } else { + LOG.info("No lookups to be loaded at this point"); + stateRef.set(new LookupUpdateState(ImmutableMap.of(), ImmutableList.of(), ImmutableList.of())); + } } private List getLookupListFromCoordinator(String tier) @@ -441,21 +444,21 @@ private List getLookupListFromCoordinator(String tier) response.getContent() ); return null; + } + + // Older version of getSpecificTier returns a list of lookup names. + // Lookup loading is performed via snapshot if older version is present. + // This check is only for backward compatibility and should be removed in a future release + if (response.getContent().startsWith("[")) { + return null; } else { - // Older version of getSpecificTier returns a list of lookup names. - // Lookup loading is performed via snapshot if older version is present. - // This check is only for backward compatibility and should be removed in a future release - if (response.getContent().startsWith("[")) { - return null; - } else { - Map lookupMap = jsonMapper.readValue( - response.getContent(), - LOOKUPS_ALL_REFERENCE - ); - if (!lookupMap.isEmpty()) { - for (Map.Entry e : lookupMap.entrySet()) { - lookupBeanList.add(new LookupBean(e.getKey(), null, e.getValue())); - } + Map lookupMap = jsonMapper.readValue( + response.getContent(), + LOOKUPS_ALL_REFERENCE + ); + if (!lookupMap.isEmpty()) { + for (Map.Entry e : lookupMap.entrySet()) { + lookupBeanList.add(new LookupBean(e.getKey(), null, e.getValue())); } } } From 447efc36bbe95b2fd40891deef8cddd3b848352b Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Thu, 28 Sep 2017 11:50:53 -0500 Subject: [PATCH 10/20] Merge conflicts --- docs/content/querying/lookups.md | 4 +- .../io/druid/query/lookup/LookupConfig.java | 14 +- .../druid/query/lookup/LookupConfigTest.java | 2 +- .../query/lookup/LookupReferencesManager.java | 142 +++++++++++------- .../expression/TestExpressionMacroTable.java | 5 +- .../lookup/LookupReferencesManagerTest.java | 12 +- 6 files changed, 104 insertions(+), 75 deletions(-) diff --git a/docs/content/querying/lookups.md b/docs/content/querying/lookups.md index 473c3615d859..08a73ea2ae3d 100644 --- a/docs/content/querying/lookups.md +++ b/docs/content/querying/lookups.md @@ -323,8 +323,8 @@ It is possible to save the configuration across restarts such that a node will n |Property|Description|Default| |--------|-----------|-------| |`druid.lookup.snapshotWorkingDir`| Working path used to store snapshot of current lookup configuration, leaving this property null will disable snapshot/bootstrap utility|null| -|`druid.lookup.numLookupLoadingThreads`| Number of threads for loading the lookups in parallel on startup.This threadpool is created only at the time of start and destroyed once startup is done. It is not kept during the lifetime of the JVM.|10| -|`druid.lookup.enableLookupSyncOnStartup`|Enable the lookup synchronization process with coordinator on startup.|false| +|`druid.lookup.numLookupLoadingThreads`| Number of threads for loading the lookups in parallel on startup. This thread pool is destroyed once startup is done. It is not kept during the lifetime of the JVM|Available Processors / 2| +|`druid.lookup.enableLookupSyncOnStartup`|Enable the lookup synchronization process with coordinator on startup. The queryable nodes will fetch and load the lookups from the coordinator instead of waiting for the coordinator to load the lookups for them. This option is recommended only in cases where the queryable nodes have lookups configured|false| ## Introspect a Lookup diff --git a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java index c417d12aaa06..7a3f1abacec5 100644 --- a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java +++ b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java @@ -30,10 +30,10 @@ public class LookupConfig private final String snapshotWorkingDir; @JsonProperty - private int numLookupLoadingThreads = 10; + private boolean enableLookupSyncOnStartup = false; - @JsonProperty - private boolean enableLookupSyncOnStartup; + @JsonProperty("numLookupLoadingThreads") + private int numLookupLoadingThreads = Runtime.getRuntime().availableProcessors() / 2; /** * @param snapshotWorkingDir working directory to store lookups snapshot file, passing null or empty string will disable the snapshot utility @@ -43,13 +43,11 @@ public class LookupConfig @JsonCreator public LookupConfig( @JsonProperty("snapshotWorkingDir") String snapshotWorkingDir, - @JsonProperty("numLookupLoadingThreads") int numLookupLoadingThreads, - @JsonProperty("enableLookupSyncOnStartup") Boolean enableLookupSyncOnStartup + @JsonProperty("enableLookupSyncOnStartup") boolean enableLookupSyncOnStartup ) { this.snapshotWorkingDir = Strings.nullToEmpty(snapshotWorkingDir); - this.numLookupLoadingThreads = numLookupLoadingThreads; - this.enableLookupSyncOnStartup = enableLookupSyncOnStartup == null ? false : enableLookupSyncOnStartup; + this.enableLookupSyncOnStartup = enableLookupSyncOnStartup; } public String getSnapshotWorkingDir() @@ -90,7 +88,7 @@ public String toString() return "LookupConfig{" + "snapshotWorkingDir='" + getSnapshotWorkingDir() + '\'' + " numLookupLoadingThreads='" + getNumLookupLoadingThreads() + '\'' + - " disableLookupSync='" + getEnableLookupSyncOnStartup() + '\'' + + " enableLookupSyncOnStartup='" + getEnableLookupSyncOnStartup() + '\'' + '}'; } } diff --git a/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java b/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java index 214ab9ba772a..d58d7ff1b662 100644 --- a/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java +++ b/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java @@ -39,7 +39,7 @@ public class LookupConfigTest @Test public void TestSerDesr() throws IOException { - LookupConfig lookupConfig = new LookupConfig(temporaryFolder.newFile().getAbsolutePath(), NUM_THREADS, LOOKUP_DISABLE); + LookupConfig lookupConfig = new LookupConfig(temporaryFolder.newFile().getAbsolutePath(), false); Assert.assertEquals(lookupConfig, mapper.reader(LookupConfig.class).readValue(mapper.writeValueAsString(lookupConfig))); } } 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 c909f1944cb4..5b1efed819f4 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -108,6 +108,8 @@ public class LookupReferencesManager private final LookupConfig lookupConfig; + private ListeningScheduledExecutorService executorService; + @Inject public LookupReferencesManager( LookupConfig lookupConfig, @Json ObjectMapper objectMapper, @@ -333,69 +335,23 @@ private void updateToLoadAndDrop( private void takeSnapshot(Map lookupMap) { if (lookupSnapshotTaker != null) { - List lookups = new ArrayList<>(lookupMap.size()); - for (Map.Entry e : lookupMap.entrySet()) { - lookups.add(new LookupBean(e.getKey(), null, e.getValue())); - } - - lookupSnapshotTaker.takeSnapshot(lookups); + lookupSnapshotTaker.takeSnapshot(getLookupBeanList(lookupMap)); } } private void loadAllLookupsAndInitStateRef() { - List lookupBeanList = new ArrayList<>(); - if (lookupConfig.getEnableLookupSyncOnStartup()) { - String tier = lookupListeningAnnouncerConfig.getLookupTier(); - lookupBeanList = getLookupListFromCoordinator(tier); - if (lookupBeanList == null) { - LOG.info("Coordinator is unavailable. Loading saved snapshot instead"); - lookupBeanList = getLookupListFromSnapshot(); - } - } else { - lookupBeanList = getLookupListFromSnapshot(); - } + List lookupBeanList = getLookupsListFromLookupConfig(); if (lookupBeanList != null) { ImmutableMap.Builder builder = ImmutableMap.builder(); - ListeningScheduledExecutorService executorService = MoreExecutors.listeningDecorator( + executorService = MoreExecutors.listeningDecorator( Executors.newScheduledThreadPool( lookupConfig.getNumLookupLoadingThreads(), Execs.makeThreadFactory("LookupReferencesManager-Startup-%s") ) ); try { - List> futures = new ArrayList<>(); - LOG.info("Starting lookup loading process"); - for (LookupBean lookupBean : lookupBeanList) { - futures.add( - executorService.submit( - () -> { - - LookupExtractorFactoryContainer container = lookupBean.getContainer(); - LOG.info( - "Starting lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - if (container.getLookupExtractorFactory().start()) { - LOG.info( - "Started lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); - } else { - LOG.error( - "Failed to start lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - return null; - } - } - ) - ); - } + List> futures = startLookups(lookupBeanList); final ListenableFuture> futureList = Futures.allAsList(futures); try { futureList.get() @@ -405,7 +361,7 @@ private void loadAllLookupsAndInitStateRef() stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); } - catch (Exception ex) { + catch (InterruptedException | ExecutionException ex) { futureList.cancel(true); throw ex; } @@ -432,6 +388,14 @@ private void loadAllLookupsAndInitStateRef() } } + /** + * Returns a list of lookups from the coordinator if the coordinator is available. If it's not available, returns null. + * + * @param tier lookup tier name + * + * @return list of LookupBean objects, or null + */ + @Nullable private List getLookupListFromCoordinator(String tier) { try { @@ -456,11 +420,9 @@ private List getLookupListFromCoordinator(String tier) response.getContent(), LOOKUPS_ALL_REFERENCE ); - if (!lookupMap.isEmpty()) { - for (Map.Entry e : lookupMap.entrySet()) { - lookupBeanList.add(new LookupBean(e.getKey(), null, e.getValue())); - } - } + lookupMap.entrySet() + .stream().forEach(e -> lookupBeanList.add + (new LookupBean(e.getKey(), null, e.getValue()))); } return lookupBeanList; } @@ -470,6 +432,12 @@ private List getLookupListFromCoordinator(String tier) } } + /** + * Returns a list of lookups from the snapshot if the lookupsnapshottaker is configured. If it's not available, returns null. + * + * @return list of LookupBean objects, or null + */ + @Nullable private List getLookupListFromSnapshot() { if (lookupSnapshotTaker != null) { @@ -478,6 +446,68 @@ private List getLookupListFromSnapshot() return null; } + private List getLookupBeanList(Map lookupMap) + { + List lookups = new ArrayList<>(lookupMap.size()); + for (Map.Entry e : lookupMap.entrySet()) { + lookups.add(new LookupBean(e.getKey(), null, e.getValue())); + } + return lookups; + } + + private List getLookupsListFromLookupConfig() + { + List lookupBeanList = new ArrayList<>(); + if (lookupConfig.getEnableLookupSyncOnStartup()) { + String tier = lookupListeningAnnouncerConfig.getLookupTier(); + lookupBeanList = getLookupListFromCoordinator(tier); + if (lookupBeanList == null) { + LOG.info("Coordinator is unavailable. Loading saved snapshot instead"); + lookupBeanList = getLookupListFromSnapshot(); + } + } else { + lookupBeanList = getLookupListFromSnapshot(); + } + return lookupBeanList; + } + + private List> startLookups(List lookupBeanList) + { + List> futures = new ArrayList<>(); + LOG.info("Starting lookup loading process"); + for (LookupBean lookupBean : lookupBeanList) { + futures.add( + executorService.submit( + () -> { + + LookupExtractorFactoryContainer container = lookupBean.getContainer(); + LOG.info( + "Starting lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + if (container.getLookupExtractorFactory().start()) { + LOG.info( + "Started lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); + } else { + LOG.error( + "Failed to start lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + return null; + } + } + ) + ); + } + return futures; + } + private LookupUpdateState atomicallyUpdateStateRef(Function fn) { while (true) { diff --git a/server/src/test/java/io/druid/query/expression/TestExpressionMacroTable.java b/server/src/test/java/io/druid/query/expression/TestExpressionMacroTable.java index bef480a33845..ecc5fde256ab 100644 --- a/server/src/test/java/io/druid/query/expression/TestExpressionMacroTable.java +++ b/server/src/test/java/io/druid/query/expression/TestExpressionMacroTable.java @@ -48,7 +48,10 @@ private TestExpressionMacroTable() new TimestampFloorExprMacro(), new TimestampFormatExprMacro(), new TimestampParseExprMacro(), - new TimestampShiftExprMacro() + new TimestampShiftExprMacro(), + new TrimExprMacro.BothTrimExprMacro(), + new TrimExprMacro.LeftTrimExprMacro(), + new TrimExprMacro.RightTrimExprMacro() ) ); } 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 58519c6d08d8..3401c4855fb0 100644 --- a/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java +++ b/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java @@ -91,7 +91,7 @@ public void setUp() throws IOException container = new LookupExtractorFactoryContainer("v0", lookupExtractorFactory); mapper.registerSubtypes(MapLookupExtractorFactory.class); lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), LOOKUP_THREADS, LOOKUP_DISABLE), + new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), true), mapper, druidLeaderClient, config, true ); @@ -101,7 +101,7 @@ public void setUp() throws IOException public void testStartStop() throws JsonProcessingException, InterruptedException, MalformedURLException { lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(null, LOOKUP_THREADS, LOOKUP_DISABLE), + new LookupConfig(null, true), mapper, druidLeaderClient, config ); @@ -445,7 +445,7 @@ public void testGetAllLookupsState() throws Exception public void testRealModeWithMainThread() throws Exception { LookupReferencesManager lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), LOOKUP_THREADS, LOOKUP_DISABLE), + new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), true), mapper, druidLeaderClient, config ); Map lookupMap = new HashMap<>(); @@ -572,9 +572,7 @@ public void testLoadLookupOnCoordinatorFailure() throws Exception lookupReferencesManager.stop(); lookupReferencesManager = new LookupReferencesManager( new LookupConfig( - lookupReferencesManager.lookupSnapshotTaker.getPersistFile().getParent(), - LOOKUP_THREADS, - LOOKUP_DISABLE + lookupReferencesManager.lookupSnapshotTaker.getPersistFile().getParent(), true ), mapper, druidLeaderClient, config, true @@ -595,7 +593,7 @@ public void testLoadLookupOnCoordinatorFailure() throws Exception public void testDisableLookupSync() throws Exception { LookupReferencesManager lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), LOOKUP_THREADS, true), + new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), true), mapper, druidLeaderClient, config ); Map lookupMap = new HashMap<>(); From 93a955878d894cbf5795de7fdbde861e3836216d Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Thu, 28 Sep 2017 17:14:02 -0500 Subject: [PATCH 11/20] Remove lookup config constructor --- .../io/druid/query/lookup/LookupConfig.java | 19 ++----- .../druid/query/lookup/LookupConfigTest.java | 2 +- .../query/lookup/LookupReferencesManager.java | 5 +- .../lookup/LookupReferencesManagerTest.java | 51 ++++++++++++++++--- 4 files changed, 51 insertions(+), 26 deletions(-) diff --git a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java index 7a3f1abacec5..18090e24d0d8 100644 --- a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java +++ b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java @@ -19,17 +19,15 @@ package io.druid.query.lookup; -import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.base.Strings; public class LookupConfig { - @JsonProperty - private final String snapshotWorkingDir; + @JsonProperty("snapshotWorkingDir") + private String snapshotWorkingDir; - @JsonProperty + @JsonProperty("enableLookupSyncOnStartup") private boolean enableLookupSyncOnStartup = false; @JsonProperty("numLookupLoadingThreads") @@ -40,19 +38,10 @@ public class LookupConfig * @param numLookupLoadingThreads number of threads for loading the lookups as part of the synchronization process * @param enableLookupSyncOnStartup decides whether the lookup synchronization process should be enabled at startup */ - @JsonCreator - public LookupConfig( - @JsonProperty("snapshotWorkingDir") String snapshotWorkingDir, - @JsonProperty("enableLookupSyncOnStartup") boolean enableLookupSyncOnStartup - ) - { - this.snapshotWorkingDir = Strings.nullToEmpty(snapshotWorkingDir); - this.enableLookupSyncOnStartup = enableLookupSyncOnStartup; - } public String getSnapshotWorkingDir() { - return snapshotWorkingDir; + return (snapshotWorkingDir == null ? "" : snapshotWorkingDir); } public int getNumLookupLoadingThreads() diff --git a/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java b/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java index d58d7ff1b662..a44d807cf5b8 100644 --- a/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java +++ b/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java @@ -39,7 +39,7 @@ public class LookupConfigTest @Test public void TestSerDesr() throws IOException { - LookupConfig lookupConfig = new LookupConfig(temporaryFolder.newFile().getAbsolutePath(), false); + LookupConfig lookupConfig = new LookupConfig(); Assert.assertEquals(lookupConfig, mapper.reader(LookupConfig.class).readValue(mapper.writeValueAsString(lookupConfig))); } } 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 5b1efed819f4..3ce5dd6266dc 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -420,9 +420,8 @@ private List getLookupListFromCoordinator(String tier) response.getContent(), LOOKUPS_ALL_REFERENCE ); - lookupMap.entrySet() - .stream().forEach(e -> lookupBeanList.add - (new LookupBean(e.getKey(), null, e.getValue()))); + lookupMap.forEach((k, v) -> lookupBeanList.add(new LookupBean(k, null, v))); + } return lookupBeanList; } 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 3401c4855fb0..cdd8a5078e68 100644 --- a/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java +++ b/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java @@ -90,8 +90,22 @@ public void setUp() throws IOException ); container = new LookupExtractorFactoryContainer("v0", lookupExtractorFactory); mapper.registerSubtypes(MapLookupExtractorFactory.class); + String temporaryPath = temporaryFolder.newFolder().getAbsolutePath(); lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), true), + new LookupConfig() + { + @Override + public boolean getEnableLookupSyncOnStartup() + { + return true; + } + + @Override + public String getSnapshotWorkingDir() + { + return temporaryPath; + } + }, mapper, druidLeaderClient, config, true ); @@ -101,7 +115,13 @@ public void setUp() throws IOException public void testStartStop() throws JsonProcessingException, InterruptedException, MalformedURLException { lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(null, true), + new LookupConfig() { + @Override + public boolean getEnableLookupSyncOnStartup() + { + return true; + } + }, mapper, druidLeaderClient, config ); @@ -445,7 +465,14 @@ public void testGetAllLookupsState() throws Exception public void testRealModeWithMainThread() throws Exception { LookupReferencesManager lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), true), + new LookupConfig() + { + @Override + public boolean getEnableLookupSyncOnStartup() + { + return true; + } + }, mapper, druidLeaderClient, config ); Map lookupMap = new HashMap<>(); @@ -571,9 +598,19 @@ public void testLoadLookupOnCoordinatorFailure() throws Exception lookupReferencesManager.handlePendingNotices(); lookupReferencesManager.stop(); lookupReferencesManager = new LookupReferencesManager( - new LookupConfig( - lookupReferencesManager.lookupSnapshotTaker.getPersistFile().getParent(), true - ), + new LookupConfig() + { + @Override + public boolean getEnableLookupSyncOnStartup() + { + return true; + } + @Override + public String getSnapshotWorkingDir() + { + return lookupReferencesManager.lookupSnapshotTaker.getPersistFile().getParent(); + } + }, mapper, druidLeaderClient, config, true ); @@ -593,7 +630,7 @@ public void testLoadLookupOnCoordinatorFailure() throws Exception public void testDisableLookupSync() throws Exception { LookupReferencesManager lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(temporaryFolder.newFolder().getAbsolutePath(), true), + new LookupConfig(), mapper, druidLeaderClient, config ); Map lookupMap = new HashMap<>(); From 1cdbec3fff96732598646cdda7a5d557067fbf5c Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Mon, 2 Oct 2017 08:57:58 -0500 Subject: [PATCH 12/20] Revert Lookupconfig constructor changes --- docs/content/querying/lookups.md | 2 +- .../io/druid/query/lookup/LookupConfig.java | 14 +++++- .../druid/query/lookup/LookupConfigTest.java | 2 +- .../lookup/LookupReferencesManagerTest.java | 48 ++----------------- 4 files changed, 19 insertions(+), 47 deletions(-) diff --git a/docs/content/querying/lookups.md b/docs/content/querying/lookups.md index 08a73ea2ae3d..b28accac7cc1 100644 --- a/docs/content/querying/lookups.md +++ b/docs/content/querying/lookups.md @@ -324,7 +324,7 @@ It is possible to save the configuration across restarts such that a node will n |--------|-----------|-------| |`druid.lookup.snapshotWorkingDir`| Working path used to store snapshot of current lookup configuration, leaving this property null will disable snapshot/bootstrap utility|null| |`druid.lookup.numLookupLoadingThreads`| Number of threads for loading the lookups in parallel on startup. This thread pool is destroyed once startup is done. It is not kept during the lifetime of the JVM|Available Processors / 2| -|`druid.lookup.enableLookupSyncOnStartup`|Enable the lookup synchronization process with coordinator on startup. The queryable nodes will fetch and load the lookups from the coordinator instead of waiting for the coordinator to load the lookups for them. This option is recommended only in cases where the queryable nodes have lookups configured|false| +|`druid.lookup.enableLookupSyncOnStartup`|Enable the lookup synchronization process with coordinator on startup. The queryable nodes will fetch and load the lookups from the coordinator instead of waiting for the coordinator to load the lookups for them. This option is recommended only in cases where the queryable nodes have lookups configured|true| ## Introspect a Lookup diff --git a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java index 18090e24d0d8..6c91b6fb57ab 100644 --- a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java +++ b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java @@ -19,7 +19,9 @@ package io.druid.query.lookup; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Strings; public class LookupConfig { @@ -28,7 +30,7 @@ public class LookupConfig private String snapshotWorkingDir; @JsonProperty("enableLookupSyncOnStartup") - private boolean enableLookupSyncOnStartup = false; + private boolean enableLookupSyncOnStartup = true; @JsonProperty("numLookupLoadingThreads") private int numLookupLoadingThreads = Runtime.getRuntime().availableProcessors() / 2; @@ -39,9 +41,17 @@ public class LookupConfig * @param enableLookupSyncOnStartup decides whether the lookup synchronization process should be enabled at startup */ + @JsonCreator + public LookupConfig( + @JsonProperty("snapshotWorkingDir") String snapshotWorkingDir + ) + { + this.snapshotWorkingDir = Strings.nullToEmpty(snapshotWorkingDir); + } + public String getSnapshotWorkingDir() { - return (snapshotWorkingDir == null ? "" : snapshotWorkingDir); + return snapshotWorkingDir; } public int getNumLookupLoadingThreads() diff --git a/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java b/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java index a44d807cf5b8..e7f2402f1818 100644 --- a/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java +++ b/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java @@ -39,7 +39,7 @@ public class LookupConfigTest @Test public void TestSerDesr() throws IOException { - LookupConfig lookupConfig = new LookupConfig(); + LookupConfig lookupConfig = new LookupConfig(temporaryFolder.newFile().getAbsolutePath()); Assert.assertEquals(lookupConfig, mapper.reader(LookupConfig.class).readValue(mapper.writeValueAsString(lookupConfig))); } } 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 cdd8a5078e68..a5e275eed1f8 100644 --- a/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java +++ b/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java @@ -92,20 +92,7 @@ public void setUp() throws IOException mapper.registerSubtypes(MapLookupExtractorFactory.class); String temporaryPath = temporaryFolder.newFolder().getAbsolutePath(); lookupReferencesManager = new LookupReferencesManager( - new LookupConfig() - { - @Override - public boolean getEnableLookupSyncOnStartup() - { - return true; - } - - @Override - public String getSnapshotWorkingDir() - { - return temporaryPath; - } - }, + new LookupConfig(temporaryFolder.newFolder().getAbsolutePath()), mapper, druidLeaderClient, config, true ); @@ -115,13 +102,7 @@ public String getSnapshotWorkingDir() public void testStartStop() throws JsonProcessingException, InterruptedException, MalformedURLException { lookupReferencesManager = new LookupReferencesManager( - new LookupConfig() { - @Override - public boolean getEnableLookupSyncOnStartup() - { - return true; - } - }, + new LookupConfig(null), mapper, druidLeaderClient, config ); @@ -465,14 +446,7 @@ public void testGetAllLookupsState() throws Exception public void testRealModeWithMainThread() throws Exception { LookupReferencesManager lookupReferencesManager = new LookupReferencesManager( - new LookupConfig() - { - @Override - public boolean getEnableLookupSyncOnStartup() - { - return true; - } - }, + new LookupConfig(temporaryFolder.newFolder().getAbsolutePath()), mapper, druidLeaderClient, config ); Map lookupMap = new HashMap<>(); @@ -598,19 +572,7 @@ public void testLoadLookupOnCoordinatorFailure() throws Exception lookupReferencesManager.handlePendingNotices(); lookupReferencesManager.stop(); lookupReferencesManager = new LookupReferencesManager( - new LookupConfig() - { - @Override - public boolean getEnableLookupSyncOnStartup() - { - return true; - } - @Override - public String getSnapshotWorkingDir() - { - return lookupReferencesManager.lookupSnapshotTaker.getPersistFile().getParent(); - } - }, + new LookupConfig(lookupReferencesManager.lookupSnapshotTaker.getPersistFile().getParent()), mapper, druidLeaderClient, config, true ); @@ -630,7 +592,7 @@ public String getSnapshotWorkingDir() public void testDisableLookupSync() throws Exception { LookupReferencesManager lookupReferencesManager = new LookupReferencesManager( - new LookupConfig(), + new LookupConfig(null), mapper, druidLeaderClient, config ); Map lookupMap = new HashMap<>(); From e23b283bcfbd15b27a2b3491778de11100d24447 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Mon, 2 Oct 2017 21:37:10 -0500 Subject: [PATCH 13/20] Add tests to LookupConfig --- docs/content/querying/lookups.md | 2 +- .../io/druid/query/lookup/LookupConfig.java | 10 +++---- .../druid/query/lookup/LookupConfigTest.java | 28 +++++++++++++++++-- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/docs/content/querying/lookups.md b/docs/content/querying/lookups.md index b28accac7cc1..96ffd7f48cdf 100644 --- a/docs/content/querying/lookups.md +++ b/docs/content/querying/lookups.md @@ -324,7 +324,7 @@ It is possible to save the configuration across restarts such that a node will n |--------|-----------|-------| |`druid.lookup.snapshotWorkingDir`| Working path used to store snapshot of current lookup configuration, leaving this property null will disable snapshot/bootstrap utility|null| |`druid.lookup.numLookupLoadingThreads`| Number of threads for loading the lookups in parallel on startup. This thread pool is destroyed once startup is done. It is not kept during the lifetime of the JVM|Available Processors / 2| -|`druid.lookup.enableLookupSyncOnStartup`|Enable the lookup synchronization process with coordinator on startup. The queryable nodes will fetch and load the lookups from the coordinator instead of waiting for the coordinator to load the lookups for them. This option is recommended only in cases where the queryable nodes have lookups configured|true| +|`druid.lookup.enableLookupSyncOnStartup`|Enable the lookup synchronization process with coordinator on startup. The queryable nodes will fetch and load the lookups from the coordinator instead of waiting for the coordinator to load the lookups for them. Users may opt to disable this option if there are no lookups configured in the cluster.|true| ## Introspect a Lookup diff --git a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java index 6c91b6fb57ab..7f424c1e987c 100644 --- a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java +++ b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java @@ -36,11 +36,10 @@ public class LookupConfig private int numLookupLoadingThreads = Runtime.getRuntime().availableProcessors() / 2; /** - * @param snapshotWorkingDir working directory to store lookups snapshot file, passing null or empty string will disable the snapshot utility - * @param numLookupLoadingThreads number of threads for loading the lookups as part of the synchronization process + * @param snapshotWorkingDir working directory to store lookups snapshot file, passing null or empty string will disable the snapshot utility + * @param numLookupLoadingThreads number of threads for loading the lookups as part of the synchronization process * @param enableLookupSyncOnStartup decides whether the lookup synchronization process should be enabled at startup */ - @JsonCreator public LookupConfig( @JsonProperty("snapshotWorkingDir") String snapshotWorkingDir @@ -64,7 +63,6 @@ public boolean getEnableLookupSyncOnStartup() return enableLookupSyncOnStartup; } - @Override public boolean equals(Object o) { @@ -77,7 +75,9 @@ public boolean equals(Object o) LookupConfig that = (LookupConfig) o; - return getSnapshotWorkingDir().equals(that.getSnapshotWorkingDir()); + return snapshotWorkingDir.equals(that.snapshotWorkingDir) && + enableLookupSyncOnStartup == that.enableLookupSyncOnStartup && + numLookupLoadingThreads == that.numLookupLoadingThreads; } diff --git a/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java b/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java index e7f2402f1818..c2e39bf0a0b3 100644 --- a/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java +++ b/processing/src/test/java/io/druid/query/lookup/LookupConfigTest.java @@ -32,14 +32,36 @@ public class LookupConfigTest { ObjectMapper mapper = TestHelper.getJsonMapper(); - private final int NUM_THREADS = 10; - private final boolean LOOKUP_DISABLE = false; @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Test public void TestSerDesr() throws IOException { LookupConfig lookupConfig = new LookupConfig(temporaryFolder.newFile().getAbsolutePath()); - Assert.assertEquals(lookupConfig, mapper.reader(LookupConfig.class).readValue(mapper.writeValueAsString(lookupConfig))); + Assert.assertEquals( + lookupConfig, + mapper.reader(LookupConfig.class).readValue(mapper.writeValueAsString(lookupConfig)) + ); + } + + @Test + public void testSerdeWithNonDefaults() throws Exception + { + String json = "{\n" + + " \"enableLookupSyncOnStartup\": false,\n" + + " \"snapshotWorkingDir\": \"/tmp\",\n" + + " \"numLookupLoadingThreads\": 4 \n" + + "}\n"; + LookupConfig config = mapper.readValue( + mapper.writeValueAsString( + mapper.readValue(json, LookupConfig.class) + ), + LookupConfig.class + ); + + Assert.assertEquals("/tmp", config.getSnapshotWorkingDir()); + Assert.assertEquals(false, config.getEnableLookupSyncOnStartup()); + Assert.assertEquals(4, config.getNumLookupLoadingThreads()); } } From a06a87ce697bdda988c558b0abc88cc5b398b3f8 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Tue, 3 Oct 2017 11:46:38 -0500 Subject: [PATCH 14/20] Make executorservice local --- .../query/lookup/LookupReferencesManager.java | 125 ++++++++---------- 1 file changed, 56 insertions(+), 69 deletions(-) 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 3ce5dd6266dc..839dad89448a 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -108,8 +108,6 @@ public class LookupReferencesManager private final LookupConfig lookupConfig; - private ListeningScheduledExecutorService executorService; - @Inject public LookupReferencesManager( LookupConfig lookupConfig, @Json ObjectMapper objectMapper, @@ -343,45 +341,7 @@ private void loadAllLookupsAndInitStateRef() { List lookupBeanList = getLookupsListFromLookupConfig(); if (lookupBeanList != null) { - ImmutableMap.Builder builder = ImmutableMap.builder(); - executorService = MoreExecutors.listeningDecorator( - Executors.newScheduledThreadPool( - lookupConfig.getNumLookupLoadingThreads(), - Execs.makeThreadFactory("LookupReferencesManager-Startup-%s") - ) - ); - try { - List> futures = startLookups(lookupBeanList); - final ListenableFuture> futureList = Futures.allAsList(futures); - try { - futureList.get() - .stream() - .filter(Objects::nonNull) - .forEach(builder::put); - - stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); - } - catch (InterruptedException | ExecutionException ex) { - futureList.cancel(true); - throw ex; - } - } - catch (Exception e) { - LOG.error(e, "Failed to finish lookup load process."); - } - finally { - try { - executorService.shutdownNow(); - if (!executorService.awaitTermination(60, TimeUnit.SECONDS)) { - LOG.warn("Lookup loading didn't complete in 60 seconds!"); - } - } - catch (InterruptedException e) { - LOG.error("Lookup loading interrupted. "); - Thread.currentThread().interrupt(); - - } - } + startLookups(lookupBeanList); } else { LOG.info("No lookups to be loaded at this point"); stateRef.set(new LookupUpdateState(ImmutableMap.of(), ImmutableList.of(), ImmutableList.of())); @@ -470,41 +430,68 @@ private List getLookupsListFromLookupConfig() return lookupBeanList; } - private List> startLookups(List lookupBeanList) + private void startLookups(List lookupBeanList) { - List> futures = new ArrayList<>(); - LOG.info("Starting lookup loading process"); - for (LookupBean lookupBean : lookupBeanList) { - futures.add( - executorService.submit( - () -> { - - LookupExtractorFactoryContainer container = lookupBean.getContainer(); - LOG.info( - "Starting lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - if (container.getLookupExtractorFactory().start()) { + ImmutableMap.Builder builder = ImmutableMap.builder(); + ListeningScheduledExecutorService executorService = MoreExecutors.listeningDecorator( + Executors.newScheduledThreadPool( + lookupConfig.getNumLookupLoadingThreads(), + Execs.makeThreadFactory("LookupReferencesManager-Startup-%s") + ) + ); + try { + List> futures = new ArrayList<>(); + LOG.info("Starting lookup loading process"); + for (LookupBean lookupBean : lookupBeanList) { + futures.add( + executorService.submit( + () -> { + + LookupExtractorFactoryContainer container = lookupBean.getContainer(); LOG.info( - "Started lookup [%s]:[%s]", + "Starting lookup [%s]:[%s]", lookupBean.getName(), container ); - return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); - } else { - LOG.error( - "Failed to start lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - return null; + if (container.getLookupExtractorFactory().start()) { + LOG.info( + "Started lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); + } else { + LOG.error( + "Failed to start lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + return null; + } } - } - ) - ); + ) + ); + } + final ListenableFuture> futureList = Futures.allAsList(futures); + try { + futureList.get() + .stream() + .filter(Objects::nonNull) + .forEach(builder::put); + + stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); + } + catch (InterruptedException | ExecutionException ex) { + futureList.cancel(true); + throw ex; + } + } + catch (Exception e) { + LOG.error(e, "Failed to finish lookup load process."); + } + finally { + executorService.shutdownNow(); } - return futures; } private LookupUpdateState atomicallyUpdateStateRef(Function fn) From 8eca9fda4f0e31a740669ab31c5a8a9f72761d9d Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Tue, 3 Oct 2017 21:39:59 -0500 Subject: [PATCH 15/20] Update LRM --- .../java/io/druid/query/lookup/LookupReferencesManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 839dad89448a..b6c8b718a1bf 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -29,6 +29,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.inject.Inject; @@ -433,7 +434,7 @@ private List getLookupsListFromLookupConfig() private void startLookups(List lookupBeanList) { ImmutableMap.Builder builder = ImmutableMap.builder(); - ListeningScheduledExecutorService executorService = MoreExecutors.listeningDecorator( + ListeningExecutorService executorService = MoreExecutors.listeningDecorator( Executors.newScheduledThreadPool( lookupConfig.getNumLookupLoadingThreads(), Execs.makeThreadFactory("LookupReferencesManager-Startup-%s") From 3ef208682e347f1ae2a03e869204afaa628e8fc7 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 4 Oct 2017 15:26:48 -0500 Subject: [PATCH 16/20] Move ListeningScheduledExecutorService to ExecutorCompletionService --- .../query/lookup/LookupReferencesManager.java | 102 +++++++++--------- 1 file changed, 49 insertions(+), 53 deletions(-) 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 b6c8b718a1bf..a7f4d27356ec 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -27,11 +27,6 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.util.concurrent.Futures; -import com.google.common.util.concurrent.ListenableFuture; -import com.google.common.util.concurrent.ListeningExecutorService; -import com.google.common.util.concurrent.ListeningScheduledExecutorService; -import com.google.common.util.concurrent.MoreExecutors; import com.google.inject.Inject; import com.metamx.emitter.EmittingLogger; import com.metamx.http.client.response.FullResponseHolder; @@ -56,14 +51,16 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.concurrent.ExecutionException; -import java.util.concurrent.Executors; +import java.util.concurrent.ExecutorCompletionService; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.LockSupport; import java.util.function.Function; +import java.util.stream.IntStream; /** * This class provide a basic {@link LookupExtractorFactory} references manager. @@ -434,58 +431,57 @@ private List getLookupsListFromLookupConfig() private void startLookups(List lookupBeanList) { ImmutableMap.Builder builder = ImmutableMap.builder(); - ListeningExecutorService executorService = MoreExecutors.listeningDecorator( - Executors.newScheduledThreadPool( - lookupConfig.getNumLookupLoadingThreads(), - Execs.makeThreadFactory("LookupReferencesManager-Startup-%s") - ) + ExecutorService executorService = Execs.multiThreaded( + lookupConfig.getNumLookupLoadingThreads(), + "LookupReferencesManager-Startup-%s" ); + ExecutorCompletionService completionService = new ExecutorCompletionService(executorService); try { - List> futures = new ArrayList<>(); LOG.info("Starting lookup loading process"); for (LookupBean lookupBean : lookupBeanList) { - futures.add( - executorService.submit( - () -> { - - LookupExtractorFactoryContainer container = lookupBean.getContainer(); - LOG.info( - "Starting lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - if (container.getLookupExtractorFactory().start()) { - LOG.info( - "Started lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); - } else { - LOG.error( - "Failed to start lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - return null; - } - } - ) - ); - } - final ListenableFuture> futureList = Futures.allAsList(futures); - try { - futureList.get() - .stream() - .filter(Objects::nonNull) - .forEach(builder::put); + completionService.submit( + () -> { - stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); - } - catch (InterruptedException | ExecutionException ex) { - futureList.cancel(true); - throw ex; + LookupExtractorFactoryContainer container = lookupBean.getContainer(); + LOG.info( + "Starting lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + if (container.getLookupExtractorFactory().start()) { + LOG.info( + "Started lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); + } else { + LOG.error( + "Failed to start lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + return null; + } + } + ); } + IntStream.range(0, lookupBeanList.size()) + .forEach(i -> { + try { + final Future> completedFuture = completionService + .take(); + final AbstractMap.SimpleImmutableEntry lookupResult = completedFuture + .get(); + if (lookupResult != null) { + builder.put(lookupResult); + } + } + catch (InterruptedException | ExecutionException e) { + LOG.error(e, "Lookup loading interrupted."); + } + }); + stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); } catch (Exception e) { LOG.error(e, "Failed to finish lookup load process."); From a966b8aeca38c649663c34a8494a3e9a8ed03395 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 4 Oct 2017 16:21:38 -0500 Subject: [PATCH 17/20] Move exception to outer block --- .../query/lookup/LookupReferencesManager.java | 87 ++++++++++--------- 1 file changed, 46 insertions(+), 41 deletions(-) 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 a7f4d27356ec..c2bd73bae589 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -60,7 +60,6 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.LockSupport; import java.util.function.Function; -import java.util.stream.IntStream; /** * This class provide a basic {@link LookupExtractorFactory} references manager. @@ -436,55 +435,61 @@ private void startLookups(List lookupBeanList) "LookupReferencesManager-Startup-%s" ); ExecutorCompletionService completionService = new ExecutorCompletionService(executorService); + List> futures = new ArrayList<>(); try { LOG.info("Starting lookup loading process"); for (LookupBean lookupBean : lookupBeanList) { - completionService.submit( - () -> { - - LookupExtractorFactoryContainer container = lookupBean.getContainer(); - LOG.info( - "Starting lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - if (container.getLookupExtractorFactory().start()) { - LOG.info( - "Started lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); - } else { - LOG.error( - "Failed to start lookup [%s]:[%s]", - lookupBean.getName(), - container - ); - return null; - } - } + futures.add( + completionService.submit( + () -> { + LookupExtractorFactoryContainer container = lookupBean.getContainer(); + LOG.info( + "Starting lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + if (container.getLookupExtractorFactory().start()) { + LOG.info( + "Started lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); + } else { + LOG.error( + "Failed to start lookup [%s]:[%s]", + lookupBean.getName(), + container + ); + return null; + } + } + ) ); } - IntStream.range(0, lookupBeanList.size()) - .forEach(i -> { - try { - final Future> completedFuture = completionService - .take(); - final AbstractMap.SimpleImmutableEntry lookupResult = completedFuture - .get(); - if (lookupResult != null) { - builder.put(lookupResult); - } - } - catch (InterruptedException | ExecutionException e) { - LOG.error(e, "Lookup loading interrupted."); - } - }); + for (int i = 0; i < futures.size(); i++) { + try { + final Future> completedFuture = completionService + .take(); + final AbstractMap.SimpleImmutableEntry lookupResult = completedFuture + .get(); + if (lookupResult != null) { + builder.put(lookupResult); + } + } + catch (ExecutionException e) { + LOG.error(e, "Execution error during lookup loading."); + } + } stateRef.set(new LookupUpdateState(builder.build(), ImmutableList.of(), ImmutableList.of())); } catch (Exception e) { LOG.error(e, "Failed to finish lookup load process."); + for (Future future : futures) { + if (!future.isDone()) { + future.cancel(true); + } + } } finally { executorService.shutdownNow(); From 2c6762e31329493c1fd9bcca387a36e6dae087ab Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 4 Oct 2017 16:31:01 -0500 Subject: [PATCH 18/20] Remove check to see future is done --- .../java/io/druid/query/lookup/LookupReferencesManager.java | 2 -- 1 file changed, 2 deletions(-) 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 c2bd73bae589..9963ff26b703 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -486,9 +486,7 @@ private void startLookups(List lookupBeanList) catch (Exception e) { LOG.error(e, "Failed to finish lookup load process."); for (Future future : futures) { - if (!future.isDone()) { future.cancel(true); - } } } finally { From c6b967ba0819cbf974379d0be0172974f44f7d3e Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Fri, 6 Oct 2017 22:16:25 -0500 Subject: [PATCH 19/20] Remove unnecessary assignment --- .../java/io/druid/query/lookup/LookupReferencesManager.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 9963ff26b703..d55194220cbb 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -44,6 +44,7 @@ import org.jboss.netty.handler.codec.http.HttpResponseStatus; import javax.annotation.Nullable; +import java.io.IOException; import java.net.MalformedURLException; import java.util.AbstractMap; import java.util.ArrayList; @@ -413,7 +414,7 @@ private List getLookupBeanList(Map getLookupsListFromLookupConfig() { - List lookupBeanList = new ArrayList<>(); + List lookupBeanList; if (lookupConfig.getEnableLookupSyncOnStartup()) { String tier = lookupListeningAnnouncerConfig.getLookupTier(); lookupBeanList = getLookupListFromCoordinator(tier); @@ -506,7 +507,7 @@ private LookupUpdateState atomicallyUpdateStateRef(Function Date: Thu, 12 Oct 2017 12:54:12 -0500 Subject: [PATCH 20/20] Add logging --- .../java/io/druid/query/lookup/LookupReferencesManager.java | 3 ++- .../io/druid/query/lookup/LookupReferencesManagerTest.java | 4 +--- 2 files changed, 3 insertions(+), 4 deletions(-) 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 d55194220cbb..961447ab356f 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -372,6 +372,7 @@ private List getLookupListFromCoordinator(String tier) // Lookup loading is performed via snapshot if older version is present. // This check is only for backward compatibility and should be removed in a future release if (response.getContent().startsWith("[")) { + LOG.info("Failed to retrieve lookup information from coordinator. Attempting to load lookups using snapshot instead"); return null; } else { Map lookupMap = jsonMapper.readValue( @@ -487,7 +488,7 @@ private void startLookups(List lookupBeanList) catch (Exception e) { LOG.error(e, "Failed to finish lookup load process."); for (Future future : futures) { - future.cancel(true); + future.cancel(true); } } finally { 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 a5e275eed1f8..4395145b4edd 100644 --- a/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java +++ b/server/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java @@ -19,7 +19,6 @@ package io.druid.query.lookup; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; import com.metamx.emitter.EmittingLogger; @@ -39,7 +38,6 @@ import org.junit.rules.TemporaryFolder; import java.io.IOException; -import java.net.MalformedURLException; import java.net.URL; import java.util.HashMap; import java.util.Map; @@ -99,7 +97,7 @@ public void setUp() throws IOException } @Test - public void testStartStop() throws JsonProcessingException, InterruptedException, MalformedURLException + public void testStartStop() throws InterruptedException, IOException { lookupReferencesManager = new LookupReferencesManager( new LookupConfig(null),