Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 0166B200BA6 for ; Tue, 18 Oct 2016 09:09:34 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id F412C160ADC; Tue, 18 Oct 2016 07:09:33 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 1E4A7160ACC for ; Tue, 18 Oct 2016 09:09:32 +0200 (CEST) Received: (qmail 25637 invoked by uid 500); 18 Oct 2016 07:09:32 -0000 Mailing-List: contact commits-help@aurora.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@aurora.apache.org Delivered-To: mailing list commits@aurora.apache.org Received: (qmail 25628 invoked by uid 99); 18 Oct 2016 07:09:32 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 18 Oct 2016 07:09:32 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id DD946E3934; Tue, 18 Oct 2016 07:09:31 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: serb@apache.org To: commits@aurora.apache.org Message-Id: <73ff792238af4ff693bceac89372b2fc@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: aurora git commit: Fix the -enable_revocable_ram flag Date: Tue, 18 Oct 2016 07:09:31 +0000 (UTC) archived-at: Tue, 18 Oct 2016 07:09:34 -0000 Repository: aurora Updated Branches: refs/heads/master ac8b8021f -> ad77de1e6 Fix the -enable_revocable_ram flag The mentioned flag has been introduced in https://reviews.apache.org/r/51807/. Unfortunately, as detailed in the bug report, my testing was not thorough enough. Problem description: * The flag is used the `ResourceType` enum constructor. This implies the flag value needs to be available during class loading. * Values supplied via the scheduler command line are only set at runtime, right at the beginning `main` [1]. * Luckily, there is a check in our arg parsing library that warns if a value is changed after it has already been read. In other words: We get an exception if we change the flag, because it has already been read during class loading. This patch corrects this issue by treating the arguments as a supplier which can be read lazily at runtime. The patch also extends the existing e2e test for revocable resources to also consider RAM. [1] https://github.com/apache/aurora/blob/b417be38fe1fcae6b85f7e91cea961ab272adf3f/src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java#L197 Bugs closed: AURORA-1794 Reviewed at https://reviews.apache.org/r/52821/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/ad77de1e Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/ad77de1e Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/ad77de1e Branch: refs/heads/master Commit: ad77de1e61786f97d7204d9d0c0b8965cdd16396 Parents: ac8b802 Author: Stephan Erb Authored: Tue Oct 18 09:08:57 2016 +0200 Committer: Stephan Erb Committed: Tue Oct 18 09:08:57 2016 +0200 ---------------------------------------------------------------------- RELEASE-NOTES.md | 1 + .../java/org/apache/aurora/common/args/Arg.java | 3 ++- .../vagrant/mesos_config/etc_mesos-slave/modules | 2 +- examples/vagrant/upstart/aurora-scheduler.conf | 1 + .../scheduler/resources/ResourceSettings.java | 4 ++++ .../aurora/scheduler/resources/ResourceType.java | 18 ++++++++++-------- 6 files changed, 19 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/ad77de1e/RELEASE-NOTES.md ---------------------------------------------------------------------- diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 368f917..b050778 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -6,6 +6,7 @@ - The Aurora client is now using the Thrift binary protocol to communicate with the scheduler. - Introduce a new `--ip` option to bind the Thermos observer to a specific rather than all interfaces. +- Fix error that prevents the scheduler from being launched with `-enable_revocable_ram`. ### Deprecations and removals: http://git-wip-us.apache.org/repos/asf/aurora/blob/ad77de1e/commons-args/src/main/java/org/apache/aurora/common/args/Arg.java ---------------------------------------------------------------------- diff --git a/commons-args/src/main/java/org/apache/aurora/common/args/Arg.java b/commons-args/src/main/java/org/apache/aurora/common/args/Arg.java index 8e915c6..4da9159 100644 --- a/commons-args/src/main/java/org/apache/aurora/common/args/Arg.java +++ b/commons-args/src/main/java/org/apache/aurora/common/args/Arg.java @@ -14,6 +14,7 @@ package org.apache.aurora.common.args; import javax.annotation.Nullable; +import java.util.function.Supplier; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -22,7 +23,7 @@ import com.google.common.base.Preconditions; * Wrapper class for the value of an argument. For proper behavior, an {@code Arg} should always * be annotated with {@link CmdLine}, which will define the command line interface to the argument. */ -public class Arg { +public class Arg implements Supplier { @Nullable private final T defaultValue; @Nullable private T value; http://git-wip-us.apache.org/repos/asf/aurora/blob/ad77de1e/examples/vagrant/mesos_config/etc_mesos-slave/modules ---------------------------------------------------------------------- diff --git a/examples/vagrant/mesos_config/etc_mesos-slave/modules b/examples/vagrant/mesos_config/etc_mesos-slave/modules index 4352bca..53b8435 100644 --- a/examples/vagrant/mesos_config/etc_mesos-slave/modules +++ b/examples/vagrant/mesos_config/etc_mesos-slave/modules @@ -5,7 +5,7 @@ "name": "org_apache_mesos_FixedResourceEstimator", "parameters": { "key": "resources", - "value": "cpus:3" + "value": "cpus:3;mem:512" } } } http://git-wip-us.apache.org/repos/asf/aurora/blob/ad77de1e/examples/vagrant/upstart/aurora-scheduler.conf ---------------------------------------------------------------------- diff --git a/examples/vagrant/upstart/aurora-scheduler.conf b/examples/vagrant/upstart/aurora-scheduler.conf index 4d88881..e68a801 100644 --- a/examples/vagrant/upstart/aurora-scheduler.conf +++ b/examples/vagrant/upstart/aurora-scheduler.conf @@ -51,5 +51,6 @@ exec bin/aurora-scheduler \ -mesos_role=aurora-role \ -populate_discovery_info=true \ -receive_revocable_resources=true \ + -enable_revocable_ram=true \ -allow_gpu_resource=true \ -offer_filter_duration=0secs http://git-wip-us.apache.org/repos/asf/aurora/blob/ad77de1e/src/main/java/org/apache/aurora/scheduler/resources/ResourceSettings.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/resources/ResourceSettings.java b/src/main/java/org/apache/aurora/scheduler/resources/ResourceSettings.java index c49fd06..4e2c425 100644 --- a/src/main/java/org/apache/aurora/scheduler/resources/ResourceSettings.java +++ b/src/main/java/org/apache/aurora/scheduler/resources/ResourceSettings.java @@ -13,6 +13,8 @@ */ package org.apache.aurora.scheduler.resources; +import java.util.function.Supplier; + import org.apache.aurora.common.args.Arg; import org.apache.aurora.common.args.CmdLine; @@ -31,6 +33,8 @@ final class ResourceSettings { @CmdLine(name = "enable_revocable_ram", help = "Treat RAM as a revocable resource.") static final Arg ENABLE_REVOCABLE_RAM = Arg.create(false); + static final Supplier NOT_REVOCABLE = () -> false; + private ResourceSettings() { } http://git-wip-us.apache.org/repos/asf/aurora/blob/ad77de1e/src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java b/src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java index e1a5dce..915c83d 100644 --- a/src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java +++ b/src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java @@ -15,6 +15,7 @@ package org.apache.aurora.scheduler.resources; import java.util.EnumSet; import java.util.Optional; +import java.util.function.Supplier; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; @@ -38,6 +39,7 @@ import static org.apache.aurora.scheduler.resources.MesosResourceConverter.SCALA import static org.apache.aurora.scheduler.resources.ResourceMapper.PORT_MAPPER; import static org.apache.aurora.scheduler.resources.ResourceSettings.ENABLE_REVOCABLE_CPUS; import static org.apache.aurora.scheduler.resources.ResourceSettings.ENABLE_REVOCABLE_RAM; +import static org.apache.aurora.scheduler.resources.ResourceSettings.NOT_REVOCABLE; /** * Describes Mesos resource types and their Aurora traits. @@ -57,7 +59,7 @@ public enum ResourceType implements TEnum { "core(s)", 16, false, - ENABLE_REVOCABLE_CPUS.get()), + ENABLE_REVOCABLE_CPUS), /** * RAM resource. @@ -72,7 +74,7 @@ public enum ResourceType implements TEnum { "MB", Amount.of(24, GB).as(MB), false, - ENABLE_REVOCABLE_RAM.get()), + ENABLE_REVOCABLE_RAM), /** * DISK resource. @@ -87,7 +89,7 @@ public enum ResourceType implements TEnum { "MB", Amount.of(450, GB).as(MB), false, - false), + NOT_REVOCABLE), /** * Port resource. @@ -102,7 +104,7 @@ public enum ResourceType implements TEnum { "count", 1000, true, - false), + NOT_REVOCABLE), /** * GPU resource. @@ -117,7 +119,7 @@ public enum ResourceType implements TEnum { "core(s)", 4, false, - false); + NOT_REVOCABLE); /** * Correspondent thrift {@link org.apache.aurora.gen.Resource} enum value. @@ -167,7 +169,7 @@ public enum ResourceType implements TEnum { /** * Indicates if a resource can be Mesos-revocable. */ - private final boolean isMesosRevocable; + private final Supplier isMesosRevocable; private static ImmutableMap byField = Maps.uniqueIndex(EnumSet.allOf(ResourceType.class), ResourceType::getValue); @@ -199,7 +201,7 @@ public enum ResourceType implements TEnum { String auroraUnit, int scalingRange, boolean isMultipleAllowed, - boolean isMesosRevocable) { + Supplier isMesosRevocable) { this.value = value; this.mesosResourceConverter = requireNonNull(mesosResourceConverter); @@ -320,7 +322,7 @@ public enum ResourceType implements TEnum { * @return True if a resource can be Mesos-revocable, false otherwise. */ public boolean isMesosRevocable() { - return isMesosRevocable; + return isMesosRevocable.get(); } /**