diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2017-11-10 13:23:15 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-11-10 13:23:15 +0100 |
commit | 12f7a4363ffd5877ce62cd835dd111703f3b87bf (patch) | |
tree | 00cda5c3bbb14f48788c1041c7076fb9773ba689 | |
parent | cf48e7bc46e30150db7a1a9af704f4d46151116f (diff) | |
parent | d5975c4506b82f2af9b58c07b5feeaac9acfcc7b (diff) |
Merge pull request #4082 from vespa-engine/revert-4081-hmusum/add-option-to-disable-filedistributor
Revert "Add 'disabled' option for filedistribution"
5 files changed, 37 insertions, 78 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/FileDistributionOptions.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/FileDistributionOptions.java index 0160978773d..b58406e1935 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/FileDistributionOptions.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/FileDistributionOptions.java @@ -7,39 +7,28 @@ import com.yahoo.cloud.config.filedistribution.FiledistributorConfig; /** * Options for controlling the behavior of the file distribution services. - * * @author tonytv */ public class FileDistributionOptions implements FiledistributorConfig.Producer { - public static FileDistributionOptions defaultOptions() { return new FileDistributionOptions(); } - private FileDistributionOptions() { - } - - private BinaryScaledAmount uploadBitRate = new BinaryScaledAmount(); - private BinaryScaledAmount downloadBitRate = new BinaryScaledAmount(); - private boolean disabled = false; + private FileDistributionOptions() {} + private BinaryScaledAmount uploadbitrate = new BinaryScaledAmount(); + private BinaryScaledAmount downloadbitrate = new BinaryScaledAmount(); - public void downloadBitRate(BinaryScaledAmount amount) { + //Called through reflection + public void downloadbitrate(BinaryScaledAmount amount) { ensureNonNegative(amount); - downloadBitRate = amount; + downloadbitrate = amount; } - public void uploadBitRate(BinaryScaledAmount amount) { + //Called through reflection + public void uploadbitrate(BinaryScaledAmount amount) { ensureNonNegative(amount); - uploadBitRate = amount; - } - - public void disabled(boolean value) { - disabled = value; - } - - public boolean disabled() { - return disabled; + uploadbitrate = amount; } private void ensureNonNegative(BinaryScaledAmount amount) { @@ -49,12 +38,12 @@ public class FileDistributionOptions implements FiledistributorConfig.Producer { private int byteRate(BinaryScaledAmount bitRate) { BinaryScaledAmount byteRate = bitRate.divide(8); - return (int) byteRate.as(BinaryPrefix.unit); + return (int)byteRate.as(BinaryPrefix.unit); } @Override public void getConfig(FiledistributorConfig.Builder builder) { - builder.maxuploadspeed((double) byteRate(uploadBitRate)); - builder.maxdownloadspeed((double) byteRate(downloadBitRate)); + builder.maxuploadspeed((double)byteRate(uploadbitrate)); + builder.maxdownloadspeed((double)byteRate(downloadbitrate)); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomFileDistributionOptionsBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomFileDistributionOptionsBuilder.java index 8a5d6846a64..55c3f569900 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomFileDistributionOptionsBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomFileDistributionOptionsBuilder.java @@ -6,42 +6,47 @@ import com.yahoo.text.XML; import com.yahoo.vespa.model.admin.FileDistributionOptions; import org.w3c.dom.Element; -import java.util.Optional; - /** - * Builds file distribution options. - * - * @author Tony Vaagenes - * @author hmusum + * Builds a file distribution options. + * @author tonytv */ public class DomFileDistributionOptionsBuilder { - private static void throwExceptionForElementInFileDistribution(String subElement, String reason) { throw new RuntimeException("In element '" + subElement + "' contained in 'filedistribution': " + reason); } - private Optional<BinaryScaledAmount> getAmount(String name, Element fileDistributionElement) { - Element optionElement = XML.getChild(fileDistributionElement, name); + private static void callSetter(FileDistributionOptions options, String name, BinaryScaledAmount amount) { + try { + options.getClass().getMethod(name, BinaryScaledAmountParser.class).invoke(options, amount); + } catch (IllegalArgumentException e) { + throwExceptionForElementInFileDistribution(name, e.getMessage()); + } + catch (Exception e) { + if (e instanceof RuntimeException) + throw (RuntimeException)e; + else + throw new RuntimeException(e); + } + } + + private static void setIfPresent(FileDistributionOptions options, String name, Element fileDistributionElement) { try { + Element optionElement = XML.getChild(fileDistributionElement, name); if (optionElement != null) { String valueString = XML.getValue(optionElement); - return Optional.of(BinaryScaledAmountParser.parse(valueString)); + BinaryScaledAmount amount = BinaryScaledAmountParser.parse(valueString); + callSetter(options, name, amount); } } catch (NumberFormatException e) { throwExceptionForElementInFileDistribution(name, "Expected a valid number. (Message = " + e.getMessage() + ")."); } - return Optional.empty(); } public FileDistributionOptions build(Element fileDistributionElement) { FileDistributionOptions options = FileDistributionOptions.defaultOptions(); if (fileDistributionElement != null) { - getAmount("uploadbitrate", fileDistributionElement).ifPresent(options::uploadBitRate); - getAmount("downloadbitrate", fileDistributionElement).ifPresent(options::downloadBitRate); - Element disable = XML.getChild(fileDistributionElement, "disabled"); - if (disable != null) { - options.disabled(Boolean.valueOf(XML.getValue(disable))); - } + setIfPresent(options, "uploadbitrate", fileDistributionElement); + setIfPresent(options, "downloadbitrate", fileDistributionElement); } return options; } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributorService.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributorService.java index 98e230cb8ba..6c2da51f3e3 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributorService.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributorService.java @@ -54,10 +54,8 @@ public class FileDistributorService extends AbstractService implements @Override public String getStartupCommand() { - // If disabled config proxy should act as file distributor, so don't start this service - return (fileDistributionOptions.disabled()) - ? null - : "exec $ROOT/sbin/vespa-filedistributor" + " --configid " + getConfigId(); + return "exec $ROOT/sbin/vespa-filedistributor" + + " --configid " + getConfigId(); } @Override diff --git a/config-model/src/main/resources/schema/admin.rnc b/config-model/src/main/resources/schema/admin.rnc index 1b0d487a388..cdf26807bcb 100644 --- a/config-model/src/main/resources/schema/admin.rnc +++ b/config-model/src/main/resources/schema/admin.rnc @@ -83,8 +83,7 @@ LogServer = element logserver { FileDistribution = element filedistribution { element uploadbitrate { xsd:string { pattern = "\d+(\.\d*)?\s*[kmgKMG]?" } }? & - element downloadbitrate { xsd:string { pattern = "\d+(\.\d*)?\s*[kmgKMG]?" } }? & - element disabled { xsd:boolean }? + element downloadbitrate { xsd:string { pattern = "\d+(\.\d*)?\s*[kmgKMG]?" } }? } Metrics = element metrics { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java index 946624a1cdb..b5375ccbce4 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java @@ -275,36 +275,4 @@ public class AdminTestCase { // 1 logforwarder on each host assertTrue(configIds.toString(), configIds.contains("admin/logforwarder.0")); } - - @Test - public void disableFiledistributorService() throws Exception { - String hosts = "<hosts>" - + " <host name=\"localhost\">" - + " <alias>node0</alias>" - + " </host>" - + "</hosts>"; - - String services = "<services>" + - " <admin version='2.0'>" + - " <adminserver hostalias='node0' />" + - " <filedistribution>" + - " <disabled>true</disabled>" + - " </filedistribution>" + - " </admin>" + - "</services>"; - - VespaModel vespaModel = new VespaModelCreatorWithMockPkg(hosts, services).create(); - String localhost = HostName.getLocalhost(); - String localhostConfigId = "hosts/" + localhost; - - // Verify services in the sentinel config - SentinelConfig.Builder b = new SentinelConfig.Builder(); - vespaModel.getConfig(b, localhostConfigId); - SentinelConfig sentinelConfig = new SentinelConfig(b); - assertThat(sentinelConfig.service().size(), is(3)); - assertThat(sentinelConfig.service(0).name(), is("logserver")); - assertThat(sentinelConfig.service(1).name(), is("slobrok")); - assertThat(sentinelConfig.service(2).name(), is("logd")); - // No filedistributor service - } } |