From 92a0e866562662d5be4b3aa01da2a115bc9a2a00 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Tue, 14 Apr 2020 08:33:45 +0200 Subject: Clean up and simplify FileDistributor No functional changes --- .../java/com/yahoo/config/model/test/MockRoot.java | 3 +- .../com/yahoo/vespa/model/AbstractService.java | 1 + .../java/com/yahoo/vespa/model/VespaModel.java | 3 +- .../FileDistributionConfigProducer.java | 3 +- .../model/filedistribution/FileDistributor.java | 66 ++++++++-------------- .../model/content/utils/ContentClusterUtils.java | 2 +- .../filedistribution/FileDistributorTestCase.java | 16 ++++-- 7 files changed, 40 insertions(+), 54 deletions(-) (limited to 'config-model') 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 13f271ebe9d..f9492398f9f 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 @@ -26,6 +26,7 @@ import java.io.IOException; import java.io.StringReader; import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.Set; @@ -66,7 +67,7 @@ public class MockRoot extends AbstractConfigProducerRoot { super(rootConfigId); hostSystem = new HostSystem(this, "hostsystem", deployState.getProvisioner(), deployState.getDeployLogger()); this.deployState = deployState; - fileDistributor = new FileDistributor(deployState.getFileRegistry(), null); + fileDistributor = new FileDistributor(deployState.getFileRegistry(), List.of()); } public FileDistributionConfigProducer getFileDistributionConfigProducer() { 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 72c475fb091..75b33e184d9 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 @@ -461,6 +461,7 @@ public abstract class AbstractService extends AbstractConfigProducer fileDistributionConfigProviders = new IdentityHashMap<>(); private final FileDistributor fileDistributor; - public FileDistributionConfigProducer(AbstractConfigProducer ancestor, FileRegistry fileRegistry, List configServerSpec) { + public FileDistributionConfigProducer(AbstractConfigProducer ancestor, FileRegistry fileRegistry, + List configServerSpec) { this(ancestor, new FileDistributor(fileRegistry, configServerSpec)); } 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 index 576b009c846..591d2b2851d 100644 --- 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 @@ -20,60 +20,39 @@ public class FileDistributor { private final FileRegistry fileRegistry; private final List configServerSpecs; - /** A map from files to the hosts to which that file should be distributed */ + /** A map from file reference to the hosts to which that file reference should be distributed */ private final Map> filesToHosts = new LinkedHashMap<>(); + public FileDistributor(FileRegistry fileRegistry, List configServerSpecs) { + this.fileRegistry = fileRegistry; + this.configServerSpecs = configServerSpecs; + } + /** * Adds the given file to the associated application packages' registry of file and marks the file - * for distribution to the given hosts. + * for distribution to the given host. * Note: This class receives ownership of the given collection. * * @return the reference to the file, created by the application package */ - public FileReference sendFileToHosts(String relativePath, Collection hosts) { - FileReference reference = fileRegistry.addFile(relativePath); - addToFilesToDistribute(reference, hosts); - - return reference; + public FileReference sendFileToHost(String relativePath, Host host) { + return addFileReference(fileRegistry.addFile(relativePath), host); } /** * Adds the given file to the associated application packages' registry of file and marks the file - * for distribution to the given hosts. + * for distribution to the given host. * Note: This class receives ownership of the given collection. * * @return the reference to the file, created by the application package */ - public FileReference sendUriToHosts(String uri, Collection hosts) { - FileReference reference = fileRegistry.addUri(uri); - if (reference != null) { - addToFilesToDistribute(reference, hosts); - } - - return reference; - } - - /** Same as sendFileToHost(relativePath,Collections.singletonList(host) */ - public FileReference sendFileToHost(String relativePath, Host host) { - return sendFileToHosts(relativePath, Arrays.asList(host)); - } - public FileReference sendUriToHost(String uri, Host host) { - return sendUriToHosts(uri, Arrays.asList(host)); - } - - private void addToFilesToDistribute(FileReference reference, Collection hosts) { - Set oldHosts = getHosts(reference); - oldHosts.addAll(hosts); + return addFileReference(fileRegistry.addUri(uri), host); } - private Set getHosts(FileReference reference) { - return filesToHosts.computeIfAbsent(reference, k -> new HashSet<>()); - } - - public FileDistributor(FileRegistry fileRegistry, List configServerSpecs) { - this.fileRegistry = fileRegistry; - this.configServerSpecs = configServerSpecs; + private FileReference addFileReference(FileReference reference, Host host) { + filesToHosts.computeIfAbsent(reference, k -> new HashSet<>()).add(host); + return reference; } /** Returns the files which has been marked for distribution to the given host */ @@ -107,16 +86,15 @@ public class FileDistributor { // should only be called during deploy public void sendDeployedFiles(FileDistribution dbHandler) { String fileSourceHost = fileSourceHost(); - for (Host host : getTargetHosts()) { - if ( ! host.getHostname().equals(fileSourceHost)) { - dbHandler.startDownload(host.getHostname(), ConfigProxy.BASEPORT, filesToSendToHost(host)); - } - } + + getTargetHosts().stream() + .filter(host -> ! host.getHostname().equals(fileSourceHost)) + .forEach(host -> dbHandler.startDownload(host.getHostname(), ConfigProxy.BASEPORT, filesToSendToHost(host))); + // Ask other config servers to download, for redundancy - if (configServerSpecs != null) - configServerSpecs.stream() - .filter(configServerSpec -> !configServerSpec.getHostName().equals(fileSourceHost)) - .forEach(spec -> dbHandler.startDownload(spec.getHostName(), spec.getConfigServerPort(), allFilesToSend())); + configServerSpecs.stream() + .filter(spec -> !spec.getHostName().equals(fileSourceHost)) + .forEach(spec -> dbHandler.startDownload(spec.getHostName(), spec.getConfigServerPort(), allFilesToSend())); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java index db9153fcf23..b5915ecf033 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java @@ -60,7 +60,7 @@ public class ContentClusterUtils { public static ContentCluster createCluster(String clusterXml, MockRoot root) { Document doc = XML.getDocument(clusterXml); Admin admin = new Admin(root, new DefaultMonitoring("vespa", 60), new Metrics(), false, - new FileDistributionConfigProducer(root, new MockFileRegistry(), null), + new FileDistributionConfigProducer(root, new MockFileRegistry(), List.of()), root.getDeployState().isHosted()); ConfigModelContext context = ConfigModelContext.create(null, root.getDeployState(), null,null, root, null); 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/FileDistributorTestCase.java index 131a5344116..39ded69b710 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/FileDistributorTestCase.java @@ -10,31 +10,34 @@ import org.junit.Test; import java.io.File; import java.util.Arrays; import java.util.HashSet; +import java.util.List; import java.util.Set; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotNull; /** * @author bratseth */ public class FileDistributorTestCase { + @Test public void fileDistributor() { MockHosts hosts = new MockHosts(); - FileDistributor fileDistributor = new FileDistributor(new MockFileRegistry(), null); + FileDistributor fileDistributor = new FileDistributor(new MockFileRegistry(), List.of()); String file1 = "component/path1"; String file2 = "component/path2"; - FileReference ref1 = fileDistributor.sendFileToHosts(file1, Arrays.asList(hosts.host1, hosts.host2)); - FileReference ref2 = fileDistributor.sendFileToHosts(file2, Arrays.asList(hosts.host3)); + FileReference ref1 = fileDistributor.sendFileToHost(file1, hosts.host1); + fileDistributor.sendFileToHost(file1, hosts.host2); // same file reference as above + FileReference ref2 = fileDistributor.sendFileToHost(file2, hosts.host3); assertEquals(new HashSet<>(Arrays.asList(hosts.host1, hosts.host2, hosts.host3)), fileDistributor.getTargetHosts()); - assertTrue( ref1 != null ); - assertTrue( ref2 != null ); + assertNotNull(ref1); + assertNotNull(ref2); MockFileDistribution dbHandler = new MockFileDistribution(); fileDistributor.sendDeployedFiles(dbHandler); @@ -54,4 +57,5 @@ public class FileDistributorTestCase { return null; } } + } -- cgit v1.2.3