From 5d41b298ac116b0c6ef77021c9ac9ed626fbecfd Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 6 Sep 2017 15:12:08 -0500 Subject: [PATCH 01/21] 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 4b1070233709..f201e9328816 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; @@ -72,7 +72,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 b3e7cb39098f..b890c5d03794 100644 --- a/processing/src/main/java/io/druid/query/extraction/ExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/ExtractionFn.java @@ -23,7 +23,6 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo; import io.druid.guice.annotations.ExtensionPoint; import io.druid.query.lookup.LookupExtractionFn; -import io.druid.query.lookup.RegisteredLookupExtractionFn; import javax.annotation.Nullable; @@ -40,7 +39,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 151c65fbbedc..18a0a3a5b6a8 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; @@ -47,6 +48,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; @@ -82,7 +84,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 10da6deacde8..cc2d05543a06 100644 --- a/server/src/main/java/io/druid/server/http/LookupCoordinatorResource.java +++ b/server/src/main/java/io/druid/server/http/LookupCoordinatorResource.java @@ -266,7 +266,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 { @@ -287,7 +289,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 27908c311e0b..413bdc31fc7a 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; @@ -134,7 +134,7 @@ public void configure(final Binder binder) binder.bind(LookupReferencesManager.class) .toInstance( - TestExprMacroTable.createTestLookupReferencesManager( + TestExpressionMacroTable.createTestLookupReferencesManager( ImmutableMap.of( "a", "xa", "abc", "xabc" From 8f2a360119be5d619a2bc0d57976f483faaad610 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 6 Sep 2017 15:58:35 -0500 Subject: [PATCH 02/21] 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 317e4e36267688689a2c00273050ad98689360a1 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 6 Sep 2017 20:56:53 -0500 Subject: [PATCH 03/21] 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 67314388277f2d6eabe16f10db9ee235d636aaac Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 13 Sep 2017 11:15:01 -0500 Subject: [PATCH 04/21] 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 f3ad2624dd6e9ed952d3ec3391543b43863999cd Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 13 Sep 2017 16:01:00 -0500 Subject: [PATCH 05/21] 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 756098b08e13251a8f401b95d80a72c20a55a047 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Thu, 14 Sep 2017 22:45:57 -0500 Subject: [PATCH 06/21] 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 1db89144c49e6b330256aa777708b1ab4c1d8c8d Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Fri, 15 Sep 2017 08:37:33 -0500 Subject: [PATCH 07/21] 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 727a70652fe4cd704be7d817effbdbec53e47476 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Fri, 15 Sep 2017 08:59:12 -0500 Subject: [PATCH 08/21] 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 6d9278fe445ca004276ee640a79a19c9fa06ff67 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 20 Sep 2017 21:33:31 -0500 Subject: [PATCH 09/21] 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 e5fe3c232a4bd5b09de95fb35517cf69f47f1413 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 20 Sep 2017 21:36:16 -0500 Subject: [PATCH 10/21] Update LookupConfig --- .../src/main/java/io/druid/query/lookup/LookupConfig.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 c417d12aaa06..f3e8fb66a580 100644 --- a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java +++ b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java @@ -44,12 +44,12 @@ public class LookupConfig 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() From 99c6f607f2374de2ad16fa1dfb8b127f7d19f8d3 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Thu, 28 Sep 2017 11:50:53 -0500 Subject: [PATCH 11/21] Refactoring and doc changes --- docs/content/querying/lookups.md | 2 +- .../io/druid/query/lookup/LookupConfig.java | 10 +- .../druid/query/lookup/LookupConfigTest.java | 2 +- .../query/lookup/LookupReferencesManager.java | 139 +++++++++++------- .../expression/TestExpressionMacroTable.java | 5 +- .../lookup/LookupReferencesManagerTest.java | 12 +- 6 files changed, 100 insertions(+), 70 deletions(-) diff --git a/docs/content/querying/lookups.md b/docs/content/querying/lookups.md index ecfc7d012b5d..08a73ea2ae3d 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.|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|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 ddba5164908a..7a3f1abacec5 100644 --- a/processing/src/main/java/io/druid/query/lookup/LookupConfig.java +++ b/processing/src/main/java/io/druid/query/lookup/LookupConfig.java @@ -29,12 +29,12 @@ public class LookupConfig @JsonProperty private final String snapshotWorkingDir; - @JsonProperty - private int numLookupLoadingThreads = Runtime.getRuntime().availableProcessors() / 2; - @JsonProperty private boolean enableLookupSyncOnStartup = false; + @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 * @param numLookupLoadingThreads number of threads for loading the lookups as part of the synchronization process @@ -43,12 +43,10 @@ public class LookupConfig @JsonCreator public LookupConfig( @JsonProperty("snapshotWorkingDir") String snapshotWorkingDir, - @JsonProperty("numLookupLoadingThreads") int numLookupLoadingThreads, @JsonProperty("enableLookupSyncOnStartup") boolean enableLookupSyncOnStartup ) { this.snapshotWorkingDir = Strings.nullToEmpty(snapshotWorkingDir); - this.numLookupLoadingThreads = numLookupLoadingThreads; this.enableLookupSyncOnStartup = enableLookupSyncOnStartup; } @@ -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 2b5224dc880b..c0574c701ce2 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, @@ -337,69 +339,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() @@ -409,7 +365,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; } @@ -426,6 +382,13 @@ 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) { @@ -451,9 +414,9 @@ private List getLookupListFromCoordinator(String tier) response.getContent(), LOOKUPS_ALL_REFERENCE ); - 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; } @@ -463,6 +426,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) { @@ -471,6 +440,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 9876ae0a41ced923717591b25aea537eb55575df Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Thu, 28 Sep 2017 17:14:02 -0500 Subject: [PATCH 12/21] 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 c0574c701ce2..63e976c1a2cc 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -414,9 +414,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 2ea72af73ac128a9b9011dc843dcaaf427753e13 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Mon, 2 Oct 2017 08:57:58 -0500 Subject: [PATCH 13/21] 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 bff5b093707ab0bd3a49df4bb159eda419d33170 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Mon, 2 Oct 2017 21:37:10 -0500 Subject: [PATCH 14/21] 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 37ae0c9d740b5e26c4ca27e7e406f210a897300e Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Tue, 3 Oct 2017 11:46:38 -0500 Subject: [PATCH 15/21] Make executorservice local --- .../query/lookup/LookupReferencesManager.java | 115 +++++++++--------- 1 file changed, 56 insertions(+), 59 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 63e976c1a2cc..e9475dbbd010 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, @@ -347,35 +345,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 { - executorService.shutdownNow(); - } + startLookups(lookupBeanList); } else { LOG.info("No lookups to be loaded at this point"); stateRef.set(new LookupUpdateState(ImmutableMap.of(), ImmutableList.of(), ImmutableList.of())); @@ -464,41 +434,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]", - lookupBean.getName(), - container - ); - return new AbstractMap.SimpleImmutableEntry<>(lookupBean.getName(), container); - } else { - LOG.error( - "Failed to start lookup [%s]:[%s]", + "Starting 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 eae35a565b16b26455bc23d641a5e51687a8eb83 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Tue, 3 Oct 2017 21:39:59 -0500 Subject: [PATCH 16/21] 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 e9475dbbd010..c28f2cd0c645 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; @@ -437,7 +438,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 f80fbd969159513400cc0535725e05bd11efe91a Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 4 Oct 2017 15:26:48 -0500 Subject: [PATCH 17/21] 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 c28f2cd0c645..eb76132ab9bd 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. @@ -438,58 +435,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 8aa5d4f9a863ee6a7865354adc2a14cb04a0e705 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 4 Oct 2017 16:21:38 -0500 Subject: [PATCH 18/21] 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 eb76132ab9bd..06a510b8b977 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. @@ -440,55 +439,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 840c6445c06718f00c1e0c79ab0547fe27770b7c Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 4 Oct 2017 16:31:01 -0500 Subject: [PATCH 19/21] 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 06a510b8b977..630ce34110c2 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -490,9 +490,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 ef3ead5e19128079f1b90eb7faa5717008b4a2f0 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Fri, 6 Oct 2017 22:16:25 -0500 Subject: [PATCH 20/21] 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 630ce34110c2..102e21249abe 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; @@ -417,7 +418,7 @@ private List getLookupBeanList(Map getLookupsListFromLookupConfig() { - List lookupBeanList = new ArrayList<>(); + List lookupBeanList; if (lookupConfig.getEnableLookupSyncOnStartup()) { String tier = lookupListeningAnnouncerConfig.getLookupTier(); lookupBeanList = getLookupListFromCoordinator(tier); @@ -510,7 +511,7 @@ private LookupUpdateState atomicallyUpdateStateRef(Function Date: Thu, 12 Oct 2017 12:54:12 -0500 Subject: [PATCH 21/21] 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 102e21249abe..0badc265020b 100644 --- a/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/io/druid/query/lookup/LookupReferencesManager.java @@ -376,6 +376,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( @@ -491,7 +492,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),