From 1aaea9a0d70fdfd5d000e1683f27250b56627f9c Mon Sep 17 00:00:00 2001 From: fjy Date: Mon, 24 Nov 2014 10:52:30 -0800 Subject: [PATCH] address code review --- .../indexing/overlord/RemoteTaskRunnerFactory.java | 10 +++++----- .../SimpleResourceManagementStrategy.java | 12 ++++++------ .../indexing/overlord/http/OverlordResource.java | 12 ++++++------ ...ehaviourConfig.java => WorkerBehaviorConfig.java} | 6 +++--- .../SimpleResourceManagementStrategyTest.java | 6 +++--- ...ConfigTest.java => WorkerBehaviorConfigTest.java} | 6 +++--- services/src/main/java/io/druid/cli/CliOverlord.java | 6 ++++++ 7 files changed, 32 insertions(+), 26 deletions(-) rename indexing-service/src/main/java/io/druid/indexing/overlord/setup/{WorkerBehaviourConfig.java => WorkerBehaviorConfig.java} (95%) rename indexing-service/src/test/java/io/druid/indexing/overlord/setup/{WorkerBehaviourConfigTest.java => WorkerBehaviorConfigTest.java} (94%) diff --git a/indexing-service/src/main/java/io/druid/indexing/overlord/RemoteTaskRunnerFactory.java b/indexing-service/src/main/java/io/druid/indexing/overlord/RemoteTaskRunnerFactory.java index 6767f85606f..e68aa15b431 100644 --- a/indexing-service/src/main/java/io/druid/indexing/overlord/RemoteTaskRunnerFactory.java +++ b/indexing-service/src/main/java/io/druid/indexing/overlord/RemoteTaskRunnerFactory.java @@ -27,7 +27,7 @@ import io.druid.curator.cache.SimplePathChildrenCacheFactory; import io.druid.guice.annotations.Global; import io.druid.indexing.overlord.config.RemoteTaskRunnerConfig; import io.druid.indexing.overlord.setup.FillCapacityWorkerSelectStrategy; -import io.druid.indexing.overlord.setup.WorkerBehaviourConfig; +import io.druid.indexing.overlord.setup.WorkerBehaviorConfig; import io.druid.indexing.overlord.setup.WorkerSelectStrategy; import io.druid.server.initialization.ZkPathsConfig; import org.apache.curator.framework.CuratorFramework; @@ -50,7 +50,7 @@ public class RemoteTaskRunnerFactory implements TaskRunnerFactory final ZkPathsConfig zkPaths, final ObjectMapper jsonMapper, @Global final HttpClient httpClient, - final Supplier workerBehaviourConfigSupplier + final Supplier workerBehaviourConfigSupplier ) { this.curator = curator; @@ -60,9 +60,9 @@ public class RemoteTaskRunnerFactory implements TaskRunnerFactory this.httpClient = httpClient; if (workerBehaviourConfigSupplier != null) { // Backwards compatibility - final WorkerBehaviourConfig workerBehaviourConfig = workerBehaviourConfigSupplier.get(); - if (workerBehaviourConfig != null) { - this.strategy = workerBehaviourConfig.getSelectStrategy(); + final WorkerBehaviorConfig workerBehaviorConfig = workerBehaviourConfigSupplier.get(); + if (workerBehaviorConfig != null) { + this.strategy = workerBehaviorConfig.getSelectStrategy(); } else { this.strategy = new FillCapacityWorkerSelectStrategy(); } diff --git a/indexing-service/src/main/java/io/druid/indexing/overlord/autoscaling/SimpleResourceManagementStrategy.java b/indexing-service/src/main/java/io/druid/indexing/overlord/autoscaling/SimpleResourceManagementStrategy.java index 3adb8388dd7..7a8491af27f 100644 --- a/indexing-service/src/main/java/io/druid/indexing/overlord/autoscaling/SimpleResourceManagementStrategy.java +++ b/indexing-service/src/main/java/io/druid/indexing/overlord/autoscaling/SimpleResourceManagementStrategy.java @@ -34,7 +34,7 @@ import com.metamx.emitter.EmittingLogger; import io.druid.indexing.overlord.RemoteTaskRunnerWorkItem; import io.druid.indexing.overlord.TaskRunnerWorkItem; import io.druid.indexing.overlord.ZkWorker; -import io.druid.indexing.overlord.setup.WorkerBehaviourConfig; +import io.druid.indexing.overlord.setup.WorkerBehaviorConfig; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -49,7 +49,7 @@ public class SimpleResourceManagementStrategy implements ResourceManagementStrat private static final EmittingLogger log = new EmittingLogger(SimpleResourceManagementStrategy.class); private final SimpleResourceManagementConfig config; - private final Supplier workerConfigRef; + private final Supplier workerConfigRef; private final ScalingStats scalingStats; private final Object lock = new Object(); @@ -63,7 +63,7 @@ public class SimpleResourceManagementStrategy implements ResourceManagementStrat @Inject public SimpleResourceManagementStrategy( SimpleResourceManagementConfig config, - Supplier workerConfigRef + Supplier workerConfigRef ) { this.config = config; @@ -76,7 +76,7 @@ public class SimpleResourceManagementStrategy implements ResourceManagementStrat { synchronized (lock) { boolean didProvision = false; - final WorkerBehaviourConfig workerConfig = workerConfigRef.get(); + final WorkerBehaviorConfig workerConfig = workerConfigRef.get(); if (workerConfig == null) { log.warn("No workerConfig available, cannot provision new workers."); return false; @@ -142,7 +142,7 @@ public class SimpleResourceManagementStrategy implements ResourceManagementStrat public boolean doTerminate(Collection pendingTasks, Collection zkWorkers) { synchronized (lock) { - final WorkerBehaviourConfig workerConfig = workerConfigRef.get(); + final WorkerBehaviorConfig workerConfig = workerConfigRef.get(); if (workerConfig == null) { log.warn("No workerConfig available, cannot terminate workers."); return false; @@ -279,7 +279,7 @@ public class SimpleResourceManagementStrategy implements ResourceManagementStrat } private void updateTargetWorkerCount( - final WorkerBehaviourConfig workerConfig, + final WorkerBehaviorConfig workerConfig, final Collection pendingTasks, final Collection zkWorkers ) diff --git a/indexing-service/src/main/java/io/druid/indexing/overlord/http/OverlordResource.java b/indexing-service/src/main/java/io/druid/indexing/overlord/http/OverlordResource.java index f5c5a745123..2e2ddaf5684 100644 --- a/indexing-service/src/main/java/io/druid/indexing/overlord/http/OverlordResource.java +++ b/indexing-service/src/main/java/io/druid/indexing/overlord/http/OverlordResource.java @@ -42,7 +42,7 @@ import io.druid.indexing.overlord.TaskRunner; import io.druid.indexing.overlord.TaskRunnerWorkItem; import io.druid.indexing.overlord.TaskStorageQueryAdapter; import io.druid.indexing.overlord.autoscaling.ResourceManagementScheduler; -import io.druid.indexing.overlord.setup.WorkerBehaviourConfig; +import io.druid.indexing.overlord.setup.WorkerBehaviorConfig; import io.druid.indexing.overlord.setup.WorkerSetupData; import io.druid.metadata.EntryExistsException; import io.druid.tasklogs.TaskLogStreamer; @@ -78,7 +78,7 @@ public class OverlordResource private final TaskLogStreamer taskLogStreamer; private final JacksonConfigManager configManager; - private AtomicReference workerConfigRef = null; + private AtomicReference workerConfigRef = null; @Deprecated private AtomicReference workerSetupDataRef = null; @@ -195,7 +195,7 @@ public class OverlordResource public Response getWorkerConfig() { if (workerConfigRef == null) { - workerConfigRef = configManager.watch(WorkerBehaviourConfig.CONFIG_KEY, WorkerBehaviourConfig.class); + workerConfigRef = configManager.watch(WorkerBehaviorConfig.CONFIG_KEY, WorkerBehaviorConfig.class); } return Response.ok(workerConfigRef.get()).build(); @@ -205,14 +205,14 @@ public class OverlordResource @Path("/worker") @Consumes("application/json") public Response setWorkerConfig( - final WorkerBehaviourConfig workerBehaviourConfig + final WorkerBehaviorConfig workerBehaviorConfig ) { - if (!configManager.set(WorkerBehaviourConfig.CONFIG_KEY, workerBehaviourConfig)) { + if (!configManager.set(WorkerBehaviorConfig.CONFIG_KEY, workerBehaviorConfig)) { return Response.status(Response.Status.BAD_REQUEST).build(); } - log.info("Updating Worker configs: %s", workerBehaviourConfig); + log.info("Updating Worker configs: %s", workerBehaviorConfig); return Response.ok().build(); } diff --git a/indexing-service/src/main/java/io/druid/indexing/overlord/setup/WorkerBehaviourConfig.java b/indexing-service/src/main/java/io/druid/indexing/overlord/setup/WorkerBehaviorConfig.java similarity index 95% rename from indexing-service/src/main/java/io/druid/indexing/overlord/setup/WorkerBehaviourConfig.java rename to indexing-service/src/main/java/io/druid/indexing/overlord/setup/WorkerBehaviorConfig.java index 4599184d7a1..afaf9171ea8 100644 --- a/indexing-service/src/main/java/io/druid/indexing/overlord/setup/WorkerBehaviourConfig.java +++ b/indexing-service/src/main/java/io/druid/indexing/overlord/setup/WorkerBehaviorConfig.java @@ -25,7 +25,7 @@ import io.druid.indexing.overlord.autoscaling.AutoScaler; /** */ -public class WorkerBehaviourConfig +public class WorkerBehaviorConfig { public static final String CONFIG_KEY = "worker.config"; @@ -33,7 +33,7 @@ public class WorkerBehaviourConfig private final AutoScaler autoScaler; @JsonCreator - public WorkerBehaviourConfig( + public WorkerBehaviorConfig( @JsonProperty("selectStrategy") WorkerSelectStrategy selectStrategy, @JsonProperty("autoScaler") AutoScaler autoScaler ) @@ -64,7 +64,7 @@ public class WorkerBehaviourConfig return false; } - WorkerBehaviourConfig that = (WorkerBehaviourConfig) o; + WorkerBehaviorConfig that = (WorkerBehaviorConfig) o; if (autoScaler != null ? !autoScaler.equals(that.autoScaler) : that.autoScaler != null) { return false; diff --git a/indexing-service/src/test/java/io/druid/indexing/overlord/autoscaling/SimpleResourceManagementStrategyTest.java b/indexing-service/src/test/java/io/druid/indexing/overlord/autoscaling/SimpleResourceManagementStrategyTest.java index ba7c8632511..763aba6af65 100644 --- a/indexing-service/src/test/java/io/druid/indexing/overlord/autoscaling/SimpleResourceManagementStrategyTest.java +++ b/indexing-service/src/test/java/io/druid/indexing/overlord/autoscaling/SimpleResourceManagementStrategyTest.java @@ -32,7 +32,7 @@ import io.druid.indexing.common.task.NoopTask; import io.druid.indexing.common.task.Task; import io.druid.indexing.overlord.RemoteTaskRunnerWorkItem; import io.druid.indexing.overlord.ZkWorker; -import io.druid.indexing.overlord.setup.WorkerBehaviourConfig; +import io.druid.indexing.overlord.setup.WorkerBehaviorConfig; import io.druid.indexing.worker.TaskAnnouncement; import io.druid.indexing.worker.Worker; import io.druid.jackson.DefaultObjectMapper; @@ -59,7 +59,7 @@ public class SimpleResourceManagementStrategyTest private Task testTask; private SimpleResourceManagementConfig simpleResourceManagementConfig; private SimpleResourceManagementStrategy simpleResourceManagementStrategy; - private AtomicReference workerConfig; + private AtomicReference workerConfig; @Before public void setUp() throws Exception @@ -93,7 +93,7 @@ public class SimpleResourceManagementStrategyTest .setWorkerVersion(""); workerConfig = new AtomicReference<>( - new WorkerBehaviourConfig( + new WorkerBehaviorConfig( null, autoScaler ) diff --git a/indexing-service/src/test/java/io/druid/indexing/overlord/setup/WorkerBehaviourConfigTest.java b/indexing-service/src/test/java/io/druid/indexing/overlord/setup/WorkerBehaviorConfigTest.java similarity index 94% rename from indexing-service/src/test/java/io/druid/indexing/overlord/setup/WorkerBehaviourConfigTest.java rename to indexing-service/src/test/java/io/druid/indexing/overlord/setup/WorkerBehaviorConfigTest.java index 995a272a8aa..49e40b6367b 100644 --- a/indexing-service/src/test/java/io/druid/indexing/overlord/setup/WorkerBehaviourConfigTest.java +++ b/indexing-service/src/test/java/io/druid/indexing/overlord/setup/WorkerBehaviorConfigTest.java @@ -34,12 +34,12 @@ import org.junit.Test; import java.util.Arrays; -public class WorkerBehaviourConfigTest +public class WorkerBehaviorConfigTest { @Test public void testSerde() throws Exception { - WorkerBehaviourConfig config = new WorkerBehaviourConfig( + WorkerBehaviorConfig config = new WorkerBehaviorConfig( new FillCapacityWithAffinityWorkerSelectStrategy( new FillCapacityWithAffinityConfig( ImmutableMap.of("foo", Arrays.asList("localhost")) @@ -82,6 +82,6 @@ public class WorkerBehaviourConfigTest } } ); - Assert.assertEquals(config, mapper.readValue(mapper.writeValueAsBytes(config), WorkerBehaviourConfig.class)); + Assert.assertEquals(config, mapper.readValue(mapper.writeValueAsBytes(config), WorkerBehaviorConfig.class)); } } \ No newline at end of file diff --git a/services/src/main/java/io/druid/cli/CliOverlord.java b/services/src/main/java/io/druid/cli/CliOverlord.java index 70a1f7dadf9..213e6ca7935 100644 --- a/services/src/main/java/io/druid/cli/CliOverlord.java +++ b/services/src/main/java/io/druid/cli/CliOverlord.java @@ -35,6 +35,7 @@ import io.druid.client.indexing.IndexingServiceSelectorConfig; import io.druid.guice.IndexingServiceFirehoseModule; import io.druid.guice.IndexingServiceModuleHelper; import io.druid.guice.IndexingServiceTaskLogsModule; +import io.druid.guice.JacksonConfigProvider; import io.druid.guice.Jerseys; import io.druid.guice.JsonConfigProvider; import io.druid.guice.LazySingleton; @@ -67,6 +68,8 @@ import io.druid.indexing.overlord.autoscaling.SimpleResourceManagementStrategy; import io.druid.indexing.overlord.config.TaskQueueConfig; import io.druid.indexing.overlord.http.OverlordRedirectInfo; import io.druid.indexing.overlord.http.OverlordResource; +import io.druid.indexing.overlord.setup.WorkerBehaviorConfig; +import io.druid.indexing.overlord.setup.WorkerSetupData; import io.druid.indexing.worker.config.WorkerConfig; import io.druid.segment.realtime.firehose.ChatHandlerProvider; import io.druid.server.http.RedirectFilter; @@ -196,6 +199,9 @@ public class CliOverlord extends ServerRunnable biddy.addBinding("remote").to(RemoteTaskRunnerFactory.class).in(LazySingleton.class); binder.bind(RemoteTaskRunnerFactory.class).in(LazySingleton.class); + + JacksonConfigProvider.bind(binder, WorkerSetupData.CONFIG_KEY, WorkerSetupData.class, null); + JacksonConfigProvider.bind(binder, WorkerBehaviorConfig.CONFIG_KEY, WorkerBehaviorConfig.class, null); } private void configureAutoscale(Binder binder)