diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-08-25 13:31:56 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-08-25 13:31:56 +0200 |
commit | 238af6a23fc6fdbda856cba2951f1fe266322510 (patch) | |
tree | aea8047dcbc9e8d32561ac62225b58688c148c60 | |
parent | 3fed53d35d773fb91b2a2f9795bdc230c19fb338 (diff) | |
parent | 915171482815f5e06f995b03d112dcce14cb5df0 (diff) |
Merge pull request #18854 from vespa-engine/hmusum/cleanup-file-distribution-in-model
Decouple FileDistributor from FileDistributionConfigProducer [run-systemtest]
10 files changed, 57 insertions, 85 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java b/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java index ad3e501d3f2..753db89af1b 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.model.test; import com.yahoo.config.ConfigInstance; @@ -17,7 +17,7 @@ import com.yahoo.vespa.model.HostSystem; import com.yahoo.vespa.model.admin.Admin; import com.yahoo.vespa.model.builder.xml.dom.DomAdminV2Builder; import com.yahoo.vespa.model.filedistribution.FileDistributionConfigProducer; -import com.yahoo.vespa.model.filedistribution.FileDistributor; +import com.yahoo.vespa.model.filedistribution.FileReferencesRepository; import org.w3c.dom.Document; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -43,7 +43,7 @@ public class MockRoot extends AbstractConfigProducerRoot { private final HostSystem hostSystem; private final DeployState deployState; - private final FileDistributor fileDistributor; + private final FileReferencesRepository fileReferencesRepository; private Admin admin; public MockRoot() { @@ -66,7 +66,7 @@ public class MockRoot extends AbstractConfigProducerRoot { super(rootConfigId); hostSystem = new HostSystem(this, "hostsystem", deployState.getProvisioner(), deployState.getDeployLogger()); this.deployState = deployState; - fileDistributor = new FileDistributor(); + fileReferencesRepository = new FileReferencesRepository(); } public FileDistributionConfigProducer getFileDistributionConfigProducer() { @@ -119,15 +119,11 @@ public class MockRoot extends AbstractConfigProducerRoot { return deployState; } - public FileDistributor getFileDistributor() { - return fileDistributor; - } + public FileReferencesRepository fileReferencesRepository() { return fileReferencesRepository; } - public HostSystem hostSystem() { - return hostSystem; - } + public HostSystem hostSystem() { return hostSystem; } - public void addDescendant(String configId, AbstractConfigProducer descendant) { + public void addDescendant(String configId, AbstractConfigProducer<?> descendant) { if (id2producer.containsKey(configId)) { throw new RuntimeException ("Config ID '" + configId + "' cannot be reserved by an instance of class '" + @@ -138,7 +134,7 @@ public class MockRoot extends AbstractConfigProducerRoot { } @Override - public void addChild(AbstractConfigProducer abstractConfigProducer) { + public void addChild(AbstractConfigProducer<?> abstractConfigProducer) { super.addChild(abstractConfigProducer); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java index 2186fffd05b..e47fd339505 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java @@ -459,7 +459,7 @@ public abstract class AbstractService extends AbstractConfigProducer<AbstractCon * @param reference file reference (hash) */ public void send(FileReference reference) { - getRoot().getFileDistributor().sendFileReference(reference); + getRoot().fileReferencesRepository().add(reference); } /** The service HTTP port for health status */ diff --git a/config-model/src/main/java/com/yahoo/vespa/model/ConfigProducerRoot.java b/config-model/src/main/java/com/yahoo/vespa/model/ConfigProducerRoot.java index b30aa5aeca6..e9b587c0eb9 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/ConfigProducerRoot.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/ConfigProducerRoot.java @@ -1,10 +1,10 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model; import com.yahoo.config.ConfigInstance; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.vespa.model.admin.Admin; -import com.yahoo.vespa.model.filedistribution.FileDistributor; +import com.yahoo.vespa.model.filedistribution.FileReferencesRepository; import java.util.Set; @@ -22,7 +22,7 @@ public interface ConfigProducerRoot extends ConfigProducer { * @param id string id of descendant * @param descendant the producer to add to this root node */ - void addDescendant(String id, AbstractConfigProducer descendant); + void addDescendant(String id, AbstractConfigProducer<?> descendant); /** * @return an unmodifiable copy of the set of configIds in this root. @@ -39,7 +39,7 @@ public interface ConfigProducerRoot extends ConfigProducer { */ public <CONFIGTYPE extends ConfigInstance> CONFIGTYPE getConfig(Class<CONFIGTYPE> clazz, String configId); - FileDistributor getFileDistributor(); + FileReferencesRepository fileReferencesRepository(); Admin getAdmin(); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java index 69157cea050..8d4cb0cefad 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java @@ -55,7 +55,7 @@ import com.yahoo.vespa.model.container.search.QueryProfiles; import com.yahoo.vespa.model.content.Content; import com.yahoo.vespa.model.content.cluster.ContentCluster; import com.yahoo.vespa.model.filedistribution.FileDistributionConfigProducer; -import com.yahoo.vespa.model.filedistribution.FileDistributor; +import com.yahoo.vespa.model.filedistribution.FileReferencesRepository; import com.yahoo.vespa.model.generic.service.ServiceCluster; import com.yahoo.vespa.model.ml.ConvertedModel; import com.yahoo.vespa.model.ml.ModelName; @@ -135,7 +135,7 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri /** The validation overrides of this. This is never null. */ private final ValidationOverrides validationOverrides; - private final FileDistributor fileDistributor; + private final FileReferencesRepository fileReferencesRepository = new FileReferencesRepository(); private final Provisioned provisioned; @@ -168,10 +168,10 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri * @param deployState the global deploy state to use for this model. */ public VespaModel(ConfigModelRegistry configModelRegistry, DeployState deployState) throws IOException, SAXException { - this(configModelRegistry, deployState, true, null); + this(configModelRegistry, deployState, true); } - private VespaModel(ConfigModelRegistry configModelRegistry, DeployState deployState, boolean complete, FileDistributor fileDistributor) + private VespaModel(ConfigModelRegistry configModelRegistry, DeployState deployState, boolean complete) throws IOException, SAXException { super("vespamodel"); version = deployState.getVespaVersion(); @@ -201,7 +201,6 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri configModelRepo.readConfigModels(deployState, this, builder, root, new VespaConfigModelRegistry(configModelRegistry)); addServiceClusters(deployState, builder); setupRouting(deployState); - this.fileDistributor = root.getFileDistributionConfigProducer().getFileDistributor(); getAdmin().addPerHostServices(hostSystem.getHosts(), deployState); freezeModelTopology(); root.prepare(configModelRepo); @@ -209,14 +208,11 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri validateWrapExceptions(); hostSystem.dumpPortAllocations(); propagateRestartOnDeploy(); - // must happen after stuff above - this.allocatedHosts = AllocatedHosts.withHosts(hostSystem.getHostSpecs()); - } - else { // create a model with no services instantiated and the given file distributor - this.allocatedHosts = AllocatedHosts.withHosts(hostSystem.getHostSpecs()); - this.fileDistributor = fileDistributor; } + // else: create a model with no services instantiated (no-op) + // must be done last + this.allocatedHosts = AllocatedHosts.withHosts(hostSystem.getHostSpecs()); } @Override @@ -272,8 +268,7 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri /** Creates a mutable model with no services instantiated */ public static VespaModel createIncomplete(DeployState deployState) throws IOException, SAXException { - return new VespaModel(new NullConfigModelRegistry(), deployState, false, - new FileDistributor()); + return new VespaModel(new NullConfigModelRegistry(), deployState, false); } private void validateWrapExceptions() { @@ -381,14 +376,10 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri .collect(Collectors.toCollection(LinkedHashSet::new)); } - public FileDistributor getFileDistributor() { - return fileDistributor; - } - @Override - public Set<FileReference> fileReferences() { - return fileDistributor.allFilesToSend(); - } + public FileReferencesRepository fileReferencesRepository() { return fileReferencesRepository; } + + public Set<FileReference> fileReferences() { return fileReferencesRepository.allFileReferences(); } /** Returns this models Vespa instance */ public ApplicationConfigProducerRoot getVespa() { return root; } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java index 86bbcd12f55..8e2f0e50c6d 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java @@ -26,7 +26,6 @@ import com.yahoo.vespa.model.admin.monitoring.Monitoring; import com.yahoo.vespa.model.admin.monitoring.builder.Metrics; import com.yahoo.vespa.model.filedistribution.FileDistributionConfigProducer; import com.yahoo.vespa.model.filedistribution.FileDistributionConfigProvider; -import com.yahoo.vespa.model.filedistribution.FileDistributor; import java.io.Serializable; import java.util.ArrayList; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProducer.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProducer.java index 9c7c6f650a6..2e1902cb1ea 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProducer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProducer.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.filedistribution; import com.yahoo.config.model.producer.AbstractConfigProducer; @@ -15,15 +15,9 @@ import java.util.Map; public class FileDistributionConfigProducer extends AbstractConfigProducer<AbstractConfigProducer<?>> { private final Map<Host, FileDistributionConfigProvider> fileDistributionConfigProviders = new IdentityHashMap<>(); - private final FileDistributor fileDistributor; public FileDistributionConfigProducer(AbstractConfigProducer<?> parent) { super(parent, "filedistribution"); - this.fileDistributor = new FileDistributor(); - } - - public FileDistributor getFileDistributor() { - return fileDistributor; } public void addFileDistributionConfigProducer(Host host, FileDistributionConfigProvider fileDistributionConfigProvider) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProvider.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProvider.java index 77cb9fe8227..314c69a6e09 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProvider.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProvider.java @@ -5,7 +5,6 @@ import com.yahoo.cloud.config.filedistribution.FiledistributorrpcConfig; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.vespa.model.ConfigProxy; import com.yahoo.vespa.model.Host; -import com.yahoo.vespa.model.HostSystem; public class FileDistributionConfigProvider extends AbstractConfigProducer<AbstractConfigProducer<?>> implements FiledistributorrpcConfig.Producer { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java deleted file mode 100644 index f2985b3a08d..00000000000 --- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.model.filedistribution; - -import com.yahoo.config.FileReference; -import com.yahoo.vespa.model.Host; - -import java.util.HashSet; -import java.util.LinkedHashMap; -import java.util.LinkedHashSet; -import java.util.Map; -import java.util.Set; - -/** - * Keeps track of what files to send with file distribution - * - * @author Tony Vaagenes - */ -public class FileDistributor { - - /** A map from file reference to the hosts to which that file reference should be distributed */ - private final Set<FileReference> fileReferences = new LinkedHashSet<>(); - - public FileDistributor() { } - - public void sendFileReference(FileReference reference) { - fileReferences.add(reference); - } - - public Set<FileReference> allFilesToSend() { - return Set.copyOf(fileReferences); - } - -} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileReferencesRepository.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileReferencesRepository.java new file mode 100644 index 00000000000..bdda9cdf582 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileReferencesRepository.java @@ -0,0 +1,26 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.model.filedistribution; + +import com.yahoo.config.FileReference; + +import java.util.LinkedHashSet; +import java.util.Set; + +/** + * Keeps track of what files to send with file distribution + * + * @author Tony Vaagenes + * @author hmusum + */ +public class FileReferencesRepository { + + /** A set of file references that should be distributed */ + private final Set<FileReference> fileReferences = new LinkedHashSet<>(); + + public FileReferencesRepository() { } + + public void add(FileReference reference) { fileReferences.add(reference); } + + public Set<FileReference> allFileReferences() { return Set.copyOf(fileReferences); } + +} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/FileDistributorTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/FileReferencesRepositoryTestCase.java index 718288d9624..cffd1cb1e2e 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/FileDistributorTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/FileReferencesRepositoryTestCase.java @@ -11,21 +11,21 @@ import static org.junit.Assert.assertNotNull; /** * @author bratseth */ -public class FileDistributorTestCase { +public class FileReferencesRepositoryTestCase { @Test public void fileDistributor() { FileRegistry fileRegistry = new MockFileRegistry(); - FileDistributor fileDistributor = new FileDistributor(); + FileReferencesRepository fileReferencesRepository = new FileReferencesRepository(); String file1 = "component/path1"; String file2 = "component/path2"; FileReference ref1 = fileRegistry.addFile(file1); FileReference ref2 = fileRegistry.addFile(file2); - fileDistributor.sendFileReference(ref1); - fileDistributor.sendFileReference(ref2); - fileDistributor.sendFileReference(ref1); // same file reference as above - fileDistributor.sendFileReference(ref2); + fileReferencesRepository.add(ref1); + fileReferencesRepository.add(ref2); + fileReferencesRepository.add(ref1); // same file reference as above + fileReferencesRepository.add(ref2); assertNotNull(ref1); assertNotNull(ref2); |