diff options
53 files changed, 1016 insertions, 471 deletions
diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AbstractAssembleBundleMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AbstractAssembleBundleMojo.java new file mode 100644 index 00000000000..622cb9a49b0 --- /dev/null +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AbstractAssembleBundleMojo.java @@ -0,0 +1,96 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.plugin.mojo; + +import com.yahoo.container.plugin.util.Files; +import com.yahoo.container.plugin.util.JarFiles; +import org.apache.maven.archiver.MavenArchiveConfiguration; +import org.apache.maven.archiver.MavenArchiver; +import org.apache.maven.artifact.Artifact; +import org.apache.maven.execution.MavenSession; +import org.apache.maven.plugin.AbstractMojo; +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugins.annotations.Parameter; +import org.apache.maven.project.MavenProject; +import org.codehaus.plexus.archiver.jar.JarArchiver; + +import java.io.File; +import java.nio.channels.Channels; +import java.nio.file.Path; +import java.util.Collection; +import java.util.Enumeration; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import java.util.zip.ZipEntry; + +/** + * @author bjorncs + */ +abstract class AbstractAssembleBundleMojo extends AbstractMojo { + + @Parameter(defaultValue = "${project}") + MavenProject project; + + @Parameter(defaultValue = "${session}", readonly = true, required = true) + MavenSession session; + + @Parameter + MavenArchiveConfiguration archiveConfiguration = new MavenArchiveConfiguration(); + + void addDirectory(JarArchiver jarArchiver, Path directory) { + if (java.nio.file.Files.isDirectory(directory)) { + jarArchiver.addDirectory(directory.toFile()); + } + } + + void createArchive(JarArchiver jarArchiver, Path jarFile, Path manifestFile) throws MojoExecutionException { + archiveConfiguration.setForced(true); // force recreating the archive + archiveConfiguration.setManifestFile(manifestFile.toFile()); + MavenArchiver mavenArchiver = new MavenArchiver(); + mavenArchiver.setArchiver(jarArchiver); + mavenArchiver.setOutputFile(jarFile.toFile()); + try { + mavenArchiver.createArchive(session, project, archiveConfiguration); + } catch (Exception e) { + throw new MojoExecutionException("Error creating archive " + jarFile.toFile().getName(), e); + } + } + + void addArtifacts(JarArchiver jarArchiver, Collection<Artifact> artifacts) { + for (Artifact artifact : artifacts) { + if ("jar".equals(artifact.getType())) { + jarArchiver.addFile(artifact.getFile(), "dependencies/" + artifact.getFile().getName()); + copyConfigDefinitions(artifact.getFile(), jarArchiver); + } else { + getLog().warn("Unknown artifact type " + artifact.getType()); + } + } + } + + private void copyConfigDefinitions(File file, JarArchiver jarArchiver) { + JarFiles.withJarFile(file, jarFile -> { + for (Enumeration<JarEntry> entries = jarFile.entries(); entries.hasMoreElements();) { + JarEntry entry = entries.nextElement(); + String name = entry.getName(); + if (name.startsWith("configdefinitions/") && name.endsWith(".def")) { + copyConfigDefinition(jarFile, entry, jarArchiver); + } + } + return null; + }); + } + + private void copyConfigDefinition(JarFile jarFile, ZipEntry entry, JarArchiver jarArchiver) { + JarFiles.withInputStream(jarFile, entry, input -> { + String defPath = entry.getName().replace("/", File.separator); + File destinationFile = new File(project.getBuild().getOutputDirectory(), defPath); + destinationFile.getParentFile().mkdirs(); + + Files.withFileOutputStream(destinationFile, output -> { + output.getChannel().transferFrom(Channels.newChannel(input), 0, Long.MAX_VALUE); + return null; + }); + jarArchiver.addFile(destinationFile, entry.getName()); + return null; + }); + } +} diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AbstractGenerateOsgiManifestMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AbstractGenerateOsgiManifestMojo.java new file mode 100644 index 00000000000..d648d9ca258 --- /dev/null +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AbstractGenerateOsgiManifestMojo.java @@ -0,0 +1,210 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.plugin.mojo; + +import com.yahoo.container.plugin.classanalysis.Analyze; +import com.yahoo.container.plugin.classanalysis.ClassFileMetaData; +import com.yahoo.container.plugin.classanalysis.ExportPackageAnnotation; +import com.yahoo.container.plugin.classanalysis.PackageTally; +import com.yahoo.container.plugin.osgi.ExportPackageParser; +import com.yahoo.container.plugin.osgi.ExportPackages; +import com.yahoo.container.plugin.osgi.ImportPackages; +import com.yahoo.container.plugin.util.Strings; +import org.apache.maven.artifact.Artifact; +import org.apache.maven.artifact.versioning.ArtifactVersion; +import org.apache.maven.artifact.versioning.DefaultArtifactVersion; +import org.apache.maven.plugin.AbstractMojo; +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugins.annotations.Parameter; +import org.apache.maven.project.MavenProject; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.jar.Attributes; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import java.util.jar.Manifest; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static com.yahoo.container.plugin.util.IO.withFileOutputStream; +import static java.util.stream.Collectors.toList; + +/** + * @author bjorncs + */ +abstract class AbstractGenerateOsgiManifestMojo extends AbstractMojo { + + @Parameter(defaultValue = "${project}") + MavenProject project; + + /** + * If set to true, the artifact's version is used as default package version for ExportPackages. + * Packages from included (compile scoped) artifacts will use the version for their own artifact. + * If the package is exported with an explicit version in package-info.java, that version will be + * used regardless of this parameter. + */ + @Parameter(alias = "UseArtifactVersionForExportPackages", defaultValue = "false") + boolean useArtifactVersionForExportPackages; + + @Parameter(alias = "Bundle-Version", defaultValue = "${project.version}") + String bundleVersion; + + // TODO: default should include groupId, but that will require a lot of changes both by us and users. + @Parameter(alias = "Bundle-SymbolicName", defaultValue = "${project.artifactId}") + String bundleSymbolicName; + + @Parameter(alias = "Import-Package") + String importPackage; + + Map<String, String> generateManifestContent( + Collection<Artifact> jarArtifactsToInclude, + Map<String, ImportPackages.Import> calculatedImports, + PackageTally pluginPackageTally) { + + Map<String, Optional<String>> manualImports = getManualImports(); + for (String packageName : manualImports.keySet()) { + calculatedImports.remove(packageName); + } + Collection<ImportPackages.Import> imports = calculatedImports.values(); + + Map<String, String> ret = new HashMap<>(); + String importPackage = Stream.concat(manualImports.entrySet().stream().map(e -> asOsgiImport(e.getKey(), e.getValue())), + imports.stream().map(ImportPackages.Import::asOsgiImport)).sorted() + .collect(Collectors.joining(",")); + + String exportPackage = osgiExportPackages(pluginPackageTally.exportedPackages()).stream().sorted() + .collect(Collectors.joining(",")); + + ret.put("Created-By", "vespa container maven plugin"); + ret.put("Bundle-ManifestVersion", "2"); + ret.put("Bundle-Name", project.getName()); + ret.put("Bundle-SymbolicName", bundleSymbolicName); + ret.put("Bundle-Version", asBundleVersion(bundleVersion)); + ret.put("Bundle-Vendor", "Yahoo!"); + addIfNotEmpty(ret, "Bundle-ClassPath", bundleClassPath(jarArtifactsToInclude)); + addIfNotEmpty(ret, "Import-Package", importPackage); + addIfNotEmpty(ret, "Export-Package", exportPackage); + + return ret; + } + + PackageTally definedPackages(Collection<Artifact> jarArtifacts) { + List<PackageTally> tallies = new ArrayList<>(); + for (var artifact : jarArtifacts) { + try { + tallies.add(definedPackages(new JarFile(artifact.getFile()), artifactVersionOrNull(artifact.getVersion()))); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + return PackageTally.combine(tallies); + } + + ArtifactVersion artifactVersionOrNull(String version) { + return useArtifactVersionForExportPackages ? new DefaultArtifactVersion(version) : null; + } + + static void createManifestFile(Path outputDirectory, Map<String, String> manifestContent) { + Manifest manifest = new Manifest(); + Attributes mainAttributes = manifest.getMainAttributes(); + + mainAttributes.put(Attributes.Name.MANIFEST_VERSION, "1.0"); + manifestContent.forEach(mainAttributes::putValue); + + withFileOutputStream(outputDirectory.resolve(JarFile.MANIFEST_NAME).toFile(), out -> { + manifest.write(out); + return null; + }); + } + + static void addIfNotEmpty(Map<String, String> map, String key, String value) { + if (value != null && ! value.isEmpty()) { + map.put(key, value); + } + } + + private Collection<String> osgiExportPackages(Map<String, ExportPackageAnnotation> exportedPackages) { + return exportedPackages.entrySet().stream().map(entry -> entry.getKey() + ";version=" + entry.getValue().osgiVersion()) + .collect(Collectors.toList()); + } + + private static String asOsgiImport(String packageName, Optional<String> version) { + return version + .map(s -> packageName + ";version=" + ("\"" + s + "\"")) + .orElse(packageName); + } + + private static String bundleClassPath(Collection<Artifact> artifactsToInclude) { + return Stream.concat(Stream.of("."), artifactsToInclude.stream() + .map(artifact -> "dependencies/" + artifact.getFile().getName())) + .collect(Collectors.joining(",")); + } + + private Map<String, Optional<String>> getManualImports() { + try { + if (importPackage == null || importPackage.isBlank()) return Map.of(); + Map<String, Optional<String>> ret = new HashMap<>(); + List<ExportPackages.Export> imports = ExportPackageParser.parseExports(importPackage); + for (ExportPackages.Export imp : imports) { + Optional<String> version = getVersionThrowOthers(imp.getParameters()); + imp.getPackageNames().forEach(pn -> ret.put(pn, version)); + } + return ret; + } catch (Exception e) { + throw new RuntimeException("Error in Import-Package:" + importPackage, e); + } + } + + private static String asBundleVersion(String projectVersion) { + if (projectVersion == null) { + throw new IllegalArgumentException("Missing project version."); + } + + String[] parts = projectVersion.split("-", 2); + List<String> numericPart = Stream.of(parts[0].split("\\.")).map(s -> Strings.replaceEmptyString(s, "0")).limit(3) + .collect(toList()); + while (numericPart.size() < 3) { + numericPart.add("0"); + } + + return String.join(".", numericPart); + } + + private static Optional<String> getVersionThrowOthers(List<ExportPackages.Parameter> parameters) { + if (parameters.size() == 1 && "version".equals(parameters.get(0).getName())) { + return Optional.of(parameters.get(0).getValue()); + } else if (parameters.size() == 0) { + return Optional.empty(); + } else { + List<String> paramNames = parameters.stream().map(ExportPackages.Parameter::getName).collect(Collectors.toList()); + throw new RuntimeException("A single, optional version parameter expected, but got " + paramNames); + } + } + + private static PackageTally definedPackages(JarFile jarFile, ArtifactVersion version) throws MojoExecutionException { + List<ClassFileMetaData> analyzedClasses = new ArrayList<>(); + for (Enumeration<JarEntry> entries = jarFile.entries(); entries.hasMoreElements();) { + JarEntry entry = entries.nextElement(); + if (! entry.isDirectory() && entry.getName().endsWith(".class")) { + analyzedClasses.add(analyzeClass(jarFile, entry, version)); + } + } + return PackageTally.fromAnalyzedClassFiles(analyzedClasses); + } + + private static ClassFileMetaData analyzeClass(JarFile jarFile, JarEntry entry, ArtifactVersion version) throws MojoExecutionException { + try { + return Analyze.analyzeClass(jarFile.getInputStream(entry), version); + } catch (Exception e) { + throw new MojoExecutionException( + String.format("While analyzing the class '%s' in jar file '%s'", entry.getName(), jarFile.getName()), e); + } + } + +} diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/Artifacts.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/Artifacts.java index a0d0e143724..bc6a970140d 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/Artifacts.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/Artifacts.java @@ -18,7 +18,10 @@ class Artifacts { private final List<Artifact> jarArtifactsProvided; private final List<Artifact> nonJarArtifacts; - private ArtifactSet(List<Artifact> jarArtifactsToInclude, List<Artifact> jarArtifactsProvided, List<Artifact> nonJarArtifacts) { + private ArtifactSet( + List<Artifact> jarArtifactsToInclude, + List<Artifact> jarArtifactsProvided, + List<Artifact> nonJarArtifacts) { this.jarArtifactsToInclude = jarArtifactsToInclude; this.jarArtifactsProvided = jarArtifactsProvided; this.nonJarArtifacts = nonJarArtifacts; @@ -37,19 +40,24 @@ class Artifacts { } } - static ArtifactSet getArtifacts(MavenProject project) { + static ArtifactSet getArtifacts(MavenProject project) { return getArtifacts(project, false, null); } + static ArtifactSet getArtifacts(MavenProject project, boolean includeTestArtifacts, String testProvidedConfig) { + TestProvidedArtifacts testProvidedArtifacts = TestProvidedArtifacts.from(project.getArtifactMap(), testProvidedConfig); List<Artifact> jarArtifactsToInclude = new ArrayList<>(); List<Artifact> jarArtifactsProvided = new ArrayList<>(); List<Artifact> nonJarArtifactsToInclude = new ArrayList<>(); List<Artifact> nonJarArtifactsProvided = new ArrayList<>(); - for (Artifact artifact : project.getArtifacts()) { if ("jar".equals(artifact.getType())) { - if (Artifact.SCOPE_COMPILE.equals(artifact.getScope())) { + if (includeTestArtifacts && testProvidedArtifacts.isTestProvided(artifact)) { + jarArtifactsProvided.add(artifact); + } else if (Artifact.SCOPE_COMPILE.equals(artifact.getScope())) { jarArtifactsToInclude.add(artifact); } else if (Artifact.SCOPE_PROVIDED.equals(artifact.getScope())) { jarArtifactsProvided.add(artifact); + } else if (includeTestArtifacts && Artifact.SCOPE_TEST.equals(artifact.getScope())) { + jarArtifactsToInclude.add(artifact); } } else { if (Artifact.SCOPE_COMPILE.equals(artifact.getScope())) { @@ -64,6 +72,6 @@ class Artifacts { } static Collection<Artifact> getArtifactsToInclude(MavenProject project) { - return getArtifacts(project).getJarArtifactsToInclude(); + return getArtifacts(project, false, null).getJarArtifactsToInclude(); } } diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AssembleContainerPluginMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AssembleContainerPluginMojo.java index 13d73d58a97..bed7610e82f 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AssembleContainerPluginMojo.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AssembleContainerPluginMojo.java @@ -1,53 +1,36 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.plugin.mojo; -import com.yahoo.container.plugin.util.Files; -import com.yahoo.container.plugin.util.JarFiles; -import org.apache.maven.archiver.MavenArchiveConfiguration; -import org.apache.maven.archiver.MavenArchiver; -import org.apache.maven.execution.MavenSession; import org.apache.maven.model.Build; -import org.apache.maven.plugin.AbstractMojo; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugins.annotations.Component; import org.apache.maven.plugins.annotations.Mojo; import org.apache.maven.plugins.annotations.Parameter; import org.apache.maven.plugins.annotations.ResolutionScope; -import org.apache.maven.project.MavenProject; import org.apache.maven.project.MavenProjectHelper; import org.codehaus.plexus.archiver.jar.JarArchiver; import java.io.File; -import java.nio.channels.Channels; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.EnumMap; -import java.util.Enumeration; import java.util.Map; -import java.util.jar.JarEntry; import java.util.jar.JarFile; -import java.util.zip.ZipEntry; /** * @author Tony Vaagenes - * @author ollivir + * @author Olli Virtanen + * @author bjorncs */ @Mojo(name = "assemble-container-plugin", requiresDependencyResolution = ResolutionScope.COMPILE, threadSafe = true) -public class AssembleContainerPluginMojo extends AbstractMojo { +public class AssembleContainerPluginMojo extends AbstractAssembleBundleMojo { private static enum Dependencies { WITH, WITHOUT } - @Parameter(defaultValue = "${project}") - private MavenProject project = null; - @Component private MavenProjectHelper projectHelper; - @Parameter(defaultValue = "${session}", readonly = true, required = true) - private MavenSession session = null; - - @Parameter - private MavenArchiveConfiguration archiveConfiguration = new MavenArchiveConfiguration(); - @Parameter(alias = "UseCommonAssemblyIds", defaultValue = "false") private boolean useCommonAssemblyIds = false; @@ -74,19 +57,17 @@ public class AssembleContainerPluginMojo extends AbstractMojo { jarFiles.put(dep, jarFileInBuildDirectory(build().getFinalName(), suffix)); }); - // force recreating the archive - archiveConfiguration.setForced(true); - archiveConfiguration.setManifestFile(new File(new File(build().getOutputDirectory()), JarFile.MANIFEST_NAME)); + Path manifestFile = Paths.get(build().getOutputDirectory(), JarFile.MANIFEST_NAME); JarArchiver jarWithoutDependencies = new JarArchiver(); - addClassesDirectory(jarWithoutDependencies); - createArchive(jarFiles.get(Dependencies.WITHOUT), jarWithoutDependencies); + addDirectory(jarWithoutDependencies, Paths.get(build().getOutputDirectory())); + createArchive(jarWithoutDependencies, jarFiles.get(Dependencies.WITHOUT).toPath(), manifestFile); project.getArtifact().setFile(jarFiles.get(Dependencies.WITHOUT)); JarArchiver jarWithDependencies = new JarArchiver(); - addClassesDirectory(jarWithDependencies); - addDependencies(jarWithDependencies); - createArchive(jarFiles.get(Dependencies.WITH), jarWithDependencies); + addDirectory(jarWithDependencies, Paths.get(build().getOutputDirectory())); + addArtifacts(jarWithDependencies, Artifacts.getArtifactsToInclude(project)); + createArchive(jarWithDependencies, jarFiles.get(Dependencies.WITH).toPath(), manifestFile); if (attachBundleArtifact) { projectHelper.attachArtifact(project, @@ -96,68 +77,6 @@ public class AssembleContainerPluginMojo extends AbstractMojo { } } - private File jarFileInBuildDirectory(String name, String suffix) { - return new File(build().getDirectory(), name + suffix); - } - - private void addClassesDirectory(JarArchiver jarArchiver) { - File classesDirectory = new File(build().getOutputDirectory()); - if (classesDirectory.isDirectory()) { - jarArchiver.addDirectory(classesDirectory); - } - } - - private void createArchive(File jarFile, JarArchiver jarArchiver) throws MojoExecutionException { - MavenArchiver mavenArchiver = new MavenArchiver(); - mavenArchiver.setArchiver(jarArchiver); - mavenArchiver.setOutputFile(jarFile); - try { - mavenArchiver.createArchive(session, project, archiveConfiguration); - } catch (Exception e) { - throw new MojoExecutionException("Error creating archive " + jarFile.getName(), e); - } - } - - private void addDependencies(JarArchiver jarArchiver) { - Artifacts.getArtifactsToInclude(project).forEach(artifact -> { - if ("jar".equals(artifact.getType())) { - jarArchiver.addFile(artifact.getFile(), "dependencies/" + artifact.getFile().getName()); - copyConfigDefinitions(artifact.getFile(), jarArchiver); - } else { - getLog().warn("Unkown artifact type " + artifact.getType()); - } - }); - } - - private void copyConfigDefinitions(File file, JarArchiver jarArchiver) { - JarFiles.withJarFile(file, jarFile -> { - for (Enumeration<JarEntry> entries = jarFile.entries(); entries.hasMoreElements();) { - JarEntry entry = entries.nextElement(); - String name = entry.getName(); - if (name.startsWith("configdefinitions/") && name.endsWith(".def")) { - copyConfigDefinition(jarFile, entry, jarArchiver); - } - } - return null; - }); - } - - private void copyConfigDefinition(JarFile jarFile, ZipEntry entry, JarArchiver jarArchiver) { - JarFiles.withInputStream(jarFile, entry, input -> { - String defPath = entry.getName().replace("/", File.separator); - File destinationFile = new File(build().getOutputDirectory(), defPath); - destinationFile.getParentFile().mkdirs(); - - Files.withFileOutputStream(destinationFile, output -> { - output.getChannel().transferFrom(Channels.newChannel(input), 0, Long.MAX_VALUE); - return null; - }); - jarArchiver.addFile(destinationFile, entry.getName()); - return null; - }); - } - - private Build build() { - return project.getBuild(); - } + private File jarFileInBuildDirectory(String name, String suffix) { return new File(build().getDirectory(), name + suffix); } + private Build build() { return project.getBuild(); } } diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AssembleTestBundleMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AssembleTestBundleMojo.java new file mode 100644 index 00000000000..ab827275f53 --- /dev/null +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/AssembleTestBundleMojo.java @@ -0,0 +1,38 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.plugin.mojo; + +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugins.annotations.Mojo; +import org.apache.maven.plugins.annotations.Parameter; +import org.apache.maven.plugins.annotations.ResolutionScope; +import org.codehaus.plexus.archiver.jar.JarArchiver; + +import java.nio.file.Path; +import java.nio.file.Paths; + +import static com.yahoo.container.plugin.mojo.TestBundleUtils.archiveFile; +import static com.yahoo.container.plugin.mojo.TestBundleUtils.manifestFile; + +/** + * @author bjorncs + */ +@Mojo(name = "assemble-test-bundle", requiresDependencyResolution = ResolutionScope.TEST, threadSafe = true) +public class AssembleTestBundleMojo extends AbstractAssembleBundleMojo { + + @Parameter + private String testProvidedArtifacts; + + @Override + public void execute() throws MojoExecutionException { + Artifacts.ArtifactSet artifacts = Artifacts.getArtifacts(project, true, testProvidedArtifacts); + JarArchiver archiver = new JarArchiver(); + addDirectory(archiver, Paths.get(project.getBuild().getOutputDirectory())); + addDirectory(archiver, Paths.get(project.getBuild().getTestOutputDirectory())); + addArtifacts(archiver, artifacts.getJarArtifactsToInclude()); + Path archiveFile = archiveFile(project); + createArchive(archiver, archiveFile, manifestFile(project)); + project.getArtifact().setFile(archiveFile.toFile()); + } + + +} diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java index 41420b38360..3020184fd35 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java @@ -4,36 +4,23 @@ package com.yahoo.container.plugin.mojo; import com.google.common.collect.Sets; import com.yahoo.container.plugin.classanalysis.Analyze; import com.yahoo.container.plugin.classanalysis.ClassFileMetaData; -import com.yahoo.container.plugin.classanalysis.ExportPackageAnnotation; import com.yahoo.container.plugin.classanalysis.PackageTally; -import com.yahoo.container.plugin.osgi.ExportPackageParser; import com.yahoo.container.plugin.osgi.ExportPackages; import com.yahoo.container.plugin.osgi.ExportPackages.Export; import com.yahoo.container.plugin.osgi.ImportPackages.Import; -import com.yahoo.container.plugin.util.Strings; import org.apache.maven.artifact.Artifact; -import org.apache.maven.artifact.versioning.ArtifactVersion; -import org.apache.maven.artifact.versioning.DefaultArtifactVersion; -import org.apache.maven.plugin.AbstractMojo; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugins.annotations.Mojo; import org.apache.maven.plugins.annotations.Parameter; import org.apache.maven.plugins.annotations.ResolutionScope; -import org.apache.maven.project.MavenProject; import java.io.File; -import java.util.ArrayList; +import java.nio.file.Paths; import java.util.Collection; -import java.util.Enumeration; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.jar.Attributes; -import java.util.jar.JarEntry; -import java.util.jar.JarFile; -import java.util.jar.Manifest; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -41,7 +28,6 @@ import static com.yahoo.container.plugin.bundle.AnalyzeBundle.exportedPackagesAg import static com.yahoo.container.plugin.osgi.ExportPackages.exportsByPackageName; import static com.yahoo.container.plugin.osgi.ImportPackages.calculateImports; import static com.yahoo.container.plugin.util.Files.allDescendantFiles; -import static com.yahoo.container.plugin.util.IO.withFileOutputStream; /** @@ -49,19 +35,7 @@ import static com.yahoo.container.plugin.util.IO.withFileOutputStream; * @author ollivir */ @Mojo(name = "generate-osgi-manifest", requiresDependencyResolution = ResolutionScope.TEST, threadSafe = true) -public class GenerateOsgiManifestMojo extends AbstractMojo { - - @Parameter(defaultValue = "${project}") - private MavenProject project = null; - - /** - * If set to true, the artifact's version is used as default package version for ExportPackages. - * Packages from included (compile scoped) artifacts will use the version for their own artifact. - * If the package is exported with an explicit version in package-info.java, that version will be - * used regardless of this parameter. - */ - @Parameter(alias = "UseArtifactVersionForExportPackages", defaultValue = "false") - private boolean useArtifactVersionForExportPackages; +public class GenerateOsgiManifestMojo extends AbstractGenerateOsgiManifestMojo { @Parameter private String discApplicationClass = null; @@ -69,22 +43,12 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { @Parameter private String discPreInstallBundle = null; - @Parameter(alias = "Bundle-Version", defaultValue = "${project.version}") - private String bundleVersion = null; - - // TODO: default should include groupId, but that will require a lot of changes both by us and users. - @Parameter(alias = "Bundle-SymbolicName", defaultValue = "${project.artifactId}") - private String bundleSymbolicName = null; - @Parameter(alias = "Bundle-Activator") private String bundleActivator = null; @Parameter(alias = "X-JDisc-Privileged-Activator") private String jdiscPrivilegedActivator = null; - @Parameter(alias = "Import-Package") - private String importPackage = null; - @Parameter(alias = "WebInfUrl") private String webInfUrl = null; @@ -127,19 +91,25 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { includedPackages.definedPackages(), exportsByPackageName(exportedPackagesFromProvidedJars)); - Map<String, Optional<String>> manualImports = emptyToNone(importPackage).map(GenerateOsgiManifestMojo::getManualImports) - .orElseGet(HashMap::new); - for (String packageName : manualImports.keySet()) { - calculatedImports.remove(packageName); - } - createManifestFile(new File(project.getBuild().getOutputDirectory()), manifestContent(project, - artifactSet.getJarArtifactsToInclude(), manualImports, calculatedImports.values(), includedPackages)); + + Map<String, String> manifestContent = generateManifestContent(artifactSet.getJarArtifactsToInclude(), calculatedImports, includedPackages); + addAdditionalManifestProperties(manifestContent); + createManifestFile(Paths.get(project.getBuild().getOutputDirectory()), manifestContent); } catch (Exception e) { throw new MojoExecutionException("Failed generating osgi manifest", e); } } + private void addAdditionalManifestProperties(Map<String, String> manifestContent) { + addIfNotEmpty(manifestContent, "Bundle-Activator", bundleActivator); + addIfNotEmpty(manifestContent, "X-JDisc-Privileged-Activator", jdiscPrivilegedActivator); + addIfNotEmpty(manifestContent, "Main-Class", mainClass); + addIfNotEmpty(manifestContent, "X-JDisc-Application", discApplicationClass); + addIfNotEmpty(manifestContent, "X-JDisc-Preinstall-Bundle", trimWhitespace(Optional.ofNullable(discPreInstallBundle))); + addIfNotEmpty(manifestContent, "WebInfUrl", webInfUrl); + } + private void logDebugPackageSets(List<Export> exportedPackagesFromProvidedJars, PackageTally includedPackages) { if (getLog().isDebugEnabled()) { getLog().debug("Referenced packages = " + includedPackages.referencedPackages()); @@ -197,101 +167,10 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { } } - private Collection<String> osgiExportPackages(Map<String, ExportPackageAnnotation> exportedPackages) { - return exportedPackages.entrySet().stream().map(entry -> entry.getKey() + ";version=" + entry.getValue().osgiVersion()) - .collect(Collectors.toList()); - } - private static String trimWhitespace(Optional<String> lines) { return Stream.of(lines.orElse("").split(",")).map(String::trim).collect(Collectors.joining(",")); } - private Map<String, String> manifestContent(MavenProject project, Collection<Artifact> jarArtifactsToInclude, - Map<String, Optional<String>> manualImports, Collection<Import> imports, PackageTally pluginPackageTally) { - Map<String, String> ret = new HashMap<>(); - String importPackage = Stream.concat(manualImports.entrySet().stream().map(e -> asOsgiImport(e.getKey(), e.getValue())), - imports.stream().map(Import::asOsgiImport)).sorted() - .collect(Collectors.joining(",")); - - String exportPackage = osgiExportPackages(pluginPackageTally.exportedPackages()).stream().sorted() - .collect(Collectors.joining(",")); - - ret.put("Created-By", "vespa container maven plugin"); - ret.put("Bundle-ManifestVersion", "2"); - addIfNotEmpty(ret, "Bundle-Name", project.getName()); - addIfNotEmpty(ret, "Bundle-SymbolicName", bundleSymbolicName); - addIfNotEmpty(ret, "Bundle-Version", asBundleVersion(bundleVersion)); - ret.put("Bundle-Vendor", "Yahoo!"); - addIfNotEmpty(ret, "Bundle-ClassPath", bundleClassPath(jarArtifactsToInclude)); - addIfNotEmpty(ret, "Bundle-Activator", bundleActivator); - addIfNotEmpty(ret, "X-JDisc-Privileged-Activator", jdiscPrivilegedActivator); - addIfNotEmpty(ret, "Main-Class", mainClass); - addIfNotEmpty(ret, "X-JDisc-Application", discApplicationClass); - addIfNotEmpty(ret, "X-JDisc-Preinstall-Bundle", trimWhitespace(Optional.ofNullable(discPreInstallBundle))); - addIfNotEmpty(ret, "WebInfUrl", webInfUrl); - addIfNotEmpty(ret, "Import-Package", importPackage); - addIfNotEmpty(ret, "Export-Package", exportPackage); - - return ret; - } - - private static void addIfNotEmpty(Map<String, String> map, String key, String value) { - if (value != null && ! value.isEmpty()) { - map.put(key, value); - } - } - - private static String asOsgiImport(String packageName, Optional<String> version) { - return version.map(s -> packageName + ";version=" + quote(s)).orElse(packageName); - } - - private static String quote(String s) { - return "\"" + s + "\""; - } - - private static void createManifestFile(File outputDirectory, Map<String, String> manifestContent) { - Manifest manifest = toManifest(manifestContent); - - withFileOutputStream(new File(outputDirectory, JarFile.MANIFEST_NAME), outputStream -> { - manifest.write(outputStream); - return null; - }); - } - - private static Manifest toManifest(Map<String, String> manifestContent) { - Manifest manifest = new Manifest(); - Attributes mainAttributes = manifest.getMainAttributes(); - - mainAttributes.put(Attributes.Name.MANIFEST_VERSION, "1.0"); - manifestContent.forEach(mainAttributes::putValue); - - return manifest; - } - - private static String bundleClassPath(Collection<Artifact> artifactsToInclude) { - return Stream.concat(Stream.of("."), artifactsToInclude.stream().map(GenerateOsgiManifestMojo::dependencyPath)) - .collect(Collectors.joining(",")); - } - - private static String dependencyPath(Artifact artifact) { - return "dependencies/" + artifact.getFile().getName(); - } - - private static String asBundleVersion(String projectVersion) { - if (projectVersion == null) { - throw new IllegalArgumentException("Missing project version."); - } - - String[] parts = projectVersion.split("-", 2); - List<String> numericPart = Stream.of(parts[0].split("\\.")).map(s -> Strings.replaceEmptyString(s, "0")).limit(3) - .collect(Collectors.toList()); - while (numericPart.size() < 3) { - numericPart.add("0"); - } - - return String.join(".", numericPart); - } - private void warnOnUnsupportedArtifacts(Collection<Artifact> nonJarArtifacts) { List<Artifact> unsupportedArtifacts = nonJarArtifacts.stream().filter(a -> ! a.getType().equals("pom")) .collect(Collectors.toList()); @@ -301,10 +180,6 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { artifact.getId(), artifact.getType()))); } - private ArtifactVersion artifactVersionOrNull(String version) { - return useArtifactVersionForExportPackages ? new DefaultArtifactVersion(version) : null; - } - private PackageTally getProjectClassesTally() { File outputDirectory = new File(project.getBuild().getOutputDirectory()); @@ -315,71 +190,4 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { return PackageTally.fromAnalyzedClassFiles(analyzedClasses); } - - private PackageTally definedPackages(Collection<Artifact> jarArtifacts) { - List<PackageTally> tallies = new ArrayList<>(); - for (var artifact : jarArtifacts) { - try { - tallies.add(definedPackages(new JarFile(artifact.getFile()), artifactVersionOrNull(artifact.getVersion()))); - } catch (Exception e) { - throw new RuntimeException(e); - } - } - return PackageTally.combine(tallies); - } - - private static PackageTally definedPackages(JarFile jarFile, ArtifactVersion version) throws MojoExecutionException { - List<ClassFileMetaData> analyzedClasses = new ArrayList<>(); - for (Enumeration<JarEntry> entries = jarFile.entries(); entries.hasMoreElements();) { - JarEntry entry = entries.nextElement(); - if (! entry.isDirectory() && entry.getName().endsWith(".class")) { - analyzedClasses.add(analyzeClass(jarFile, entry, version)); - } - } - return PackageTally.fromAnalyzedClassFiles(analyzedClasses); - } - - private static ClassFileMetaData analyzeClass(JarFile jarFile, JarEntry entry, ArtifactVersion version) throws MojoExecutionException { - try { - return Analyze.analyzeClass(jarFile.getInputStream(entry), version); - } catch (Exception e) { - throw new MojoExecutionException( - String.format("While analyzing the class '%s' in jar file '%s'", entry.getName(), jarFile.getName()), e); - } - } - - private static Map<String, Optional<String>> getManualImports(String importPackage) { - try { - Map<String, Optional<String>> ret = new HashMap<>(); - List<Export> imports = parseImportPackages(importPackage); - for (Export imp : imports) { - Optional<String> version = getVersionThrowOthers(imp.getParameters()); - imp.getPackageNames().forEach(pn -> ret.put(pn, version)); - } - - return ret; - } catch (Exception e) { - throw new RuntimeException("Error in Import-Package:" + importPackage, e); - } - } - - private static Optional<String> getVersionThrowOthers(List<ExportPackages.Parameter> parameters) { - if (parameters.size() == 1 && "version".equals(parameters.get(0).getName())) { - return Optional.of(parameters.get(0).getValue()); - } else if (parameters.size() == 0) { - return Optional.empty(); - } else { - List<String> paramNames = parameters.stream().map(ExportPackages.Parameter::getName).collect(Collectors.toList()); - throw new RuntimeException("A single, optional version parameter expected, but got " + paramNames); - } - } - - private static List<Export> parseImportPackages(String importPackages) { - return ExportPackageParser.parseExports(importPackages); - } - - private static Optional<String> emptyToNone(String str) { - return Optional.ofNullable(str).map(String::trim).filter(s -> ! s.isEmpty()); - } - } diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateTestBundleOsgiManifestMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateTestBundleOsgiManifestMojo.java new file mode 100644 index 00000000000..811aff87b7e --- /dev/null +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateTestBundleOsgiManifestMojo.java @@ -0,0 +1,80 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.plugin.mojo; + +import com.yahoo.container.plugin.classanalysis.Analyze; +import com.yahoo.container.plugin.classanalysis.ClassFileMetaData; +import com.yahoo.container.plugin.classanalysis.PackageTally; +import com.yahoo.container.plugin.osgi.ExportPackages.Export; +import com.yahoo.container.plugin.osgi.ImportPackages; +import org.apache.maven.artifact.Artifact; +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugins.annotations.Mojo; +import org.apache.maven.plugins.annotations.Parameter; +import org.apache.maven.plugins.annotations.ResolutionScope; + +import java.io.File; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + +import static com.yahoo.container.plugin.bundle.AnalyzeBundle.exportedPackagesAggregated; +import static com.yahoo.container.plugin.mojo.TestBundleUtils.outputDirectory; +import static com.yahoo.container.plugin.osgi.ExportPackages.exportsByPackageName; +import static com.yahoo.container.plugin.osgi.ImportPackages.calculateImports; +import static com.yahoo.container.plugin.util.Files.allDescendantFiles; +import static java.util.stream.Collectors.toList; + +/** + * @author bjorncs + */ +@Mojo(name = "generate-test-bundle-osgi-manifest", requiresDependencyResolution = ResolutionScope.TEST, threadSafe = true) +public class GenerateTestBundleOsgiManifestMojo extends AbstractGenerateOsgiManifestMojo { + + @Parameter + private String testProvidedArtifacts; + + public void execute() throws MojoExecutionException { + try { + Artifacts.ArtifactSet artifactSet = Artifacts.getArtifacts(project, true, testProvidedArtifacts); + + List<File> providedJars = artifactSet.getJarArtifactsProvided().stream() + .map(Artifact::getFile) + .collect(toList()); + + List<Export> exportedPackagesFromProvidedJars = exportedPackagesAggregated(providedJars); + + PackageTally projectPackages = getProjectMainAndTestClassesTally(); + + PackageTally jarArtifactsToInclude = definedPackages(artifactSet.getJarArtifactsToInclude()); + + PackageTally includedPackages = projectPackages.combine(jarArtifactsToInclude); + + Map<String, ImportPackages.Import> calculatedImports = calculateImports(includedPackages.referencedPackages(), + includedPackages.definedPackages(), + exportsByPackageName(exportedPackagesFromProvidedJars)); + + Map<String, String> manifestContent = generateManifestContent(artifactSet.getJarArtifactsToInclude(), calculatedImports, includedPackages); + addAdditionalManifestProperties(manifestContent); + createManifestFile(outputDirectory(project), manifestContent); + + } catch (Exception e) { + throw new MojoExecutionException("Failed generating osgi manifest", e); + } + } + + private void addAdditionalManifestProperties(Map<String, String> manifestContent) { + manifestContent.put("X-JDisc-Test-Bundle-Version", "1.0"); + } + + private PackageTally getProjectMainAndTestClassesTally() { + List<ClassFileMetaData> analyzedClasses = + Stream.concat( + allDescendantFiles(new File(project.getBuild().getOutputDirectory())), + allDescendantFiles(new File(project.getBuild().getTestOutputDirectory()))) + .filter(file -> file.getName().endsWith(".class")) + .map(classFile -> Analyze.analyzeClass(classFile, null)) + .collect(toList()); + return PackageTally.fromAnalyzedClassFiles(analyzedClasses); + } + +} diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/TestBundleUtils.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/TestBundleUtils.java new file mode 100644 index 00000000000..9a3fc89bbd5 --- /dev/null +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/TestBundleUtils.java @@ -0,0 +1,25 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.plugin.mojo; + +import org.apache.maven.project.MavenProject; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.jar.JarFile; + +/** + * @author bjorncs + */ +class TestBundleUtils { + private TestBundleUtils() {} + + static Path outputDirectory(MavenProject project) { return targetDirectory(project).resolve("test-bundle/"); } + + static Path manifestFile(MavenProject project) { return outputDirectory(project).resolve(JarFile.MANIFEST_NAME); } + + static Path archiveFile(MavenProject project) { + return targetDirectory(project).resolve(project.getBuild().getFinalName() + "-tests.jar"); + } + + private static Path targetDirectory(MavenProject project) { return Paths.get(project.getBuild().getDirectory()); } +} diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/TestProvidedArtifacts.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/TestProvidedArtifacts.java new file mode 100644 index 00000000000..6f9e305f8d9 --- /dev/null +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/TestProvidedArtifacts.java @@ -0,0 +1,73 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.plugin.mojo; + +import org.apache.maven.artifact.Artifact; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Stream; + +import static java.util.stream.Collectors.toList; + +/** + * Determines the test dependencies that are provided by the Vespa/JDisc test runtime based on the resolved dependency graph and a config string. + * "Test provided" dependencies are treated as "provided" scope dependencies when building a test bundle. + * + * @author bjorncs + */ +class TestProvidedArtifacts { + + private final List<Artifact> artifacts; + + private TestProvidedArtifacts(List<Artifact> artifacts) { this.artifacts = artifacts; } + + boolean isTestProvided(Artifact artifact) { return artifacts.contains(artifact); } + + static TestProvidedArtifacts from(Map<String, Artifact> artifacts, String configString) { + if (configString == null || configString.isBlank()) return new TestProvidedArtifacts(List.of()); + return new TestProvidedArtifacts(getTestProvidedArtifacts(artifacts, configString)); + } + + private static List<Artifact> getTestProvidedArtifacts(Map<String, Artifact> artifacts, String configString) { + List<String> testProvidedArtifactStringIds = toTestProvidedArtifactStringIds(configString); + List<Artifact> testProvidedArtifacts = new ArrayList<>(); + for (Artifact artifact : artifacts.values()) { + boolean hasTestProvidedArtifactAsParent = + dependencyTrail(artifact, artifacts) + .anyMatch(parent -> testProvidedArtifactStringIds.contains(toArtifactStringId(parent))); + boolean isBlacklisted = testProvidedArtifactStringIds.contains(toBlacklistedArtifactStringId(artifact)); + if (hasTestProvidedArtifactAsParent && !isBlacklisted) { + testProvidedArtifacts.add(artifact); + } + } + return testProvidedArtifacts; + } + + private static List<String> toTestProvidedArtifactStringIds(String commaSeparatedString) { + if (commaSeparatedString == null || commaSeparatedString.isBlank()) return List.of(); + return Arrays.stream(commaSeparatedString.split(",")) + .map(String::strip) + .filter(s -> !s.isBlank()) + .collect(toList()); + } + + private static Stream<Artifact> dependencyTrail(Artifact artifact, Map<String, Artifact> otherArtifacts) { + return artifact.getDependencyTrail().stream() + .map(parentId -> otherArtifacts.get(stripVersionAndScope(parentId))) + .filter(Objects::nonNull); + } + + private static String stripVersionAndScope(String fullArtifactIdentifier) { + int firstDelimiter = fullArtifactIdentifier.indexOf(':'); + int secondDelimiter = fullArtifactIdentifier.indexOf(':', firstDelimiter + 1); + return fullArtifactIdentifier.substring(0, secondDelimiter); + } + + private static String toArtifactStringId(Artifact artifact) { return artifact.getGroupId() + ":" + artifact.getArtifactId(); } + + private static String toBlacklistedArtifactStringId(Artifact artifact) { return "!" + toArtifactStringId(artifact); } + +} diff --git a/bundle-plugin/src/test/java/com/yahoo/container/plugin/mojo/TestProvidedArtifactsTest.java b/bundle-plugin/src/test/java/com/yahoo/container/plugin/mojo/TestProvidedArtifactsTest.java new file mode 100644 index 00000000000..b60bce794f5 --- /dev/null +++ b/bundle-plugin/src/test/java/com/yahoo/container/plugin/mojo/TestProvidedArtifactsTest.java @@ -0,0 +1,68 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.plugin.mojo; + + +import org.apache.maven.artifact.Artifact; +import org.apache.maven.artifact.DefaultArtifact; +import org.apache.maven.artifact.handler.DefaultArtifactHandler; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * @author bjorncs + */ +public class TestProvidedArtifactsTest { + + private static final String GROUP_ID = "com.test"; + + @Test + public void findsAllTestProvidedDependencies() { + Map<String, Artifact> artifacts = new TreeMap<>(); + Artifact a = createArtifact(artifacts, "a"); + Artifact aa = createArtifact(artifacts, "a-a", "a"); + Artifact ab = createArtifact(artifacts, "a-b", "a"); + Artifact aaa = createArtifact(artifacts, "a-a-a", "a", "a-a"); + Artifact b = createArtifact(artifacts, "b"); + Artifact ba = createArtifact(artifacts, "b-a", "b"); + Artifact c = createArtifact(artifacts, "c"); + + String configString = "com.test:a,com.test:b-a,!com.test:a-b"; + TestProvidedArtifacts testProvidedArtifacts = TestProvidedArtifacts.from(artifacts, configString); + + assertTrue(testProvidedArtifacts.isTestProvided(a)); + assertTrue(testProvidedArtifacts.isTestProvided(aa)); + assertFalse(testProvidedArtifacts.isTestProvided(ab)); + assertTrue(testProvidedArtifacts.isTestProvided(aaa)); + assertFalse(testProvidedArtifacts.isTestProvided(b)); + assertTrue(testProvidedArtifacts.isTestProvided(ba)); + assertFalse(testProvidedArtifacts.isTestProvided(c)); + } + + private static Artifact createArtifact(Map<String, Artifact> artifacts, String artifactId, String... dependents) { + Artifact artifact = createArtifact(artifactId, dependents); + artifacts.put(simpleId(artifactId), artifact); + return artifact; + } + + private static Artifact createArtifact(String artifactId, String... dependents) { + Artifact artifact = new DefaultArtifact(GROUP_ID, artifactId, "1.0", "test", "jar", "deploy", new DefaultArtifactHandler("jar")); + List<String> dependencyTrail = new ArrayList<>(); + dependencyTrail.add(fullId("bundle-plugin")); + Arrays.stream(dependents).forEach(dependent -> dependencyTrail.add(fullId(dependent))); + dependencyTrail.add(fullId(artifactId)); + artifact.setDependencyTrail(dependencyTrail); + return artifact; + } + + private static String fullId(String artifactId) { return simpleId(artifactId) + ":1.0:compile"; } + private static String simpleId(String artifactId) { return GROUP_ID + ":" + artifactId; } + +}
\ No newline at end of file diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ExactMatch.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ExactMatch.java index 51751b2e247..d5b22988e36 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ExactMatch.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ExactMatch.java @@ -58,14 +58,13 @@ public class ExactMatch extends Processor { field.addQueryCommand("word"); } else { // exact String exactTerminator = DEFAULT_EXACT_TERMINATOR; - if (field.getMatching().getExactMatchTerminator() != null && - ! field.getMatching().getExactMatchTerminator().equals("")) - { + if (field.getMatching().getExactMatchTerminator() != null + && ! field.getMatching().getExactMatchTerminator().equals("")) { exactTerminator = field.getMatching().getExactMatchTerminator(); } else { warn(search, field, - "With 'exact' matching, an exact-terminator is needed (using \"" - + exactTerminator +"\" as terminator)"); + "With 'exact' matching, an exact-terminator is needed " + + "(using '" + exactTerminator +"' as terminator)"); } field.addQueryCommand("exact " + exactTerminator); @@ -103,6 +102,7 @@ public class ExactMatch extends Processor { } return exp; } + } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java index 2110e6476b6..8979176b66c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java @@ -41,6 +41,8 @@ import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent; import java.io.File; import java.io.FilenameFilter; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import java.time.Clock; import java.time.Duration; import java.time.Instant; @@ -504,7 +506,7 @@ public class SessionRepository { long sessionId, Optional<Long> currentlyActiveSessionId, boolean internalRedeploy) throws IOException { File userApplicationDir = getSessionAppDir(sessionId); - IOUtils.copyDirectory(applicationFile, userApplicationDir); + copyApp(applicationFile, userApplicationDir); ApplicationPackage applicationPackage = createApplication(applicationFile, userApplicationDir, applicationId, @@ -515,6 +517,20 @@ public class SessionRepository { return applicationPackage; } + private void copyApp(File sourceDir, File destinationDir) throws IOException { + if (destinationDir.exists()) + throw new RuntimeException("Destination dir " + destinationDir + " already exists"); + if (! sourceDir.isDirectory()) + throw new IllegalArgumentException(sourceDir.getAbsolutePath() + " is not a directory"); + + // Copy app it atomically: Copy to default tmp dir and move to destination + java.nio.file.Path tempDestinationDir = Files.createTempDirectory(destinationDir.getParentFile().toPath(), "app-package"); + log.log(Level.FINE, "Copying dir " + sourceDir.getAbsolutePath() + " to " + tempDestinationDir.toFile().getAbsolutePath()); + IOUtils.copyDirectory(sourceDir, tempDestinationDir.toFile()); + log.log(Level.FINE, "Moving " + tempDestinationDir + " to " + destinationDir.getAbsolutePath()); + Files.move(tempDestinationDir, destinationDir.toPath(), StandardCopyOption.ATOMIC_MOVE); + } + /** * Returns a new session instance for the given session id. */ diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/AliasTarget.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/AliasTarget.java index a5436bc3fc1..41723dbdea6 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/AliasTarget.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/AliasTarget.java @@ -57,7 +57,12 @@ public abstract class AliasTarget { /** Unpack target from given record data */ public static AliasTarget unpack(RecordData data) { - return LatencyAliasTarget.unpack(data); + String[] parts = data.asString().split("/"); + switch (parts[0]) { + case "latency": return LatencyAliasTarget.unpack(data); + case "weighted": return WeightedAliasTarget.unpack(data); + } + throw new IllegalArgumentException("Unknown alias type '" + parts[0] + "'"); } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/WeightedAliasTarget.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/WeightedAliasTarget.java new file mode 100644 index 00000000000..9d741cb2dbc --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/WeightedAliasTarget.java @@ -0,0 +1,64 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.integration.dns; + +import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.zone.ZoneId; + +import java.util.Objects; + +/** + * An implementation of {@link AliasTarget} where is requests are answered based on the weight assigned to the + * record, as a proportion of the total weight for all records having the same DNS name. + * + * The portion of received traffic is calculated as follows: (record weight / sum of the weights of all records). + * + * @author mpolden + */ +public class WeightedAliasTarget extends AliasTarget { + + private final long weight; + + public WeightedAliasTarget(HostName name, String dnsZone, ZoneId zone, long weight) { + super(name, dnsZone, zone.value()); + this.weight = weight; + } + + /** The weight of this target */ + public long weight() { + return weight; + } + + @Override + public RecordData pack() { + return RecordData.from("weighted/" + name().value() + "/" + dnsZone() + "/" + id() + "/" + weight); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + WeightedAliasTarget that = (WeightedAliasTarget) o; + return weight == that.weight; + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), weight); + } + + /** Unpack weighted alias from given record data */ + public static WeightedAliasTarget unpack(RecordData data) { + var parts = data.asString().split("/"); + if (parts.length != 5) { + throw new IllegalArgumentException("Expected data to be on format type/name/DNS-zone/zone-id/weight, " + + "but got " + data.asString()); + } + if (!"weighted".equals(parts[0])) { + throw new IllegalArgumentException("Unexpected type '" + parts[0] + "'"); + } + return new WeightedAliasTarget(HostName.from(parts[1]), parts[2], ZoneId.from(parts[3]), + Long.parseLong(parts[4])); + } + +} diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/dns/AliasTargetTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/dns/AliasTargetTest.java new file mode 100644 index 00000000000..7169222fe26 --- /dev/null +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/dns/AliasTargetTest.java @@ -0,0 +1,39 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.integration.dns; + +import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.zone.ZoneId; +import org.junit.Test; + +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +/** + * @author mpolden + */ +public class AliasTargetTest { + + @Test + public void packing() { + List<AliasTarget> tests = List.of( + new LatencyAliasTarget(HostName.from("foo.example.com"), "dns-zone-1", ZoneId.from("prod.us-north-1")), + new WeightedAliasTarget(HostName.from("bar.example.com"), "dns-zone-2", ZoneId.from("prod.us-north-2"), 50) + ); + for (var target : tests) { + AliasTarget unpacked = AliasTarget.unpack(target.pack()); + assertEquals(target, unpacked); + } + + List<RecordData> invalidData = List.of(RecordData.from(""), RecordData.from("foobar")); + for (var data : invalidData) { + try { + AliasTarget.unpack(data); + fail("Expected exception"); + } catch (IllegalArgumentException ignored) { + } + } + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java index c441188b1be..98169ba3196 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java @@ -264,6 +264,7 @@ public class RoutingController { private boolean canRouteDirectlyTo(DeploymentId deploymentId, Application application) { if (controller.system().isPublic()) return true; // Public always supports direct routing if (controller.system().isCd()) return true; // CD deploys directly so we cannot enforce all requirements below + if(deploymentId.zoneId().environment().isManuallyDeployed()) return true; // Manually deployed zones does not include any use cases where direct routing is not supported // Check Athenz service presence. The test framework uses this identity when sending requests to the // deployment's container(s). diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 95f8219b401..9e6eb9ca2e1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -625,6 +625,8 @@ public class ControllerTest { // Create application var context = tester.newDeploymentContext(); ZoneId zone = ZoneId.from("dev", "us-east-1"); + tester.controllerTester().zoneRegistry() + .setRoutingMethod(ZoneApiMock.from(zone), RoutingMethod.shared, RoutingMethod.sharedLayer4); // Deploy tester.controller().applications().deploy(context.instanceId(), zone, Optional.of(applicationPackage), DeployOptions.none()); @@ -633,6 +635,14 @@ public class ControllerTest { assertTrue("No job status added", context.instanceJobs().isEmpty()); assertEquals("DeploymentSpec is not persisted", DeploymentSpec.empty, context.application().deploymentSpec()); + + // Verify zone supports shared layer 4 and shared routing methods + Set<RoutingMethod> routingMethods = tester.controller().routing().endpointsOf(context.deploymentIdIn(zone)) + .asList() + .stream() + .map(Endpoint::routingMethod) + .collect(Collectors.toSet()); + assertEquals(routingMethods, Set.of(RoutingMethod.shared, RoutingMethod.sharedLayer4)); } @Test diff --git a/document/src/main/java/com/yahoo/document/annotation/SpanTree.java b/document/src/main/java/com/yahoo/document/annotation/SpanTree.java index 05e5ff41cf1..9df45ab6c2e 100644 --- a/document/src/main/java/com/yahoo/document/annotation/SpanTree.java +++ b/document/src/main/java/com/yahoo/document/annotation/SpanTree.java @@ -25,7 +25,7 @@ import java.util.Map; * A SpanTree holds a root node of a tree of SpanNodes, and a List of Annotations pointing to these nodes * or each other. It also has a name. * - * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a> + * @author Einar M R Rosenvinge * @see com.yahoo.document.annotation.SpanNode * @see com.yahoo.document.annotation.Annotation */ @@ -39,8 +39,7 @@ public class SpanTree implements Iterable<Annotation>, SpanNodeParent, Comparabl /** * WARNING! Only to be used by deserializers! Creates an empty SpanTree instance. */ - public SpanTree() { - } + public SpanTree() { } /** * Creates a new SpanTree with a given root node. @@ -227,18 +226,12 @@ public class SpanTree implements Iterable<Annotation>, SpanNodeParent, Comparabl root.setParent(this); } - /** - * Returns the name of this span tree. - * @return the name of this span tree. - */ + /** Returns the name of this span tree. */ public String getName() { return name; } - /** - * Returns the root node of this span tree. - * @return the root node of this span tree. - */ + /** Returns the root node of this span tree. */ public SpanNode getRoot() { return root; } diff --git a/eval/src/vespa/eval/tensor/dense/typed_cells.h b/eval/src/vespa/eval/tensor/dense/typed_cells.h index 0f22c85735e..d1d2baa535e 100644 --- a/eval/src/vespa/eval/tensor/dense/typed_cells.h +++ b/eval/src/vespa/eval/tensor/dense/typed_cells.h @@ -48,15 +48,6 @@ struct TypedCells { }; template <typename TGT, typename... Args> -decltype(auto) dispatch_0(CellType ct, Args &&...args) { - switch (ct) { - case CellType::DOUBLE: return TGT::template call<double>(std::forward<Args>(args)...); - case CellType::FLOAT: return TGT::template call<float>(std::forward<Args>(args)...); - } - abort(); -} - -template <typename TGT, typename... Args> decltype(auto) dispatch_1(const TypedCells &a, Args &&...args) { switch (a.type) { case CellType::DOUBLE: return TGT::call(a.unsafe_typify<double>(), std::forward<Args>(args)...); @@ -74,22 +65,4 @@ decltype(auto) dispatch_2(A1 &&a, const TypedCells &b, Args &&...args) { abort(); } -template <typename T, typename... Args> -decltype(auto) select_1(CellType a_type) { - switch(a_type) { - case CellType::DOUBLE: return T::template get_fun<double, Args...>(); - case CellType::FLOAT: return T::template get_fun<float, Args...>(); - } - abort(); -} - -template <typename T> -decltype(auto) select_2(CellType a_type, CellType b_type) { - switch(b_type) { - case CellType::DOUBLE: return select_1<T, double>(a_type); - case CellType::FLOAT: return select_1<T, float>(a_type); - } - abort(); -} - } // namespace diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExactExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExactExpression.java index 14b5af53b5a..31633cdc88b 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExactExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExactExpression.java @@ -27,24 +27,30 @@ public final class ExactExpression extends Expression { @Override protected void doExecute(ExecutionContext ctx) { StringFieldValue input = (StringFieldValue)ctx.getValue(); - if (input.getString().isEmpty()) { - return; - } + if (input.getString().isEmpty()) return; + StringFieldValue output = input.clone(); ctx.setValue(output); String prev = output.getString(); String next = toLowerCase(prev); - SpanList root = new SpanList(); - SpanTree tree = new SpanTree(SpanTrees.LINGUISTICS, root); + SpanTree tree = output.getSpanTree(SpanTrees.LINGUISTICS); + SpanList root; + if (tree == null) { + root = new SpanList(); + tree = new SpanTree(SpanTrees.LINGUISTICS, root); + output.setSpanTree(tree); + } + else { + root = (SpanList)tree.getRoot(); + } SpanNode node = new Span(0, prev.length()); tree.annotate(node, new Annotation(AnnotationTypes.TERM, next.equals(prev) ? null : new StringFieldValue(next))); tree.annotate(node, new Annotation(AnnotationTypes.TOKEN_TYPE, new IntegerFieldValue(TokenType.ALPHABETIC.getValue()))); root.add(node); - output.setSpanTree(tree); } @Override diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/FlattenExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/FlattenExpression.java index e43744420f8..91f46381def 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/FlattenExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/FlattenExpression.java @@ -24,6 +24,7 @@ public final class FlattenExpression extends Expression { public FlattenExpression() { super(DataType.STRING); } + @Override protected void doExecute(ExecutionContext ctx) { StringFieldValue input = (StringFieldValue)ctx.getValue(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileFinder.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileFinder.java index 121cb244715..52900e35fe2 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileFinder.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileFinder.java @@ -1,6 +1,7 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.task.util.file; +import com.yahoo.lang.MutableInteger; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import java.io.IOException; @@ -110,17 +111,19 @@ public class FileFinder { * @return true iff anything was matched and deleted */ public boolean deleteRecursively(TaskContext context) { + final int maxNumberOfDeletedPathsToLog = 20; + MutableInteger numDeleted = new MutableInteger(0); List<Path> deletedPaths = new ArrayList<>(); try { forEach(attributes -> { if (attributes.unixPath().deleteRecursively()) { - deletedPaths.add(attributes.path()); + if (numDeleted.next() <= maxNumberOfDeletedPathsToLog) deletedPaths.add(attributes.path()); } }); } finally { - if (deletedPaths.size() > 20) { - context.log(logger, "Deleted " + deletedPaths.size() + " paths under " + basePath); + if (numDeleted.get() > maxNumberOfDeletedPathsToLog) { + context.log(logger, "Deleted " + numDeleted.get() + " paths under " + basePath); } else if (deletedPaths.size() > 0) { List<Path> paths = deletedPaths.stream() .map(basePath::relativize) diff --git a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp index 407b14c5f7d..abcf35051fc 100644 --- a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp @@ -1,6 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + #include <vespa/config-attributes.h> #include <vespa/fastos/file.h> +#include <vespa/searchcommon/attribute/i_attribute_functor.h> #include <vespa/searchcommon/attribute/iattributevector.h> #include <vespa/searchcore/proton/attribute/attribute_collection_spec_factory.h> #include <vespa/searchcore/proton/attribute/attribute_manager_initializer.h> @@ -9,24 +11,22 @@ #include <vespa/searchcore/proton/attribute/exclusive_attribute_read_accessor.h> #include <vespa/searchcore/proton/attribute/imported_attributes_repo.h> #include <vespa/searchcore/proton/attribute/sequential_attributes_initializer.h> -#include <vespa/searchcore/proton/flushengine/shrink_lid_space_flush_target.h> #include <vespa/searchcore/proton/common/hw_info.h> #include <vespa/searchcore/proton/documentmetastore/documentmetastorecontext.h> +#include <vespa/searchcore/proton/flushengine/shrink_lid_space_flush_target.h> #include <vespa/searchcore/proton/initializer/initializer_task.h> #include <vespa/searchcore/proton/initializer/task_runner.h> #include <vespa/searchcore/proton/server/executor_thread_service.h> #include <vespa/searchcore/proton/test/attribute_utils.h> #include <vespa/searchcore/proton/test/attribute_vectors.h> +#include <vespa/searchlib/attribute/attribute_read_guard.h> #include <vespa/searchlib/attribute/attributefactory.h> -#include <vespa/searchcommon/attribute/i_attribute_functor.h> #include <vespa/searchlib/attribute/attributevector.hpp> -#include <vespa/searchlib/attribute/attribute_read_guard.h> #include <vespa/searchlib/attribute/imported_attribute_vector.h> #include <vespa/searchlib/attribute/imported_attribute_vector_factory.h> #include <vespa/searchlib/attribute/predicate_attribute.h> #include <vespa/searchlib/attribute/reference_attribute.h> #include <vespa/searchlib/attribute/singlenumericattribute.hpp> -#include <vespa/vespalib/util/foregroundtaskexecutor.h> #include <vespa/searchlib/common/indexmetainfo.h> #include <vespa/searchlib/index/dummyfileheadercontext.h> #include <vespa/searchlib/predicate/predicate_index.h> @@ -34,6 +34,8 @@ #include <vespa/searchlib/test/directory_handler.h> #include <vespa/searchlib/test/mock_gid_to_lid_mapping.h> #include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/util/foreground_thread_executor.h> +#include <vespa/vespalib/util/foregroundtaskexecutor.h> #include <vespa/vespalib/util/threadstackexecutor.h> #include <vespa/log/log.h> @@ -53,6 +55,7 @@ using proton::test::AttributeUtils; using proton::test::createInt32Attribute; using proton::test::Int32Attribute; using vespalib::ForegroundTaskExecutor; +using vespalib::ForegroundThreadExecutor; using search::TuneFileAttributes; using search::attribute::BasicType; using search::attribute::IAttributeContext; @@ -152,15 +155,21 @@ struct BaseFixture DirectoryHandler _dirHandler; DummyFileHeaderContext _fileHeaderContext; ForegroundTaskExecutor _attributeFieldWriter; + ForegroundThreadExecutor _shared; HwInfo _hwInfo; BaseFixture(); ~BaseFixture(); + proton::AttributeManager::SP make_manager() { + return std::make_shared<proton::AttributeManager>(test_dir, "test.subdb", TuneFileAttributes(), + _fileHeaderContext, _attributeFieldWriter, _shared, _hwInfo); + } }; BaseFixture::BaseFixture() : _dirHandler(test_dir), _fileHeaderContext(), _attributeFieldWriter(), + _shared(), _hwInfo() { } @@ -185,8 +194,7 @@ struct AttributeManagerFixture }; AttributeManagerFixture::AttributeManagerFixture(BaseFixture &bf) - : _msp(std::make_shared<proton::AttributeManager>(test_dir, "test.subdb", TuneFileAttributes(), - bf._fileHeaderContext, bf._attributeFieldWriter, bf._hwInfo)), + : _msp(bf.make_manager()), _m(*_msp), _builder() {} @@ -503,11 +511,7 @@ TEST_F("require that new attributes after reconfig are initialized", Fixture) TEST_F("require that removed attributes cannot resurrect", BaseFixture) { - proton::AttributeManager::SP am1( - new proton::AttributeManager(test_dir, "test.subdb", - TuneFileAttributes(), - f._fileHeaderContext, - f._attributeFieldWriter, f._hwInfo)); + auto am1 = f.make_manager(); { AttributeVector::SP a1 = am1->addAttribute({"a1", INT32_SINGLE}, 0); fillAttribute(a1, 2, 10, 15); @@ -801,9 +805,7 @@ TEST_F("require that attribute vector of wrong type is dropped", BaseFixture) predicateParams2.setArity(4); predicate2.setPredicateParams(predicateParams2); - auto am1(std::make_shared<proton::AttributeManager> - (test_dir, "test.subdb", TuneFileAttributes(), - f._fileHeaderContext, f._attributeFieldWriter, f._hwInfo)); + auto am1 = f.make_manager(); am1->addAttribute({"a1", INT32_SINGLE}, 1); am1->addAttribute({"a2", INT32_SINGLE}, 2); am1->addAttribute({"a3", generic_tensor}, 3); @@ -840,17 +842,13 @@ void assertShrinkTargetSerial(proton::AttributeManager &mgr, const vespalib::str TEST_F("require that we can guess flushed serial number for shrink flushtarget", BaseFixture) { - auto am1(std::make_shared<proton::AttributeManager> - (test_dir, "test.subdb", TuneFileAttributes(), - f._fileHeaderContext, f._attributeFieldWriter, f._hwInfo)); + auto am1 = f.make_manager(); am1->addAttribute({"a1", INT32_SINGLE}, 1); am1->addAttribute({"a2", INT32_SINGLE}, 2); TEST_DO(assertShrinkTargetSerial(*am1, "a1", 0)); TEST_DO(assertShrinkTargetSerial(*am1, "a2", 1)); am1->flushAll(10); - am1 = std::make_shared<proton::AttributeManager> - (test_dir, "test.subdb", TuneFileAttributes(), - f._fileHeaderContext, f._attributeFieldWriter, f._hwInfo); + am1 = f.make_manager(); am1->addAttribute({"a1", INT32_SINGLE}, 1); am1->addAttribute({"a2", INT32_SINGLE}, 2); TEST_DO(assertShrinkTargetSerial(*am1, "a1", 10)); @@ -859,9 +857,7 @@ TEST_F("require that we can guess flushed serial number for shrink flushtarget", TEST_F("require that shrink flushtarget is handed over to new attribute manager", BaseFixture) { - auto am1(std::make_shared<proton::AttributeManager> - (test_dir, "test.subdb", TuneFileAttributes(), - f._fileHeaderContext, f._attributeFieldWriter, f._hwInfo)); + auto am1 = f.make_manager(); am1->addAttribute({"a1", INT32_SINGLE}, 4); AttrSpecList newSpec; newSpec.push_back(AttributeSpec("a1", INT32_SINGLE)); diff --git a/searchcore/src/tests/proton/attribute/attribute_populator/attribute_populator_test.cpp b/searchcore/src/tests/proton/attribute/attribute_populator/attribute_populator_test.cpp index 788aa4fb1ee..f81644c3f55 100644 --- a/searchcore/src/tests/proton/attribute/attribute_populator/attribute_populator_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_populator/attribute_populator_test.cpp @@ -1,22 +1,25 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("attribute_populator_test"); -#include <vespa/vespalib/testkit/testapp.h> -#include <vespa/document/repo/configbuilder.h> #include <vespa/document/fieldvalue/intfieldvalue.h> +#include <vespa/document/repo/configbuilder.h> #include <vespa/searchcore/proton/attribute/attribute_populator.h> #include <vespa/searchcore/proton/attribute/attributemanager.h> #include <vespa/searchcore/proton/common/hw_info.h> #include <vespa/searchcore/proton/test/test.h> -#include <vespa/vespalib/util/foregroundtaskexecutor.h> #include <vespa/searchlib/index/dummyfileheadercontext.h> #include <vespa/searchlib/test/directory_handler.h> +#include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/util/foreground_thread_executor.h> +#include <vespa/vespalib/util/foregroundtaskexecutor.h> #include <vespa/vespalib/util/stringfmt.h> +#include <vespa/log/log.h> +LOG_SETUP("attribute_populator_test"); + using document::config_builder::DocumenttypesConfigBuilderHelper; using document::config_builder::Struct; using vespalib::ForegroundTaskExecutor; +using vespalib::ForegroundThreadExecutor; using namespace document; using namespace proton; using namespace search; @@ -62,6 +65,7 @@ struct Fixture DirectoryHandler _testDir; DummyFileHeaderContext _fileHeader; ForegroundTaskExecutor _attributeFieldWriter; + ForegroundThreadExecutor _shared; HwInfo _hwInfo; AttributeManager::SP _mgr; std::unique_ptr<AttributePopulator> _pop; @@ -70,10 +74,10 @@ struct Fixture : _testDir(TEST_DIR), _fileHeader(), _attributeFieldWriter(), + _shared(), _hwInfo(), - _mgr(new AttributeManager(TEST_DIR, "test.subdb", - TuneFileAttributes(), - _fileHeader, _attributeFieldWriter, _hwInfo)), + _mgr(new AttributeManager(TEST_DIR, "test.subdb", TuneFileAttributes(), + _fileHeader, _attributeFieldWriter, _shared, _hwInfo)), _pop(), _ctx() { diff --git a/searchcore/src/tests/proton/attribute/attribute_test.cpp b/searchcore/src/tests/proton/attribute/attribute_test.cpp index dabab649497..6a12d2daa20 100644 --- a/searchcore/src/tests/proton/attribute/attribute_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_test.cpp @@ -42,6 +42,7 @@ #include <vespa/vespalib/io/fileutil.h> #include <vespa/vespalib/test/insertion_operators.h> #include <vespa/vespalib/util/exceptions.h> +#include <vespa/vespalib/util/foreground_thread_executor.h> #include <vespa/vespalib/util/foregroundtaskexecutor.h> #include <vespa/vespalib/util/sequencedtaskexecutorobserver.h> @@ -78,6 +79,7 @@ using search::tensor::TensorAttribute; using search::test::DirectoryHandler; using std::string; using vespalib::ForegroundTaskExecutor; +using vespalib::ForegroundThreadExecutor; using vespalib::SequencedTaskExecutorObserver; using vespalib::eval::TensorSpec; using vespalib::eval::ValueType; @@ -121,12 +123,12 @@ fillAttribute(const AttributeVector::SP &attr, uint32_t from, uint32_t to, int64 const std::shared_ptr<IDestructorCallback> emptyCallback; - class AttributeWriterTest : public ::testing::Test { public: DirectoryHandler _dirHandler; std::unique_ptr<ForegroundTaskExecutor> _attributeFieldWriterReal; std::unique_ptr<SequencedTaskExecutorObserver> _attributeFieldWriter; + ForegroundThreadExecutor _shared; std::shared_ptr<MockAttributeManager> _mgr; std::unique_ptr<AttributeWriter> _aw; @@ -134,6 +136,7 @@ public: : _dirHandler(test_dir), _attributeFieldWriterReal(), _attributeFieldWriter(), + _shared(), _mgr(), _aw() { @@ -146,6 +149,7 @@ public: _attributeFieldWriter = std::make_unique<SequencedTaskExecutorObserver>(*_attributeFieldWriterReal); _mgr = std::make_shared<MockAttributeManager>(); _mgr->set_writer(*_attributeFieldWriter); + _mgr->set_shared_executor(_shared); allocAttributeWriter(); } void allocAttributeWriter() { @@ -573,6 +577,7 @@ public: DirectoryHandler _dirHandler; DummyFileHeaderContext _fileHeaderContext; ForegroundTaskExecutor _attributeFieldWriter; + ForegroundThreadExecutor _shared; HwInfo _hwInfo; proton::AttributeManager::SP _baseMgr; FilterAttributeManager _filterMgr; @@ -581,11 +586,13 @@ public: : _dirHandler(test_dir), _fileHeaderContext(), _attributeFieldWriter(), + _shared(), _hwInfo(), _baseMgr(new proton::AttributeManager(test_dir, "test.subdb", TuneFileAttributes(), _fileHeaderContext, _attributeFieldWriter, + _shared, _hwInfo)), _filterMgr(ACCEPTED_ATTRIBUTES, _baseMgr) { @@ -887,6 +894,11 @@ public: startAttributeField("a1"). addTensor(std::unique_ptr<vespalib::tensor::Tensor>()).endField().endDocument(); } + void expect_shared_executor_tasks(size_t exp_accepted_tasks) { + auto stats = _shared.getStats(); + EXPECT_EQ(exp_accepted_tasks, stats.acceptedTasks); + EXPECT_EQ(0, stats.rejectedTasks); + } }; TEST_F(TwoPhasePutTest, handles_put_in_two_phases_when_specified_for_tensor_attribute) @@ -895,16 +907,13 @@ TEST_F(TwoPhasePutTest, handles_put_in_two_phases_when_specified_for_tensor_attr put(1, *doc, 1); expect_tensor_attr_calls(1, 1); - assertExecuteHistory({1, 0}); + expect_shared_executor_tasks(1); + assertExecuteHistory({0}); put(2, *doc, 2); expect_tensor_attr_calls(2, 2); - assertExecuteHistory({1, 0, 0, 0}); - - put(3, *doc, 3); - expect_tensor_attr_calls(3, 3); - // Note that the prepare step is executed round-robin between the 2 threads. - assertExecuteHistory({1, 0, 0, 0, 1, 0}); + expect_shared_executor_tasks(2); + assertExecuteHistory({0, 0}); } TEST_F(TwoPhasePutTest, put_is_ignored_when_serial_number_is_older_or_equal_to_attribute) @@ -913,7 +922,8 @@ TEST_F(TwoPhasePutTest, put_is_ignored_when_serial_number_is_older_or_equal_to_a attr->commit(7, 7); put(7, *doc, 1); expect_tensor_attr_calls(0, 0); - assertExecuteHistory({1, 0}); + expect_shared_executor_tasks(1); + assertExecuteHistory({0}); } TEST_F(TwoPhasePutTest, document_is_cleared_if_field_is_not_set) @@ -921,7 +931,8 @@ TEST_F(TwoPhasePutTest, document_is_cleared_if_field_is_not_set) auto doc = make_no_field_doc(); put(1, *doc, 1); expect_tensor_attr_calls(0, 0, 1); - assertExecuteHistory({1, 0}); + expect_shared_executor_tasks(1); + assertExecuteHistory({0}); } TEST_F(TwoPhasePutTest, document_is_cleared_if_tensor_in_field_is_not_set) @@ -929,7 +940,8 @@ TEST_F(TwoPhasePutTest, document_is_cleared_if_tensor_in_field_is_not_set) auto doc = make_no_tensor_doc(); put(1, *doc, 1); expect_tensor_attr_calls(0, 0, 1); - assertExecuteHistory({1, 0}); + expect_shared_executor_tasks(1); + assertExecuteHistory({0}); } diff --git a/searchcore/src/tests/proton/attribute/attributeflush_test.cpp b/searchcore/src/tests/proton/attribute/attributeflush_test.cpp index 4604129248b..adf2f4a7d7d 100644 --- a/searchcore/src/tests/proton/attribute/attributeflush_test.cpp +++ b/searchcore/src/tests/proton/attribute/attributeflush_test.cpp @@ -8,13 +8,14 @@ #include <vespa/searchcore/proton/flushengine/shrink_lid_space_flush_target.h> #include <vespa/searchlib/attribute/attributefactory.h> #include <vespa/searchlib/attribute/integerbase.h> -#include <vespa/vespalib/util/foregroundtaskexecutor.h> #include <vespa/searchlib/common/indexmetainfo.h> #include <vespa/searchlib/index/dummyfileheadercontext.h> #include <vespa/searchlib/test/directory_handler.h> #include <vespa/vespalib/datastore/datastorebase.h> #include <vespa/vespalib/io/fileutil.h> #include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/util/foreground_thread_executor.h> +#include <vespa/vespalib/util/foregroundtaskexecutor.h> #include <vespa/vespalib/util/threadstackexecutor.h> #include <vespa/log/log.h> @@ -246,6 +247,7 @@ struct BaseFixture test::DirectoryHandler _dirHandler; DummyFileHeaderContext _fileHeaderContext; ForegroundTaskExecutor _attributeFieldWriter; + ForegroundThreadExecutor _shared; HwInfo _hwInfo; BaseFixture(); BaseFixture(const HwInfo &hwInfo); @@ -256,12 +258,14 @@ BaseFixture::BaseFixture() : _dirHandler(test_dir), _fileHeaderContext(), _attributeFieldWriter(), + _shared(), _hwInfo() { } BaseFixture::BaseFixture(const HwInfo &hwInfo) : _dirHandler(test_dir), _fileHeaderContext(), _attributeFieldWriter(), + _shared(), _hwInfo(hwInfo) {} BaseFixture::~BaseFixture() = default; @@ -289,7 +293,7 @@ struct AttributeManagerFixture AttributeManagerFixture::AttributeManagerFixture(BaseFixture &bf) : _msp(std::make_shared<AttributeManager>(test_dir, "test.subdb", TuneFileAttributes(), - bf._fileHeaderContext, bf._attributeFieldWriter, bf._hwInfo)), + bf._fileHeaderContext, bf._attributeFieldWriter, bf._shared, bf._hwInfo)), _m(*_msp) {} AttributeManagerFixture::~AttributeManagerFixture() = default; diff --git a/searchcore/src/tests/proton/attribute/attributes_state_explorer/attributes_state_explorer_test.cpp b/searchcore/src/tests/proton/attribute/attributes_state_explorer/attributes_state_explorer_test.cpp index 01452f1361e..094ae1a1400 100644 --- a/searchcore/src/tests/proton/attribute/attributes_state_explorer/attributes_state_explorer_test.cpp +++ b/searchcore/src/tests/proton/attribute/attributes_state_explorer/attributes_state_explorer_test.cpp @@ -1,16 +1,17 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchcore/proton/attribute/attribute_manager_explorer.h> #include <vespa/searchcore/proton/attribute/attributemanager.h> #include <vespa/searchcore/proton/common/hw_info.h> -#include <vespa/searchcore/proton/test/attribute_vectors.h> #include <vespa/searchcore/proton/test/attribute_utils.h> -#include <vespa/vespalib/util/foregroundtaskexecutor.h> +#include <vespa/searchcore/proton/test/attribute_vectors.h> +#include <vespa/searchlib/attribute/singlenumericattribute.hpp> #include <vespa/searchlib/index/dummyfileheadercontext.h> #include <vespa/searchlib/test/directory_handler.h> #include <vespa/vespalib/test/insertion_operators.h> -#include <vespa/searchlib/attribute/singlenumericattribute.hpp> +#include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/util/foreground_thread_executor.h> +#include <vespa/vespalib/util/foregroundtaskexecutor.h> #include <vespa/log/log.h> LOG_SETUP("attributes_state_explorer_test"); @@ -19,6 +20,7 @@ using namespace proton; using namespace proton::test; using search::AttributeVector; using vespalib::ForegroundTaskExecutor; +using vespalib::ForegroundThreadExecutor; using search::TuneFileAttributes; using search::index::DummyFileHeaderContext; using search::test::DirectoryHandler; @@ -30,6 +32,7 @@ struct Fixture DirectoryHandler _dirHandler; DummyFileHeaderContext _fileHeaderContext; ForegroundTaskExecutor _attributeFieldWriter; + ForegroundThreadExecutor _shared; HwInfo _hwInfo; AttributeManager::SP _mgr; AttributeManagerExplorer _explorer; @@ -37,10 +40,12 @@ struct Fixture : _dirHandler(TEST_DIR), _fileHeaderContext(), _attributeFieldWriter(), + _shared(), _hwInfo(), _mgr(new AttributeManager(TEST_DIR, "test.subdb", TuneFileAttributes(), _fileHeaderContext, _attributeFieldWriter, + _shared, _hwInfo)), _explorer(_mgr) { diff --git a/searchcore/src/tests/proton/attribute/exclusive_attribute_read_accessor/exclusive_attribute_read_accessor_test.cpp b/searchcore/src/tests/proton/attribute/exclusive_attribute_read_accessor/exclusive_attribute_read_accessor_test.cpp index 6f91b1a6f6f..19af6a699c4 100644 --- a/searchcore/src/tests/proton/attribute/exclusive_attribute_read_accessor/exclusive_attribute_read_accessor_test.cpp +++ b/searchcore/src/tests/proton/attribute/exclusive_attribute_read_accessor/exclusive_attribute_read_accessor_test.cpp @@ -38,7 +38,7 @@ TEST_F("require that attribute write thread is blocked while guard is held", Fix { ReadGuard::UP guard = f.accessor.takeGuard(); Gate gate; - f.writer->execute(f.writer->getExecutorId(f.attribute->getNamePrefix()), [&gate]() { gate.countDown(); }); + f.writer->execute(f.writer->getExecutorIdFromName(f.attribute->getNamePrefix()), [&gate]() { gate.countDown(); }); bool reachedZero = gate.await(100); EXPECT_FALSE(reachedZero); EXPECT_EQUAL(1u, gate.getCount()); diff --git a/searchcore/src/tests/proton/docsummary/docsummary.cpp b/searchcore/src/tests/proton/docsummary/docsummary.cpp index b6d6d2437d8..7d27c3b21f4 100644 --- a/searchcore/src/tests/proton/docsummary/docsummary.cpp +++ b/searchcore/src/tests/proton/docsummary/docsummary.cpp @@ -791,7 +791,7 @@ Test::requireThatAttributesAreUsed() search::AttributeVector *bjAttr = attributeManager->getWritableAttribute("bj"); auto bjTensorAttr = dynamic_cast<search::tensor::TensorAttribute *>(bjAttr); - attributeFieldWriter.execute(attributeFieldWriter.getExecutorId(bjAttr->getNamePrefix()), + attributeFieldWriter.execute(attributeFieldWriter.getExecutorIdFromName(bjAttr->getNamePrefix()), [&]() { bjTensorAttr->setTensor(3, *make_tensor(TensorSpec("tensor(x{},y{})") .add({{"x", "a"}, {"y", "b"}}, 4))); diff --git a/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp b/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp index 89930a6b1a3..15f7d5798ea 100644 --- a/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp +++ b/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp @@ -192,7 +192,7 @@ Fixture::initViewSet(ViewSet &views) views._reconfigurer, views._writeService, _summaryExecutor, TuneFileIndexManager(), TuneFileAttributes(), views._fileHeaderContext); auto attrMgr = make_shared<AttributeManager>(BASE_DIR, "test.subdb", TuneFileAttributes(), views._fileHeaderContext, - views._writeService.attributeFieldWriter(),views._hwInfo); + views._writeService.attributeFieldWriter(), views._writeService.shared(), views._hwInfo); auto summaryMgr = make_shared<SummaryManager> (_summaryExecutor, search::LogDocumentStore::Config(), search::GrowStrategy(), BASE_DIR, views._docTypeName, TuneFileSummary(), views._fileHeaderContext,views._noTlSyncer, search::IBucketizer::SP()); @@ -273,7 +273,7 @@ struct MyFastAccessFeedView _writeService, *_lidReuseDelayer, _commitTimeTracker); StoreOnlyFeedView::PersistentParams params(1, 1, DocTypeName(DOC_TYPE), 0, SubDbType::NOTREADY); auto mgr = make_shared<AttributeManager>(BASE_DIR, "test.subdb", TuneFileAttributes(), _fileHeaderContext, - _writeService.attributeFieldWriter(), _hwInfo); + _writeService.attributeFieldWriter(), _writeService.shared(), _hwInfo); IAttributeWriter::SP writer(new AttributeWriter(mgr)); FastAccessFeedView::Context fastUpdateCtx(writer, _docIdLimit); _feedView.set(FastAccessFeedView::SP(new FastAccessFeedView(storeOnlyCtx, params, fastUpdateCtx)));; diff --git a/searchcore/src/tests/proton/reprocessing/attribute_reprocessing_initializer/attribute_reprocessing_initializer_test.cpp b/searchcore/src/tests/proton/reprocessing/attribute_reprocessing_initializer/attribute_reprocessing_initializer_test.cpp index 898825016b3..21bb011901e 100644 --- a/searchcore/src/tests/proton/reprocessing/attribute_reprocessing_initializer/attribute_reprocessing_initializer_test.cpp +++ b/searchcore/src/tests/proton/reprocessing/attribute_reprocessing_initializer/attribute_reprocessing_initializer_test.cpp @@ -11,11 +11,12 @@ #include <vespa/searchcore/proton/reprocessing/i_reprocessing_handler.h> #include <vespa/searchcore/proton/test/attribute_utils.h> #include <vespa/searchlib/attribute/attributefactory.h> -#include <vespa/vespalib/util/foregroundtaskexecutor.h> #include <vespa/searchlib/index/dummyfileheadercontext.h> #include <vespa/searchlib/test/directory_handler.h> #include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/test/insertion_operators.h> +#include <vespa/vespalib/util/foreground_thread_executor.h> +#include <vespa/vespalib/util/foregroundtaskexecutor.h> #include <vespa/log/log.h> LOG_SETUP("attribute_reprocessing_initializer_test"); @@ -30,6 +31,7 @@ using search::attribute::Config; using search::index::schema::DataType; using search::test::DirectoryHandler; using vespalib::ForegroundTaskExecutor; +using vespalib::ForegroundThreadExecutor; const vespalib::string TEST_DIR = "test_output"; const SerialNum INIT_SERIAL_NUM = 10; @@ -54,6 +56,7 @@ struct MyConfig { DummyFileHeaderContext _fileHeaderContext; ForegroundTaskExecutor _attributeFieldWriter; + ForegroundThreadExecutor _shared; HwInfo _hwInfo; AttributeManager::SP _mgr; search::index::Schema _schema; @@ -87,10 +90,10 @@ struct MyConfig MyConfig::MyConfig() : _fileHeaderContext(), _attributeFieldWriter(), + _shared(), _hwInfo(), _mgr(new AttributeManager(TEST_DIR, "test.subdb", TuneFileAttributes(), - _fileHeaderContext, - _attributeFieldWriter, _hwInfo)), + _fileHeaderContext, _attributeFieldWriter, _shared, _hwInfo)), _schema() {} MyConfig::~MyConfig() = default; @@ -132,6 +135,7 @@ public: DirectoryHandler _dirHandler; DummyFileHeaderContext _fileHeaderContext; ForegroundTaskExecutor _attributeFieldWriter; + ForegroundThreadExecutor _shared; HwInfo _hwInfo; AttributeManager::SP _mgr; MyConfig _oldCfg; @@ -143,10 +147,10 @@ public: : _dirHandler(TEST_DIR), _fileHeaderContext(), _attributeFieldWriter(), + _shared(), _hwInfo(), - _mgr(new AttributeManager(TEST_DIR, "test.subdb", TuneFileAttributes(), - _fileHeaderContext, - _attributeFieldWriter, _hwInfo)), + _mgr(new AttributeManager(TEST_DIR, "test.subdb", TuneFileAttributes(), _fileHeaderContext, + _attributeFieldWriter, _shared, _hwInfo)), _oldCfg(), _newCfg(), _inspector(_oldCfg, _newCfg), diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp index 9b54ae816e0..a49b27caf36 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp @@ -12,9 +12,10 @@ #include <vespa/searchcore/proton/common/attribute_updater.h> #include <vespa/searchlib/attribute/attributevector.hpp> #include <vespa/searchlib/attribute/imported_attribute_vector.h> -#include <vespa/searchlib/tensor/prepare_result.h> #include <vespa/searchlib/common/idestructorcallback.h> +#include <vespa/searchlib/tensor/prepare_result.h> #include <vespa/vespalib/stllike/hash_map.hpp> +#include <vespa/vespalib/util/threadexecutor.h> #include <future> #include <vespa/log/log.h> @@ -281,7 +282,7 @@ public: FieldContext::FieldContext(ISequencedTaskExecutor &writer, AttributeVector *attr) : _name(attr->getName()), - _executorId(writer.getExecutorId(attr->getNamePrefix())), + _executorId(writer.getExecutorIdFromName(attr->getNamePrefix())), _attr(attr), _use_two_phase_put(use_two_phase_put_for_attribute(*attr)) { @@ -607,8 +608,7 @@ AttributeWriter::internalPut(SerialNum serialNum, const Document &doc, DocumentI assert(wc.getFields().size() == 1); auto prepare_task = std::make_unique<PreparePutTask>(serialNum, lid, wc.getFields()[0], extractor); auto complete_task = std::make_unique<CompletePutTask>(*prepare_task, immediateCommit, onWriteDone); - // We use the local docid to create an executor id to round-robin between the threads. - _attributeFieldWriter.executeTask(_attributeFieldWriter.getExecutorId(lid), std::move(prepare_task)); + _shared_executor.execute(std::move(prepare_task)); _attributeFieldWriter.executeTask(wc.getExecutorId(), std::move(complete_task)); } else { if (allAttributes || wc.hasStructFieldAttribute()) { @@ -633,6 +633,7 @@ AttributeWriter::internalRemove(SerialNum serialNum, DocumentIdT lid, bool immed AttributeWriter::AttributeWriter(proton::IAttributeManager::SP mgr) : _mgr(std::move(mgr)), _attributeFieldWriter(_mgr->getAttributeFieldWriter()), + _shared_executor(_mgr->get_shared_executor()), _writeContexts(), _dataType(nullptr), _hasStructFieldAttribute(false), @@ -645,7 +646,7 @@ AttributeWriter::AttributeWriter(proton::IAttributeManager::SP mgr) void AttributeWriter::setupAttriuteMapping() { for (auto attr : getWritableAttributes()) { vespalib::stringref name = attr->getName(); - _attrMap[name] = AttrWithId(attr, _attributeFieldWriter.getExecutorId(attr->getNamePrefix())); + _attrMap[name] = AttrWithId(attr, _attributeFieldWriter.getExecutorIdFromName(attr->getNamePrefix())); } } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.h index 726379220e3..eaf8abe4872 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.h @@ -26,6 +26,7 @@ private: using FieldValue = document::FieldValue; const IAttributeManager::SP _mgr; vespalib::ISequencedTaskExecutor &_attributeFieldWriter; + vespalib::ThreadExecutor& _shared_executor; using ExecutorId = vespalib::ISequencedTaskExecutor::ExecutorId; public: /** diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp index 319ae2dcad1..504f6841daf 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp @@ -63,7 +63,9 @@ std::shared_ptr<ShrinkLidSpaceFlushTarget> allocShrinker(const AttributeVector:: using Type = IFlushTarget::Type; using Component = IFlushTarget::Component; - auto shrinkwrap = std::make_shared<ThreadedCompactableLidSpace>(attr, attributeFieldWriter, attributeFieldWriter.getExecutorId(attr->getNamePrefix())); + auto shrinkwrap = std::make_shared<ThreadedCompactableLidSpace>(attr, attributeFieldWriter, + attributeFieldWriter.getExecutorIdFromName( + attr->getNamePrefix())); const vespalib::string &name = attr->getName(); auto dir = diskLayout.createAttributeDir(name); search::SerialNum shrinkSerialNum = estimateShrinkSerialNum(*attr); @@ -223,6 +225,7 @@ AttributeManager::AttributeManager(const vespalib::string &baseDir, const TuneFileAttributes &tuneFileAttributes, const FileHeaderContext &fileHeaderContext, vespalib::ISequencedTaskExecutor &attributeFieldWriter, + vespalib::ThreadExecutor& shared_executor, const HwInfo &hwInfo) : proton::IAttributeManager(), _attributes(), @@ -235,17 +238,18 @@ AttributeManager::AttributeManager(const vespalib::string &baseDir, _factory(std::make_shared<AttributeFactory>()), _interlock(std::make_shared<search::attribute::Interlock>()), _attributeFieldWriter(attributeFieldWriter), + _shared_executor(shared_executor), _hwInfo(hwInfo), _importedAttributes() { } - AttributeManager::AttributeManager(const vespalib::string &baseDir, const vespalib::string &documentSubDbName, const search::TuneFileAttributes &tuneFileAttributes, const search::common::FileHeaderContext &fileHeaderContext, vespalib::ISequencedTaskExecutor &attributeFieldWriter, + vespalib::ThreadExecutor& shared_executor, const IAttributeFactory::SP &factory, const HwInfo &hwInfo) : proton::IAttributeManager(), @@ -259,6 +263,7 @@ AttributeManager::AttributeManager(const vespalib::string &baseDir, _factory(factory), _interlock(std::make_shared<search::attribute::Interlock>()), _attributeFieldWriter(attributeFieldWriter), + _shared_executor(shared_executor), _hwInfo(hwInfo), _importedAttributes() { @@ -278,6 +283,7 @@ AttributeManager::AttributeManager(const AttributeManager &currMgr, _factory(currMgr._factory), _interlock(currMgr._interlock), _attributeFieldWriter(currMgr._attributeFieldWriter), + _shared_executor(currMgr._shared_executor), _hwInfo(currMgr._hwInfo), _importedAttributes() { @@ -537,6 +543,11 @@ AttributeManager::getAttributeFieldWriter() const return _attributeFieldWriter; } +vespalib::ThreadExecutor& +AttributeManager::get_shared_executor() const +{ + return _shared_executor; +} AttributeVector * AttributeManager::getWritableAttribute(const vespalib::string &name) const @@ -548,14 +559,12 @@ AttributeManager::getWritableAttribute(const vespalib::string &name) const return itr->second.getAttribute().get(); } - const std::vector<AttributeVector *> & AttributeManager::getWritableAttributes() const { return _writableAttributes; } - void AttributeManager::asyncForEachAttribute(std::shared_ptr<IConstAttributeFunctor> func) const { @@ -564,7 +573,7 @@ AttributeManager::asyncForEachAttribute(std::shared_ptr<IConstAttributeFunctor> continue; } AttributeVector::SP attrsp = attr.second.getAttribute(); - _attributeFieldWriter.execute(_attributeFieldWriter.getExecutorId(attrsp->getNamePrefix()), + _attributeFieldWriter.execute(_attributeFieldWriter.getExecutorIdFromName(attrsp->getNamePrefix()), [attrsp, func]() { (*func)(*attrsp); }); } } @@ -577,7 +586,7 @@ AttributeManager::asyncForAttribute(const vespalib::string &name, std::unique_pt } AttributeVector::SP attrsp = itr->second.getAttribute(); vespalib::string attrName = attrsp->getNamePrefix(); - _attributeFieldWriter.execute(_attributeFieldWriter.getExecutorId(attrName), + _attributeFieldWriter.execute(_attributeFieldWriter.getExecutorIdFromName(attrName), [attr=std::move(attrsp), func=std::move(func)]() { (*func)(*attr); }); } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.h b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.h index 350b986add4..10c017c84d3 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.h @@ -17,6 +17,8 @@ namespace search::common { class FileHeaderContext; } namespace searchcorespi { class IFlushTarget; } +namespace vespalib { class ThreadExecutor; } + namespace proton { class AttributeDiskLayout; @@ -78,6 +80,7 @@ private: IAttributeFactory::SP _factory; std::shared_ptr<search::attribute::Interlock> _interlock; vespalib::ISequencedTaskExecutor &_attributeFieldWriter; + vespalib::ThreadExecutor& _shared_executor; HwInfo _hwInfo; std::unique_ptr<ImportedAttributesRepo> _importedAttributes; @@ -104,6 +107,7 @@ public: const search::TuneFileAttributes &tuneFileAttributes, const search::common::FileHeaderContext & fileHeaderContext, vespalib::ISequencedTaskExecutor &attributeFieldWriter, + vespalib::ThreadExecutor& shared_executor, const HwInfo &hwInfo); AttributeManager(const vespalib::string &baseDir, @@ -111,6 +115,7 @@ public: const search::TuneFileAttributes &tuneFileAttributes, const search::common::FileHeaderContext & fileHeaderContext, vespalib::ISequencedTaskExecutor &attributeFieldWriter, + vespalib::ThreadExecutor& shared_executor, const IAttributeFactory::SP &factory, const HwInfo &hwInfo); @@ -166,6 +171,8 @@ public: vespalib::ISequencedTaskExecutor &getAttributeFieldWriter() const override; + vespalib::ThreadExecutor& get_shared_executor() const override; + search::AttributeVector *getWritableAttribute(const vespalib::string &name) const override; const std::vector<search::AttributeVector *> &getWritableAttributes() const override; diff --git a/searchcore/src/vespa/searchcore/proton/attribute/exclusive_attribute_read_accessor.cpp b/searchcore/src/vespa/searchcore/proton/attribute/exclusive_attribute_read_accessor.cpp index 9543897407d..16f06bdde93 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/exclusive_attribute_read_accessor.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/exclusive_attribute_read_accessor.cpp @@ -51,7 +51,7 @@ ExclusiveAttributeReadAccessor::takeGuard() { GateSP entranceGate = std::make_shared<Gate>(); GateSP exitGate = std::make_shared<Gate>(); - _attributeFieldWriter.execute(_attributeFieldWriter.getExecutorId(_attribute->getNamePrefix()), + _attributeFieldWriter.execute(_attributeFieldWriter.getExecutorIdFromName(_attribute->getNamePrefix()), [this, entranceGate, exitGate]() { attributeWriteBlockingTask(_attribute, entranceGate, exitGate); }); entranceGate->await(); return std::make_unique<Guard>(*_attribute, exitGate); diff --git a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp index 7c02ec66014..3b1269b031c 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp @@ -161,13 +161,17 @@ FilterAttributeManager::getFlushedSerialNum(const vespalib::string &name) const return 0; } - vespalib::ISequencedTaskExecutor & FilterAttributeManager::getAttributeFieldWriter() const { return _mgr->getAttributeFieldWriter(); } +vespalib::ThreadExecutor& +FilterAttributeManager::get_shared_executor() const +{ + return _mgr->get_shared_executor(); +} search::AttributeVector * FilterAttributeManager::getWritableAttribute(const vespalib::string &name) const @@ -196,7 +200,7 @@ FilterAttributeManager::asyncForEachAttribute(std::shared_ptr<IConstAttributeFun search::AttributeVector::SP attrsp = guard.getSP(); // Name must be extracted in document db master thread or attribute // writer thread - attributeFieldWriter.execute(attributeFieldWriter.getExecutorId(attrsp->getNamePrefix()), + attributeFieldWriter.execute(attributeFieldWriter.getExecutorIdFromName(attrsp->getNamePrefix()), [attrsp, func]() { (*func)(*attrsp); }); } } @@ -207,7 +211,7 @@ FilterAttributeManager::asyncForAttribute(const vespalib::string &name, std::uni if (!attr) { return; } vespalib::ISequencedTaskExecutor &attributeFieldWriter = getAttributeFieldWriter(); vespalib::string attrName = (*attr)->getNamePrefix(); - attributeFieldWriter.execute(attributeFieldWriter.getExecutorId(attrName), + attributeFieldWriter.execute(attributeFieldWriter.getExecutorIdFromName(attrName), [attr=std::move(attr), func=std::move(func)]() mutable { (*func)(**attr); }); diff --git a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.h b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.h index b183f7ed3b3..0b478a34144 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.h @@ -47,6 +47,7 @@ public: void pruneRemovedFields(search::SerialNum serialNum) override; const IAttributeFactory::SP &getFactory() const override; vespalib::ISequencedTaskExecutor & getAttributeFieldWriter() const override; + vespalib::ThreadExecutor& get_shared_executor() const override; search::AttributeVector * getWritableAttribute(const vespalib::string &name) const override; const std::vector<search::AttributeVector *> & getWritableAttributes() const override; diff --git a/searchcore/src/vespa/searchcore/proton/attribute/flushableattribute.cpp b/searchcore/src/vespa/searchcore/proton/attribute/flushableattribute.cpp index 2b5f4b028dc..4edb64b861a 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/flushableattribute.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/flushableattribute.cpp @@ -230,7 +230,7 @@ FlushableAttribute::initFlush(SerialNum currentSerial) // Called by document db executor std::promise<IFlushTarget::Task::UP> promise; std::future<IFlushTarget::Task::UP> future = promise.get_future(); - _attributeFieldWriter.execute(_attributeFieldWriter.getExecutorId(_attr->getNamePrefix()), + _attributeFieldWriter.execute(_attributeFieldWriter.getExecutorIdFromName(_attr->getNamePrefix()), [&]() { promise.set_value(internalInitFlush(currentSerial)); }); return future.get(); } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_manager.h b/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_manager.h index 25d0a508438..fa5c52617fd 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_manager.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_manager.h @@ -14,7 +14,10 @@ namespace search { class IDestructorCallback;} namespace search::attribute { class IAttributeFunctor; } -namespace vespalib { class ISequencedTaskExecutor; } +namespace vespalib { + class ISequencedTaskExecutor; + class ThreadExecutor; +} namespace proton { @@ -74,6 +77,8 @@ struct IAttributeManager : public search::IAttributeManager virtual vespalib::ISequencedTaskExecutor &getAttributeFieldWriter() const = 0; + virtual vespalib::ThreadExecutor& get_shared_executor() const = 0; + /* * Get pointer to named writable attribute. If attribute isn't * found or is an extra attribute then nullptr is returned. diff --git a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_listener.cpp b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_listener.cpp index 7d5eabe7de8..57e284cc61b 100644 --- a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_listener.cpp +++ b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_listener.cpp @@ -11,7 +11,7 @@ GidToLidChangeListener::GidToLidChangeListener(vespalib::ISequencedTaskExecutor const vespalib::string &name, const vespalib::string &docTypeName) : _attributeFieldWriter(attributeFieldWriter), - _executorId(_attributeFieldWriter.getExecutorId(attr->getNamePrefix())), + _executorId(_attributeFieldWriter.getExecutorIdFromName(attr->getNamePrefix())), _attr(std::move(attr)), _refCount(refCount), _name(name), diff --git a/searchcore/src/vespa/searchcore/proton/server/fast_access_doc_subdb.cpp b/searchcore/src/vespa/searchcore/proton/server/fast_access_doc_subdb.cpp index 542923043ef..063561347b4 100644 --- a/searchcore/src/vespa/searchcore/proton/server/fast_access_doc_subdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/fast_access_doc_subdb.cpp @@ -69,6 +69,7 @@ FastAccessDocSubDB::createAttributeManagerInitializer(const DocumentDBConfig &co configSnapshot.getTuneFileDocumentDBSP()->_attr, _fileHeaderContext, _writeService.attributeFieldWriter(), + _writeService.shared(), attrFactory, _hwInfo); return std::make_shared<AttributeManagerInitializer>(configSerialNum, diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp index 962ee65c10d..1e579739d85 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp @@ -102,8 +102,8 @@ diskMemUsageSamplerConfig(const ProtonConfig &proton, const HwInfo &hwInfo) } size_t -deriveCompactionCompressionThreads(const ProtonConfig &proton, - const HwInfo::Cpu &cpuInfo) { +derive_shared_threads(const ProtonConfig &proton, + const HwInfo::Cpu &cpuInfo) { size_t scaledCores = (size_t)std::ceil(cpuInfo.cores() * proton.feeding.concurrency); // We need at least 1 guaranteed free worker in order to ensure progress so #documentsdbs + 1 should suffice, @@ -301,7 +301,7 @@ Proton::init(const BootstrapConfig::SP & configSnapshot) vespalib::string fileConfigId; _warmupExecutor = std::make_unique<vespalib::ThreadStackExecutor>(4, 128*1024, index_warmup_executor); - const size_t sharedThreads = deriveCompactionCompressionThreads(protonConfig, hwInfo.cpu()); + const size_t sharedThreads = derive_shared_threads(protonConfig, hwInfo.cpu()); _sharedExecutor = std::make_shared<vespalib::BlockingThreadStackExecutor>(sharedThreads, 128*1024, sharedThreads*16, proton_shared_executor); _compile_cache_executor_binding = vespalib::eval::CompileCache::bind(_sharedExecutor); InitializeThreads initializeThreads; diff --git a/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h b/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h index 8e5d3018532..3c301e66057 100644 --- a/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h +++ b/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h @@ -14,13 +14,15 @@ private: std::vector<search::AttributeVector*> _writables; std::unique_ptr<ImportedAttributesRepo> _importedAttributes; vespalib::ISequencedTaskExecutor* _writer; + vespalib::ThreadExecutor* _shared; public: MockAttributeManager() : _mock(), _writables(), _importedAttributes(), - _writer() + _writer(), + _shared() {} search::AttributeVector::SP addAttribute(const vespalib::string &name, const search::AttributeVector::SP &attr) { @@ -28,11 +30,12 @@ public: _writables.push_back(attr.get()); return attr; } - void set_writer(vespalib::ISequencedTaskExecutor& writer) { _writer = &writer; } - + void set_shared_executor(vespalib::ThreadExecutor& shared) { + _shared = &shared; + } search::AttributeGuard::UP getAttribute(const vespalib::string &name) const override { return _mock.getAttribute(name); } @@ -69,6 +72,10 @@ public: assert(_writer != nullptr); return *_writer; } + vespalib::ThreadExecutor& get_shared_executor() const override { + assert(_shared != nullptr); + return *_shared; + } search::AttributeVector *getWritableAttribute(const vespalib::string &name) const override { auto attr = getAttribute(name); if (attr) { diff --git a/searchcorespi/src/vespa/searchcorespi/index/ithreadingservice.h b/searchcorespi/src/vespa/searchcorespi/index/ithreadingservice.h index 2ace9a8ac6b..18c376a4c2b 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/ithreadingservice.h +++ b/searchcorespi/src/vespa/searchcorespi/index/ithreadingservice.h @@ -8,26 +8,36 @@ namespace vespalib { class ISequencedTaskExecutor; } namespace searchcorespi::index { /** - * Interface for the thread model used for write tasks. + * Interface for the thread model used for write tasks for a single document database. * * We have multiple write threads: * - * 1. The master write thread used for the majority of write tasks. + * 1. The "master" write thread used for the majority of write tasks. * - * 2. The index write thread used for doing changes to the memory + * 2. The "index" write thread used for doing changes to the memory * index, either directly (for data not bound to a field) or via * index field inverter executor or index field writer executor. * - * 3. The index field inverter executor is used to populate field + * 3. The "summary" thread is used for doing changes to the document store. + * + * 4. The "index field inverter" executor is used to populate field * inverters with data from document fields. Scheduled tasks for * the same field are executed in sequence. * - * 4. The index field writer executor is used to sort data in field + * 5. The "index field writer" executor is used to sort data in field * inverters before pushing the data to the memory field indexes. * Scheduled tasks for the same field are executed in sequence. * - * The master write thread is always the one giving tasks to the index - * write thread. + * 6. The "attribute field writer" executor is used to write data to attribute vectors. + * Each attribute is always handled by the same thread, + * and scheduled tasks for the same attribute are executed in sequence. + * + * The master write thread is always the one giving tasks to the other write threads above. + * + * In addition this interface exposes the "shared" executor that is used by all document databases. + * This is among others used for compressing / de-compressing documents in the document store, + * merging files as part of disk index fusion, and running the prepare step when doing two-phase + * puts against a tensor attribute with a HNSW index. * * The index write thread extracts fields from documents and gives * task to the index field inverter executor and the index field diff --git a/staging_vespalib/src/tests/sequencedtaskexecutor/adaptive_sequenced_executor_test.cpp b/staging_vespalib/src/tests/sequencedtaskexecutor/adaptive_sequenced_executor_test.cpp index 10f3f6089e3..2ca49105610 100644 --- a/staging_vespalib/src/tests/sequencedtaskexecutor/adaptive_sequenced_executor_test.cpp +++ b/staging_vespalib/src/tests/sequencedtaskexecutor/adaptive_sequenced_executor_test.cpp @@ -63,6 +63,8 @@ public: } }; +vespalib::stringref ZERO("0"); + TEST_F("testExecute", Fixture) { std::shared_ptr<TestObj> tv(std::make_shared<TestObj>()); EXPECT_EQUAL(0, tv->_val); @@ -97,7 +99,7 @@ TEST_F("require that task with different component ids are not serialized", Fixt std::shared_ptr<TestObj> tv(std::make_shared<TestObj>()); EXPECT_EQUAL(0, tv->_val); f._threads.execute(0, [&]() { usleep(2000); tv->modify(0, 14); }); - f._threads.execute(2, [&]() { tv->modify(14, 42); }); + f._threads.execute(1, [&]() { tv->modify(14, 42); }); tv->wait(2); if (tv->_fail != 1) { continue; @@ -118,8 +120,8 @@ TEST_F("require that task with same string component id are serialized", Fixture std::shared_ptr<TestObj> tv(std::make_shared<TestObj>()); EXPECT_EQUAL(0, tv->_val); auto test2 = [&]() { tv->modify(14, 42); }; - f._threads.execute(f._threads.getExecutorId("0"), [&]() { usleep(2000); tv->modify(0, 14); }); - f._threads.execute(f._threads.getExecutorId("0"), test2); + f._threads.execute(f._threads.getExecutorIdFromName(ZERO), [&]() { usleep(2000); tv->modify(0, 14); }); + f._threads.execute(f._threads.getExecutorIdFromName(ZERO), test2); tv->wait(2); EXPECT_EQUAL(0, tv->_fail); EXPECT_EQUAL(42, tv->_val); @@ -136,8 +138,8 @@ int detectSerializeFailure(Fixture &f, vespalib::stringref altComponentId, int t for (tryCnt = 0; tryCnt < tryLimit; ++tryCnt) { std::shared_ptr<TestObj> tv(std::make_shared<TestObj>()); EXPECT_EQUAL(0, tv->_val); - f._threads.execute(f._threads.getExecutorId("0"), [&]() { usleep(2000); tv->modify(0, 14); }); - f._threads.execute(f._threads.getExecutorId(altComponentId), [&]() { tv->modify(14, 42); }); + f._threads.execute(f._threads.getExecutorIdFromName(ZERO), [&]() { usleep(2000); tv->modify(0, 14); }); + f._threads.execute(f._threads.getExecutorIdFromName(altComponentId), [&]() { tv->modify(14, 42); }); tv->wait(2); if (tv->_fail != 1) { continue; @@ -156,10 +158,10 @@ vespalib::string makeAltComponentId(Fixture &f) { int tryCnt = 0; char altComponentId[20]; - ISequencedTaskExecutor::ExecutorId executorId0 = f._threads.getExecutorId("0"); + ISequencedTaskExecutor::ExecutorId executorId0 = f._threads.getExecutorIdFromName(ZERO); for (tryCnt = 1; tryCnt < 100; ++tryCnt) { sprintf(altComponentId, "%d", tryCnt); - if (f._threads.getExecutorId(altComponentId) == executorId0) { + if (f._threads.getExecutorIdFromName(altComponentId) == executorId0) { break; } } @@ -236,13 +238,9 @@ TEST("require that you get correct number of executors") { TEST("require that you distribute well") { AdaptiveSequencedExecutor seven(7, 1, 0, 10); EXPECT_EQUAL(7u, seven.getNumExecutors()); - EXPECT_EQUAL(97u, seven.getComponentHashSize()); - EXPECT_EQUAL(0u, seven.getComponentEffectiveHashSize()); for (uint32_t id=0; id < 1000; id++) { - EXPECT_EQUAL((id%97)%7, seven.getExecutorId(id).getId()); + EXPECT_EQUAL(id%7, seven.getExecutorId(id).getId()); } - EXPECT_EQUAL(97u, seven.getComponentHashSize()); - EXPECT_EQUAL(97u, seven.getComponentEffectiveHashSize()); } } diff --git a/staging_vespalib/src/tests/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp b/staging_vespalib/src/tests/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp index 70d0f1c743d..6128386837d 100644 --- a/staging_vespalib/src/tests/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp +++ b/staging_vespalib/src/tests/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp @@ -65,6 +65,8 @@ public: } }; +vespalib::stringref ZERO("0"); + TEST_F("testExecute", Fixture) { std::shared_ptr<TestObj> tv(std::make_shared<TestObj>()); EXPECT_EQUAL(0, tv->_val); @@ -120,8 +122,8 @@ TEST_F("require that task with same string component id are serialized", Fixture std::shared_ptr<TestObj> tv(std::make_shared<TestObj>()); EXPECT_EQUAL(0, tv->_val); auto test2 = [=]() { tv->modify(14, 42); }; - f._threads->execute(f._threads->getExecutorId("0"), [=]() { usleep(2000); tv->modify(0, 14); }); - f._threads->execute(f._threads->getExecutorId("0"), test2); + f._threads->execute(f._threads->getExecutorIdFromName(ZERO), [=]() { usleep(2000); tv->modify(0, 14); }); + f._threads->execute(f._threads->getExecutorIdFromName(ZERO), test2); tv->wait(2); EXPECT_EQUAL(0, tv->_fail); EXPECT_EQUAL(42, tv->_val); @@ -138,8 +140,8 @@ int detectSerializeFailure(Fixture &f, vespalib::stringref altComponentId, int t for (tryCnt = 0; tryCnt < tryLimit; ++tryCnt) { std::shared_ptr<TestObj> tv(std::make_shared<TestObj>()); EXPECT_EQUAL(0, tv->_val); - f._threads->execute(f._threads->getExecutorId("0"), [=]() { usleep(2000); tv->modify(0, 14); }); - f._threads->execute(f._threads->getExecutorId(altComponentId), [=]() { tv->modify(14, 42); }); + f._threads->execute(f._threads->getExecutorIdFromName(ZERO), [=]() { usleep(2000); tv->modify(0, 14); }); + f._threads->execute(f._threads->getExecutorIdFromName(altComponentId), [=]() { tv->modify(14, 42); }); tv->wait(2); if (tv->_fail != 1) { continue; @@ -158,10 +160,10 @@ vespalib::string makeAltComponentId(Fixture &f) { int tryCnt = 0; char altComponentId[20]; - ISequencedTaskExecutor::ExecutorId executorId0 = f._threads->getExecutorId("0"); + ISequencedTaskExecutor::ExecutorId executorId0 = f._threads->getExecutorIdFromName(ZERO); for (tryCnt = 1; tryCnt < 100; ++tryCnt) { sprintf(altComponentId, "%d", tryCnt); - if (f._threads->getExecutorId(altComponentId) == executorId0) { + if (f._threads->getExecutorIdFromName(altComponentId) == executorId0) { break; } } diff --git a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp index 50bc3b020a8..3e87749c794 100644 --- a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp @@ -256,6 +256,11 @@ AdaptiveSequencedExecutor::~AdaptiveSequencedExecutor() assert(_worker_stack.empty()); } +ISequencedTaskExecutor::ExecutorId +AdaptiveSequencedExecutor::getExecutorId(uint64_t component) const { + return ExecutorId(component % _strands.size()); +} + void AdaptiveSequencedExecutor::executeTask(ExecutorId id, Task::UP task) { diff --git a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h index bc3457a72ef..a4d3ac97758 100644 --- a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h +++ b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h @@ -117,6 +117,7 @@ public: AdaptiveSequencedExecutor(size_t num_strands, size_t num_threads, size_t max_waiting, size_t max_pending); ~AdaptiveSequencedExecutor() override; + ExecutorId getExecutorId(uint64_t component) const override; void executeTask(ExecutorId id, Task::UP task) override; void sync() override; void setTaskLimit(uint32_t task_limit) override; diff --git a/staging_vespalib/src/vespa/vespalib/util/foreground_thread_executor.h b/staging_vespalib/src/vespa/vespalib/util/foreground_thread_executor.h new file mode 100644 index 00000000000..575552971fa --- /dev/null +++ b/staging_vespalib/src/vespa/vespalib/util/foreground_thread_executor.h @@ -0,0 +1,31 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/vespalib/util/threadexecutor.h> +#include <atomic> + +namespace vespalib { + +/** + * Implementation of the ThreadExecutor interface that runs all tasks in the foreground by the calling thread. + */ +class ForegroundThreadExecutor : public vespalib::ThreadExecutor { +private: + std::atomic<size_t> _accepted; + +public: + ForegroundThreadExecutor() : _accepted(0) { } + Task::UP execute(Task::UP task) override { + task->run(); + ++_accepted; + return Task::UP(); + } + size_t getNumThreads() const override { return 0; } + Stats getStats() override { + return ExecutorStats(ExecutorStats::QueueSizeT(), _accepted.load(std::memory_order_relaxed), 0); + } + virtual void setTaskLimit(uint32_t taskLimit) override { (void) taskLimit; } +}; + +} diff --git a/staging_vespalib/src/vespa/vespalib/util/isequencedtaskexecutor.cpp b/staging_vespalib/src/vespa/vespalib/util/isequencedtaskexecutor.cpp index d05702cc85b..f8f1f64fac5 100644 --- a/staging_vespalib/src/vespa/vespalib/util/isequencedtaskexecutor.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/isequencedtaskexecutor.cpp @@ -23,7 +23,7 @@ ISequencedTaskExecutor::ISequencedTaskExecutor(uint32_t numExecutors) ISequencedTaskExecutor::~ISequencedTaskExecutor() = default; ISequencedTaskExecutor::ExecutorId -ISequencedTaskExecutor::getExecutorId(vespalib::stringref componentId) const { +ISequencedTaskExecutor::getExecutorIdFromName(vespalib::stringref componentId) const { vespalib::hash<vespalib::stringref> hashfun; return getExecutorId(hashfun(componentId)); } diff --git a/staging_vespalib/src/vespa/vespalib/util/isequencedtaskexecutor.h b/staging_vespalib/src/vespa/vespalib/util/isequencedtaskexecutor.h index cd2a6c6f0d8..034e1520b8d 100644 --- a/staging_vespalib/src/vespa/vespalib/util/isequencedtaskexecutor.h +++ b/staging_vespalib/src/vespa/vespalib/util/isequencedtaskexecutor.h @@ -37,10 +37,10 @@ public: * @param componentId component id * @return executor id */ - ExecutorId getExecutorId(uint64_t componentId) const; + virtual ExecutorId getExecutorId(uint64_t componentId) const; uint32_t getNumExecutors() const { return _numExecutors; } - ExecutorId getExecutorId(vespalib::stringref componentId) const; + ExecutorId getExecutorIdFromName(vespalib::stringref componentId) const; /** * Schedule a task to run after all previously scheduled tasks with diff --git a/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.cpp b/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.cpp index a0c2f0ac237..0fd78d8dcf6 100644 --- a/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.cpp @@ -18,7 +18,8 @@ std::unique_ptr<ISequencedTaskExecutor> SequencedTaskExecutor::create(uint32_t threads, uint32_t taskLimit, OptimizeFor optimize, uint32_t kindOfWatermark, duration reactionTime) { if (optimize == OptimizeFor::ADAPTIVE) { - return std::make_unique<AdaptiveSequencedExecutor>(threads, threads, kindOfWatermark, taskLimit); + size_t num_strands = std::min(taskLimit, threads*32); + return std::make_unique<AdaptiveSequencedExecutor>(num_strands, threads, kindOfWatermark, taskLimit); } else { auto executors = std::make_unique<std::vector<std::unique_ptr<SyncableThreadExecutor>>>(); executors->reserve(threads); |