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);