From bf21a316ca6fba5709ed2544ea8136502ed7b592 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Tue, 3 Dec 2019 12:23:19 +0100 Subject: [PATCH 01/30] Adding support for autoscaling in GCE --- distribution/pom.xml | 2 + docs/configuration/index.md | 8 +- .../extensions-contrib/gce-extensions.md | 103 ++++ docs/development/extensions.md | 1 + extensions-contrib/gce-extensions/pom.xml | 116 ++++ .../autoscaling/gce/GCEAutoScaler.java | 496 ++++++++++++++++++ .../autoscaling/gce/GCEEnvironmentConfig.java | 121 +++++ .../overlord/autoscaling/gce/GCEModule.java | 42 ++ .../overlord/autoscaling/gce/GCEUtils.java | 72 +++ ...rg.apache.druid.initialization.DruidModule | 16 + .../autoscaling/gce/GCEAutoScalerTest.java | 472 +++++++++++++++++ .../autoscaling/gce/GCEUtilsTest.java | 96 ++++ extensions-core/google-extensions/pom.xml | 2 +- licenses.yaml | 16 +- pom.xml | 12 +- website/i18n/en.json | 3 + website/sidebars.json | 1 + 17 files changed, 1571 insertions(+), 8 deletions(-) create mode 100644 docs/development/extensions-contrib/gce-extensions.md create mode 100644 extensions-contrib/gce-extensions/pom.xml create mode 100644 extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScaler.java create mode 100644 extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEEnvironmentConfig.java create mode 100644 extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEModule.java create mode 100644 extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtils.java create mode 100644 extensions-contrib/gce-extensions/src/main/resources/META-INF/services/org.apache.druid.initialization.DruidModule create mode 100644 extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScalerTest.java create mode 100644 extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtilsTest.java diff --git a/distribution/pom.xml b/distribution/pom.xml index a47ead33057b..2b5c3d5d760b 100644 --- a/distribution/pom.xml +++ b/distribution/pom.xml @@ -423,6 +423,8 @@ org.apache.druid.extensions.contrib:druid-moving-average-query -c org.apache.druid.extensions.contrib:druid-tdigestsketch + -c + org.apache.druid.extensions.contrib:gce-extensions diff --git a/docs/configuration/index.md b/docs/configuration/index.md index 982ff5ebd9d8..e1300d19bf6a 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -891,7 +891,7 @@ There are additional configs for autoscaling (if it is enabled): |Property|Description|Default| |--------|-----------|-------| -|`druid.indexer.autoscale.strategy`|Choices are "noop" or "ec2". Sets the strategy to run when autoscaling is required.|noop| +|`druid.indexer.autoscale.strategy`|Choices are "noop", "ec2" or "gce". Sets the strategy to run when autoscaling is required.|noop| |`druid.indexer.autoscale.doAutoscale`|If set to "true" autoscaling will be enabled.|false| |`druid.indexer.autoscale.provisionPeriod`|How often to check whether or not new MiddleManagers should be added.|PT1M| |`druid.indexer.autoscale.terminatePeriod`|How often to check when MiddleManagers should be removed.|PT5M| @@ -1115,7 +1115,9 @@ field. If not provided, the default is to not use it at all. ##### Autoscaler -Amazon's EC2 is currently the only supported autoscaler. +Amazon's EC2 together with Google's GCE are currently the only supported autoscalers. + +EC2's autoscaler properties are: |Property|Description|Default| |--------|-----------|-------| @@ -1125,6 +1127,8 @@ Amazon's EC2 is currently the only supported autoscaler. |`nodeData`|A JSON object that describes how to launch new nodes.|none; required| |`userData`|A JSON object that describes how to configure new nodes. If you have set druid.indexer.autoscale.workerVersion, this must have a versionReplacementString. Otherwise, a versionReplacementString is not necessary.|none; optional| +For GCE's properties, please refer to the [gce-extensions](../extensions-contrib/gce-extensions.md). + ## Data Server This section contains the configuration options for the processes that reside on Data servers (MiddleManagers/Peons and Historicals) in the suggested [three-server configuration](../design/processes.html#server-types). diff --git a/docs/development/extensions-contrib/gce-extensions.md b/docs/development/extensions-contrib/gce-extensions.md new file mode 100644 index 000000000000..fd22b68ed050 --- /dev/null +++ b/docs/development/extensions-contrib/gce-extensions.md @@ -0,0 +1,103 @@ +--- +id: gce-extensions +title: "GCE Extensions" +--- + + + + +To use this Apache Druid (incubating) extension, make sure to [include](../../development/extensions.md#loading-extensions) `gce-extensions`. + +At the moment, this extension enables only Druid to autoscale instances in GCE. + +The extension manages the instances to be scaled up and down through the use of the [Managed Instance Groups](https://cloud.google.com/compute/docs/instance-groups/creating-groups-of-managed-instances#resize_managed_group) +of GCE (MIG from now on). This choice hs been made to ease the configuration of the machines and simplify their +management. + +For this reason, in order to use this extension, the user must have created +1. An instance template with the right machine type and image to bu used to run the MiddleManager +2. A MIG that has been configured to use the instance template created in the point above + +Moreover, in order to be able to rescale the machines in the MIG, the Overlord must run with a service account +guaranteeing the following two scopes from the [Compute Engine API](https://developers.google.com/identity/protocols/googlescopes#computev1) +- `https://www.googleapis.com/auth/cloud-platform` +- `https://www.googleapis.com/auth/compute` + +## Overlord Dynamic Configuration + +The Overlord can dynamically change worker behavior. + +The JSON object can be submitted to the Overlord via a POST request at: + +``` +http://:/druid/indexer/v1/worker +``` + +Optional Header Parameters for auditing the config change can also be specified. + +|Header Param Name| Description | Default | +|----------|-------------|---------| +|`X-Druid-Author`| author making the config change|""| +|`X-Druid-Comment`| comment describing the change being done|""| + +A sample worker config spec is shown below: + +```json +{ + "autoScaler": { + "envConfig" : { + "numInstances" : 1, + "projectId" : "super-project", + "zoneName" : "us-central-1", + "managedInstanceGroupName" : "druid-middlemanagers" + }, + "maxNumWorkers" : 4, + "minNumWorkers" : 2, + "type" : "gce" + } +} +``` + +The configuration of the autoscaler is quite simple and it is made of two levels only. + +The external level specifies the `type`—always `gce` in this case— and two numeric values, +the `maxNumWorkers` and `minNumWorkers` used to define the bounduaries in between which the +number of instances must be at any time. + +The internal level is the `envConfig` and it is used to specify + +- The `numInstances` used to specify how many workers will be spawned at each +request to provision more workers. This is safe to be left to `1` +- The `projectId` used to specify the name of the project in which the MIG resides +- The `zoneName` used to identify in which zone of the worlds the MIG is +- The `managedInstanceGroupName` used to specify the MIG cotaining the instances created or +removed + +Please refer to the Overlord Dynamic Configuration section in the main [documentation](../../configuration/index.md) +for parameters other than the ones specified here, such as `selectStrategy` etc. + +## Known limitations + +- The module internally uses the [ListManagedInstances](https://cloud.google.com/compute/docs/reference/rest/v1/instanceGroupManagers/listManagedInstances) + call from the API and, while the documentation of the API states that the call can be paged through using the + `pageToken` argument, the responses to such call do not provide any `nextPageToken` to set such parameter. This means + that the extension can operate safely with a maximum of 500 MiddleManagers instances at any time (the maximum number + of instances to be returned for each call). + \ No newline at end of file diff --git a/docs/development/extensions.md b/docs/development/extensions.md index 54a50ffb98c5..574f67dafdc5 100644 --- a/docs/development/extensions.md +++ b/docs/development/extensions.md @@ -91,6 +91,7 @@ All of these community extensions can be downloaded using [pull-deps](../operati |druid-influxdb-emitter|InfluxDB metrics emitter|[link](../development/extensions-contrib/influxdb-emitter.md)| |druid-momentsketch|Support for approximate quantile queries using the [momentsketch](https://github.com/stanford-futuredata/momentsketch) library|[link](../development/extensions-contrib/momentsketch-quantiles.md)| |druid-tdigestsketch|Support for approximate sketch aggregators based on [T-Digest](https://github.com/tdunning/t-digest)|[link](../development/extensions-contrib/tdigestsketch-quantiles.md)| +|gce-extensions|GCE Extensions|[link](../development/extensions-contrib/gce-extensions.md)| ## Promoting community extensions to core extensions diff --git a/extensions-contrib/gce-extensions/pom.xml b/extensions-contrib/gce-extensions/pom.xml new file mode 100644 index 000000000000..6be57eaa9f50 --- /dev/null +++ b/extensions-contrib/gce-extensions/pom.xml @@ -0,0 +1,116 @@ + + + + + org.apache.druid + druid + 0.17.0-incubating-SNAPSHOT + ../../pom.xml + + 4.0.0 + + org.apache.druid.extensions.contrib + gce-extensions + gce-extensions + Extension to support the autoscaling in GCE + + + UTF-8 + + + + org.apache.druid + druid-core + ${project.parent.version} + provided + + + org.apache.druid + druid-indexing-service + ${project.parent.version} + provided + + + org.apache.druid + druid-aws-common + ${project.parent.version} + provided + + + org.apache.druid + druid-processing + ${project.parent.version} + provided + + + com.google.code.findbugs + jsr305 + provided + + + com.fasterxml.jackson.core + jackson-annotations + provided + + + com.google.guava + guava + provided + + + com.google.inject + guice + provided + + + com.fasterxml.jackson.core + jackson-databind + provided + + + com.google.apis + google-api-services-compute + v1-rev214-1.25.0 + compile + + + com.google.auth + google-auth-library-oauth2-http + 0.18.0 + + + com.google.auth + google-auth-library-credentials + 0.18.0 + + + + junit + junit + test + + + org.easymock + easymock + test + + + diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScaler.java new file mode 100644 index 000000000000..a799f8db1288 --- /dev/null +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScaler.java @@ -0,0 +1,496 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.indexing.overlord.autoscaling.gce; + +import com.fasterxml.jackson.annotation.JacksonInject; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonTypeName; +import com.google.api.client.googleapis.auth.oauth2.GoogleCredential; +import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport; +import com.google.api.client.http.HttpTransport; +import com.google.api.client.json.JsonFactory; +import com.google.api.client.json.jackson2.JacksonFactory; +import com.google.api.services.compute.Compute; +import com.google.api.services.compute.ComputeScopes; +import com.google.api.services.compute.model.Instance; +import com.google.api.services.compute.model.InstanceGroupManagersDeleteInstancesRequest; +import com.google.api.services.compute.model.InstanceGroupManagersListManagedInstancesResponse; +import com.google.api.services.compute.model.InstanceList; +import com.google.api.services.compute.model.ManagedInstance; +import com.google.api.services.compute.model.Operation; +import com.google.common.net.InetAddresses; +import org.apache.druid.indexing.overlord.autoscaling.AutoScaler; +import org.apache.druid.indexing.overlord.autoscaling.AutoScalingData; +import org.apache.druid.indexing.overlord.autoscaling.SimpleWorkerProvisioningConfig; +import org.apache.druid.java.util.emitter.EmittingLogger; + +import java.io.IOException; +import java.security.GeneralSecurityException; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; + +/** + * This module permits the autoscaling of the workers in GCE + * + * General notes: + * - The IPs are IPs as in Internet Protocol, and they look like 1.2.3.4 + * - The IDs are the names of the instances of instances created, they look like prefix-abcd, + * where the prefix is chosen by you and abcd is a suffix assigned by GCE + */ +@JsonTypeName("gce") +public class GCEAutoScaler implements AutoScaler +{ + private static final EmittingLogger log = new EmittingLogger(GCEAutoScaler.class); + + private final GCEEnvironmentConfig envConfig; + private final int minNumWorkers; + private final int maxNumWorkers; + private final SimpleWorkerProvisioningConfig config; // For future use + + private Compute cachedComputeService = null; + + @JsonCreator + public GCEAutoScaler( + @JsonProperty("minNumWorkers") int minNumWorkers, + @JsonProperty("maxNumWorkers") int maxNumWorkers, + @JsonProperty("envConfig") GCEEnvironmentConfig envConfig, + @JacksonInject SimpleWorkerProvisioningConfig config + ) + { + this.minNumWorkers = minNumWorkers; + this.maxNumWorkers = maxNumWorkers; + this.envConfig = envConfig; + this.config = config; + } + + /** + * CAVEAT: this is meant to be used only for testing passing a mock version of Compute + */ + public GCEAutoScaler( + int minNumWorkers, + int maxNumWorkers, + GCEEnvironmentConfig envConfig, + SimpleWorkerProvisioningConfig config, + Compute compute + ) + { + this(minNumWorkers, maxNumWorkers, envConfig, config); + this.cachedComputeService = compute; + } + + @Override + @JsonProperty + public int getMinNumWorkers() + { + return minNumWorkers; + } + + @Override + @JsonProperty + public int getMaxNumWorkers() + { + return maxNumWorkers; + } + + @Override + @JsonProperty + public GCEEnvironmentConfig getEnvConfig() + { + return envConfig; + } + + private synchronized Compute createComputeService() + throws IOException, GeneralSecurityException, InterruptedException + { + final int max_retries = 5; + final long retries_interval = 5 * 1000; // 5 secs. + + int retries = 0; + while (cachedComputeService == null && retries < max_retries) { + if (retries > 0) { + Thread.sleep(retries_interval); + } + + log.info("Creating new ComputeService [%d/%d]", retries + 1, max_retries); + + try { + HttpTransport httpTransport = GoogleNetHttpTransport.newTrustedTransport(); + JsonFactory jsonFactory = JacksonFactory.getDefaultInstance(); + GoogleCredential credential = GoogleCredential.getApplicationDefault( + httpTransport, + jsonFactory + ); + if (credential.createScopedRequired()) { + List scopes = new ArrayList<>(); + scopes.add(ComputeScopes.CLOUD_PLATFORM); + scopes.add(ComputeScopes.COMPUTE); + credential = credential.createScoped(scopes); + } + + if (credential.getClientAuthentication() != null) { + log.error("Not using a service account, terminating"); + System.exit(1); + } + + cachedComputeService = new Compute.Builder(httpTransport, jsonFactory, credential) + .setApplicationName("DruidAutoscaler") + .build(); + + retries++; + } + catch (Throwable e) { + log.error(e, "Got Exception in creating the ComputeService"); + throw e; + } + } + return cachedComputeService; + } + + // Used to wait for an operation to finish + private Operation.Error waitForOperationEnd( + Compute compute, + Operation operation) throws Exception + { + final long pollInterval = 5 * 1000; // 5 sec + + String status = operation.getStatus(); + String opId = operation.getName(); + while (operation != null && !"DONE".equals(status)) { + Thread.sleep(pollInterval); + Compute.ZoneOperations.Get get = compute.zoneOperations().get( + envConfig.getProjectId(), + envConfig.getZoneName(), + opId + ); + operation = get.execute(); + if (operation != null) { + status = operation.getStatus(); + } + } + return operation == null ? null : operation.getError(); + } + + /** + * When called resizes envConfig.getManagedInstanceGroupName() increasing it by creating + * envConfig.getNumInstances() new workers (unless the maximum is reached). Return the + * IDs of the workers created + */ + @Override + public AutoScalingData provision() + { + final String project = envConfig.getProjectId(); + final String zone = envConfig.getZoneName(); + final int numInstances = envConfig.getNumInstances(); + final String managedInstanceGroupName = envConfig.getManagedInstanceGroupName(); + + try { + List before = getRunningInstances(); + log.debug("Existing instances [%s]", String.join(",", before)); + + int toSize = Math.min(before.size() + numInstances, getMaxNumWorkers()); + if (before.size() >= toSize) { + // nothing to scale + return new AutoScalingData(new ArrayList<>()); + } + log.info("Asked to provision instances, will resize to %d", toSize); + + Compute computeService = createComputeService(); + Compute.InstanceGroupManagers.Resize request = + computeService.instanceGroupManagers().resize(project, zone, + managedInstanceGroupName, toSize); + + Operation response = request.execute(); + Operation.Error err = waitForOperationEnd(computeService, response); + if (err == null || err.isEmpty()) { + List after = getRunningInstances(); + after.removeAll(before); // these should be the new ones + log.debug("Added instances [%s]", String.join(",", after)); + return new AutoScalingData(after); + } else { + log.error("Unable to provision instances: %s", err.toPrettyString()); + } + } + catch (Exception e) { + log.error(e, "Unable to provision any gce instances."); + } + + return new AutoScalingData(new ArrayList<>()); + } + + /** + * Terminats the instances in the list of IPs provided by the caller + */ + @Override + public AutoScalingData terminate(List ips) + { + log.info("Asked to terminate: [%s]", String.join(",", ips)); + + if (ips.isEmpty()) { + return new AutoScalingData(new ArrayList<>()); + } + + List nodeIds = ipToIdLookup(ips); // if they are not IPs, they will be unchanged + try { + return new AutoScalingData(idToIpLookup( + terminateWithIds(nodeIds != null ? nodeIds : new ArrayList<>()).getNodeIds()) + ); + } + catch (Exception e) { + log.error(e, "Unable to terminate any instances."); + } + + return new AutoScalingData(new ArrayList<>()); + } + + private List namesToInstances(List names) + { + List instances = new ArrayList<>(); + for (String name : names) { + instances.add( + // convert the name into a URL's path to be used in calls to the API + String.format(Locale.US, "zones/%s/instances/%s", envConfig.getZoneName(), name) + ); + } + return instances; + } + + /** + * Terminats the instances in the list of IDs provided by the caller + */ + @Override + public AutoScalingData terminateWithIds(List ids) + { + log.info("Asked to terminate IDs: [%s]", String.join(",", ids)); + + if (ids.isEmpty()) { + return new AutoScalingData(new ArrayList<>()); + } + + try { + final String project = envConfig.getProjectId(); + final String zone = envConfig.getZoneName(); + final String managedInstanceGroupName = envConfig.getManagedInstanceGroupName(); + + List before = getRunningInstances(); + + InstanceGroupManagersDeleteInstancesRequest requestBody = + new InstanceGroupManagersDeleteInstancesRequest(); + requestBody.setInstances(namesToInstances(ids)); + + Compute computeService = createComputeService(); + Compute.InstanceGroupManagers.DeleteInstances request = + computeService + .instanceGroupManagers() + .deleteInstances(project, zone, managedInstanceGroupName, requestBody); + + Operation response = request.execute(); + Operation.Error err = waitForOperationEnd(computeService, response); + if (err == null || err.isEmpty()) { + List after = getRunningInstances(); + before.removeAll(after); // keep only the ones no more present + return new AutoScalingData(before); + } else { + log.error("Unable to terminate instances: %s", err.toPrettyString()); + } + } + catch (Exception e) { + log.error(e, "Unable to terminate any instances."); + } + + return new AutoScalingData(new ArrayList<>()); + } + + // Returns the list of the IDs of the machines running in the MIG + private List getRunningInstances() + { + ArrayList ids = new ArrayList<>(); + try { + final String project = envConfig.getProjectId(); + final String zone = envConfig.getZoneName(); + final String managedInstanceGroupName = envConfig.getManagedInstanceGroupName(); + + Compute computeService = createComputeService(); + Compute.InstanceGroupManagers.ListManagedInstances request = + computeService + .instanceGroupManagers() + .listManagedInstances(project, zone, managedInstanceGroupName); + // Notice that while the doc says otherwise, there is not nextPageToken to page + // through results and so everything needs to be in the same page + request.setMaxResults(500L); // 500 is sadly the max + InstanceGroupManagersListManagedInstancesResponse response = request.execute(); + for (ManagedInstance mi : response.getManagedInstances()) { + ids.add(GCEUtils.extractNameFromInstance(mi.getInstance())); + } + log.debug("Found running instances [%s]", String.join(",", ids)); + } + catch (Exception e) { + log.error(e, "Unable to get instances."); + } + return ids; + } + + /** + * Converts the IPs to IDs + */ + @Override + public List ipToIdLookup(List ips) + { + log.debug("Asked IPs -> IDs for: [%s]", String.join(",", ips)); + + if (ips.isEmpty()) { + return new ArrayList<>(); + } + + // If the first one is not an IP, just assume all the other ones are not as well and just + // return them as they are. This check is here because Druid does not check if IPs are + // actually IPs and can send IDs to this function instead + if (!InetAddresses.isInetAddress(ips.get(0))) { + log.debug("Not IPs, doing nothing"); + return ips; + } + + final String project = envConfig.getProjectId(); + final String zone = envConfig.getZoneName(); + try { + Compute computeService = createComputeService(); + Compute.Instances.List request = computeService.instances().list(project, zone); + // Cannot filter by IP atm, see below + // request.setFilter(GCEUtils.buildFilter(ips, "networkInterfaces[0].networkIP")); + + List instanceIds = new ArrayList<>(); + InstanceList response; + do { + response = request.execute(); + if (response.getItems() == null) { + continue; + } + for (Instance instance : response.getItems()) { + // This stupid look up is needed because atm it is not possible to filter + // by IP, see https://issuetracker.google.com/issues/73455339 + if (ips.contains(instance.getNetworkInterfaces().get(0).getNetworkIP())) { + instanceIds.add(instance.getName()); + } + } + request.setPageToken(response.getNextPageToken()); + } while (response.getNextPageToken() != null); + + log.debug("Converted to [%s]", String.join(",", instanceIds)); + return instanceIds; + } + catch (Exception e) { + log.error(e, "Unable to convert IPs to IDs."); + } + + return new ArrayList<>(); + } + + /** + * Converts the IDs to IPs - this is actually never called from the outside but it is called once + * from inside the class if terminate is used instead of terminateWithIds + */ + @Override + public List idToIpLookup(List nodeIds) + { + log.debug("Asked IDs -> IPs for: [%s]", String.join(",", nodeIds)); + + if (nodeIds.isEmpty()) { + return new ArrayList<>(); + } + + final String project = envConfig.getProjectId(); + final String zone = envConfig.getZoneName(); + + try { + Compute computeService = createComputeService(); + Compute.Instances.List request = computeService.instances().list(project, zone); + request.setFilter(GCEUtils.buildFilter(nodeIds, "name")); + + List instanceIps = new ArrayList<>(); + InstanceList response; + do { + response = request.execute(); + if (response.getItems() == null) { + continue; + } + for (Instance instance : response.getItems()) { + // Assuming that every server has at least one network interface... + String ip = instance.getNetworkInterfaces().get(0).getNetworkIP(); + // ...even though some IPs are reported as null on the spot but later they are ok, + // so we skip the ones that are null. fear not, they are picked up later this just + // prevents to have a machine called 'null' around which makes the caller wait for + // it for maxScalingDuration time before doing anything else + if (ip != null && !"null".equals(ip)) { + instanceIps.add(ip); + } else { + // log and skip it + log.warn("Call returned null IP for %s, skipping", instance.getName()); + } + } + request.setPageToken(response.getNextPageToken()); + } while (response.getNextPageToken() != null); + + return instanceIps; + } + catch (Exception e) { + log.error(e, "Unable to convert IDs to IPs."); + } + + return new ArrayList<>(); + } + + @Override + public String toString() + { + return "gceAutoScaler={" + + "envConfig=" + envConfig + + ", maxNumWorkers=" + maxNumWorkers + + ", minNumWorkers=" + minNumWorkers + + '}'; + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + GCEAutoScaler that = (GCEAutoScaler) o; + + return (envConfig != null ? envConfig.equals(that.envConfig) : that.envConfig == null) && + minNumWorkers == that.minNumWorkers && + maxNumWorkers == that.maxNumWorkers; + } + + @Override + public int hashCode() + { + int result = 0; + result = 31 * result + (envConfig != null ? envConfig.hashCode() : 0); + result = 31 * result + minNumWorkers; + result = 31 * result + maxNumWorkers; + return result; + } +} diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEEnvironmentConfig.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEEnvironmentConfig.java new file mode 100644 index 000000000000..81767b26c9de --- /dev/null +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEEnvironmentConfig.java @@ -0,0 +1,121 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.indexing.overlord.autoscaling.gce; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +/** + */ +public class GCEEnvironmentConfig +{ + /** + * numInstances: the number of workers to try to spawn at each call to provision + * projectId: the id of the project where to operate + * zoneName: the name of the zone where to operata + * instanceTemplate: the template to use when creating the instances + * minworkers: the minimum number of workers in the pool (*) + * maxWorkers: the maximum number of workers in the pool (*) + * + * (*) both used by the caller of the AutoScaler to know if it makes sense to call + * provision / terminate or if there is no hope that something would be done + */ + private final int numInstances; + private final String projectId; + private final String zoneName; + private final String managedInstanceGroupName; + + @JsonCreator + public GCEEnvironmentConfig( + @JsonProperty("numInstances") int numInstances, + @JsonProperty("projectId") String projectId, + @JsonProperty("zoneName") String zoneName, + @JsonProperty("managedInstanceGroupName") String managedInstanceGroupName + ) + { + this.numInstances = numInstances; + this.projectId = projectId; + this.zoneName = zoneName; + this.managedInstanceGroupName = managedInstanceGroupName; + } + + @JsonProperty + public int getNumInstances() + { + return numInstances; + } + + + @JsonProperty + String getZoneName() + { + return zoneName; + } + + @JsonProperty + String getProjectId() + { + return projectId; + } + + @JsonProperty + String getManagedInstanceGroupName() + { + return managedInstanceGroupName; + } + + @Override + public String toString() + { + return "GCEEnvironmentConfig={" + + "projectId=" + projectId + + ", zoneName=" + zoneName + + ", numInstances=" + numInstances + + ", managedInstanceGroupName=" + managedInstanceGroupName + + '}'; + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + GCEEnvironmentConfig that = (GCEEnvironmentConfig) o; + return (numInstances == that.numInstances && + projectId.equals(that.projectId) && + zoneName.equals(that.zoneName) && + managedInstanceGroupName.equals(that.managedInstanceGroupName)); + } + + @Override + public int hashCode() + { + int result = projectId != null ? projectId.hashCode() : 0; + result = 31 * result + (zoneName != null ? zoneName.hashCode() : 0); + result = 31 * result + (managedInstanceGroupName != null ? managedInstanceGroupName.hashCode() : 0); + result = 31 * result + numInstances; + return result; + } +} diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEModule.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEModule.java new file mode 100644 index 000000000000..9c2d1c30b0ee --- /dev/null +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEModule.java @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.indexing.overlord.autoscaling.gce; + +import com.fasterxml.jackson.databind.Module; +import com.fasterxml.jackson.databind.module.SimpleModule; +import com.google.inject.Binder; +import org.apache.druid.initialization.DruidModule; + +import java.util.Collections; +import java.util.List; + +public class GCEModule implements DruidModule +{ + @Override + public List getJacksonModules() + { + return Collections.singletonList(new SimpleModule("DruidGCEModule").registerSubtypes(GCEAutoScaler.class)); + } + + @Override + public void configure(Binder binder) + { + } +} diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtils.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtils.java new file mode 100644 index 000000000000..08e637a73990 --- /dev/null +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtils.java @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.indexing.overlord.autoscaling.gce; + +import java.util.Iterator; +import java.util.List; +import java.util.Locale; + +/** + * Simple collection of utilities extracted to ease testing and simplify the GCEAutoScaler class + */ +public class GCEUtils +{ + + /** + * converts https://www.googleapis.com/compute/v1/projects/X/zones/Y/instances/name-of-the-thing + * into just `name-of-the-thing` as it is needed by the other pieces of the API + */ + public static String extractNameFromInstance(String instance) + { + String name = instance; + if (instance != null && !instance.isEmpty()) { + int lastSlash = instance.lastIndexOf('/'); + if (lastSlash > -1) { + name = instance.substring(lastSlash + 1); + } else { + name = instance; // let's assume not the URI like thing + } + } + return name; + } + + /** + * Converts a list of terms to a 'OR' list of terms to look for a specific 'key' + */ + public static String buildFilter(List list, String key) + { + if (list == null || list.isEmpty() || key == null || key.isEmpty()) { + throw new IllegalArgumentException("Arguments cannot be empty of null"); + } + Iterator it = list.iterator(); + + StringBuilder sb = new StringBuilder(); + sb.append(String.format(Locale.US, "(%s = \"%s\")", key, it.next())); + while (it.hasNext()) { + sb.append(" OR ").append(String.format(Locale.US, "(%s = \"%s\")", key, it.next())); + } + return sb.toString(); + } + + // cannot build it! + private GCEUtils() + { + } +} diff --git a/extensions-contrib/gce-extensions/src/main/resources/META-INF/services/org.apache.druid.initialization.DruidModule b/extensions-contrib/gce-extensions/src/main/resources/META-INF/services/org.apache.druid.initialization.DruidModule new file mode 100644 index 000000000000..40992ac78867 --- /dev/null +++ b/extensions-contrib/gce-extensions/src/main/resources/META-INF/services/org.apache.druid.initialization.DruidModule @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF 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. + +org.apache.druid.indexing.overlord.autoscaling.gce.GCEModule diff --git a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScalerTest.java b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScalerTest.java new file mode 100644 index 000000000000..574c0232e6bd --- /dev/null +++ b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScalerTest.java @@ -0,0 +1,472 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.indexing.overlord.autoscaling.gce; + +import com.fasterxml.jackson.databind.BeanProperty; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.InjectableValues; +import com.fasterxml.jackson.databind.Module; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.api.services.compute.Compute; +import com.google.api.services.compute.model.Instance; +import com.google.api.services.compute.model.InstanceGroupManagersDeleteInstancesRequest; +import com.google.api.services.compute.model.InstanceGroupManagersListManagedInstancesResponse; +import com.google.api.services.compute.model.InstanceList; +import com.google.api.services.compute.model.ManagedInstance; +import com.google.api.services.compute.model.NetworkInterface; +import com.google.api.services.compute.model.Operation; +import org.apache.druid.indexing.overlord.autoscaling.AutoScaler; +import org.apache.druid.indexing.overlord.autoscaling.AutoScalingData; +import org.apache.druid.jackson.DefaultObjectMapper; +import org.easymock.EasyMock; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Locale; + +/** + */ +public class GCEAutoScalerTest +{ + private Compute mockCompute = null; + // id -> ip & ip -> id + private Compute.Instances mockInstances = null; + private Compute.Instances.List mockIpToIdRequest = null; + private Compute.Instances.List mockIdToIpRequest = null; + // running instances + private Compute.InstanceGroupManagers mockInstanceGroupManagers = null; + private Compute.InstanceGroupManagers.ListManagedInstances mockInstancesRequest = null; + // terminate + private Compute.InstanceGroupManagers.DeleteInstances mockDeleteRequest = null; + //provision + private Compute.InstanceGroupManagers.Resize mockResizeRequest = null; + + @Before + public void setUp() + { + // for every test let's create all (only a subset needed for each test tho) + + mockCompute = EasyMock.createMock(Compute.class); + + mockInstances = EasyMock.createMock(Compute.Instances.class); + mockIpToIdRequest = EasyMock.createMock(Compute.Instances.List.class); + mockIdToIpRequest = EasyMock.createMock(Compute.Instances.List.class); + + mockInstanceGroupManagers = EasyMock.createMock(Compute.InstanceGroupManagers.class); + mockInstancesRequest = EasyMock.createMock( + Compute.InstanceGroupManagers.ListManagedInstances.class + ); + + mockDeleteRequest = EasyMock.createMock(Compute.InstanceGroupManagers.DeleteInstances.class); + + mockResizeRequest = EasyMock.createMock(Compute.InstanceGroupManagers.Resize.class); + } + + @After + public void tearDown() + { + // not calling verify here as we use different bits and pieces in each test + } + + private static void verifyAutoScaler(final GCEAutoScaler autoScaler) + { + Assert.assertEquals(1, autoScaler.getEnvConfig().getNumInstances()); + Assert.assertEquals(4, autoScaler.getMaxNumWorkers()); + Assert.assertEquals(2, autoScaler.getMinNumWorkers()); + Assert.assertEquals("winkie-country", autoScaler.getEnvConfig().getZoneName()); + Assert.assertEquals("super-project", autoScaler.getEnvConfig().getProjectId()); + Assert.assertEquals("druid-mig", autoScaler.getEnvConfig().getManagedInstanceGroupName()); + } + + @Test + public void testConfig() + { + final String json = "{\n" + + " \"envConfig\" : {\n" + + " \"numInstances\" : 1,\n" + + " \"projectId\" : \"super-project\",\n" + + " \"zoneName\" : \"winkie-country\",\n" + + " \"managedInstanceGroupName\" : \"druid-mig\"\n" + + " },\n" + + " \"maxNumWorkers\" : 4,\n" + + " \"minNumWorkers\" : 2,\n" + + " \"type\" : \"gce\"\n" + + "}"; + + final ObjectMapper objectMapper = new DefaultObjectMapper() + .registerModules((Iterable) new GCEModule().getJacksonModules()); + objectMapper.setInjectableValues( + new InjectableValues() + { + @Override + public Object findInjectableValue( + Object o, + DeserializationContext deserializationContext, + BeanProperty beanProperty, + Object o1 + ) + { + return null; + } + } + ); + + try { + final GCEAutoScaler autoScaler = + (GCEAutoScaler) objectMapper.readValue(json, AutoScaler.class); + verifyAutoScaler(autoScaler); + + final GCEAutoScaler roundTripAutoScaler = (GCEAutoScaler) objectMapper.readValue( + objectMapper.writeValueAsBytes(autoScaler), + AutoScaler.class + ); + verifyAutoScaler(roundTripAutoScaler); + + Assert.assertEquals("Round trip equals", autoScaler, roundTripAutoScaler); + } + catch (Exception e) { + Assert.fail(String.format(Locale.US, "Got exception in test %s", e.getMessage())); + } + } + + private Instance makeInstance(String name, String ip) + { + Instance instance = new Instance(); + instance.setName(name); + NetworkInterface net = new NetworkInterface(); + net.setNetworkIP(ip); + instance.setNetworkInterfaces(Collections.singletonList(net)); + return instance; + } + + @Test + public void testIpToId() throws IOException // for the mock calls, not really throwing + { + GCEAutoScaler autoScaler = new GCEAutoScaler( + 2, + 4, + new GCEEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + null, + mockCompute // <-- I pretend to be Google + ); + + // empty IPs + List ips1 = Collections.emptyList(); + List ids1 = autoScaler.ipToIdLookup(ips1); + Assert.assertEquals(0, ids1.size()); + + // actually not IPs + List ips2 = Collections.singletonList("foo-bar-baz"); + List ids2 = autoScaler.ipToIdLookup(ips2); + Assert.assertEquals(ips2, ids2); + + // actually IPs + Instance i1 = makeInstance("foo", "1.2.3.5"); // not the one we look for + Instance i2 = makeInstance("bar", "1.2.3.4"); // the one we do look for + InstanceList mockResponse = new InstanceList(); + mockResponse.setNextPageToken(null); + mockResponse.setItems(Arrays.asList(i1, i2)); + + EasyMock.expect(mockIpToIdRequest.execute()).andReturn(mockResponse); + EasyMock.expect(mockIpToIdRequest.setPageToken(EasyMock.anyString())).andReturn( + mockIpToIdRequest // the method needs to return something, what is actually irrelevant here + ); + EasyMock.replay(mockIpToIdRequest); + + EasyMock.expect(mockInstances.list("proj-x", "us-central-1")).andReturn(mockIpToIdRequest); + EasyMock.replay(mockInstances); + + EasyMock.expect(mockCompute.instances()).andReturn(mockInstances); + EasyMock.replay(mockCompute); + + List ips3 = Collections.singletonList("1.2.3.4"); + List ids3 = autoScaler.ipToIdLookup(ips3); + Assert.assertEquals(1, ids3.size()); + Assert.assertEquals("bar", ids3.get(0)); + + EasyMock.verify(mockCompute); + EasyMock.verify(mockInstances); + EasyMock.verify(mockIpToIdRequest); + } + + @Test + public void testIdToIp() throws IOException // for the mock calls, not really throwing + { + GCEAutoScaler autoScaler = new GCEAutoScaler( + 2, + 4, + new GCEEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + null, + mockCompute // <-- I pretend to be Google + ); + + // empty IPs + List ids1 = Collections.emptyList(); + List ips1 = autoScaler.idToIpLookup(ids1); + Assert.assertEquals(0, ips1.size()); + + // actually IDs + Instance i1 = makeInstance("foo", "null"); // invalid ip, not returned + Instance i2 = makeInstance("bar", "1.2.3.4"); // valid ip, returned + InstanceList mockResponse = new InstanceList(); + mockResponse.setNextPageToken(null); + mockResponse.setItems(Arrays.asList(i1, i2)); + + EasyMock.expect(mockIdToIpRequest.setFilter("(name = \"foo\") OR (name = \"bar\")")).andReturn( + mockIdToIpRequest // the method needs to return something but it is actually irrelevant + ); + EasyMock.expect(mockIdToIpRequest.execute()).andReturn(mockResponse); + EasyMock.expect(mockIdToIpRequest.setPageToken(EasyMock.anyString())).andReturn( + mockIdToIpRequest // the method needs to return something but it is actually irrelevant + ); + EasyMock.replay(mockIdToIpRequest); + + EasyMock.expect(mockInstances.list("proj-x", "us-central-1")).andReturn(mockIdToIpRequest); + EasyMock.replay(mockInstances); + + EasyMock.expect(mockCompute.instances()).andReturn(mockInstances); + EasyMock.replay(mockCompute); + + List ids3 = Arrays.asList("foo", "bar"); + List ips3 = autoScaler.idToIpLookup(ids3); + Assert.assertEquals(1, ips3.size()); + Assert.assertEquals("1.2.3.4", ips3.get(0)); + + EasyMock.verify(mockCompute); + EasyMock.verify(mockInstances); + EasyMock.verify(mockIdToIpRequest); + } + + private InstanceGroupManagersListManagedInstancesResponse createRunningInstances( + List instances + ) + { + InstanceGroupManagersListManagedInstancesResponse mockResponse = + new InstanceGroupManagersListManagedInstancesResponse(); + mockResponse.setManagedInstances(new ArrayList<>()); + for (String x : instances) { + ManagedInstance mi = new ManagedInstance(); + mi.setInstance(x); + mockResponse.getManagedInstances().add(mi); + } + return mockResponse; + } + + @Test + public void testTerminateWithIds() throws IOException // for the mock calls, not really throwing + { + GCEAutoScaler autoScaler = new GCEAutoScaler( + 2, + 4, + new GCEEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + null, + mockCompute // <-- I pretend to be Google + ); + + // set up getRunningInstances results + InstanceGroupManagersListManagedInstancesResponse beforeRunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar", + "http://xyz/baz" + )); + InstanceGroupManagersListManagedInstancesResponse afterRunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar" + )); + + EasyMock.expect(mockInstancesRequest.execute()).andReturn(beforeRunningInstance); // 1st call + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.expect(mockInstancesRequest.execute()).andReturn(afterRunningInstance); // 2nd call + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.replay(mockInstancesRequest); + + + EasyMock.expect(mockInstanceGroupManagers.listManagedInstances( + "proj-x", + "us-central-1", + "druid-mig" + )).andReturn(mockInstancesRequest).times(2); + + // set up the delete operation + Operation mockResponse = new Operation(); + mockResponse.setStatus("DONE"); + mockResponse.setError(new Operation.Error()); + + EasyMock.expect(mockDeleteRequest.execute()).andReturn(mockResponse); + EasyMock.replay(mockDeleteRequest); + + InstanceGroupManagersDeleteInstancesRequest requestBody = + new InstanceGroupManagersDeleteInstancesRequest(); + requestBody.setInstances(Collections.singletonList("zones/us-central-1/instances/baz")); + + EasyMock.expect(mockInstanceGroupManagers.deleteInstances( + "proj-x", + "us-central-1", + "druid-mig", + requestBody + )).andReturn(mockDeleteRequest); + + EasyMock.replay(mockInstanceGroupManagers); + + // called twice in getRunningInstances... + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + // ...and once in terminateWithIds + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + + // and that's all folks! + EasyMock.replay(mockCompute); + + AutoScalingData autoScalingData = + autoScaler.terminateWithIds(Collections.singletonList("baz")); + Assert.assertEquals(1, autoScalingData.getNodeIds().size()); + Assert.assertEquals("baz", autoScalingData.getNodeIds().get(0)); + + EasyMock.verify(mockCompute); + EasyMock.verify(mockInstanceGroupManagers); + EasyMock.verify(mockDeleteRequest); + EasyMock.verify(mockInstancesRequest); + } + + @Test + public void testProvision() throws IOException // for the mock calls, not really throwing + { + GCEAutoScaler autoScaler = new GCEAutoScaler( + 2, + 4, + new GCEEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + null, + mockCompute // <-- I pretend to be Google + ); + + // set up getRunningInstances results + InstanceGroupManagersListManagedInstancesResponse beforeRunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar" + )); + InstanceGroupManagersListManagedInstancesResponse afterRunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar", + "http://xyz/baz" + )); + + EasyMock.expect(mockInstancesRequest.execute()).andReturn(beforeRunningInstance); // 1st call + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.expect(mockInstancesRequest.execute()).andReturn(afterRunningInstance); // 2nd call + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.replay(mockInstancesRequest); + + EasyMock.expect(mockInstanceGroupManagers.listManagedInstances( + "proj-x", + "us-central-1", + "druid-mig" + )).andReturn(mockInstancesRequest).times(2); + + // set up the resize operation + Operation mockResponse = new Operation(); + mockResponse.setStatus("DONE"); + mockResponse.setError(new Operation.Error()); + + EasyMock.expect(mockResizeRequest.execute()).andReturn(mockResponse); + EasyMock.replay(mockResizeRequest); + + EasyMock.expect(mockInstanceGroupManagers.resize( + "proj-x", + "us-central-1", + "druid-mig", + 3 + )).andReturn(mockResizeRequest); + + EasyMock.replay(mockInstanceGroupManagers); + + // called twice in getRunningInstances... + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + // ...and once in provision + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + + // and that's all folks! + EasyMock.replay(mockCompute); + + AutoScalingData autoScalingData = autoScaler.provision(); + Assert.assertEquals(1, autoScalingData.getNodeIds().size()); + Assert.assertEquals("baz", autoScalingData.getNodeIds().get(0)); + + EasyMock.verify(mockCompute); + EasyMock.verify(mockInstanceGroupManagers); + EasyMock.verify(mockResizeRequest); + EasyMock.verify(mockInstancesRequest); + } + + @Test + public void testProvisionSkipped() throws IOException // for the mock calls, not really throwing + { + GCEAutoScaler autoScaler = new GCEAutoScaler( + 2, + 4, + new GCEEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + null, + mockCompute // <-- I pretend to be Google + ); + + // set up getRunningInstances results + InstanceGroupManagersListManagedInstancesResponse beforeRunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar", + "http://xyz/baz", + "http://xyz/zab" // already max instances, will not scale + )); + + EasyMock.expect(mockInstancesRequest.execute()).andReturn(beforeRunningInstance); + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.replay(mockInstancesRequest); + + EasyMock.expect(mockInstanceGroupManagers.listManagedInstances( + "proj-x", + "us-central-1", + "druid-mig" + )).andReturn(mockInstancesRequest); + + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + EasyMock.replay(mockInstanceGroupManagers); + + // and that's all folks! + EasyMock.replay(mockCompute); + + AutoScalingData autoScalingData = autoScaler.provision(); + Assert.assertEquals(0, autoScalingData.getNodeIds().size()); + + EasyMock.verify(mockCompute); + EasyMock.verify(mockInstancesRequest); + EasyMock.verify(mockInstanceGroupManagers); + } +} diff --git a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtilsTest.java b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtilsTest.java new file mode 100644 index 000000000000..d5d2839a10cc --- /dev/null +++ b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtilsTest.java @@ -0,0 +1,96 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.indexing.overlord.autoscaling.gce; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; + +/** + */ +public class GCEUtilsTest +{ + @Test + public void testExtractNameFromInstance() + { + String instance0 = + "https://www.googleapis.com/compute/v1/projects/X/zones/Y/instances/name-of-the-thing"; + Assert.assertEquals("name-of-the-thing", GCEUtils.extractNameFromInstance(instance0)); + + String instance1 = "https://www.googleapis.com/compute/v1/projects/X/zones/Y/instances/"; + Assert.assertEquals("", GCEUtils.extractNameFromInstance(instance1)); + + String instance2 = "name-of-the-thing"; + Assert.assertEquals("name-of-the-thing", GCEUtils.extractNameFromInstance(instance2)); + + String instance3 = null; + Assert.assertEquals(null, GCEUtils.extractNameFromInstance(instance3)); + + String instance4 = ""; + Assert.assertEquals("", GCEUtils.extractNameFromInstance(instance4)); + } + + @Test + public void testBuildFilter() + { + List list0 = null; + try { + String x = GCEUtils.buildFilter(list0, "name"); + Assert.fail("Exception should have been thrown!"); + } + catch (IllegalArgumentException e) { + // ok to be here! + } + + List list1 = new ArrayList<>(); + try { + String x = GCEUtils.buildFilter(list1, "name"); + Assert.fail("Exception should have been thrown!"); + } + catch (IllegalArgumentException e) { + // ok to be here! + } + + List list2 = new ArrayList<>(); + list2.add("foo"); + try { + String x = GCEUtils.buildFilter(list2, null); + Assert.fail("Exception should have been thrown!"); + } + catch (IllegalArgumentException e) { + // ok to be here! + } + + List list3 = new ArrayList<>(); + list3.add("foo"); + Assert.assertEquals("(name = \"foo\")", GCEUtils.buildFilter(list3, "name")); + + List list4 = new ArrayList<>(); + list4.add("foo"); + list4.add("bar"); + Assert.assertEquals( + "(name = \"foo\") OR (name = \"bar\")", + GCEUtils.buildFilter(list4, "name") + ); + } + +} diff --git a/extensions-core/google-extensions/pom.xml b/extensions-core/google-extensions/pom.xml index e17da6df7fd6..c820aa9c7457 100644 --- a/extensions-core/google-extensions/pom.xml +++ b/extensions-core/google-extensions/pom.xml @@ -34,7 +34,7 @@ - v1-rev79-${com.google.apis.client.version} + v1-rev158-${com.google.apis.client.version} diff --git a/licenses.yaml b/licenses.yaml index a23bb4eaf4d3..a41a24da7060 100644 --- a/licenses.yaml +++ b/licenses.yaml @@ -4086,12 +4086,22 @@ name: Google Cloud Storage JSON API license_category: binary module: extensions/druid-google-extensions license_name: Apache License version 2.0 -version: v1-rev79-1.22.0 +version: v1-rev158-1.25.0 libraries: - com.google.apis: google-api-services-storage --- +name: Google Compute Engine API +license_category: binary +module: extensions/gce-extensions +license_name: Apache License version 2.0 +version: v1-rev214-1.25.0 +libraries: + - com.google.apis: google-api-services-compute + +--- + name: "Jackson Module: Guice" license_category: binary module: java-core @@ -4106,7 +4116,7 @@ name: Google APIs Client Library For Java license_category: binary module: java-core license_name: Apache License version 2.0 -version: 1.22.0 +version: 1.25.0 libraries: - com.google.api-client: google-api-client @@ -4116,7 +4126,7 @@ name: Google HTTP Client Library For Java license_category: binary module: java-core license_name: Apache License version 2.0 -version: 1.22.0 +version: 1.25.0 libraries: - com.google.http-client: google-http-client - com.google.http-client: google-http-client-jackson2 diff --git a/pom.xml b/pom.xml index 4ab46a8884fe..ab9cdbf6531c 100644 --- a/pom.xml +++ b/pom.xml @@ -113,8 +113,8 @@ 3.4.14 2.5.7 - 1.22.0 - + 1.25.0 + v1-rev214-1.25.0 apache.snapshots Apache Snapshot Repository https://repository.apache.org/snapshots @@ -187,6 +187,7 @@ extensions-contrib/moving-average-query extensions-contrib/tdigestsketch extensions-contrib/influxdb-emitter + extensions-contrib/gce-extensions distribution @@ -1192,6 +1193,13 @@ resilience4j-bulkhead ${resilience4j.version} + + + com.google.apis + google-api-services-compute + ${com.google.apis.compute.version} + provided + org.testng diff --git a/website/i18n/en.json b/website/i18n/en.json index c769b68bdcb7..0c689546c6a0 100644 --- a/website/i18n/en.json +++ b/website/i18n/en.json @@ -143,6 +143,9 @@ "development/extensions-contrib/time-min-max": { "title": "Timestamp Min/Max aggregators" }, + "development/extensions-contrib/gce-extensions": { + "title": "GCE Extensions" + }, "development/extensions-core/approximate-histograms": { "title": "Approximate Histogram aggregators" }, diff --git a/website/sidebars.json b/website/sidebars.json index 004619af1f6c..da8a73458417 100644 --- a/website/sidebars.json +++ b/website/sidebars.json @@ -214,6 +214,7 @@ "development/extensions-contrib/tdigestsketch-quantiles", "development/extensions-contrib/thrift", "development/extensions-contrib/time-min-max", + "development/extensions-contrib/gce-extensions", "ingestion/standalone-realtime" ] } From 7d42d04f46b0eb0d2d2fb3b4873f8adf0f11eb52 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Wed, 4 Dec 2019 12:25:14 +0100 Subject: [PATCH 02/30] adding extra google deps also in gce pom --- extensions-contrib/gce-extensions/pom.xml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/extensions-contrib/gce-extensions/pom.xml b/extensions-contrib/gce-extensions/pom.xml index 6be57eaa9f50..e86857644fc5 100644 --- a/extensions-contrib/gce-extensions/pom.xml +++ b/extensions-contrib/gce-extensions/pom.xml @@ -101,6 +101,21 @@ google-auth-library-credentials 0.18.0 + + com.google.http-client + google-http-client + 1.25.0 + + + com.google.http-client + google-http-client-jackson2 + 1.25.0 + + + com.google.api-client + google-api-client + 1.25.0 + junit From e7475b7d498d7d2ada37520ecfc4758a3f474524 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Wed, 4 Dec 2019 14:28:03 +0100 Subject: [PATCH 03/30] fix link in doc --- docs/configuration/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration/index.md b/docs/configuration/index.md index e1300d19bf6a..e17a83ac34d8 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -1127,7 +1127,7 @@ EC2's autoscaler properties are: |`nodeData`|A JSON object that describes how to launch new nodes.|none; required| |`userData`|A JSON object that describes how to configure new nodes. If you have set druid.indexer.autoscale.workerVersion, this must have a versionReplacementString. Otherwise, a versionReplacementString is not necessary.|none; optional| -For GCE's properties, please refer to the [gce-extensions](../extensions-contrib/gce-extensions.md). +For GCE's properties, please refer to the [gce-extensions](../development/extensions-contrib/gce-extensions.md). ## Data Server From bfc705c19e278e2997b20885944c76d58a7561c6 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Wed, 4 Dec 2019 16:25:52 +0100 Subject: [PATCH 04/30] remove unused deps --- extensions-contrib/gce-extensions/pom.xml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/extensions-contrib/gce-extensions/pom.xml b/extensions-contrib/gce-extensions/pom.xml index e86857644fc5..95767545e136 100644 --- a/extensions-contrib/gce-extensions/pom.xml +++ b/extensions-contrib/gce-extensions/pom.xml @@ -91,16 +91,6 @@ v1-rev214-1.25.0 compile - - com.google.auth - google-auth-library-oauth2-http - 0.18.0 - - - com.google.auth - google-auth-library-credentials - 0.18.0 - com.google.http-client google-http-client From c8eda74f46c08740a612ac3f76a137f5312b0294 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Wed, 4 Dec 2019 16:40:39 +0100 Subject: [PATCH 05/30] adding terms to spelling file --- docs/development/extensions-contrib/gce-extensions.md | 6 +++--- website/.spelling | 6 ++++++ website/i18n/en.json | 6 +++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/docs/development/extensions-contrib/gce-extensions.md b/docs/development/extensions-contrib/gce-extensions.md index fd22b68ed050..687f6bb37912 100644 --- a/docs/development/extensions-contrib/gce-extensions.md +++ b/docs/development/extensions-contrib/gce-extensions.md @@ -28,7 +28,7 @@ To use this Apache Druid (incubating) extension, make sure to [include](../../de At the moment, this extension enables only Druid to autoscale instances in GCE. The extension manages the instances to be scaled up and down through the use of the [Managed Instance Groups](https://cloud.google.com/compute/docs/instance-groups/creating-groups-of-managed-instances#resize_managed_group) -of GCE (MIG from now on). This choice hs been made to ease the configuration of the machines and simplify their +of GCE (MIG from now on). This choice has been made to ease the configuration of the machines and simplify their management. For this reason, in order to use this extension, the user must have created @@ -78,7 +78,7 @@ A sample worker config spec is shown below: The configuration of the autoscaler is quite simple and it is made of two levels only. The external level specifies the `type`—always `gce` in this case— and two numeric values, -the `maxNumWorkers` and `minNumWorkers` used to define the bounduaries in between which the +the `maxNumWorkers` and `minNumWorkers` used to define the boundaries in between which the number of instances must be at any time. The internal level is the `envConfig` and it is used to specify @@ -87,7 +87,7 @@ The internal level is the `envConfig` and it is used to specify request to provision more workers. This is safe to be left to `1` - The `projectId` used to specify the name of the project in which the MIG resides - The `zoneName` used to identify in which zone of the worlds the MIG is -- The `managedInstanceGroupName` used to specify the MIG cotaining the instances created or +- The `managedInstanceGroupName` used to specify the MIG containing the instances created or removed Please refer to the Overlord Dynamic Configuration section in the main [documentation](../../configuration/index.md) diff --git a/website/.spelling b/website/.spelling index 531d0833b470..dd783f44059f 100644 --- a/website/.spelling +++ b/website/.spelling @@ -1548,6 +1548,7 @@ EventReceiverFirehose File.getFreeSpace File.getTotalSpace ForkJoinPool +GCE HadoopIndexTasks HttpEmitter HttpPostEmitter @@ -1556,6 +1557,7 @@ JRE8u60 KeyManager L1 L2 +ListManagedInstances LoadSpec LoggingEmitter Los_Angeles @@ -1597,6 +1599,8 @@ affinityConfig allowAll ANDed array_mod +autoscale +autoscalers batch_index_task cgroup classloader @@ -1634,6 +1638,8 @@ floatMax floatMin floatSum freeSpacePercent +gce +gce-extensions getCanonicalHostName groupBy hdfs diff --git a/website/i18n/en.json b/website/i18n/en.json index 0c689546c6a0..f4e86f66edee 100644 --- a/website/i18n/en.json +++ b/website/i18n/en.json @@ -101,6 +101,9 @@ "development/extensions-contrib/distinctcount": { "title": "DistinctCount Aggregator" }, + "development/extensions-contrib/gce-extensions": { + "title": "GCE Extensions" + }, "development/extensions-contrib/graphite": { "title": "Graphite Emitter" }, @@ -143,9 +146,6 @@ "development/extensions-contrib/time-min-max": { "title": "Timestamp Min/Max aggregators" }, - "development/extensions-contrib/gce-extensions": { - "title": "GCE Extensions" - }, "development/extensions-core/approximate-histograms": { "title": "Approximate Histogram aggregators" }, From a18511077b81954036236c60492e156d7bc4f362 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Thu, 13 Feb 2020 14:58:03 +0100 Subject: [PATCH 06/30] version in pom 0.17.0-incubating-SNAPSHOT --> 0.18.0-SNAPSHOT --- extensions-contrib/gce-extensions/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions-contrib/gce-extensions/pom.xml b/extensions-contrib/gce-extensions/pom.xml index 95767545e136..f454438dbeb2 100644 --- a/extensions-contrib/gce-extensions/pom.xml +++ b/extensions-contrib/gce-extensions/pom.xml @@ -22,7 +22,7 @@ org.apache.druid druid - 0.17.0-incubating-SNAPSHOT + 0.18.0-SNAPSHOT ../../pom.xml 4.0.0 From 5607d175bf9981ba64eea3a650e3207567238ed2 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Fri, 14 Feb 2020 17:27:59 +0100 Subject: [PATCH 07/30] GCEXyz -> GceXyz in naming for consistency --- ...{GCEAutoScaler.java => GceAutoScaler.java} | 24 +++++++------- ...tConfig.java => GceEnvironmentConfig.java} | 8 ++--- .../gce/{GCEModule.java => GceModule.java} | 4 +-- .../gce/{GCEUtils.java => GceUtils.java} | 6 ++-- ...rg.apache.druid.initialization.DruidModule | 2 +- ...ScalerTest.java => GceAutoScalerTest.java} | 32 +++++++++---------- .../{GCEUtilsTest.java => GceUtilsTest.java} | 22 ++++++------- 7 files changed, 49 insertions(+), 49 deletions(-) rename extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/{GCEAutoScaler.java => GceAutoScaler.java} (96%) rename extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/{GCEEnvironmentConfig.java => GceEnvironmentConfig.java} (95%) rename extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/{GCEModule.java => GceModule.java} (93%) rename extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/{GCEUtils.java => GceUtils.java} (96%) rename extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/{GCEAutoScalerTest.java => GceAutoScalerTest.java} (94%) rename extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/{GCEUtilsTest.java => GceUtilsTest.java} (79%) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java similarity index 96% rename from extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScaler.java rename to extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index a799f8db1288..842eff775ba6 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -57,11 +57,11 @@ * where the prefix is chosen by you and abcd is a suffix assigned by GCE */ @JsonTypeName("gce") -public class GCEAutoScaler implements AutoScaler +public class GceAutoScaler implements AutoScaler { - private static final EmittingLogger log = new EmittingLogger(GCEAutoScaler.class); + private static final EmittingLogger log = new EmittingLogger(GceAutoScaler.class); - private final GCEEnvironmentConfig envConfig; + private final GceEnvironmentConfig envConfig; private final int minNumWorkers; private final int maxNumWorkers; private final SimpleWorkerProvisioningConfig config; // For future use @@ -69,10 +69,10 @@ public class GCEAutoScaler implements AutoScaler private Compute cachedComputeService = null; @JsonCreator - public GCEAutoScaler( + public GceAutoScaler( @JsonProperty("minNumWorkers") int minNumWorkers, @JsonProperty("maxNumWorkers") int maxNumWorkers, - @JsonProperty("envConfig") GCEEnvironmentConfig envConfig, + @JsonProperty("envConfig") GceEnvironmentConfig envConfig, @JacksonInject SimpleWorkerProvisioningConfig config ) { @@ -85,10 +85,10 @@ public GCEAutoScaler( /** * CAVEAT: this is meant to be used only for testing passing a mock version of Compute */ - public GCEAutoScaler( + public GceAutoScaler( int minNumWorkers, int maxNumWorkers, - GCEEnvironmentConfig envConfig, + GceEnvironmentConfig envConfig, SimpleWorkerProvisioningConfig config, Compute compute ) @@ -113,7 +113,7 @@ public int getMaxNumWorkers() @Override @JsonProperty - public GCEEnvironmentConfig getEnvConfig() + public GceEnvironmentConfig getEnvConfig() { return envConfig; } @@ -338,7 +338,7 @@ private List getRunningInstances() request.setMaxResults(500L); // 500 is sadly the max InstanceGroupManagersListManagedInstancesResponse response = request.execute(); for (ManagedInstance mi : response.getManagedInstances()) { - ids.add(GCEUtils.extractNameFromInstance(mi.getInstance())); + ids.add(GceUtils.extractNameFromInstance(mi.getInstance())); } log.debug("Found running instances [%s]", String.join(",", ids)); } @@ -374,7 +374,7 @@ public List ipToIdLookup(List ips) Compute computeService = createComputeService(); Compute.Instances.List request = computeService.instances().list(project, zone); // Cannot filter by IP atm, see below - // request.setFilter(GCEUtils.buildFilter(ips, "networkInterfaces[0].networkIP")); + // request.setFilter(GceUtils.buildFilter(ips, "networkInterfaces[0].networkIP")); List instanceIds = new ArrayList<>(); InstanceList response; @@ -422,7 +422,7 @@ public List idToIpLookup(List nodeIds) try { Compute computeService = createComputeService(); Compute.Instances.List request = computeService.instances().list(project, zone); - request.setFilter(GCEUtils.buildFilter(nodeIds, "name")); + request.setFilter(GceUtils.buildFilter(nodeIds, "name")); List instanceIps = new ArrayList<>(); InstanceList response; @@ -477,7 +477,7 @@ public boolean equals(Object o) return false; } - GCEAutoScaler that = (GCEAutoScaler) o; + GceAutoScaler that = (GceAutoScaler) o; return (envConfig != null ? envConfig.equals(that.envConfig) : that.envConfig == null) && minNumWorkers == that.minNumWorkers && diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEEnvironmentConfig.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java similarity index 95% rename from extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEEnvironmentConfig.java rename to extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java index 81767b26c9de..1c76ff0e2aeb 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEEnvironmentConfig.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java @@ -24,7 +24,7 @@ /** */ -public class GCEEnvironmentConfig +public class GceEnvironmentConfig { /** * numInstances: the number of workers to try to spawn at each call to provision @@ -43,7 +43,7 @@ public class GCEEnvironmentConfig private final String managedInstanceGroupName; @JsonCreator - public GCEEnvironmentConfig( + public GceEnvironmentConfig( @JsonProperty("numInstances") int numInstances, @JsonProperty("projectId") String projectId, @JsonProperty("zoneName") String zoneName, @@ -84,7 +84,7 @@ String getManagedInstanceGroupName() @Override public String toString() { - return "GCEEnvironmentConfig={" + + return "GceEnvironmentConfig={" + "projectId=" + projectId + ", zoneName=" + zoneName + ", numInstances=" + numInstances + @@ -102,7 +102,7 @@ public boolean equals(Object o) return false; } - GCEEnvironmentConfig that = (GCEEnvironmentConfig) o; + GceEnvironmentConfig that = (GceEnvironmentConfig) o; return (numInstances == that.numInstances && projectId.equals(that.projectId) && zoneName.equals(that.zoneName) && diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEModule.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceModule.java similarity index 93% rename from extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEModule.java rename to extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceModule.java index 9c2d1c30b0ee..ce1d1c1b20fd 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEModule.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceModule.java @@ -27,12 +27,12 @@ import java.util.Collections; import java.util.List; -public class GCEModule implements DruidModule +public class GceModule implements DruidModule { @Override public List getJacksonModules() { - return Collections.singletonList(new SimpleModule("DruidGCEModule").registerSubtypes(GCEAutoScaler.class)); + return Collections.singletonList(new SimpleModule("DruidGCEModule").registerSubtypes(GceAutoScaler.class)); } @Override diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtils.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceUtils.java similarity index 96% rename from extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtils.java rename to extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceUtils.java index 08e637a73990..131f5a07d450 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtils.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceUtils.java @@ -24,9 +24,9 @@ import java.util.Locale; /** - * Simple collection of utilities extracted to ease testing and simplify the GCEAutoScaler class + * Simple collection of utilities extracted to ease testing and simplify the GceAutoScaler class */ -public class GCEUtils +public class GceUtils { /** @@ -66,7 +66,7 @@ public static String buildFilter(List list, String key) } // cannot build it! - private GCEUtils() + private GceUtils() { } } diff --git a/extensions-contrib/gce-extensions/src/main/resources/META-INF/services/org.apache.druid.initialization.DruidModule b/extensions-contrib/gce-extensions/src/main/resources/META-INF/services/org.apache.druid.initialization.DruidModule index 40992ac78867..c0fecfd530bd 100644 --- a/extensions-contrib/gce-extensions/src/main/resources/META-INF/services/org.apache.druid.initialization.DruidModule +++ b/extensions-contrib/gce-extensions/src/main/resources/META-INF/services/org.apache.druid.initialization.DruidModule @@ -13,4 +13,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -org.apache.druid.indexing.overlord.autoscaling.gce.GCEModule +org.apache.druid.indexing.overlord.autoscaling.gce.GceModule diff --git a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScalerTest.java b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java similarity index 94% rename from extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScalerTest.java rename to extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java index 574c0232e6bd..ca7e4fcb1516 100644 --- a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEAutoScalerTest.java +++ b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java @@ -50,7 +50,7 @@ /** */ -public class GCEAutoScalerTest +public class GceAutoScalerTest { private Compute mockCompute = null; // id -> ip & ip -> id @@ -92,7 +92,7 @@ public void tearDown() // not calling verify here as we use different bits and pieces in each test } - private static void verifyAutoScaler(final GCEAutoScaler autoScaler) + private static void verifyAutoScaler(final GceAutoScaler autoScaler) { Assert.assertEquals(1, autoScaler.getEnvConfig().getNumInstances()); Assert.assertEquals(4, autoScaler.getMaxNumWorkers()); @@ -118,7 +118,7 @@ public void testConfig() + "}"; final ObjectMapper objectMapper = new DefaultObjectMapper() - .registerModules((Iterable) new GCEModule().getJacksonModules()); + .registerModules((Iterable) new GceModule().getJacksonModules()); objectMapper.setInjectableValues( new InjectableValues() { @@ -136,11 +136,11 @@ public Object findInjectableValue( ); try { - final GCEAutoScaler autoScaler = - (GCEAutoScaler) objectMapper.readValue(json, AutoScaler.class); + final GceAutoScaler autoScaler = + (GceAutoScaler) objectMapper.readValue(json, AutoScaler.class); verifyAutoScaler(autoScaler); - final GCEAutoScaler roundTripAutoScaler = (GCEAutoScaler) objectMapper.readValue( + final GceAutoScaler roundTripAutoScaler = (GceAutoScaler) objectMapper.readValue( objectMapper.writeValueAsBytes(autoScaler), AutoScaler.class ); @@ -166,10 +166,10 @@ private Instance makeInstance(String name, String ip) @Test public void testIpToId() throws IOException // for the mock calls, not really throwing { - GCEAutoScaler autoScaler = new GCEAutoScaler( + GceAutoScaler autoScaler = new GceAutoScaler( 2, 4, - new GCEEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), null, mockCompute // <-- I pretend to be Google ); @@ -216,10 +216,10 @@ public void testIpToId() throws IOException // for the mock calls, not really th @Test public void testIdToIp() throws IOException // for the mock calls, not really throwing { - GCEAutoScaler autoScaler = new GCEAutoScaler( + GceAutoScaler autoScaler = new GceAutoScaler( 2, 4, - new GCEEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), null, mockCompute // <-- I pretend to be Google ); @@ -279,10 +279,10 @@ private InstanceGroupManagersListManagedInstancesResponse createRunningInstances @Test public void testTerminateWithIds() throws IOException // for the mock calls, not really throwing { - GCEAutoScaler autoScaler = new GCEAutoScaler( + GceAutoScaler autoScaler = new GceAutoScaler( 2, 4, - new GCEEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), null, mockCompute // <-- I pretend to be Google ); @@ -357,10 +357,10 @@ public void testTerminateWithIds() throws IOException // for the mock calls, not @Test public void testProvision() throws IOException // for the mock calls, not really throwing { - GCEAutoScaler autoScaler = new GCEAutoScaler( + GceAutoScaler autoScaler = new GceAutoScaler( 2, 4, - new GCEEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), null, mockCompute // <-- I pretend to be Google ); @@ -429,10 +429,10 @@ public void testProvision() throws IOException // for the mock calls, not really @Test public void testProvisionSkipped() throws IOException // for the mock calls, not really throwing { - GCEAutoScaler autoScaler = new GCEAutoScaler( + GceAutoScaler autoScaler = new GceAutoScaler( 2, 4, - new GCEEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), null, mockCompute // <-- I pretend to be Google ); diff --git a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtilsTest.java b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceUtilsTest.java similarity index 79% rename from extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtilsTest.java rename to extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceUtilsTest.java index d5d2839a10cc..7c88355e1a39 100644 --- a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GCEUtilsTest.java +++ b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceUtilsTest.java @@ -27,26 +27,26 @@ /** */ -public class GCEUtilsTest +public class GceUtilsTest { @Test public void testExtractNameFromInstance() { String instance0 = "https://www.googleapis.com/compute/v1/projects/X/zones/Y/instances/name-of-the-thing"; - Assert.assertEquals("name-of-the-thing", GCEUtils.extractNameFromInstance(instance0)); + Assert.assertEquals("name-of-the-thing", GceUtils.extractNameFromInstance(instance0)); String instance1 = "https://www.googleapis.com/compute/v1/projects/X/zones/Y/instances/"; - Assert.assertEquals("", GCEUtils.extractNameFromInstance(instance1)); + Assert.assertEquals("", GceUtils.extractNameFromInstance(instance1)); String instance2 = "name-of-the-thing"; - Assert.assertEquals("name-of-the-thing", GCEUtils.extractNameFromInstance(instance2)); + Assert.assertEquals("name-of-the-thing", GceUtils.extractNameFromInstance(instance2)); String instance3 = null; - Assert.assertEquals(null, GCEUtils.extractNameFromInstance(instance3)); + Assert.assertEquals(null, GceUtils.extractNameFromInstance(instance3)); String instance4 = ""; - Assert.assertEquals("", GCEUtils.extractNameFromInstance(instance4)); + Assert.assertEquals("", GceUtils.extractNameFromInstance(instance4)); } @Test @@ -54,7 +54,7 @@ public void testBuildFilter() { List list0 = null; try { - String x = GCEUtils.buildFilter(list0, "name"); + String x = GceUtils.buildFilter(list0, "name"); Assert.fail("Exception should have been thrown!"); } catch (IllegalArgumentException e) { @@ -63,7 +63,7 @@ public void testBuildFilter() List list1 = new ArrayList<>(); try { - String x = GCEUtils.buildFilter(list1, "name"); + String x = GceUtils.buildFilter(list1, "name"); Assert.fail("Exception should have been thrown!"); } catch (IllegalArgumentException e) { @@ -73,7 +73,7 @@ public void testBuildFilter() List list2 = new ArrayList<>(); list2.add("foo"); try { - String x = GCEUtils.buildFilter(list2, null); + String x = GceUtils.buildFilter(list2, null); Assert.fail("Exception should have been thrown!"); } catch (IllegalArgumentException e) { @@ -82,14 +82,14 @@ public void testBuildFilter() List list3 = new ArrayList<>(); list3.add("foo"); - Assert.assertEquals("(name = \"foo\")", GCEUtils.buildFilter(list3, "name")); + Assert.assertEquals("(name = \"foo\")", GceUtils.buildFilter(list3, "name")); List list4 = new ArrayList<>(); list4.add("foo"); list4.add("bar"); Assert.assertEquals( "(name = \"foo\") OR (name = \"bar\")", - GCEUtils.buildFilter(list4, "name") + GceUtils.buildFilter(list4, "name") ); } From ff0d8d415b41783bc129232d7b1af1b0ec997fbf Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Fri, 14 Feb 2020 18:21:13 +0100 Subject: [PATCH 08/30] add preconditions --- .../overlord/autoscaling/gce/GceAutoScaler.java | 7 +++++++ .../autoscaling/gce/GceEnvironmentConfig.java | 14 +++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index 842eff775ba6..e9ad2e5b5e55 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -36,6 +36,7 @@ import com.google.api.services.compute.model.InstanceList; import com.google.api.services.compute.model.ManagedInstance; import com.google.api.services.compute.model.Operation; +import com.google.common.base.Preconditions; import com.google.common.net.InetAddresses; import org.apache.druid.indexing.overlord.autoscaling.AutoScaler; import org.apache.druid.indexing.overlord.autoscaling.AutoScalingData; @@ -76,7 +77,13 @@ public GceAutoScaler( @JacksonInject SimpleWorkerProvisioningConfig config ) { + Preconditions.checkArgument(minNumWorkers > 0, + "minNumWorkers must be greater than 0"); this.minNumWorkers = minNumWorkers; + Preconditions.checkArgument(maxNumWorkers > 0, + "maxNumWorkers must be greater than 0"); + Preconditions.checkArgument(maxNumWorkers > minNumWorkers, + "maxNumWorkers must be greater than minNumWorkers"); this.maxNumWorkers = maxNumWorkers; this.envConfig = envConfig; this.config = config; diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java index 1c76ff0e2aeb..3d2575e73fb0 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Preconditions; /** */ @@ -50,10 +51,17 @@ public GceEnvironmentConfig( @JsonProperty("managedInstanceGroupName") String managedInstanceGroupName ) { + Preconditions.checkArgument(numInstances > 0, + "numIntances must be greater than 0"); this.numInstances = numInstances; - this.projectId = projectId; - this.zoneName = zoneName; - this.managedInstanceGroupName = managedInstanceGroupName; + this.projectId = Preconditions.checkNotNull(projectId, + "projectId must be not null"); + this.zoneName = Preconditions.checkNotNull(zoneName, + "zoneName nust be not null"); + this.managedInstanceGroupName = Preconditions.checkNotNull( + managedInstanceGroupName, + "managedInstanceGroupName must be not null" + ); } @JsonProperty From 7b0761a9173c686a3c963e8c9817458c19aee284 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Fri, 14 Feb 2020 18:23:12 +0100 Subject: [PATCH 09/30] add VisibleForTesting annotation --- .../druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index e9ad2e5b5e55..9880028405f6 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -38,6 +38,7 @@ import com.google.api.services.compute.model.Operation; import com.google.common.base.Preconditions; import com.google.common.net.InetAddresses; +import org.apache.curator.shaded.com.google.common.annotations.VisibleForTesting; import org.apache.druid.indexing.overlord.autoscaling.AutoScaler; import org.apache.druid.indexing.overlord.autoscaling.AutoScalingData; import org.apache.druid.indexing.overlord.autoscaling.SimpleWorkerProvisioningConfig; @@ -92,6 +93,7 @@ public GceAutoScaler( /** * CAVEAT: this is meant to be used only for testing passing a mock version of Compute */ + @VisibleForTesting public GceAutoScaler( int minNumWorkers, int maxNumWorkers, From 96d380418ec8469b2212ca5b7d8121dbc167184c Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Fri, 14 Feb 2020 18:26:38 +0100 Subject: [PATCH 10/30] typos in comments --- .../indexing/overlord/autoscaling/gce/GceAutoScaler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index 9880028405f6..b7c753882757 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -246,7 +246,7 @@ public AutoScalingData provision() } /** - * Terminats the instances in the list of IPs provided by the caller + * Terminates the instances in the list of IPs provided by the caller */ @Override public AutoScalingData terminate(List ips) @@ -283,7 +283,7 @@ private List namesToInstances(List names) } /** - * Terminats the instances in the list of IDs provided by the caller + * Terminates the instances in the list of IDs provided by the caller */ @Override public AutoScalingData terminateWithIds(List ids) From 6a43f4998eb869abe6566fe036e78add11cae1bd Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Mon, 17 Feb 2020 11:09:49 +0100 Subject: [PATCH 11/30] use StringUtils.format instead of String.format --- .../indexing/overlord/autoscaling/gce/GceAutoScaler.java | 4 ++-- .../druid/indexing/overlord/autoscaling/gce/GceUtils.java | 7 ++++--- .../overlord/autoscaling/gce/GceAutoScalerTest.java | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index b7c753882757..288ace593c36 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -42,13 +42,13 @@ import org.apache.druid.indexing.overlord.autoscaling.AutoScaler; import org.apache.druid.indexing.overlord.autoscaling.AutoScalingData; import org.apache.druid.indexing.overlord.autoscaling.SimpleWorkerProvisioningConfig; +import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.emitter.EmittingLogger; import java.io.IOException; import java.security.GeneralSecurityException; import java.util.ArrayList; import java.util.List; -import java.util.Locale; /** * This module permits the autoscaling of the workers in GCE @@ -276,7 +276,7 @@ private List namesToInstances(List names) for (String name : names) { instances.add( // convert the name into a URL's path to be used in calls to the API - String.format(Locale.US, "zones/%s/instances/%s", envConfig.getZoneName(), name) + StringUtils.format("zones/%s/instances/%s", envConfig.getZoneName(), name) ); } return instances; diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceUtils.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceUtils.java index 131f5a07d450..6e9109f78213 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceUtils.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceUtils.java @@ -19,9 +19,10 @@ package org.apache.druid.indexing.overlord.autoscaling.gce; +import org.apache.druid.java.util.common.StringUtils; + import java.util.Iterator; import java.util.List; -import java.util.Locale; /** * Simple collection of utilities extracted to ease testing and simplify the GceAutoScaler class @@ -58,9 +59,9 @@ public static String buildFilter(List list, String key) Iterator it = list.iterator(); StringBuilder sb = new StringBuilder(); - sb.append(String.format(Locale.US, "(%s = \"%s\")", key, it.next())); + sb.append(StringUtils.format("(%s = \"%s\")", key, it.next())); while (it.hasNext()) { - sb.append(" OR ").append(String.format(Locale.US, "(%s = \"%s\")", key, it.next())); + sb.append(" OR ").append(StringUtils.format("(%s = \"%s\")", key, it.next())); } return sb.toString(); } diff --git a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java index ca7e4fcb1516..f1564aa42a1f 100644 --- a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java +++ b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java @@ -35,6 +35,7 @@ import org.apache.druid.indexing.overlord.autoscaling.AutoScaler; import org.apache.druid.indexing.overlord.autoscaling.AutoScalingData; import org.apache.druid.jackson.DefaultObjectMapper; +import org.apache.druid.java.util.common.StringUtils; import org.easymock.EasyMock; import org.junit.After; import org.junit.Assert; @@ -46,7 +47,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Locale; /** */ @@ -149,7 +149,7 @@ public Object findInjectableValue( Assert.assertEquals("Round trip equals", autoScaler, roundTripAutoScaler); } catch (Exception e) { - Assert.fail(String.format(Locale.US, "Got exception in test %s", e.getMessage())); + Assert.fail(StringUtils.format("Got exception in test %s", e.getMessage())); } } From 8d03c8426a1e8398c62e925cb68b8757bbff36be Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Mon, 17 Feb 2020 11:55:40 +0100 Subject: [PATCH 12/30] use custom exception instead of exit --- .../autoscaling/gce/GceAutoScaler.java | 5 ++- .../autoscaling/gce/GceServiceException.java | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceServiceException.java diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index 288ace593c36..b03611bf5d2c 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -128,7 +128,7 @@ public GceEnvironmentConfig getEnvConfig() } private synchronized Compute createComputeService() - throws IOException, GeneralSecurityException, InterruptedException + throws IOException, GeneralSecurityException, InterruptedException, GceServiceException { final int max_retries = 5; final long retries_interval = 5 * 1000; // 5 secs. @@ -156,8 +156,7 @@ private synchronized Compute createComputeService() } if (credential.getClientAuthentication() != null) { - log.error("Not using a service account, terminating"); - System.exit(1); + throw new GceServiceException("Not using a service account"); } cachedComputeService = new Compute.Builder(httpTransport, jsonFactory, credential) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceServiceException.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceServiceException.java new file mode 100644 index 000000000000..e618d46ee2f4 --- /dev/null +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceServiceException.java @@ -0,0 +1,32 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.indexing.overlord.autoscaling.gce; + + +/** + * Provides a specialized Exception type for the GCE module + */ +public class GceServiceException extends Exception +{ + public GceServiceException(String message) + { + super(message); + } +} From 721f4c052dc73595dd816bea81f5ef951c1d4582 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Mon, 17 Feb 2020 12:38:09 +0100 Subject: [PATCH 13/30] factorize interval time between retries --- .../overlord/autoscaling/gce/GceAutoScaler.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index b03611bf5d2c..e8efb6510071 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -70,6 +70,8 @@ public class GceAutoScaler implements AutoScaler private Compute cachedComputeService = null; + private final long POLL_INTERVAL_MS = 5 * 1000; // 5 sec + @JsonCreator public GceAutoScaler( @JsonProperty("minNumWorkers") int minNumWorkers, @@ -130,16 +132,15 @@ public GceEnvironmentConfig getEnvConfig() private synchronized Compute createComputeService() throws IOException, GeneralSecurityException, InterruptedException, GceServiceException { - final int max_retries = 5; - final long retries_interval = 5 * 1000; // 5 secs. + final int maxRetries = 5; int retries = 0; - while (cachedComputeService == null && retries < max_retries) { + while (cachedComputeService == null && retries < maxRetries) { if (retries > 0) { - Thread.sleep(retries_interval); + Thread.sleep(this.POLL_INTERVAL_MS); } - log.info("Creating new ComputeService [%d/%d]", retries + 1, max_retries); + log.info("Creating new ComputeService [%d/%d]", retries + 1, maxRetries); try { HttpTransport httpTransport = GoogleNetHttpTransport.newTrustedTransport(); @@ -178,12 +179,10 @@ private Operation.Error waitForOperationEnd( Compute compute, Operation operation) throws Exception { - final long pollInterval = 5 * 1000; // 5 sec - String status = operation.getStatus(); String opId = operation.getName(); while (operation != null && !"DONE".equals(status)) { - Thread.sleep(pollInterval); + Thread.sleep(this.POLL_INTERVAL_MS); Compute.ZoneOperations.Get get = compute.zoneOperations().get( envConfig.getProjectId(), envConfig.getZoneName(), From a1029af49a2375a5733a9ad591605795b326f86e Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Mon, 17 Feb 2020 16:08:11 +0100 Subject: [PATCH 14/30] making literal value a constant --- .../indexing/overlord/autoscaling/gce/GceAutoScaler.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index e8efb6510071..d45091b63093 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -329,6 +329,8 @@ public AutoScalingData terminateWithIds(List ids) // Returns the list of the IDs of the machines running in the MIG private List getRunningInstances() { + static long maxResults = 500L; // 500 is sadly the max, see below + ArrayList ids = new ArrayList<>(); try { final String project = envConfig.getProjectId(); @@ -342,7 +344,7 @@ private List getRunningInstances() .listManagedInstances(project, zone, managedInstanceGroupName); // Notice that while the doc says otherwise, there is not nextPageToken to page // through results and so everything needs to be in the same page - request.setMaxResults(500L); // 500 is sadly the max + request.setMaxResults(maxResults); InstanceGroupManagersListManagedInstancesResponse response = request.execute(); for (ManagedInstance mi : response.getManagedInstances()) { ids.add(GceUtils.extractNameFromInstance(mi.getInstance())); From 35dea2737be1aaffd1fe8c0e16fd6c9b119741fe Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Mon, 17 Feb 2020 17:06:02 +0100 Subject: [PATCH 15/30] iter all network interfaces --- .../indexing/overlord/autoscaling/gce/GceAutoScaler.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index d45091b63093..fc554d0e6b90 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -35,6 +35,7 @@ import com.google.api.services.compute.model.InstanceGroupManagersListManagedInstancesResponse; import com.google.api.services.compute.model.InstanceList; import com.google.api.services.compute.model.ManagedInstance; +import com.google.api.services.compute.model.NetworkInterface; import com.google.api.services.compute.model.Operation; import com.google.common.base.Preconditions; import com.google.common.net.InetAddresses; @@ -329,7 +330,7 @@ public AutoScalingData terminateWithIds(List ids) // Returns the list of the IDs of the machines running in the MIG private List getRunningInstances() { - static long maxResults = 500L; // 500 is sadly the max, see below + final long maxResults = 500L; // 500 is sadly the max, see below ArrayList ids = new ArrayList<>(); try { @@ -395,8 +396,10 @@ public List ipToIdLookup(List ips) for (Instance instance : response.getItems()) { // This stupid look up is needed because atm it is not possible to filter // by IP, see https://issuetracker.google.com/issues/73455339 - if (ips.contains(instance.getNetworkInterfaces().get(0).getNetworkIP())) { - instanceIds.add(instance.getName()); + for (NetworkInterface ni : instance.getNetworkInterfaces()) { + if (ips.contains(ni.getNetworkIP())) { + instanceIds.add(instance.getName()); + } } } request.setPageToken(response.getNextPageToken()); From 7a5aac2656f43be58b45838184c9051ae0fa3e5e Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Mon, 17 Feb 2020 17:07:53 +0100 Subject: [PATCH 16/30] use provided on google (non api) deps --- extensions-contrib/gce-extensions/pom.xml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/extensions-contrib/gce-extensions/pom.xml b/extensions-contrib/gce-extensions/pom.xml index f454438dbeb2..3ef69cf7cf45 100644 --- a/extensions-contrib/gce-extensions/pom.xml +++ b/extensions-contrib/gce-extensions/pom.xml @@ -94,17 +94,17 @@ com.google.http-client google-http-client - 1.25.0 + provided com.google.http-client google-http-client-jackson2 - 1.25.0 + provided com.google.api-client google-api-client - 1.25.0 + provided From ccede8f75d22a1709a0e6bf0799dbeae4a12dafa Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Mon, 17 Feb 2020 18:15:10 +0100 Subject: [PATCH 17/30] adding missing dep --- extensions-contrib/gce-extensions/pom.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/extensions-contrib/gce-extensions/pom.xml b/extensions-contrib/gce-extensions/pom.xml index 3ef69cf7cf45..b4b686b4d22f 100644 --- a/extensions-contrib/gce-extensions/pom.xml +++ b/extensions-contrib/gce-extensions/pom.xml @@ -106,6 +106,11 @@ google-api-client provided + + org.apache.curator + curator-client + provided + junit From e2b352bc83b11d4fa25bfe7f974b103d7596e9ca Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Tue, 18 Feb 2020 09:34:41 +0100 Subject: [PATCH 18/30] removing unneded this and use Objects methods instead o 3-way if in hash and comparison --- .../overlord/autoscaling/gce/GceAutoScaler.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index fc554d0e6b90..3d6cb7618998 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -71,7 +71,7 @@ public class GceAutoScaler implements AutoScaler private Compute cachedComputeService = null; - private final long POLL_INTERVAL_MS = 5 * 1000; // 5 sec + private static final long POLL_INTERVAL_MS = 5 * 1000; // 5 sec @JsonCreator public GceAutoScaler( @@ -138,7 +138,7 @@ private synchronized Compute createComputeService() int retries = 0; while (cachedComputeService == null && retries < maxRetries) { if (retries > 0) { - Thread.sleep(this.POLL_INTERVAL_MS); + Thread.sleep(POLL_INTERVAL_MS); } log.info("Creating new ComputeService [%d/%d]", retries + 1, maxRetries); @@ -183,7 +183,7 @@ private Operation.Error waitForOperationEnd( String status = operation.getStatus(); String opId = operation.getName(); while (operation != null && !"DONE".equals(status)) { - Thread.sleep(this.POLL_INTERVAL_MS); + Thread.sleep(POLL_INTERVAL_MS); Compute.ZoneOperations.Get get = compute.zoneOperations().get( envConfig.getProjectId(), envConfig.getZoneName(), @@ -491,7 +491,7 @@ public boolean equals(Object o) GceAutoScaler that = (GceAutoScaler) o; - return (envConfig != null ? envConfig.equals(that.envConfig) : that.envConfig == null) && + return Object.equals(envConfig, that.envConfig) && minNumWorkers == that.minNumWorkers && maxNumWorkers == that.maxNumWorkers; } @@ -500,7 +500,7 @@ public boolean equals(Object o) public int hashCode() { int result = 0; - result = 31 * result + (envConfig != null ? envConfig.hashCode() : 0); + result = 31 * result + Object.hashCode(envConfig); result = 31 * result + minNumWorkers; result = 31 * result + maxNumWorkers; return result; From e67e8676d518722e2112184f936a761d5d83a646 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Tue, 18 Feb 2020 09:39:26 +0100 Subject: [PATCH 19/30] adding import --- .../indexing/overlord/autoscaling/gce/GceAutoScaler.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index 3d6cb7618998..43fb068419e8 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -50,6 +50,7 @@ import java.security.GeneralSecurityException; import java.util.ArrayList; import java.util.List; +import java.util.Objects; /** * This module permits the autoscaling of the workers in GCE @@ -491,7 +492,7 @@ public boolean equals(Object o) GceAutoScaler that = (GceAutoScaler) o; - return Object.equals(envConfig, that.envConfig) && + return Objects.equals(envConfig, that.envConfig) && minNumWorkers == that.minNumWorkers && maxNumWorkers == that.maxNumWorkers; } @@ -500,7 +501,7 @@ public boolean equals(Object o) public int hashCode() { int result = 0; - result = 31 * result + Object.hashCode(envConfig); + result = 31 * result + Objects.hashCode(envConfig); result = 31 * result + minNumWorkers; result = 31 * result + maxNumWorkers; return result; From 0964bad411ebd5cdc132d80b80fbd88ab378d604 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Thu, 20 Feb 2020 11:43:24 +0100 Subject: [PATCH 20/30] adding retries around getRunningInstances and adding limit for operation end waiting --- .../autoscaling/gce/GceAutoScaler.java | 44 ++++- .../autoscaling/gce/GceEnvironmentConfig.java | 2 +- .../autoscaling/gce/GceAutoScalerTest.java | 167 ++++++++++++++++++ 3 files changed, 205 insertions(+), 8 deletions(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index 43fb068419e8..d923d334c36e 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -73,6 +73,8 @@ public class GceAutoScaler implements AutoScaler private Compute cachedComputeService = null; private static final long POLL_INTERVAL_MS = 5 * 1000; // 5 sec + private static final int RUNNING_INSTANCES_MAX_RETRIES = 10; + private static final int OPERATION_END_MAX_RETRIES = 10; @JsonCreator public GceAutoScaler( @@ -183,7 +185,11 @@ private Operation.Error waitForOperationEnd( { String status = operation.getStatus(); String opId = operation.getName(); - while (operation != null && !"DONE".equals(status)) { + for (int i = 0; i < OPERATION_END_MAX_RETRIES; i++) { + if (operation == null || "DONE".equals(status)) { + return operation == null ? null : operation.getError(); + } + log.info("Waiting for operation %s to end", opId); Thread.sleep(POLL_INTERVAL_MS); Compute.ZoneOperations.Get get = compute.zoneOperations().get( envConfig.getProjectId(), @@ -195,7 +201,9 @@ private Operation.Error waitForOperationEnd( status = operation.getStatus(); } } - return operation == null ? null : operation.getError(); + throw new InterruptedException( + StringUtils.format("Timed out waiting for operation %s to complete", opId) + ); } /** @@ -230,9 +238,20 @@ public AutoScalingData provision() Operation response = request.execute(); Operation.Error err = waitForOperationEnd(computeService, response); if (err == null || err.isEmpty()) { - List after = getRunningInstances(); + List after = null; + // as the waitForOperationEnd only waits for the operation to be scheduled + // this loop waits until the requested machines actually go up (or up to a + // certain amount of retries in checking) + for (int i = 0; i < RUNNING_INSTANCES_MAX_RETRIES; i++) { + after = getRunningInstances(); + if (after.size() == toSize) { + break; + } + log.info("Machines not up yet, waiting"); + Thread.sleep(POLL_INTERVAL_MS); + } after.removeAll(before); // these should be the new ones - log.debug("Added instances [%s]", String.join(",", after)); + log.info("Added instances [%s]", String.join(",", after)); return new AutoScalingData(after); } else { log.error("Unable to provision instances: %s", err.toPrettyString()); @@ -314,7 +333,18 @@ public AutoScalingData terminateWithIds(List ids) Operation response = request.execute(); Operation.Error err = waitForOperationEnd(computeService, response); if (err == null || err.isEmpty()) { - List after = getRunningInstances(); + List after = null; + // as the waitForOperationEnd only waits for the operation to be scheduled + // this loop waits until the requested machines actually go down (or up to a + // certain amount of retries in checking) + for (int i = 0; i < RUNNING_INSTANCES_MAX_RETRIES; i++) { + after = getRunningInstances(); + if (after.size() == (before.size() - ids.size())) { + break; + } + log.info("Machines not down yet, waiting"); + Thread.sleep(POLL_INTERVAL_MS); + } before.removeAll(after); // keep only the ones no more present return new AutoScalingData(before); } else { @@ -365,7 +395,7 @@ private List getRunningInstances() @Override public List ipToIdLookup(List ips) { - log.debug("Asked IPs -> IDs for: [%s]", String.join(",", ips)); + log.info("Asked IPs -> IDs for: [%s]", String.join(",", ips)); if (ips.isEmpty()) { return new ArrayList<>(); @@ -423,7 +453,7 @@ public List ipToIdLookup(List ips) @Override public List idToIpLookup(List nodeIds) { - log.debug("Asked IDs -> IPs for: [%s]", String.join(",", nodeIds)); + log.info("Asked IDs -> IPs for: [%s]", String.join(",", nodeIds)); if (nodeIds.isEmpty()) { return new ArrayList<>(); diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java index 3d2575e73fb0..bb1d042342c6 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java @@ -52,7 +52,7 @@ public GceEnvironmentConfig( ) { Preconditions.checkArgument(numInstances > 0, - "numIntances must be greater than 0"); + "numInstances must be greater than 0"); this.numInstances = numInstances; this.projectId = Preconditions.checkNotNull(projectId, "projectId must be not null"); diff --git a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java index f1564aa42a1f..cac9270a275c 100644 --- a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java +++ b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java @@ -354,6 +354,93 @@ public void testTerminateWithIds() throws IOException // for the mock calls, not EasyMock.verify(mockInstancesRequest); } + @Test + public void testTerminateWithIdsWithMissingRemoval() throws IOException // for the mock calls, not really throwing + { + GceAutoScaler autoScaler = new GceAutoScaler( + 2, + 4, + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + null, + mockCompute // <-- I pretend to be Google + ); + + // set up getRunningInstances results + InstanceGroupManagersListManagedInstancesResponse beforeRunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar", + "http://xyz/baz" + )); + InstanceGroupManagersListManagedInstancesResponse after1RunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar", + "http://xyz/baz" + )); // not changing anything, will trigger the loop around getRunningInstances + InstanceGroupManagersListManagedInstancesResponse after2RunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar" + )); // now the machine got dropped! + + EasyMock.expect(mockInstancesRequest.execute()).andReturn(beforeRunningInstance); // 1st call + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.expect(mockInstancesRequest.execute()).andReturn(after1RunningInstance); // 2nd call, the next is needed + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.expect(mockInstancesRequest.execute()).andReturn(after2RunningInstance); // 3rd call, this unblocks + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.replay(mockInstancesRequest); + + + EasyMock.expect(mockInstanceGroupManagers.listManagedInstances( + "proj-x", + "us-central-1", + "druid-mig" + )).andReturn(mockInstancesRequest).times(3); + + // set up the delete operation + Operation mockResponse = new Operation(); + mockResponse.setStatus("DONE"); + mockResponse.setError(new Operation.Error()); + + EasyMock.expect(mockDeleteRequest.execute()).andReturn(mockResponse); + EasyMock.replay(mockDeleteRequest); + + InstanceGroupManagersDeleteInstancesRequest requestBody = + new InstanceGroupManagersDeleteInstancesRequest(); + requestBody.setInstances(Collections.singletonList("zones/us-central-1/instances/baz")); + + EasyMock.expect(mockInstanceGroupManagers.deleteInstances( + "proj-x", + "us-central-1", + "druid-mig", + requestBody + )).andReturn(mockDeleteRequest); + + EasyMock.replay(mockInstanceGroupManagers); + + // called three times in getRunningInstances... + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + // ...and once in terminateWithIds + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + + // and that's all folks! + EasyMock.replay(mockCompute); + + AutoScalingData autoScalingData = + autoScaler.terminateWithIds(Collections.singletonList("baz")); + Assert.assertEquals(1, autoScalingData.getNodeIds().size()); + Assert.assertEquals("baz", autoScalingData.getNodeIds().get(0)); + + EasyMock.verify(mockCompute); + EasyMock.verify(mockInstanceGroupManagers); + EasyMock.verify(mockDeleteRequest); + EasyMock.verify(mockInstancesRequest); + } + @Test public void testProvision() throws IOException // for the mock calls, not really throwing { @@ -469,4 +556,84 @@ public void testProvisionSkipped() throws IOException // for the mock calls, not EasyMock.verify(mockInstancesRequest); EasyMock.verify(mockInstanceGroupManagers); } + + @Test + public void testProvisionWithMissingNewInstances() throws IOException // for the mock calls, not really throwing + { + GceAutoScaler autoScaler = new GceAutoScaler( + 2, + 4, + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + null, + mockCompute // <-- I pretend to be Google + ); + + // set up getRunningInstances results + InstanceGroupManagersListManagedInstancesResponse beforeRunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar" + )); + InstanceGroupManagersListManagedInstancesResponse after1RunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar" + )); // not changing anything, will trigger the loop around getRunningInstances + InstanceGroupManagersListManagedInstancesResponse after2RunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar", + "http://xyz/baz" + )); // now the new machine is here! + + EasyMock.expect(mockInstancesRequest.execute()).andReturn(beforeRunningInstance); // 1st call + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.expect(mockInstancesRequest.execute()).andReturn(after1RunningInstance); // 2nd call, the next is needed + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.expect(mockInstancesRequest.execute()).andReturn(after2RunningInstance); // 3rd call, this unblocks + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.replay(mockInstancesRequest); + + EasyMock.expect(mockInstanceGroupManagers.listManagedInstances( + "proj-x", + "us-central-1", + "druid-mig" + )).andReturn(mockInstancesRequest).times(3); + + // set up the resize operation + Operation mockResponse = new Operation(); + mockResponse.setStatus("DONE"); + mockResponse.setError(new Operation.Error()); + + EasyMock.expect(mockResizeRequest.execute()).andReturn(mockResponse); + EasyMock.replay(mockResizeRequest); + + EasyMock.expect(mockInstanceGroupManagers.resize( + "proj-x", + "us-central-1", + "druid-mig", + 3 + )).andReturn(mockResizeRequest); + + EasyMock.replay(mockInstanceGroupManagers); + + // called three times in getRunningInstances... + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + // ...and once in provision + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + + // and that's all folks! + EasyMock.replay(mockCompute); + + AutoScalingData autoScalingData = autoScaler.provision(); + Assert.assertEquals(1, autoScalingData.getNodeIds().size()); + Assert.assertEquals("baz", autoScalingData.getNodeIds().get(0)); + + EasyMock.verify(mockCompute); + EasyMock.verify(mockInstanceGroupManagers); + EasyMock.verify(mockResizeRequest); + EasyMock.verify(mockInstancesRequest); + } } From 8b0ae54369f4cf5457a6681c1d12d10a3a97eef8 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Wed, 26 Feb 2020 17:56:49 +0100 Subject: [PATCH 21/30] refactor GceEnvironmentConfig.hashCode --- .../overlord/autoscaling/gce/GceEnvironmentConfig.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java index bb1d042342c6..2bc9df3e78d4 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java @@ -23,6 +23,8 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; +import java.util.Objects; + /** */ public class GceEnvironmentConfig @@ -120,9 +122,10 @@ public boolean equals(Object o) @Override public int hashCode() { - int result = projectId != null ? projectId.hashCode() : 0; - result = 31 * result + (zoneName != null ? zoneName.hashCode() : 0); - result = 31 * result + (managedInstanceGroupName != null ? managedInstanceGroupName.hashCode() : 0); + int result = 0; + result = 31 * result + Objects.hashCode(projectId); + result = 31 * result + Objects.hashCode(zoneName); + result = 31 * result + Objects.hashCode(managedInstanceGroupName); result = 31 * result + numInstances; return result; } From c58b2d8cbc03142a10d6b273b8339e59d52e71c8 Mon Sep 17 00:00:00 2001 From: Francesco Nidito <11637948+frnidito@users.noreply.github.com> Date: Fri, 17 Apr 2020 10:29:56 +0200 Subject: [PATCH 22/30] 0.18.0-SNAPSHOT -> 0.19.0-SNAPSHOT --- extensions-contrib/gce-extensions/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions-contrib/gce-extensions/pom.xml b/extensions-contrib/gce-extensions/pom.xml index b4b686b4d22f..cca5046599bd 100644 --- a/extensions-contrib/gce-extensions/pom.xml +++ b/extensions-contrib/gce-extensions/pom.xml @@ -22,7 +22,7 @@ org.apache.druid druid - 0.18.0-SNAPSHOT + 0.19.0-SNAPSHOT ../../pom.xml 4.0.0 From 8337a5f770119690101c6cadba05d606bb82c2d0 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Mon, 20 Apr 2020 10:41:32 +0200 Subject: [PATCH 23/30] removing unused config --- .../druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index d923d334c36e..b76d191d4d47 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -68,7 +68,6 @@ public class GceAutoScaler implements AutoScaler private final GceEnvironmentConfig envConfig; private final int minNumWorkers; private final int maxNumWorkers; - private final SimpleWorkerProvisioningConfig config; // For future use private Compute cachedComputeService = null; @@ -93,7 +92,6 @@ public GceAutoScaler( "maxNumWorkers must be greater than minNumWorkers"); this.maxNumWorkers = maxNumWorkers; this.envConfig = envConfig; - this.config = config; } /** From 6ad5992a1a6a6af0b2642f9b1fed72867529e2f6 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Mon, 20 Apr 2020 11:12:52 +0200 Subject: [PATCH 24/30] adding tests to hash and equals --- extensions-contrib/gce-extensions/pom.xml | 5 ++++ .../autoscaling/gce/GceAutoScaler.java | 10 +++----- .../autoscaling/gce/GceEnvironmentConfig.java | 2 +- .../autoscaling/gce/GceAutoScalerTest.java | 24 +++++++++++++------ 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/extensions-contrib/gce-extensions/pom.xml b/extensions-contrib/gce-extensions/pom.xml index cca5046599bd..87a0a98d3ff4 100644 --- a/extensions-contrib/gce-extensions/pom.xml +++ b/extensions-contrib/gce-extensions/pom.xml @@ -122,5 +122,10 @@ easymock test + + nl.jqno.equalsverifier + equalsverifier + test + diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index b76d191d4d47..34e3bbaeb031 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -19,7 +19,6 @@ package org.apache.druid.indexing.overlord.autoscaling.gce; -import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; @@ -42,7 +41,6 @@ import org.apache.curator.shaded.com.google.common.annotations.VisibleForTesting; import org.apache.druid.indexing.overlord.autoscaling.AutoScaler; import org.apache.druid.indexing.overlord.autoscaling.AutoScalingData; -import org.apache.druid.indexing.overlord.autoscaling.SimpleWorkerProvisioningConfig; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.emitter.EmittingLogger; @@ -61,7 +59,7 @@ * where the prefix is chosen by you and abcd is a suffix assigned by GCE */ @JsonTypeName("gce") -public class GceAutoScaler implements AutoScaler +public final class GceAutoScaler implements AutoScaler { private static final EmittingLogger log = new EmittingLogger(GceAutoScaler.class); @@ -79,8 +77,7 @@ public class GceAutoScaler implements AutoScaler public GceAutoScaler( @JsonProperty("minNumWorkers") int minNumWorkers, @JsonProperty("maxNumWorkers") int maxNumWorkers, - @JsonProperty("envConfig") GceEnvironmentConfig envConfig, - @JacksonInject SimpleWorkerProvisioningConfig config + @JsonProperty("envConfig") GceEnvironmentConfig envConfig ) { Preconditions.checkArgument(minNumWorkers > 0, @@ -102,11 +99,10 @@ public GceAutoScaler( int minNumWorkers, int maxNumWorkers, GceEnvironmentConfig envConfig, - SimpleWorkerProvisioningConfig config, Compute compute ) { - this(minNumWorkers, maxNumWorkers, envConfig, config); + this(minNumWorkers, maxNumWorkers, envConfig); this.cachedComputeService = compute; } diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java index 2bc9df3e78d4..f8394b169f8a 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java @@ -27,7 +27,7 @@ /** */ -public class GceEnvironmentConfig +public final class GceEnvironmentConfig { /** * numInstances: the number of workers to try to spawn at each call to provision diff --git a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java index cac9270a275c..40744222c71f 100644 --- a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java +++ b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java @@ -32,6 +32,7 @@ import com.google.api.services.compute.model.ManagedInstance; import com.google.api.services.compute.model.NetworkInterface; import com.google.api.services.compute.model.Operation; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.indexing.overlord.autoscaling.AutoScaler; import org.apache.druid.indexing.overlord.autoscaling.AutoScalingData; import org.apache.druid.jackson.DefaultObjectMapper; @@ -153,6 +154,14 @@ public Object findInjectableValue( } } + @Test + public void test_config_equals() + { + EqualsVerifier.forClass(GceEnvironmentConfig.class).withNonnullFields( + "projectId", "zoneName", "managedInstanceGroupName", "numInstances" + ).verify(); + } + private Instance makeInstance(String name, String ip) { Instance instance = new Instance(); @@ -170,7 +179,6 @@ public void testIpToId() throws IOException // for the mock calls, not really th 2, 4, new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - null, mockCompute // <-- I pretend to be Google ); @@ -220,7 +228,6 @@ public void testIdToIp() throws IOException // for the mock calls, not really th 2, 4, new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - null, mockCompute // <-- I pretend to be Google ); @@ -283,7 +290,6 @@ public void testTerminateWithIds() throws IOException // for the mock calls, not 2, 4, new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - null, mockCompute // <-- I pretend to be Google ); @@ -361,7 +367,6 @@ public void testTerminateWithIdsWithMissingRemoval() throws IOException // for t 2, 4, new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - null, mockCompute // <-- I pretend to be Google ); @@ -448,7 +453,6 @@ public void testProvision() throws IOException // for the mock calls, not really 2, 4, new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - null, mockCompute // <-- I pretend to be Google ); @@ -520,7 +524,6 @@ public void testProvisionSkipped() throws IOException // for the mock calls, not 2, 4, new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - null, mockCompute // <-- I pretend to be Google ); @@ -564,7 +567,6 @@ public void testProvisionWithMissingNewInstances() throws IOException // for the 2, 4, new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - null, mockCompute // <-- I pretend to be Google ); @@ -636,4 +638,12 @@ public void testProvisionWithMissingNewInstances() throws IOException // for the EasyMock.verify(mockResizeRequest); EasyMock.verify(mockInstancesRequest); } + + @Test + public void test_equals() + { + EqualsVerifier.forClass(GceAutoScaler.class).withNonnullFields( + "envConfig", "maxNumWorkers", "minNumWorkers" + ).withIgnoredFields("cachedComputeService").verify(); + } } From d2fe900914b935d8a3c4c75a3d0858ad8ec1ae79 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Mon, 20 Apr 2020 11:17:38 +0200 Subject: [PATCH 25/30] adding nullable to waitForOperationEnd --- .../druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index 34e3bbaeb031..260f3492dd92 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -44,6 +44,7 @@ import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.emitter.EmittingLogger; +import javax.annotation.Nullable; import java.io.IOException; import java.security.GeneralSecurityException; import java.util.ArrayList; @@ -173,6 +174,7 @@ private synchronized Compute createComputeService() } // Used to wait for an operation to finish + @Nullable private Operation.Error waitForOperationEnd( Compute compute, Operation operation) throws Exception From 802cc9ff135f43a1d9b1e51a96b05e7be92de0a2 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Mon, 20 Apr 2020 13:06:50 +0200 Subject: [PATCH 26/30] adding testTerminate --- .../autoscaling/gce/GceAutoScaler.java | 4 +- .../autoscaling/gce/GceAutoScalerTest.java | 98 ++++++++++++++++++- 2 files changed, 97 insertions(+), 5 deletions(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index 260f3492dd92..1d2ba0d810ed 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -274,9 +274,7 @@ public AutoScalingData terminate(List ips) List nodeIds = ipToIdLookup(ips); // if they are not IPs, they will be unchanged try { - return new AutoScalingData(idToIpLookup( - terminateWithIds(nodeIds != null ? nodeIds : new ArrayList<>()).getNodeIds()) - ); + return terminateWithIds(nodeIds != null ? nodeIds : new ArrayList<>()); } catch (Exception e) { log.error(e, "Unable to terminate any instances."); diff --git a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java index 40744222c71f..70f122818e2f 100644 --- a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java +++ b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java @@ -155,7 +155,7 @@ public Object findInjectableValue( } @Test - public void test_config_equals() + public void testConfigEquals() { EqualsVerifier.forClass(GceEnvironmentConfig.class).withNonnullFields( "projectId", "zoneName", "managedInstanceGroupName", "numInstances" @@ -360,6 +360,100 @@ public void testTerminateWithIds() throws IOException // for the mock calls, not EasyMock.verify(mockInstancesRequest); } + @Test + public void testTerminate() throws IOException // for the mock calls, not really throwing + { + GceAutoScaler autoScaler = new GceAutoScaler( + 2, + 4, + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), + mockCompute // <-- I pretend to be Google + ); + + // testing the ip --> id part + Instance i0 = makeInstance("baz", "1.2.3.6"); + InstanceList mockInstanceListResponse = new InstanceList(); + mockInstanceListResponse.setNextPageToken(null); + mockInstanceListResponse.setItems(Collections.singletonList(i0)); + + EasyMock.expect(mockIpToIdRequest.execute()).andReturn(mockInstanceListResponse); + EasyMock.expect(mockIpToIdRequest.setPageToken(EasyMock.anyString())).andReturn( + mockIpToIdRequest // the method needs to return something, what is actually irrelevant here + ); + EasyMock.replay(mockIpToIdRequest); + + EasyMock.expect(mockInstances.list("proj-x", "us-central-1")).andReturn(mockIpToIdRequest); + + EasyMock.expect(mockCompute.instances()).andReturn(mockInstances); + EasyMock.replay(mockInstances); + + // testing the delete part + InstanceGroupManagersListManagedInstancesResponse beforeRunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar", + "http://xyz/baz" + )); + InstanceGroupManagersListManagedInstancesResponse afterRunningInstance = + createRunningInstances(Arrays.asList( + "http://xyz/foo", + "http://xyz/bar" + )); + + EasyMock.expect(mockInstancesRequest.execute()).andReturn(beforeRunningInstance); // 1st call + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.expect(mockInstancesRequest.execute()).andReturn(afterRunningInstance); // 2nd call + EasyMock.expect(mockInstancesRequest.setMaxResults(500L)).andReturn(mockInstancesRequest); + EasyMock.replay(mockInstancesRequest); + + EasyMock.expect(mockInstanceGroupManagers.listManagedInstances( + "proj-x", + "us-central-1", + "druid-mig" + )).andReturn(mockInstancesRequest).times(2); + + // set up the delete operation + Operation mockResponse = new Operation(); + mockResponse.setStatus("DONE"); + mockResponse.setError(new Operation.Error()); + + EasyMock.expect(mockDeleteRequest.execute()).andReturn(mockResponse); + EasyMock.replay(mockDeleteRequest); + + InstanceGroupManagersDeleteInstancesRequest requestBody = + new InstanceGroupManagersDeleteInstancesRequest(); + requestBody.setInstances(Collections.singletonList("zones/us-central-1/instances/baz")); + + EasyMock.expect(mockInstanceGroupManagers.deleteInstances( + "proj-x", + "us-central-1", + "druid-mig", + requestBody + )).andReturn(mockDeleteRequest); + + EasyMock.replay(mockInstanceGroupManagers); + + // called twice in getRunningInstances... + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + // ...and once in terminateWithIds + EasyMock.expect(mockCompute.instanceGroupManagers()).andReturn(mockInstanceGroupManagers); + + // and that's all folks! + EasyMock.replay(mockCompute); + + AutoScalingData autoScalingData = + autoScaler.terminate(Collections.singletonList("1.2.3.6")); + Assert.assertEquals(1, autoScalingData.getNodeIds().size()); + Assert.assertEquals("baz", autoScalingData.getNodeIds().get(0)); + + EasyMock.verify(mockCompute); + EasyMock.verify(mockIpToIdRequest); + EasyMock.verify(mockInstanceGroupManagers); + EasyMock.verify(mockDeleteRequest); + EasyMock.verify(mockInstancesRequest); + } + @Test public void testTerminateWithIdsWithMissingRemoval() throws IOException // for the mock calls, not really throwing { @@ -640,7 +734,7 @@ public void testProvisionWithMissingNewInstances() throws IOException // for the } @Test - public void test_equals() + public void testEquals() { EqualsVerifier.forClass(GceAutoScaler.class).withNonnullFields( "envConfig", "maxNumWorkers", "minNumWorkers" From f756cbebcf32d90fbc3fde33b0968105caa95bba Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Mon, 20 Apr 2020 16:26:25 +0200 Subject: [PATCH 27/30] adding unit tests for createComputeService --- .../autoscaling/gce/GceAutoScaler.java | 66 +++--- .../autoscaling/gce/GceEnvironmentConfig.java | 2 +- .../autoscaling/gce/GceAutoScalerTest.java | 202 ++++++++++++++---- 3 files changed, 185 insertions(+), 85 deletions(-) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index 1d2ba0d810ed..c7977ffd84f7 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -38,7 +38,6 @@ import com.google.api.services.compute.model.Operation; import com.google.common.base.Preconditions; import com.google.common.net.InetAddresses; -import org.apache.curator.shaded.com.google.common.annotations.VisibleForTesting; import org.apache.druid.indexing.overlord.autoscaling.AutoScaler; import org.apache.druid.indexing.overlord.autoscaling.AutoScalingData; import org.apache.druid.java.util.common.StringUtils; @@ -60,7 +59,7 @@ * where the prefix is chosen by you and abcd is a suffix assigned by GCE */ @JsonTypeName("gce") -public final class GceAutoScaler implements AutoScaler +public class GceAutoScaler implements AutoScaler { private static final EmittingLogger log = new EmittingLogger(GceAutoScaler.class); @@ -92,21 +91,6 @@ public GceAutoScaler( this.envConfig = envConfig; } - /** - * CAVEAT: this is meant to be used only for testing passing a mock version of Compute - */ - @VisibleForTesting - public GceAutoScaler( - int minNumWorkers, - int maxNumWorkers, - GceEnvironmentConfig envConfig, - Compute compute - ) - { - this(minNumWorkers, maxNumWorkers, envConfig); - this.cachedComputeService = compute; - } - @Override @JsonProperty public int getMinNumWorkers() @@ -128,6 +112,32 @@ public GceEnvironmentConfig getEnvConfig() return envConfig; } + @Nullable + Compute createComputeServiceImpl() + throws IOException, GeneralSecurityException, GceServiceException + { + HttpTransport httpTransport = GoogleNetHttpTransport.newTrustedTransport(); + JsonFactory jsonFactory = JacksonFactory.getDefaultInstance(); + GoogleCredential credential = GoogleCredential.getApplicationDefault( + httpTransport, + jsonFactory + ); + if (credential.createScopedRequired()) { + List scopes = new ArrayList<>(); + scopes.add(ComputeScopes.CLOUD_PLATFORM); + scopes.add(ComputeScopes.COMPUTE); + credential = credential.createScoped(scopes); + } + + if (credential.getClientAuthentication() != null) { + throw new GceServiceException("Not using a service account"); + } + + return new Compute.Builder(httpTransport, jsonFactory, credential) + .setApplicationName("DruidAutoscaler") + .build(); + } + private synchronized Compute createComputeService() throws IOException, GeneralSecurityException, InterruptedException, GceServiceException { @@ -142,27 +152,7 @@ private synchronized Compute createComputeService() log.info("Creating new ComputeService [%d/%d]", retries + 1, maxRetries); try { - HttpTransport httpTransport = GoogleNetHttpTransport.newTrustedTransport(); - JsonFactory jsonFactory = JacksonFactory.getDefaultInstance(); - GoogleCredential credential = GoogleCredential.getApplicationDefault( - httpTransport, - jsonFactory - ); - if (credential.createScopedRequired()) { - List scopes = new ArrayList<>(); - scopes.add(ComputeScopes.CLOUD_PLATFORM); - scopes.add(ComputeScopes.COMPUTE); - credential = credential.createScoped(scopes); - } - - if (credential.getClientAuthentication() != null) { - throw new GceServiceException("Not using a service account"); - } - - cachedComputeService = new Compute.Builder(httpTransport, jsonFactory, credential) - .setApplicationName("DruidAutoscaler") - .build(); - + cachedComputeService = createComputeServiceImpl(); retries++; } catch (Throwable e) { diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java index f8394b169f8a..2bc9df3e78d4 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceEnvironmentConfig.java @@ -27,7 +27,7 @@ /** */ -public final class GceEnvironmentConfig +public class GceEnvironmentConfig { /** * numInstances: the number of workers to try to spawn at each call to provision diff --git a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java index 70f122818e2f..4c7e78597f1b 100644 --- a/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java +++ b/extensions-contrib/gce-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScalerTest.java @@ -44,6 +44,7 @@ import org.junit.Test; import java.io.IOException; +import java.security.GeneralSecurityException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -159,7 +160,7 @@ public void testConfigEquals() { EqualsVerifier.forClass(GceEnvironmentConfig.class).withNonnullFields( "projectId", "zoneName", "managedInstanceGroupName", "numInstances" - ).verify(); + ).usingGetClass().verify(); } private Instance makeInstance(String name, String ip) @@ -173,14 +174,24 @@ private Instance makeInstance(String name, String ip) } @Test - public void testIpToId() throws IOException // for the mock calls, not really throwing + public void testIpToId() + throws IOException, GeneralSecurityException, GceServiceException { - GceAutoScaler autoScaler = new GceAutoScaler( + GceAutoScaler autoScaler = EasyMock.createMockBuilder(GceAutoScaler.class).withConstructor( + int.class, + int.class, + GceEnvironmentConfig.class + ).withArgs( 2, 4, - new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - mockCompute // <-- I pretend to be Google - ); + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig") + ).addMockedMethod( + "createComputeServiceImpl" + ).createMock(); + + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(mockCompute); + EasyMock.replay(autoScaler); // empty IPs List ips1 = Collections.emptyList(); @@ -222,14 +233,24 @@ public void testIpToId() throws IOException // for the mock calls, not really th } @Test - public void testIdToIp() throws IOException // for the mock calls, not really throwing + public void testIdToIp() + throws IOException, GeneralSecurityException, GceServiceException { - GceAutoScaler autoScaler = new GceAutoScaler( + GceAutoScaler autoScaler = EasyMock.createMockBuilder(GceAutoScaler.class).withConstructor( + int.class, + int.class, + GceEnvironmentConfig.class + ).withArgs( 2, 4, - new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - mockCompute // <-- I pretend to be Google - ); + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig") + ).addMockedMethod( + "createComputeServiceImpl" + ).createMock(); + + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(mockCompute); + EasyMock.replay(autoScaler); // empty IPs List ids1 = Collections.emptyList(); @@ -284,14 +305,24 @@ private InstanceGroupManagersListManagedInstancesResponse createRunningInstances } @Test - public void testTerminateWithIds() throws IOException // for the mock calls, not really throwing + public void testTerminateWithIds() + throws IOException, GeneralSecurityException, GceServiceException { - GceAutoScaler autoScaler = new GceAutoScaler( + GceAutoScaler autoScaler = EasyMock.createMockBuilder(GceAutoScaler.class).withConstructor( + int.class, + int.class, + GceEnvironmentConfig.class + ).withArgs( 2, 4, - new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - mockCompute // <-- I pretend to be Google - ); + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig") + ).addMockedMethod( + "createComputeServiceImpl" + ).createMock(); + + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(mockCompute); + EasyMock.replay(autoScaler); // set up getRunningInstances results InstanceGroupManagersListManagedInstancesResponse beforeRunningInstance = @@ -361,14 +392,24 @@ public void testTerminateWithIds() throws IOException // for the mock calls, not } @Test - public void testTerminate() throws IOException // for the mock calls, not really throwing + public void testTerminate() + throws IOException, GeneralSecurityException, GceServiceException { - GceAutoScaler autoScaler = new GceAutoScaler( + GceAutoScaler autoScaler = EasyMock.createMockBuilder(GceAutoScaler.class).withConstructor( + int.class, + int.class, + GceEnvironmentConfig.class + ).withArgs( 2, 4, - new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - mockCompute // <-- I pretend to be Google - ); + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig") + ).addMockedMethod( + "createComputeServiceImpl" + ).createMock(); + + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(mockCompute); + EasyMock.replay(autoScaler); // testing the ip --> id part Instance i0 = makeInstance("baz", "1.2.3.6"); @@ -455,14 +496,24 @@ public void testTerminate() throws IOException // for the mock calls, not really } @Test - public void testTerminateWithIdsWithMissingRemoval() throws IOException // for the mock calls, not really throwing + public void testTerminateWithIdsWithMissingRemoval() + throws IOException, GeneralSecurityException, GceServiceException { - GceAutoScaler autoScaler = new GceAutoScaler( + GceAutoScaler autoScaler = EasyMock.createMockBuilder(GceAutoScaler.class).withConstructor( + int.class, + int.class, + GceEnvironmentConfig.class + ).withArgs( 2, 4, - new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - mockCompute // <-- I pretend to be Google - ); + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig") + ).addMockedMethod( + "createComputeServiceImpl" + ).createMock(); + + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(mockCompute); + EasyMock.replay(autoScaler); // set up getRunningInstances results InstanceGroupManagersListManagedInstancesResponse beforeRunningInstance = @@ -541,14 +592,24 @@ public void testTerminateWithIdsWithMissingRemoval() throws IOException // for t } @Test - public void testProvision() throws IOException // for the mock calls, not really throwing + public void testProvision() + throws IOException, GeneralSecurityException, GceServiceException { - GceAutoScaler autoScaler = new GceAutoScaler( - 2, - 4, - new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - mockCompute // <-- I pretend to be Google - ); + GceAutoScaler autoScaler = EasyMock.createMockBuilder(GceAutoScaler.class).withConstructor( + int.class, + int.class, + GceEnvironmentConfig.class + ).withArgs( + 2, + 4, + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig") + ).addMockedMethod( + "createComputeServiceImpl" + ).createMock(); + + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(mockCompute); + EasyMock.replay(autoScaler); // set up getRunningInstances results InstanceGroupManagersListManagedInstancesResponse beforeRunningInstance = @@ -612,14 +673,24 @@ public void testProvision() throws IOException // for the mock calls, not really } @Test - public void testProvisionSkipped() throws IOException // for the mock calls, not really throwing + public void testProvisionSkipped() + throws IOException, GeneralSecurityException, GceServiceException { - GceAutoScaler autoScaler = new GceAutoScaler( - 2, - 4, - new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - mockCompute // <-- I pretend to be Google - ); + GceAutoScaler autoScaler = EasyMock.createMockBuilder(GceAutoScaler.class).withConstructor( + int.class, + int.class, + GceEnvironmentConfig.class + ).withArgs( + 2, + 4, + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig") + ).addMockedMethod( + "createComputeServiceImpl" + ).createMock(); + + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(mockCompute); + EasyMock.replay(autoScaler); // set up getRunningInstances results InstanceGroupManagersListManagedInstancesResponse beforeRunningInstance = @@ -655,14 +726,24 @@ public void testProvisionSkipped() throws IOException // for the mock calls, not } @Test - public void testProvisionWithMissingNewInstances() throws IOException // for the mock calls, not really throwing + public void testProvisionWithMissingNewInstances() + throws IOException, GeneralSecurityException, GceServiceException { - GceAutoScaler autoScaler = new GceAutoScaler( + GceAutoScaler autoScaler = EasyMock.createMockBuilder(GceAutoScaler.class).withConstructor( + int.class, + int.class, + GceEnvironmentConfig.class + ).withArgs( 2, 4, - new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig"), - mockCompute // <-- I pretend to be Google - ); + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig") + ).addMockedMethod( + "createComputeServiceImpl" + ).createMock(); + + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(mockCompute); + EasyMock.replay(autoScaler); // set up getRunningInstances results InstanceGroupManagersListManagedInstancesResponse beforeRunningInstance = @@ -738,6 +819,35 @@ public void testEquals() { EqualsVerifier.forClass(GceAutoScaler.class).withNonnullFields( "envConfig", "maxNumWorkers", "minNumWorkers" - ).withIgnoredFields("cachedComputeService").verify(); + ).withIgnoredFields("cachedComputeService").usingGetClass().verify(); } + + @Test + public void testFailedComputeCreation() + throws IOException, GeneralSecurityException, GceServiceException + { + GceAutoScaler autoScaler = EasyMock.createMockBuilder(GceAutoScaler.class).withConstructor( + int.class, + int.class, + GceEnvironmentConfig.class + ).withArgs( + 2, + 4, + new GceEnvironmentConfig(1, "proj-x", "us-central-1", "druid-mig") + ).addMockedMethod( + "createComputeServiceImpl" + ).createMock(); + + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.expect(autoScaler.createComputeServiceImpl()).andReturn(null); + EasyMock.replay(autoScaler); + + List ips = Collections.singletonList("1.2.3.4"); + List ids = autoScaler.ipToIdLookup(ips); + Assert.assertEquals(0, ids.size()); // Exception caught in execution results in empty result + } + } From 77ff4d5891f9bf2d16ea3f0319409a8d7bbcce61 Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Tue, 21 Apr 2020 09:32:55 +0200 Subject: [PATCH 28/30] increasing retries in unrelated integration-test to prevent sporadic failure (hopefully) --- .../druid/tests/coordinator/duty/ITAutoCompactionTest.java | 2 +- .../java/org/apache/druid/server/initialization/JettyTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index 592c91034daf..5932fe8d050a 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -237,7 +237,7 @@ private void verifyQuery(String queryResource) throws Exception fullDatasourceName ); - queryHelper.testQueriesFromString(queryResponseTemplate, 2); + queryHelper.testQueriesFromString(queryResponseTemplate, 5); } private void submitCompactionConfig(Integer maxRowsPerSegment, Period skipOffsetFromLatest) throws Exception diff --git a/server/src/test/java/org/apache/druid/server/initialization/JettyTest.java b/server/src/test/java/org/apache/druid/server/initialization/JettyTest.java index 206c3186e02c..2ab9f7c52ce2 100644 --- a/server/src/test/java/org/apache/druid/server/initialization/JettyTest.java +++ b/server/src/test/java/org/apache/druid/server/initialization/JettyTest.java @@ -511,7 +511,7 @@ private void waitForJettyServerModuleActiveConnectionsZero(JettyServerModule jsm { // it can take a bit to close the connection, so maybe sleep for a while and hope it closes final int sleepTimeMills = 10; - final int totalSleeps = 5_000 / sleepTimeMills; + final int totalSleeps = 10_000 / sleepTimeMills; int count = 0; while (jsm.getActiveConnections() > 0 && count++ < totalSleeps) { Thread.sleep(sleepTimeMills); From 546cb0ef62d1a627a7bbccb8739e0b3d6fef6eff Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Wed, 22 Apr 2020 09:08:44 +0200 Subject: [PATCH 29/30] reverting queryResponseTemplate change --- .../druid/tests/coordinator/duty/ITAutoCompactionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index 5932fe8d050a..592c91034daf 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -237,7 +237,7 @@ private void verifyQuery(String queryResource) throws Exception fullDatasourceName ); - queryHelper.testQueriesFromString(queryResponseTemplate, 5); + queryHelper.testQueriesFromString(queryResponseTemplate, 2); } private void submitCompactionConfig(Integer maxRowsPerSegment, Period skipOffsetFromLatest) throws Exception From 1791ee6eb27e7ab447845ad3cbd4f4ab491042fa Mon Sep 17 00:00:00 2001 From: Francesco Nidito Date: Wed, 22 Apr 2020 09:16:39 +0200 Subject: [PATCH 30/30] adding comment for Compute.Builder.build() returning null --- .../druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java index c7977ffd84f7..3c8f52915017 100644 --- a/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java +++ b/extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java @@ -144,6 +144,9 @@ private synchronized Compute createComputeService() final int maxRetries = 5; int retries = 0; + // This retry loop is here to catch the cases in which the underlying call to + // Compute.Builder(...).build() returns null, case that has been experienced + // sporadically at start time while (cachedComputeService == null && retries < maxRetries) { if (retries > 0) { Thread.sleep(POLL_INTERVAL_MS);