diff options
43 files changed, 594 insertions, 889 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java index 6e6f027b520..0b99496a9b4 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java @@ -50,14 +50,12 @@ public class ClusterControllerCluster extends AbstractConfigProducer<ClusterCont public void getConfig(ZookeeperServerConfig.Builder builder) { builder.clientPort(ZK_CLIENT_PORT); builder.juteMaxBuffer(1024 * 1024); // 1 Mb should be more than enough for cluster controller - boolean oldQuorumExists = containerCluster.getContainers().stream() // More than half the previous hosts must be present in the new config for quorum to persist. - .filter(container -> previousHosts.contains(container.getHostName())) // Set intersection is symmetric. - .count() > previousHosts.size() / 2; for (ClusterControllerContainer container : containerCluster.getContainers()) { ZookeeperServerConfig.Server.Builder serverBuilder = new ZookeeperServerConfig.Server.Builder(); serverBuilder.hostname(container.getHostName()); serverBuilder.id(container.index()); - serverBuilder.joining(oldQuorumExists && ! previousHosts.contains(container.getHostName())); + serverBuilder.joining( ! previousHosts.isEmpty() && ! previousHosts.contains(container.getHostName())); + serverBuilder.retired(container.isRetired()); builder.server(serverBuilder); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java new file mode 100644 index 00000000000..048d2ccde6a --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java @@ -0,0 +1,120 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.model.application.validation; + +import aQute.bnd.header.Parameters; +import aQute.bnd.osgi.Domain; +import aQute.bnd.version.VersionRange; +import com.yahoo.config.application.api.ApplicationPackage; +import com.yahoo.config.application.api.ComponentInfo; +import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.path.Path; +import com.yahoo.vespa.model.VespaModel; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NodeList; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; + +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathExpressionException; +import javax.xml.xpath.XPathFactory; +import java.io.IOException; +import java.io.StringReader; +import java.nio.file.Paths; +import java.util.Optional; +import java.util.function.BiConsumer; +import java.util.function.Consumer; +import java.util.jar.JarFile; +import java.util.jar.Manifest; +import java.util.logging.Level; +import java.util.regex.Pattern; + +/** + * Base class for OSGi bundle validator. Uses BND library for some of the validation. + * + * @author bjorncs + */ +public abstract class AbstractBundleValidator extends Validator { + + protected abstract void validateManifest(DeployState state, JarFile jar, Manifest mf); + protected abstract void validatePomXml(DeployState state, JarFile jar, Document pom); + + @Override + public final void validate(VespaModel model, DeployState state) { + ApplicationPackage app = state.getApplicationPackage(); + for (ComponentInfo info : app.getComponentsInfo(state.getVespaVersion())) { + Path path = Path.fromString(info.getPathRelativeToAppDir()); + try { + state.getDeployLogger() + .log(Level.FINE, String.format("Validating bundle at '%s'", path)); + JarFile jarFile = new JarFile(app.getFileReference(path)); + validateJarFile(state, jarFile); + } catch (IOException e) { + throw new IllegalArgumentException( + "Failed to validate JAR file '" + path.last() + "'", e); + } + } + } + + final void validateJarFile(DeployState state, JarFile jar) throws IOException { + Manifest manifest = jar.getManifest(); + if (manifest == null) { + throw new IllegalArgumentException("Non-existing or invalid manifest in " + filename(jar)); + } + validateManifest(state, jar, manifest); + getPomXmlContent(state.getDeployLogger(), jar) + .ifPresent(pom -> validatePomXml(state, jar, pom)); + } + + protected final String filename(JarFile jarFile) { return Paths.get(jarFile.getName()).getFileName().toString(); } + + protected final void forEachPomXmlElement(Document pom, String xpath, Consumer<Element> consumer) throws XPathExpressionException { + NodeList dependencies = (NodeList) XPathFactory.newDefaultInstance().newXPath() + .compile("/project/" + xpath) + .evaluate(pom, XPathConstants.NODESET); + for (int i = 0; i < dependencies.getLength(); i++) { + Element element = (Element) dependencies.item(i); + consumer.accept(element); + } + } + + protected final void forEachImportPackage(Manifest mf, BiConsumer<String, VersionRange> consumer) { + Parameters importPackage = Domain.domain(mf).getImportPackage(); + importPackage.forEach((packageName, attrs) -> { + VersionRange versionRange = attrs.getVersion() != null + ? VersionRange.parseOSGiVersionRange(attrs.getVersion()) + : null; + consumer.accept(packageName, versionRange); + }); + } + + protected final void log(DeployState state, Level level, String fmt, Object... args) { + state.getDeployLogger().log(level, String.format(fmt, args)); + } + + private static final Pattern POM_FILE_LOCATION = Pattern.compile("META-INF/maven/.+?/.+?/pom.xml"); + private Optional<Document> getPomXmlContent(DeployLogger deployLogger, JarFile jar) { + return jar.stream() + .filter(f -> POM_FILE_LOCATION.matcher(f.getName()).matches()) + .findFirst() + .map(f -> { + try { + String text = new String(jar.getInputStream(f).readAllBytes()); + return DocumentBuilderFactory.newDefaultInstance().newDocumentBuilder() + .parse(new InputSource(new StringReader(text))); + } catch (ParserConfigurationException e) { + throw new RuntimeException(e); + } catch (SAXException e) { + deployLogger.log(Level.INFO, String.format("Unable to parse pom.xml from %s", filename(jar))); + return null; + } catch (IOException e) { + deployLogger.log(Level.INFO, + String.format("Unable to read '%s' from '%s'", f.getName(), jar.getName())); + return null; + } + }); + } +} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/BundleValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/BundleValidator.java index fe5ffb4e544..a1024425124 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/BundleValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/BundleValidator.java @@ -1,28 +1,10 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; -import aQute.bnd.header.Parameters; -import aQute.bnd.osgi.Domain; import aQute.bnd.version.VersionRange; -import com.yahoo.config.application.api.ApplicationPackage; -import com.yahoo.config.application.api.ComponentInfo; -import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.deploy.DeployState; -import com.yahoo.path.Path; -import com.yahoo.vespa.model.VespaModel; import org.w3c.dom.Document; -import org.w3c.dom.Element; -import org.w3c.dom.NodeList; -import org.xml.sax.InputSource; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.xpath.XPathConstants; -import javax.xml.xpath.XPathExpressionException; -import javax.xml.xpath.XPathFactory; -import java.io.IOException; -import java.io.StringReader; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -30,9 +12,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; -import java.util.function.Consumer; import java.util.function.Predicate; import java.util.jar.Attributes; import java.util.jar.JarFile; @@ -42,44 +22,17 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; /** - * A validator for bundles. Uses BND library for some of the validation. + * A validator for bundles. * * @author hmusum * @author bjorncs */ -public class BundleValidator extends Validator { +public class BundleValidator extends AbstractBundleValidator { public BundleValidator() {} @Override - public void validate(VespaModel model, DeployState deployState) { - ApplicationPackage app = deployState.getApplicationPackage(); - for (ComponentInfo info : app.getComponentsInfo(deployState.getVespaVersion())) { - Path path = Path.fromString(info.getPathRelativeToAppDir()); - try { - DeployLogger deployLogger = deployState.getDeployLogger(); - deployLogger.log(Level.FINE, String.format("Validating bundle at '%s'", path)); - JarFile jarFile = new JarFile(app.getFileReference(path)); - validateJarFile(deployLogger, deployState.isHosted(), jarFile); - } catch (IOException e) { - throw new IllegalArgumentException( - "Failed to validate JAR file '" + path.last() + "'", e); - } - } - } - - void validateJarFile(DeployLogger deployLogger, boolean isHosted, JarFile jarFile) throws IOException { - Manifest manifest = jarFile.getManifest(); - String filename = Paths.get(jarFile.getName()).getFileName().toString(); - if (manifest == null) { - throw new IllegalArgumentException("Non-existing or invalid manifest in " + filename); - } - validateManifest(deployLogger, filename, manifest); - getPomXmlContent(deployLogger, jarFile) - .ifPresent(pomXml -> validatePomXml(deployLogger, isHosted, filename, pomXml)); - } - - private void validateManifest(DeployLogger deployLogger, String filename, Manifest mf) { + protected void validateManifest(DeployState state, JarFile jar, Manifest mf) { // Check for required OSGI headers Attributes attributes = mf.getMainAttributes(); HashSet<String> mfAttributes = new HashSet<>(); @@ -91,30 +44,26 @@ public class BundleValidator extends Validator { for (String header : requiredOSGIHeaders) { if (!mfAttributes.contains(header)) { throw new IllegalArgumentException("Required OSGI header '" + header + - "' was not found in manifest in '" + filename + "'"); + "' was not found in manifest in '" + filename(jar) + "'"); } } if (attributes.getValue("Bundle-Version").endsWith(".SNAPSHOT")) { - deployLogger.logApplicationPackage(Level.WARNING, "Deploying snapshot bundle " + filename + - ".\nTo use this bundle, you must include the qualifier 'SNAPSHOT' in the version specification in services.xml."); + log(state, Level.WARNING, + "Deploying snapshot bundle " + filename(jar) + ".\nTo use this bundle, you must include the " + + "qualifier 'SNAPSHOT' in the version specification in services.xml."); } if (attributes.getValue("Import-Package") != null) { - validateImportedPackages(deployLogger, filename, mf); + validateImportedPackages(state, jar, mf); } } - private static void validateImportedPackages(DeployLogger deployLogger, String filename, Manifest manifest) { - Domain osgiHeaders = Domain.domain(manifest); - Parameters importPackage = osgiHeaders.getImportPackage(); - Map<DeprecatedProvidedBundle, List<String>> deprecatedPackagesInUse = new HashMap<>(); - - importPackage.forEach((packageName, attrs) -> { - VersionRange versionRange = attrs.getVersion() != null - ? VersionRange.parseOSGiVersionRange(attrs.getVersion()) - : null; + @Override protected void validatePomXml(DeployState state, JarFile jar, Document pom) {} + private void validateImportedPackages(DeployState state, JarFile jar, Manifest manifest) { + Map<DeprecatedProvidedBundle, List<String>> deprecatedPackagesInUse = new HashMap<>(); + forEachImportPackage(manifest, (packageName, versionRange) -> { for (DeprecatedProvidedBundle deprecatedBundle : DeprecatedProvidedBundle.values()) { for (Predicate<String> matcher : deprecatedBundle.javaPackageMatchers) { if (matcher.test(packageName) @@ -125,111 +74,16 @@ public class BundleValidator extends Validator { } } }); - deprecatedPackagesInUse.forEach((artifact, packagesInUse) -> { - deployLogger.logApplicationPackage(Level.WARNING, - String.format("For JAR file '%s': \n" + - "Manifest imports the following Java packages from '%s': %s. \n" + - "%s", - filename, artifact.name, packagesInUse, artifact.description)); + log(state, Level.WARNING, "JAR file '%s' imports the packages %s from '%s'. \n%s", + filename(jar), packagesInUse, artifact.name, artifact.description); }); } - private static final Pattern POM_FILE_LOCATION = Pattern.compile("META-INF/maven/.+?/.+?/pom.xml"); - - private Optional<String> getPomXmlContent(DeployLogger deployLogger, JarFile jarFile) { - return jarFile.stream() - .filter(f -> POM_FILE_LOCATION.matcher(f.getName()).matches()) - .findFirst() - .map(f -> { - try { - return new String(jarFile.getInputStream(f).readAllBytes()); - } catch (IOException e) { - deployLogger.log(Level.INFO, - String.format("Unable to read '%s' from '%s'", f.getName(), jarFile.getName())); - return null; - } - }); - } - - private void validatePomXml(DeployLogger deployLogger, boolean isHosted, String jarFilename, String pomXmlContent) { - if (isHosted) { - try { - Document pom = DocumentBuilderFactory.newDefaultInstance().newDocumentBuilder() - .parse(new InputSource(new StringReader(pomXmlContent))); - validateDependencies(deployLogger, jarFilename, pom); - validateRepositories(deployLogger, jarFilename, pom); - } catch (ParserConfigurationException e) { - throw new RuntimeException(e); - } catch (Exception e) { - deployLogger.log(Level.INFO, String.format("Unable to parse pom.xml from %s", jarFilename)); - } - } - } - - private static void validateDependencies(DeployLogger deployLogger, String jarFilename, Document pom) throws XPathExpressionException { - forEachPomXmlElement(pom, "dependencies/dependency", dependency -> { - String groupId = dependency.getElementsByTagName("groupId").item(0).getTextContent(); - String artifactId = dependency.getElementsByTagName("artifactId").item(0).getTextContent(); - for (DeprecatedMavenArtifact deprecatedArtifact : DeprecatedMavenArtifact.values()) { - if (groupId.equals(deprecatedArtifact.groupId) && artifactId.equals(deprecatedArtifact.artifactId)) { - deployLogger.logApplicationPackage(Level.WARNING, - String.format( - "The pom.xml of bundle '%s' includes a dependency to the artifact '%s:%s'. \n%s", - jarFilename, groupId, artifactId, deprecatedArtifact.description)); - } - } - }); - } - - private static void validateRepositories(DeployLogger deployLogger, String jarFilename, Document pom) throws XPathExpressionException { - forEachPomXmlElement(pom, "pluginRepositories/pluginRepository", - repository -> validateRepository(deployLogger, jarFilename, "pluginRepositories", repository)); - forEachPomXmlElement(pom, "repositories/repository", - repository -> validateRepository(deployLogger, jarFilename, "repositories", repository)); - } - - private static void validateRepository(DeployLogger deployLogger, String jarFilename, String parentElementName, - Element element) { - String url = element.getElementsByTagName("url").item(0).getTextContent(); - if (url.contains("vespa-maven-libs-release-local")) { - deployLogger.logApplicationPackage(Level.WARNING, - String.format("<%s> in pom.xml of '%s' uses deprecated Maven repository '%s'.\n See announcement.", - parentElementName, jarFilename, url)); - } - } - - private static void forEachPomXmlElement(Document pom, String xpath, Consumer<Element> consumer) throws XPathExpressionException { - NodeList dependencies = (NodeList) XPathFactory.newDefaultInstance().newXPath() - .compile("/project/" + xpath) - .evaluate(pom, XPathConstants.NODESET); - for (int i = 0; i < dependencies.getLength(); i++) { - Element element = (Element) dependencies.item(i); - consumer.accept(element); - } - } - - private enum DeprecatedMavenArtifact { - VESPA_HTTP_CLIENT_EXTENSION("com.yahoo.vespa", "vespa-http-client-extensions", - "This artifact will be removed in Vespa 8. " + - "Programmatic use can be safely removed from system/staging tests. " + - "See internal Vespa 8 release notes for details."); - - final String groupId; - final String artifactId; - final String description; - - DeprecatedMavenArtifact(String groupId, String artifactId, String description) { - this.groupId = groupId; - this.artifactId = artifactId; - this.description = description; - } - } - private enum DeprecatedProvidedBundle { ORG_JSON("org.json:json", - "The org.json library will no longer provided by jdisc runtime on Vespa 8. " + - "See https://docs.vespa.ai/en/vespa8-release-notes.html#container-runtime.", + "This bundle is no longer provided on Vespa 8 - " + + "see https://docs.vespa.ai/en/vespa8-release-notes.html#container-runtime.", Set.of("org\\.json")); final String name; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java index 780de5dbf04..5bf7ea0b290 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java @@ -309,8 +309,9 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat ZookeeperServerConfig.Server.Builder serverBuilder = new ZookeeperServerConfig.Server.Builder(); serverBuilder.hostname(container.getHostName()) .id(container.index()) - .joining(!previousHosts.isEmpty() && - !previousHosts.contains(container.getHostName())); + .joining( ! previousHosts.isEmpty() && + ! previousHosts.contains(container.getHostName())) + .retired(container.isRetired()); builder.server(serverBuilder); builder.dynamicReconfiguration(true); } diff --git a/config-model/src/test/cfg/application/validation/testjars/pom-xml-warnings/META-INF/MANIFEST.MF b/config-model/src/test/cfg/application/validation/testjars/pom-xml-warnings/META-INF/MANIFEST.MF deleted file mode 100644 index 1f88a5e6477..00000000000 --- a/config-model/src/test/cfg/application/validation/testjars/pom-xml-warnings/META-INF/MANIFEST.MF +++ /dev/null @@ -1,7 +0,0 @@ -Manifest-Version: 1.0 -Created-By: 1.6.0_20 (Apple Inc.) -Bundle-ManifestVersion: 2 -Bundle-Name: mybundle -Bundle-SymbolicName: mybundle -Bundle-Version: 0 - diff --git a/config-model/src/test/cfg/application/validation/testjars/pom-xml-warnings/META-INF/maven/com.yahoo.test/mybundle/pom.xml b/config-model/src/test/cfg/application/validation/testjars/pom-xml-warnings/META-INF/maven/com.yahoo.test/mybundle/pom.xml deleted file mode 100644 index 167751f55c4..00000000000 --- a/config-model/src/test/cfg/application/validation/testjars/pom-xml-warnings/META-INF/maven/com.yahoo.test/mybundle/pom.xml +++ /dev/null @@ -1,28 +0,0 @@ -<?xml version="1.0"?> -<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> - <modelVersion>4.0.0</modelVersion> - <groupId>com.yahoo.test</groupId> - <artifactId>mybundle</artifactId> - <packaging>container-plugin</packaging> - <version>1.0.0</version> - <dependencies> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>vespa-http-client-extensions</artifactId> - <scope>test</scope> - </dependency> - </dependencies> - - <pluginRepositories> - <pluginRepository> - <id>my-plugin-repository</id> - <url>http://myartifactory:8000/artifactory/vespa-maven-libs-release-local</url> - </pluginRepository> - </pluginRepositories> - <repositories> - <repository> - <id>my-repository</id> - <url>http://myartifactory:8000/artifactory/vespa-maven-libs-release-local</url> - </repository> - </repositories> -</project>
\ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java index 10f883bdc75..ba70b7493a2 100644 --- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java @@ -2054,7 +2054,7 @@ public class ModelProvisioningTest { assertTrue("Initial servers are not joining", config.build().server().stream().noneMatch(ZookeeperServerConfig.Server::joining)); } { - VespaModel nextModel = tester.createModel(Zone.defaultZone(), servicesXml.apply(5), true, false, false, 0, Optional.of(model), new DeployState.Builder()); + VespaModel nextModel = tester.createModel(Zone.defaultZone(), servicesXml.apply(3), true, false, false, 0, Optional.of(model), new DeployState.Builder(), "node-1-3-10-04", "node-1-3-10-03"); ApplicationContainerCluster cluster = nextModel.getContainerClusters().get("zk"); ZookeeperServerConfig.Builder config = new ZookeeperServerConfig.Builder(); cluster.getContainers().forEach(c -> c.getConfig(config)); @@ -2067,6 +2067,14 @@ public class ModelProvisioningTest { 4, true), config.build().server().stream().collect(Collectors.toMap(ZookeeperServerConfig.Server::id, ZookeeperServerConfig.Server::joining))); + assertEquals("Retired nodes are retired", + Map.of(0, false, + 1, true, + 2, true, + 3, false, + 4, false), + config.build().server().stream().collect(Collectors.toMap(ZookeeperServerConfig.Server::id, + ZookeeperServerConfig.Server::retired))); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/BundleValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/BundleValidatorTest.java index eeae7dfe0ee..b5b93be6cd7 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/BundleValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/BundleValidatorTest.java @@ -2,7 +2,7 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.application.api.DeployLogger; -import com.yahoo.config.model.application.provider.BaseDeployLogger; +import com.yahoo.config.model.deploy.DeployState; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -29,7 +29,7 @@ public class BundleValidatorTest { // Valid jar file JarFile ok = createTemporaryJarFile("ok"); BundleValidator bundleValidator = new BundleValidator(); - bundleValidator.validateJarFile(new BaseDeployLogger(), false, ok); + bundleValidator.validateJarFile(DeployState.createTestState(), ok); // No manifest validateWithException("nomanifest", "Non-existing or invalid manifest in nomanifest.jar"); @@ -39,7 +39,7 @@ public class BundleValidatorTest { try { JarFile jarFile = createTemporaryJarFile(jarName); BundleValidator bundleValidator = new BundleValidator(); - bundleValidator.validateJarFile(new BaseDeployLogger(), false, jarFile); + bundleValidator.validateJarFile(DeployState.createTestState(), jarFile); assert (false); } catch (IllegalArgumentException e) { assertEquals(e.getMessage(), exceptionMessage); @@ -50,47 +50,27 @@ public class BundleValidatorTest { public void require_that_deploying_snapshot_bundle_gives_warning() throws IOException { final StringBuffer buffer = new StringBuffer(); - DeployLogger logger = createDeployLogger(buffer); + DeployState state = createDeployState(buffer); JarFile jarFile = createTemporaryJarFile("snapshot_bundle"); - new BundleValidator().validateJarFile(logger, false, jarFile); + new BundleValidator().validateJarFile(state, jarFile); assertTrue(buffer.toString().contains("Deploying snapshot bundle")); } @Test public void outputs_deploy_warning_on_import_of_packages_from_deprecated_artifact() throws IOException { final StringBuffer buffer = new StringBuffer(); - DeployLogger logger = createDeployLogger(buffer); + DeployState state = createDeployState(buffer); BundleValidator validator = new BundleValidator(); JarFile jarFile = createTemporaryJarFile("import-warnings"); - validator.validateJarFile(logger, true, jarFile); + validator.validateJarFile(state, jarFile); assertThat(buffer.toString()) - .contains("For JAR file 'import-warnings.jar': \n" + - "Manifest imports the following Java packages from 'org.json:json': [org.json]. \n" + - "The org.json library will no longer provided by jdisc runtime on Vespa 8. See https://docs.vespa.ai/en/vespa8-release-notes.html#container-runtime."); + .contains("JAR file 'import-warnings.jar' imports the packages [org.json] from 'org.json:json'. \n" + + "This bundle is no longer provided on Vespa 8 - see https://docs.vespa.ai/en/vespa8-release-notes.html#container-runtime."); } - @Test - public void outputs_deploy_warnings_for_pom_xml() throws IOException { - StringBuffer buffer = new StringBuffer(); - DeployLogger logger = createDeployLogger(buffer); - BundleValidator validator = new BundleValidator(); - JarFile jarFile = createTemporaryJarFile("pom-xml-warnings"); - validator.validateJarFile(logger, true, jarFile); - String output = buffer.toString(); - assertThat(output) - .contains("The pom.xml of bundle 'pom-xml-warnings.jar' includes a dependency to the artifact " + - "'com.yahoo.vespa:vespa-http-client-extensions'. \n" + - "This artifact will be removed in Vespa 8. " + - "Programmatic use can be safely removed from system/staging tests. " + - "See internal Vespa 8 release notes for details.\n"); - assertThat(output) - .contains("\n" + - "<pluginRepositories> in pom.xml of 'pom-xml-warnings.jar' uses deprecated Maven repository " + - "'http://myartifactory:8000/artifactory/vespa-maven-libs-release-local'.\n See announcement."); - assertThat(output) - .contains("\n" + - "<repositories> in pom.xml of 'pom-xml-warnings.jar' uses deprecated Maven repository " + - "'http://myartifactory:8000/artifactory/vespa-maven-libs-release-local'.\n See announcement."); + private DeployState createDeployState(StringBuffer buffer) { + DeployLogger logger = (__, message) -> buffer.append(message).append('\n'); + return DeployState.createTestState(logger); } private JarFile createTemporaryJarFile(String testArtifact) throws IOException { @@ -114,7 +94,4 @@ public class BundleValidatorTest { return new JarFile(jarFile.toFile()); } - private DeployLogger createDeployLogger(StringBuffer buffer) { - return (__, message) -> buffer.append(message).append('\n'); - } } diff --git a/configdefinitions/src/vespa/zookeeper-server.def b/configdefinitions/src/vespa/zookeeper-server.def index 4d02dc67dfa..a2c4ec3b2db 100644 --- a/configdefinitions/src/vespa/zookeeper-server.def +++ b/configdefinitions/src/vespa/zookeeper-server.def @@ -32,10 +32,13 @@ juteMaxBuffer int default=52428800 myid int restart server[].id int server[].hostname string +server[].clientPort int default=2181 server[].quorumPort int default=2182 server[].electionPort int default=2183 # Whether this server is joining an existing cluster server[].joining bool default=false +# Whether this server is retired, and about to be removed +server[].retired bool default=false # Needed when upgrading from ZooKeeper 3.4 to 3.5, see https://issues.apache.org/jira/browse/ZOOKEEPER-3056, # and in general where there is a zookeeper ensemble running that has had few transactions. diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java index 9d51c5ca9d1..f0b681fd9c9 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.controller.api.integration.configserver; import com.yahoo.slime.Inspector; import com.yahoo.slime.SlimeUtils; -import org.apache.hc.core5.http.ClassicHttpRequest; import java.util.stream.Stream; @@ -41,7 +40,8 @@ public class ConfigServerException extends RuntimeException { PARENT_HOST_NOT_READY, CERTIFICATE_NOT_READY, LOAD_BALANCER_NOT_READY, - INCOMPLETE_RESPONSE + INCOMPLETE_RESPONSE, + CONFIG_NOT_CONVERGED } public static ConfigServerException readException(byte[] body, String context) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index a6d1800bf71..7b1cc1fd84d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -253,6 +253,7 @@ public class InternalStepRunner implements StepRunner { return result; case ACTIVATION_CONFLICT: case APPLICATION_LOCK_FAILURE: + case CONFIG_NOT_CONVERGED: logger.log("Deployment failed with possibly transient error " + e.code() + ", will retry: " + e.getMessage()); return result; diff --git a/searchlib/src/vespa/searchlib/diskindex/field_mergers_state.cpp b/searchlib/src/vespa/searchlib/diskindex/field_mergers_state.cpp index e62367c0f37..280800f1b58 100644 --- a/searchlib/src/vespa/searchlib/diskindex/field_mergers_state.cpp +++ b/searchlib/src/vespa/searchlib/diskindex/field_mergers_state.cpp @@ -5,15 +5,15 @@ #include "field_merger_task.h" #include "fusion_output_index.h" #include <vespa/searchcommon/common/schema.h> -#include <vespa/vespalib/util/threadexecutor.h> +#include <vespa/vespalib/util/executor.h> +#include <cassert> namespace search::diskindex { -FieldMergersState::FieldMergersState(const FusionOutputIndex& fusion_out_index, vespalib::ThreadExecutor& executor, std::shared_ptr<IFlushToken> flush_token) +FieldMergersState::FieldMergersState(const FusionOutputIndex& fusion_out_index, vespalib::Executor& executor, std::shared_ptr<IFlushToken> flush_token) : _fusion_out_index(fusion_out_index), _executor(executor), _flush_token(std::move(flush_token)), - _concurrent(std::max(1ul, _executor.getNumThreads() / 2)), _done(_fusion_out_index.get_schema().getNumIndexFields()), _failed(0u), _field_mergers(_fusion_out_index.get_schema().getNumIndexFields()) @@ -28,7 +28,6 @@ FieldMergersState::~FieldMergersState() FieldMerger& FieldMergersState::alloc_field_merger(uint32_t id) { - _concurrent.wait(); assert(id < _field_mergers.size()); auto field_merger = std::make_unique<FieldMerger>(id, _fusion_out_index, _flush_token); auto& result = *field_merger; @@ -46,7 +45,6 @@ FieldMergersState::destroy_field_merger(FieldMerger& field_merger) old_merger = std::move(_field_mergers[id]); assert(old_merger.get() == &field_merger); old_merger.reset(); - _concurrent.post(); _done.countDown(); } diff --git a/searchlib/src/vespa/searchlib/diskindex/field_mergers_state.h b/searchlib/src/vespa/searchlib/diskindex/field_mergers_state.h index 34c50c4d3e5..f4bad9a2b8c 100644 --- a/searchlib/src/vespa/searchlib/diskindex/field_mergers_state.h +++ b/searchlib/src/vespa/searchlib/diskindex/field_mergers_state.h @@ -2,12 +2,12 @@ #pragma once -#include <vespa/document/util/queue.h> #include <vespa/vespalib/util/count_down_latch.h> #include <atomic> +#include <vector> namespace search { class IFlushToken; } -namespace vespalib { class ThreadExecutor; } +namespace vespalib { class Executor; } namespace search::diskindex { @@ -20,16 +20,15 @@ class FusionOutputIndex; */ class FieldMergersState { const FusionOutputIndex& _fusion_out_index; - vespalib::ThreadExecutor& _executor; + vespalib::Executor& _executor; std::shared_ptr<IFlushToken> _flush_token; - document::Semaphore _concurrent; vespalib::CountDownLatch _done; std::atomic<uint32_t> _failed; std::vector<std::unique_ptr<FieldMerger>> _field_mergers; void destroy_field_merger(FieldMerger& field_merger); public: - FieldMergersState(const FusionOutputIndex& fusion_out_index, vespalib::ThreadExecutor& executor, std::shared_ptr<IFlushToken> flush_token); + FieldMergersState(const FusionOutputIndex& fusion_out_index, vespalib::Executor& executor, std::shared_ptr<IFlushToken> flush_token); ~FieldMergersState(); FieldMerger& alloc_field_merger(uint32_t id); void field_merger_done(FieldMerger& field_merger, bool failed); diff --git a/searchlib/src/vespa/searchlib/diskindex/fusion.cpp b/searchlib/src/vespa/searchlib/diskindex/fusion.cpp index 0fa634ef072..7d8bd4bc799 100644 --- a/searchlib/src/vespa/searchlib/diskindex/fusion.cpp +++ b/searchlib/src/vespa/searchlib/diskindex/fusion.cpp @@ -10,7 +10,6 @@ #include <vespa/searchlib/index/schemautil.h> #include <vespa/searchlib/util/dirtraverse.h> #include <vespa/vespalib/io/fileutil.h> -#include <vespa/vespalib/util/count_down_latch.h> #include <vespa/vespalib/util/error.h> #include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/util/lambdatask.h> @@ -72,7 +71,7 @@ Fusion::Fusion(const Schema& schema, const vespalib::string& dir, Fusion::~Fusion() = default; bool -Fusion::mergeFields(vespalib::ThreadExecutor & executor, std::shared_ptr<IFlushToken> flush_token) +Fusion::mergeFields(vespalib::Executor & executor, std::shared_ptr<IFlushToken> flush_token) { FieldMergersState field_mergers_state(_fusion_out_index, executor, flush_token); const Schema &schema = getSchema(); @@ -104,7 +103,7 @@ Fusion::readSchemaFiles() } bool -Fusion::merge(vespalib::ThreadExecutor& executor, std::shared_ptr<IFlushToken> flush_token) +Fusion::merge(vespalib::Executor& executor, std::shared_ptr<IFlushToken> flush_token) { FastOS_StatInfo statInfo; if (!FastOS_File::Stat(_fusion_out_index.get_path().c_str(), &statInfo)) { diff --git a/searchlib/src/vespa/searchlib/diskindex/fusion.h b/searchlib/src/vespa/searchlib/diskindex/fusion.h index 1f5c4471950..04edf77ea81 100644 --- a/searchlib/src/vespa/searchlib/diskindex/fusion.h +++ b/searchlib/src/vespa/searchlib/diskindex/fusion.h @@ -3,7 +3,7 @@ #pragma once #include "fusion_output_index.h" -#include <vespa/vespalib/util/threadexecutor.h> +#include <vespa/vespalib/util/executor.h> namespace search { class IFlushToken; @@ -25,7 +25,7 @@ class Fusion private: using Schema = index::Schema; - bool mergeFields(vespalib::ThreadExecutor& executor, std::shared_ptr<IFlushToken> flush_token); + bool mergeFields(vespalib::Executor& executor, std::shared_ptr<IFlushToken> flush_token); bool readSchemaFiles(); bool checkSchemaCompat(); @@ -43,7 +43,7 @@ public: ~Fusion(); void set_dynamic_k_pos_index_format(bool dynamic_k_pos_index_format) { _fusion_out_index.set_dynamic_k_pos_index_format(dynamic_k_pos_index_format); } void set_force_small_merge_chunk(bool force_small_merge_chunk) { _fusion_out_index.set_force_small_merge_chunk(force_small_merge_chunk); } - bool merge(vespalib::ThreadExecutor& executor, std::shared_ptr<IFlushToken> flush_token); + bool merge(vespalib::Executor& executor, std::shared_ptr<IFlushToken> flush_token); }; } 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 5fc6d2a69ae..da31f1c1a79 100644 --- a/staging_vespalib/src/tests/sequencedtaskexecutor/adaptive_sequenced_executor_test.cpp +++ b/staging_vespalib/src/tests/sequencedtaskexecutor/adaptive_sequenced_executor_test.cpp @@ -18,7 +18,7 @@ class Fixture public: AdaptiveSequencedExecutor _threads; - Fixture() : _threads(2, 2, 0, 1000) { } + Fixture(bool is_max_pending_hard=true) : _threads(2, 2, 0, 1000, is_max_pending_hard) { } }; @@ -231,12 +231,12 @@ TEST_F("require that executeLambda works", Fixture) } TEST("require that you get correct number of executors") { - AdaptiveSequencedExecutor seven(7, 1, 0, 10); + AdaptiveSequencedExecutor seven(7, 1, 0, 10, true); EXPECT_EQUAL(7u, seven.getNumExecutors()); } TEST("require that you distribute well") { - AdaptiveSequencedExecutor seven(7, 1, 0, 10); + AdaptiveSequencedExecutor seven(7, 1, 0, 10, true); EXPECT_EQUAL(7u, seven.getNumExecutors()); for (uint32_t id=0; id < 1000; id++) { EXPECT_EQUAL(id%7, seven.getExecutorId(id).getId()); diff --git a/staging_vespalib/src/tests/sequencedtaskexecutor/sequencedtaskexecutor_benchmark.cpp b/staging_vespalib/src/tests/sequencedtaskexecutor/sequencedtaskexecutor_benchmark.cpp index c609c538977..0f7c82ef988 100644 --- a/staging_vespalib/src/tests/sequencedtaskexecutor/sequencedtaskexecutor_benchmark.cpp +++ b/staging_vespalib/src/tests/sequencedtaskexecutor/sequencedtaskexecutor_benchmark.cpp @@ -55,7 +55,7 @@ int main(int argc, char **argv) { std::atomic<long> counter(0); std::unique_ptr<ISequencedTaskExecutor> executor; if (use_adaptive_executor) { - executor = std::make_unique<AdaptiveSequencedExecutor>(num_strands, num_threads, max_waiting, task_limit); + executor = std::make_unique<AdaptiveSequencedExecutor>(num_strands, num_threads, max_waiting, task_limit, true); } else { auto optimize = optimize_for_throughput ? vespalib::Executor::OptimizeFor::THROUGHPUT 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 4d08e14375c..1e23ba15785 100644 --- a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.cpp @@ -95,7 +95,7 @@ AdaptiveSequencedExecutor::maybe_block_self(std::unique_lock<std::mutex> &lock) while (_self.state == Self::State::BLOCKED) { _self.cond.wait(lock); } - while ((_self.state == Self::State::OPEN) && (_self.pending_tasks >= _cfg.max_pending)) { + while ((_self.state == Self::State::OPEN) && _cfg.is_above_max_pending(_self.pending_tasks)) { _self.state = Self::State::BLOCKED; while (_self.state == Self::State::BLOCKED) { _self.cond.wait(lock); @@ -228,7 +228,8 @@ AdaptiveSequencedExecutor::worker_main() } AdaptiveSequencedExecutor::AdaptiveSequencedExecutor(size_t num_strands, size_t num_threads, - size_t max_waiting, size_t max_pending) + size_t max_waiting, size_t max_pending, + bool is_max_pending_hard) : ISequencedTaskExecutor(num_strands), _thread_tools(std::make_unique<ThreadTools>(*this)), _mutex(), @@ -238,7 +239,7 @@ AdaptiveSequencedExecutor::AdaptiveSequencedExecutor(size_t num_strands, size_t _self(), _stats(), _idleTracker(steady_clock::now()), - _cfg(num_threads, max_waiting, max_pending) + _cfg(num_threads, max_waiting, max_pending, is_max_pending_hard) { _stats.queueSize.add(_self.pending_tasks); _thread_tools->start(num_threads); 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 ccf6ab977f3..d6244564fbd 100644 --- a/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h +++ b/staging_vespalib/src/vespa/vespalib/util/adaptive_sequenced_executor.h @@ -51,14 +51,22 @@ private: size_t max_waiting; size_t max_pending; size_t wakeup_limit; + bool is_max_pending_hard; void set_max_pending(size_t max_pending_in) { max_pending = std::max(1uL, max_pending_in); wakeup_limit = std::max(1uL, size_t(max_pending * 0.9)); assert(wakeup_limit > 0); assert(wakeup_limit <= max_pending); } - Config(size_t num_threads_in, size_t max_waiting_in, size_t max_pending_in) - : num_threads(num_threads_in), max_waiting(max_waiting_in), max_pending(1000), wakeup_limit(900) + bool is_above_max_pending(size_t pending) { + return (pending >= max_pending) && is_max_pending_hard; + } + Config(size_t num_threads_in, size_t max_waiting_in, size_t max_pending_in, bool is_max_pending_hard_in) + : num_threads(num_threads_in), + max_waiting(max_waiting_in), + max_pending(1000), + wakeup_limit(900), + is_max_pending_hard(is_max_pending_hard_in) { assert(num_threads > 0); set_max_pending(max_pending_in); @@ -143,7 +151,8 @@ private: void worker_main(); public: AdaptiveSequencedExecutor(size_t num_strands, size_t num_threads, - size_t max_waiting, size_t max_pending); + size_t max_waiting, size_t max_pending, + bool is_max_pending_hard); ~AdaptiveSequencedExecutor() override; ExecutorId getExecutorId(uint64_t component) const override; void executeTask(ExecutorId id, Task::UP task) override; diff --git a/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.cpp b/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.cpp index 88a679b4cdb..59ffad88d09 100644 --- a/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/sequencedtaskexecutor.cpp @@ -61,7 +61,7 @@ SequencedTaskExecutor::create(Runnable::init_fun_t func, uint32_t threads, uint3 { if (optimize == OptimizeFor::ADAPTIVE) { size_t num_strands = std::min(taskLimit, threads*32); - return std::make_unique<AdaptiveSequencedExecutor>(num_strands, threads, kindOfWatermark, taskLimit); + return std::make_unique<AdaptiveSequencedExecutor>(num_strands, threads, kindOfWatermark, taskLimit, is_task_limit_hard); } else { auto executors = std::vector<std::unique_ptr<SyncableThreadExecutor>>(); executors.reserve(threads); diff --git a/storage/src/tests/distributor/visitoroperationtest.cpp b/storage/src/tests/distributor/visitoroperationtest.cpp index ea99186a434..6c597b620dd 100644 --- a/storage/src/tests/distributor/visitoroperationtest.cpp +++ b/storage/src/tests/distributor/visitoroperationtest.cpp @@ -252,6 +252,20 @@ TEST_F(VisitorOperationTest, no_bucket) { runEmptyVisitor(msg)); } +TEST_F(VisitorOperationTest, none_fieldset_is_rejected) { + enable_cluster_state("distributor:1 storage:1"); + auto msg = std::make_shared<api::CreateVisitorCommand>( + makeBucketSpace(), "dumpvisitor", "instance", ""); + msg->addBucketToBeVisited(document::BucketId(16, 1)); + msg->addBucketToBeVisited(nullId); + msg->setFieldSet("[none]"); + + EXPECT_EQ("CreateVisitorReply(last=BucketId(0x0000000000000000)) " + "ReturnCode(ILLEGAL_PARAMETERS, Field set '[none]' is not supported " + "for external visitor operations. Use '[id]' to return documents with no fields set.)", + runEmptyVisitor(msg)); +} + TEST_F(VisitorOperationTest, only_super_bucket_and_progress_allowed) { enable_cluster_state("distributor:1 storage:1"); diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp index 42411d53d52..5abaad6ef9f 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "visitoroperation.h" +#include <vespa/document/fieldset/fieldsets.h> #include <vespa/storage/common/reindexing_constants.h> #include <vespa/storage/storageserver/storagemetricsset.h> #include <vespa/storage/distributor/top_level_distributor.h> @@ -347,6 +348,17 @@ VisitorOperation::verifyOperationSentToCorrectDistributor() verifyDistributorOwnsBucket(_superBucket.bid); } +void +VisitorOperation::verify_fieldset_makes_sense_for_visiting() +{ + if (_msg->getFieldSet() == document::NoFields::NAME) { + throw VisitorVerificationException( + api::ReturnCode::ILLEGAL_PARAMETERS, + "Field set '[none]' is not supported for external visitor operations. " + "Use '[id]' to return documents with no fields set."); + } +} + bool VisitorOperation::verifyCreateVisitorCommand(DistributorStripeMessageSender& sender) { @@ -354,6 +366,7 @@ VisitorOperation::verifyCreateVisitorCommand(DistributorStripeMessageSender& sen verifyOperationContainsBuckets(); verifyOperationHasSuperbucketAndProgress(); verifyOperationSentToCorrectDistributor(); + verify_fieldset_makes_sense_for_visiting(); // TODO wrap and test if (is_read_for_write() && (_msg->getMaxBucketsPerVisitor() != 1)) { throw VisitorVerificationException( diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h index c38c463a313..ce10ea87c11 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h @@ -106,6 +106,7 @@ private: void verifyOperationContainsBuckets(); void verifyOperationHasSuperbucketAndProgress(); void verifyOperationSentToCorrectDistributor(); + void verify_fieldset_makes_sense_for_visiting(); bool verifyCreateVisitorCommand(DistributorStripeMessageSender& sender); bool pickBucketsToVisit(const std::vector<BucketDatabase::Entry>& buckets); bool expandBucketContaining(); diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/api/FeedClientImpl.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/api/FeedClientImpl.java index 29ca1e2d4fd..fa214808ae3 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/api/FeedClientImpl.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/api/FeedClientImpl.java @@ -16,6 +16,8 @@ import java.time.Instant; import java.util.Optional; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.Logger; /** * Implementation of FeedClient. It is a thin layer on top of multiClusterHandler and multiClusterResultAggregator. @@ -24,6 +26,9 @@ import java.util.concurrent.TimeUnit; */ public class FeedClientImpl implements FeedClient { + private static final Logger log = Logger.getLogger(FeedClientImpl.class.getName()); + private static final AtomicBoolean warningPrinted = new AtomicBoolean(false); + private final Clock clock; private final OperationProcessor operationProcessor; private final long closeTimeoutMs; @@ -46,6 +51,10 @@ public class FeedClientImpl implements FeedClient { sessionParams, timeoutExecutor, clock); + if (warningPrinted.compareAndSet(false, true)) { + log.warning("The vespa-http-client is deprecated and will be removed in Vespa 8. " + + "See https://docs.vespa.ai/en/vespa8-release-notes.html"); + } } @Override diff --git a/zookeeper-server/CMakeLists.txt b/zookeeper-server/CMakeLists.txt index 6582b4602af..a4e4ddaeb2d 100644 --- a/zookeeper-server/CMakeLists.txt +++ b/zookeeper-server/CMakeLists.txt @@ -1,4 +1,3 @@ # Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. add_subdirectory(zookeeper-server-common) -add_subdirectory(zookeeper-server-3.6.3) add_subdirectory(zookeeper-server-3.7.0) diff --git a/zookeeper-server/pom.xml b/zookeeper-server/pom.xml index e482282388f..3efdde2cd81 100644 --- a/zookeeper-server/pom.xml +++ b/zookeeper-server/pom.xml @@ -13,7 +13,6 @@ <version>7-SNAPSHOT</version> <modules> <module>zookeeper-server-common</module> - <module>zookeeper-server-3.6.3</module> <module>zookeeper-server-3.7.0</module> </modules> <dependencies> diff --git a/zookeeper-server/zookeeper-server-3.6.3/CMakeLists.txt b/zookeeper-server/zookeeper-server-3.6.3/CMakeLists.txt deleted file mode 100644 index b7871cfbde1..00000000000 --- a/zookeeper-server/zookeeper-server-3.6.3/CMakeLists.txt +++ /dev/null @@ -1,4 +0,0 @@ -# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -install_fat_java_artifact(zookeeper-server-3.6.3) -# Needs to be included when this is the wanted default version (and symlinks for other versions need to be removed) -#install_symlink(lib/jars/zookeeper-server-3.6.3-jar-with-dependencies.jar lib/jars/zookeeper-server-jar-with-dependencies.jar) diff --git a/zookeeper-server/zookeeper-server-3.6.3/pom.xml b/zookeeper-server/zookeeper-server-3.6.3/pom.xml deleted file mode 100644 index f7e6f512f7c..00000000000 --- a/zookeeper-server/zookeeper-server-3.6.3/pom.xml +++ /dev/null @@ -1,120 +0,0 @@ -<?xml version="1.0"?> -<!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> -<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd"> - <modelVersion>4.0.0</modelVersion> - <parent> - <groupId>com.yahoo.vespa</groupId> - <artifactId>zookeeper-server</artifactId> - <version>7-SNAPSHOT</version> - <relativePath>../pom.xml</relativePath> - </parent> - <artifactId>zookeeper-server-3.6.3</artifactId> - <packaging>container-plugin</packaging> - <version>7-SNAPSHOT</version> - <dependencies> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>zookeeper-server-common</artifactId> - <version>${project.version}</version> - </dependency> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>zookeeper-client-common</artifactId> - <version>${project.version}</version> - <exclusions> - <exclusion> - <!-- Don't use ZK version from zookeeper-client-common --> - <groupId>org.apache.zookeeper</groupId> - <artifactId>zookeeper</artifactId> - </exclusion> - </exclusions> - </dependency> - <dependency> - <groupId>org.apache.zookeeper</groupId> - <artifactId>zookeeper</artifactId> - <version>3.6.3</version> - <exclusions> - <!-- - Container provides wiring for all common log libraries - Duplicate embedding results in various warnings being printed to stderr - --> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-log4j12</artifactId> - </exclusion> - <exclusion> - <groupId>log4j</groupId> - <artifactId>log4j</artifactId> - </exclusion> - </exclusions> - </dependency> - <!-- snappy-java and metrics-core are included here - to be able to work with ZooKeeper 3.6.3 due to - class loading issues --> - <dependency> - <groupId>io.dropwizard.metrics</groupId> - <artifactId>metrics-core</artifactId> - <scope>compile</scope> - <version>3.2.5</version> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - </exclusions> - </dependency> - <dependency> - <groupId>org.xerial.snappy</groupId> - <artifactId>snappy-java</artifactId> - <scope>compile</scope> - <version>1.1.7</version> - </dependency> - <dependency> - <groupId>junit</groupId> - <artifactId>junit</artifactId> - <scope>test</scope> - </dependency> - </dependencies> - <build> - <plugins> - <plugin> - <groupId>org.apache.maven.plugins</groupId> - <artifactId>maven-compiler-plugin</artifactId> - <configuration> - <compilerArgs> - <arg>-Xlint:all</arg> - <arg>-Werror</arg> - </compilerArgs> - </configuration> - </plugin> - <plugin> - <groupId>org.apache.maven.plugins</groupId> - <artifactId>maven-surefire-plugin</artifactId> - <configuration> - <redirectTestOutputToFile>${test.hide}</redirectTestOutputToFile> - <forkMode>once</forkMode> - </configuration> - </plugin> - <plugin> - <groupId>org.apache.maven.plugins</groupId> - <artifactId>maven-install-plugin</artifactId> - <configuration> - <updateReleaseInfo>true</updateReleaseInfo> - </configuration> - </plugin> - <plugin> - <groupId>com.yahoo.vespa</groupId> - <artifactId>bundle-plugin</artifactId> - <extensions>true</extensions> - <configuration> - <importPackage>com.sun.management</importPackage> - <bundleSymbolicName>zookeeper-server</bundleSymbolicName> - </configuration> - </plugin> - </plugins> - </build> -</project> diff --git a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java b/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java deleted file mode 100644 index c002ffa72ce..00000000000 --- a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.zookeeper; - -import com.google.inject.Inject; -import com.yahoo.cloud.config.ZookeeperServerConfig; -import com.yahoo.component.AbstractComponent; - -import java.nio.file.Path; -import java.time.Duration; -import java.util.concurrent.atomic.AtomicReference; - -/** - * Starts or reconfigures zookeeper cluster. - * The QuorumPeer conditionally created here is owned by the Reconfigurer; - * when it already has a peer, that peer is used here in case start or shutdown is required. - * - * @author hmusum - */ -public class ReconfigurableVespaZooKeeperServer extends AbstractComponent implements VespaZooKeeperServer { - - private final AtomicReference<QuorumPeer> peer = new AtomicReference<>(); - - @Inject - public ReconfigurableVespaZooKeeperServer(Reconfigurer reconfigurer, ZookeeperServerConfig zookeeperServerConfig) { - reconfigurer.startOrReconfigure(zookeeperServerConfig, this, VespaQuorumPeer::new, peer::set); - } - - @Override - public void shutdown() { - peer.get().shutdown(Duration.ofMinutes(1)); - } - - @Override - public void start(Path configFilePath) { - peer.get().start(configFilePath); - } - - @Override - public boolean reconfigurable() { - return true; - } - -} diff --git a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaMtlsAuthenticationProvider.java b/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaMtlsAuthenticationProvider.java deleted file mode 100644 index 66742b0e05b..00000000000 --- a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaMtlsAuthenticationProvider.java +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.zookeeper; - -import org.apache.zookeeper.KeeperException; -import org.apache.zookeeper.common.X509Exception; -import org.apache.zookeeper.data.Id; -import org.apache.zookeeper.server.ServerCnxn; -import org.apache.zookeeper.server.auth.AuthenticationProvider; -import org.apache.zookeeper.server.auth.X509AuthenticationProvider; - -import java.security.cert.X509Certificate; -import java.util.logging.Logger; - -/** - * A {@link AuthenticationProvider} to be used in combination with Vespa mTLS - * - * @author bjorncs - */ -public class VespaMtlsAuthenticationProvider extends X509AuthenticationProvider { - - private static final Logger log = Logger.getLogger(VespaMtlsAuthenticationProvider.class.getName()); - - public VespaMtlsAuthenticationProvider() throws X509Exception { super(null, null);} - - @Override - public KeeperException.Code handleAuthentication(ServerCnxn cnxn, byte[] authData) { - // Vespa's mTLS peer authorization rules are performed by the underlying trust manager implementation. - // The client is authorized once the SSL handshake has completed. - X509Certificate[] certificateChain = (X509Certificate[]) cnxn.getClientCertificateChain(); - if (certificateChain == null || certificateChain.length == 0) { - log.warning("Client not authenticated - should not be possible with clientAuth=NEED"); - return KeeperException.Code.AUTHFAILED; - } - X509Certificate certificate = certificateChain[0]; - cnxn.addAuthInfo(new Id(getScheme(), certificate.getSubjectX500Principal().getName())); - return KeeperException.Code.OK; - } - - @Override public String getScheme() { return "x509"; } - -} diff --git a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaQuorumPeer.java b/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaQuorumPeer.java deleted file mode 100644 index 47ec03367c1..00000000000 --- a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaQuorumPeer.java +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.zookeeper; - -import com.yahoo.protect.Process; -import org.apache.zookeeper.server.admin.AdminServer; -import org.apache.zookeeper.server.quorum.QuorumPeerConfig; -import org.apache.zookeeper.server.quorum.QuorumPeerMain; - -import java.io.IOException; -import java.nio.file.Path; -import java.time.Duration; -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * Starts/stops a ZooKeeper server. Extends QuorumPeerMain to be able to call initializeAndRun() and wraps - * exceptions so it can be used by code that does not depend on ZooKeeper. - * - * @author hmusum - */ -class VespaQuorumPeer extends QuorumPeerMain implements QuorumPeer { - - private static final Logger log = java.util.logging.Logger.getLogger(VespaQuorumPeer.class.getName()); - - @Override - public void start(Path path) { - initializeAndRun(new String[]{ path.toFile().getAbsolutePath()}); - } - - @Override - public void shutdown(Duration timeout) { - if (quorumPeer != null) { - log.log(Level.FINE, "Shutting down ZooKeeper server"); - try { - quorumPeer.shutdown(); - quorumPeer.join(timeout.toMillis()); // Wait for shutdown to complete - if (quorumPeer.isAlive()) - throw new IllegalStateException("Peer still alive after " + timeout); - } catch (RuntimeException | InterruptedException e) { - // If shutdown fails, we have no other option than forcing the JVM to stop and letting it be restarted. - // - // When a VespaZooKeeperServer component receives a new config, the container will try to start a new - // server with the new config, this will fail until the old server is deconstructed. If the old server - // fails to deconstruct/shut down, the new one will never start and if that happens forcing a restart is - // the better option. - Process.logAndDie("Failed to shut down ZooKeeper server properly, forcing shutdown", e); - } - } - } - - @Override - protected void initializeAndRun(String[] args) { - try { - super.initializeAndRun(args); - } catch (QuorumPeerConfig.ConfigException | IOException | AdminServer.AdminServerException e) { - throw new RuntimeException("Exception when initializing or running ZooKeeper server", e); - } - } - -} diff --git a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java b/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java deleted file mode 100644 index 27aa18c64c7..00000000000 --- a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.zookeeper; - -import com.yahoo.vespa.zookeeper.client.ZkClientConfigBuilder; -import org.apache.zookeeper.KeeperException; -import org.apache.zookeeper.admin.ZooKeeperAdmin; - -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * @author hmusum - */ -@SuppressWarnings("unused") // Created by injection -public class VespaZooKeeperAdminImpl implements VespaZooKeeperAdmin { - - private static final Logger log = java.util.logging.Logger.getLogger(VespaZooKeeperAdminImpl.class.getName()); - - @Override - public void reconfigure(String connectionSpec, String joiningServers, String leavingServers) throws ReconfigException { - ZooKeeperAdmin zooKeeperAdmin = null; - try { - zooKeeperAdmin = createAdmin(connectionSpec); - long fromConfig = -1; - // Using string parameters because the List variant of reconfigure fails to join empty lists (observed on 3.5.6, fixed in 3.7.0) - byte[] appliedConfig = zooKeeperAdmin.reconfigure(joiningServers, leavingServers, null, fromConfig, null); - log.log(Level.INFO, "Applied ZooKeeper config: " + new String(appliedConfig, StandardCharsets.UTF_8)); - } catch (KeeperException e) { - if (retryOn(e)) - throw new ReconfigException(e); - else - throw new RuntimeException(e); - } catch (IOException | InterruptedException e) { - throw new RuntimeException(e); - } finally { - if (zooKeeperAdmin != null) { - try { - zooKeeperAdmin.close(); - } catch (InterruptedException e) { /* ignore */} - } - } - } - - private ZooKeeperAdmin createAdmin(String connectionSpec) throws IOException { - return new ZooKeeperAdmin(connectionSpec, (int) sessionTimeout().toMillis(), - (event) -> log.log(Level.INFO, event.toString()), new ZkClientConfigBuilder().toConfig()); - } - - private static boolean retryOn(KeeperException e) { - return e instanceof KeeperException.ReconfigInProgress || - e instanceof KeeperException.ConnectionLossException || - e instanceof KeeperException.NewConfigNoQuorum; - } - -} - diff --git a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImpl.java b/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImpl.java deleted file mode 100644 index 8f3a5a91a43..00000000000 --- a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImpl.java +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.zookeeper; - -import com.google.inject.Inject; -import com.yahoo.cloud.config.ZookeeperServerConfig; -import com.yahoo.component.AbstractComponent; - -import java.nio.file.Path; -import java.time.Duration; - -/** - * @author Ulf Lilleengen - * @author Harald Musum - */ -public class VespaZooKeeperServerImpl extends AbstractComponent implements VespaZooKeeperServer { - - private final VespaQuorumPeer peer; - private final ZooKeeperRunner runner; - - @Inject - public VespaZooKeeperServerImpl(ZookeeperServerConfig zookeeperServerConfig) { - this.peer = new VespaQuorumPeer(); - this.runner = new ZooKeeperRunner(zookeeperServerConfig, this); - } - - @Override - public void deconstruct() { - runner.shutdown(); - super.deconstruct(); - } - - @Override - public void shutdown() { - peer.shutdown(Duration.ofMinutes(1)); - } - - @Override - public void start(Path configFilePath) { - peer.start(configFilePath); - } - - @Override - public boolean reconfigurable() { - return false; - } - -} diff --git a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/org/apache/zookeeper/common/NetUtils.java b/zookeeper-server/zookeeper-server-3.6.3/src/main/java/org/apache/zookeeper/common/NetUtils.java deleted file mode 100644 index 33ec9b1303a..00000000000 --- a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/org/apache/zookeeper/common/NetUtils.java +++ /dev/null @@ -1,94 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.zookeeper.common; - -import java.net.Inet6Address; -import java.net.InetAddress; -import java.net.InetSocketAddress; - -/** - * This class contains common utilities for netstuff. Like printing IPv6 literals correctly - */ -public class NetUtils { - - // Note: Changed from original to use hostname from InetSocketAddress if there exists one - public static String formatInetAddr(InetSocketAddress addr) { - String hostName = addr.getHostName(); - if (hostName != null) { - return String.format("%s:%s", hostName, addr.getPort()); - } - - InetAddress ia = addr.getAddress(); - - if (ia == null) { - return String.format("%s:%s", addr.getHostString(), addr.getPort()); - } - if (ia instanceof Inet6Address) { - return String.format("[%s]:%s", ia.getHostAddress(), addr.getPort()); - } else { - return String.format("%s:%s", ia.getHostAddress(), addr.getPort()); - } - } - - /** - * Separates host and port from given host port string if host port string is enclosed - * within square bracket. - * - * @param hostPort host port string - * @return String[]{host, port} if host port string is host:port - * or String[] {host, port:port} if host port string is host:port:port - * or String[] {host} if host port string is host - * or String[]{} if not a ipv6 host port string. - */ - public static String[] getIPV6HostAndPort(String hostPort) { - if (hostPort.startsWith("[")) { - int i = hostPort.lastIndexOf(']'); - if (i < 0) { - throw new IllegalArgumentException( - hostPort + " starts with '[' but has no matching ']'"); - } - String host = hostPort.substring(1, i); - if (host.isEmpty()) { - throw new IllegalArgumentException(host + " is empty."); - } - if (hostPort.length() > i + 1) { - return getHostPort(hostPort, i, host); - } - return new String[] { host }; - } else { - //Not an IPV6 host port string - return new String[] {}; - } - } - - private static String[] getHostPort(String hostPort, int indexOfClosingBracket, String host) { - // [127::1]:2181 , check separator : exits - if (hostPort.charAt(indexOfClosingBracket + 1) != ':') { - throw new IllegalArgumentException(hostPort + " does not have : after ]"); - } - // [127::1]: scenario - if (indexOfClosingBracket + 2 == hostPort.length()) { - throw new IllegalArgumentException(hostPort + " doesn't have a port after colon."); - } - //do not include - String port = hostPort.substring(indexOfClosingBracket + 2); - return new String[] { host, port }; - } -} diff --git a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/org/apache/zookeeper/server/VespaNettyServerCnxnFactory.java b/zookeeper-server/zookeeper-server-3.6.3/src/main/java/org/apache/zookeeper/server/VespaNettyServerCnxnFactory.java deleted file mode 100644 index fdfe0fe8467..00000000000 --- a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/org/apache/zookeeper/server/VespaNettyServerCnxnFactory.java +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package org.apache.zookeeper.server; - -import com.yahoo.vespa.zookeeper.Configurator; - -import java.io.IOException; -import java.net.InetSocketAddress; -import java.util.logging.Logger; - -/** - * Overrides secure setting with value from {@link Configurator}. - * Workaround for incorrect handling of clientSecurePort in combination with ZooKeeper Dynamic Reconfiguration in 3.6.2 - * See https://issues.apache.org/jira/browse/ZOOKEEPER-3577. - * - * Using package {@link org.apache.zookeeper.server} as {@link NettyServerCnxnFactory#NettyServerCnxnFactory()} is package-private. - * - * @author bjorncs - */ -public class VespaNettyServerCnxnFactory extends NettyServerCnxnFactory { - - private static final Logger log = Logger.getLogger(VespaNettyServerCnxnFactory.class.getName()); - - private final boolean isSecure; - - public VespaNettyServerCnxnFactory() { - super(); - this.isSecure = Configurator.VespaNettyServerCnxnFactory_isSecure; - boolean portUnificationEnabled = Boolean.getBoolean(NettyServerCnxnFactory.PORT_UNIFICATION_KEY); - log.info(String.format("For %h: isSecure=%b, portUnification=%b", this, isSecure, portUnificationEnabled)); - } - - @Override - public void configure(InetSocketAddress addr, int maxClientCnxns, int backlog, boolean secure) throws IOException { - log.info(String.format("For %h: configured() invoked with parameter 'secure'=%b, overridden to %b", this, secure, isSecure)); - super.configure(addr, maxClientCnxns, backlog, isSecure); - } -} diff --git a/zookeeper-server/zookeeper-server-3.7.0/pom.xml b/zookeeper-server/zookeeper-server-3.7.0/pom.xml index ac7db35e6af..01fd83a496b 100644 --- a/zookeeper-server/zookeeper-server-3.7.0/pom.xml +++ b/zookeeper-server/zookeeper-server-3.7.0/pom.xml @@ -11,6 +11,9 @@ <artifactId>zookeeper-server-3.7.0</artifactId> <packaging>container-plugin</packaging> <version>7-SNAPSHOT</version> + <properties> + <zookeeper.version>3.7.0</zookeeper.version> + </properties> <dependencies> <dependency> <groupId>com.yahoo.vespa</groupId> @@ -32,7 +35,7 @@ <dependency> <groupId>org.apache.zookeeper</groupId> <artifactId>zookeeper</artifactId> - <version>3.7.0</version> + <version>${zookeeper.version}</version> <exclusions> <!-- Container provides wiring for all common log libraries @@ -87,7 +90,6 @@ <configuration> <compilerArgs> <arg>-Xlint:all</arg> - <arg>-Werror</arg> </compilerArgs> </configuration> </plugin> @@ -97,6 +99,9 @@ <configuration> <redirectTestOutputToFile>${test.hide}</redirectTestOutputToFile> <forkMode>once</forkMode> + <systemPropertyVariables> + <zk-version>${zookeeper.version}</zk-version> + </systemPropertyVariables> </configuration> </plugin> <plugin> diff --git a/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java b/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java index c002ffa72ce..246911fdfc7 100644 --- a/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java +++ b/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java @@ -18,21 +18,21 @@ import java.util.concurrent.atomic.AtomicReference; */ public class ReconfigurableVespaZooKeeperServer extends AbstractComponent implements VespaZooKeeperServer { - private final AtomicReference<QuorumPeer> peer = new AtomicReference<>(); + private QuorumPeer peer; @Inject public ReconfigurableVespaZooKeeperServer(Reconfigurer reconfigurer, ZookeeperServerConfig zookeeperServerConfig) { - reconfigurer.startOrReconfigure(zookeeperServerConfig, this, VespaQuorumPeer::new, peer::set); + peer = reconfigurer.startOrReconfigure(zookeeperServerConfig, this, () -> peer = new VespaQuorumPeer()); } @Override public void shutdown() { - peer.get().shutdown(Duration.ofMinutes(1)); + peer.shutdown(Duration.ofMinutes(1)); } @Override public void start(Path configFilePath) { - peer.get().start(configFilePath); + peer.start(configFilePath); } @Override diff --git a/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java b/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java index 27aa18c64c7..ae7bf8d84f5 100644 --- a/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java +++ b/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java @@ -2,11 +2,15 @@ package com.yahoo.vespa.zookeeper; import com.yahoo.vespa.zookeeper.client.ZkClientConfigBuilder; +import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.ZooDefs; import org.apache.zookeeper.admin.ZooKeeperAdmin; +import org.apache.zookeeper.data.ACL; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -19,27 +23,28 @@ public class VespaZooKeeperAdminImpl implements VespaZooKeeperAdmin { private static final Logger log = java.util.logging.Logger.getLogger(VespaZooKeeperAdminImpl.class.getName()); @Override - public void reconfigure(String connectionSpec, String joiningServers, String leavingServers) throws ReconfigException { - ZooKeeperAdmin zooKeeperAdmin = null; - try { - zooKeeperAdmin = createAdmin(connectionSpec); + public void reconfigure(String connectionSpec, String servers) throws ReconfigException { + try (ZooKeeperAdmin zooKeeperAdmin = createAdmin(connectionSpec)) { long fromConfig = -1; - // Using string parameters because the List variant of reconfigure fails to join empty lists (observed on 3.5.6, fixed in 3.7.0) - byte[] appliedConfig = zooKeeperAdmin.reconfigure(joiningServers, leavingServers, null, fromConfig, null); + // Using string parameters because the List variant of reconfigure fails to join empty lists (observed on 3.5.6, fixed in 3.7.0). + log.log(Level.INFO, "Applying ZooKeeper config: " + servers); + byte[] appliedConfig = zooKeeperAdmin.reconfigure(null, null, servers, fromConfig, null); log.log(Level.INFO, "Applied ZooKeeper config: " + new String(appliedConfig, StandardCharsets.UTF_8)); - } catch (KeeperException e) { - if (retryOn(e)) - throw new ReconfigException(e); - else - throw new RuntimeException(e); - } catch (IOException | InterruptedException e) { + + // Verify by issuing a write operation; this is only accepted once new quorum is obtained. + List<ACL> acl = ZooDefs.Ids.OPEN_ACL_UNSAFE; + String node = zooKeeperAdmin.create("/reconfigure-dummy-node", new byte[0], acl, CreateMode.EPHEMERAL_SEQUENTIAL); + zooKeeperAdmin.delete(node, -1); + + log.log(Level.INFO, "Verified ZooKeeper config: " + new String(appliedConfig, StandardCharsets.UTF_8)); + } + catch ( KeeperException.ReconfigInProgress + | KeeperException.ConnectionLossException + | KeeperException.NewConfigNoQuorum e) { + throw new ReconfigException(e); + } + catch (KeeperException | IOException | InterruptedException e) { throw new RuntimeException(e); - } finally { - if (zooKeeperAdmin != null) { - try { - zooKeeperAdmin.close(); - } catch (InterruptedException e) { /* ignore */} - } } } @@ -48,11 +53,5 @@ public class VespaZooKeeperAdminImpl implements VespaZooKeeperAdmin { (event) -> log.log(Level.INFO, event.toString()), new ZkClientConfigBuilder().toConfig()); } - private static boolean retryOn(KeeperException e) { - return e instanceof KeeperException.ReconfigInProgress || - e instanceof KeeperException.ConnectionLossException || - e instanceof KeeperException.NewConfigNoQuorum; - } - } diff --git a/zookeeper-server/zookeeper-server-3.7.0/src/test/java/com/yahoo/vespa/zookeper/VespaZooKeeperTest.java b/zookeeper-server/zookeeper-server-3.7.0/src/test/java/com/yahoo/vespa/zookeper/VespaZooKeeperTest.java new file mode 100644 index 00000000000..db643d76e0d --- /dev/null +++ b/zookeeper-server/zookeeper-server-3.7.0/src/test/java/com/yahoo/vespa/zookeper/VespaZooKeeperTest.java @@ -0,0 +1,259 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.zookeper; + +import com.yahoo.cloud.config.ZookeeperServerConfig; +import com.yahoo.net.HostName; +import com.yahoo.vespa.zookeeper.ReconfigurableVespaZooKeeperServer; +import com.yahoo.vespa.zookeeper.Reconfigurer; +import com.yahoo.vespa.zookeeper.VespaZooKeeperAdminImpl; +import com.yahoo.vespa.zookeeper.client.ZkClientConfigBuilder; +import org.apache.zookeeper.CreateMode; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.ZooDefs; +import org.apache.zookeeper.admin.ZooKeeperAdmin; +import org.apache.zookeeper.data.ACL; +import org.apache.zookeeper.data.Stat; +import org.junit.Ignore; +import org.junit.Test; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.ServerSocket; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.Phaser; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.IntStream; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.concurrent.TimeUnit.SECONDS; +import static java.util.stream.Collectors.toList; +import static org.junit.Assert.assertEquals; + +public class VespaZooKeeperTest { + + static final Path tempDirRoot = getTmpDir(); + static final List<Integer> ports = new ArrayList<>(); + + /** + * Performs dynamic reconfiguration of ZooKeeper servers. + * + * First, a cluster of 3 servers is set up, and some data is written to it. + * Then, 3 new servers are added, and the first 3 marked for retirement; + * this should force the quorum to move the 3 new servers, but not disconnect the old ones. + * Next, the old servers are removed. + * Then, the cluster is reduced to size 1. + * Finally, the cluster grows to size 3 again. + * + * Throughout all of this, quorum should remain, and the data should remain the same. + */ + @Test(timeout = 120_000) + @Ignore // Unstable, some ZK server keeps resetting connections sometimes. + public void testReconfiguration() throws ExecutionException, InterruptedException, IOException, KeeperException, TimeoutException { + List<ZooKeeper> keepers = new ArrayList<>(); + for (int i = 0; i < 8; i++) keepers.add(new ZooKeeper()); + for (int i = 0; i < 8; i++) keepers.get(i).run(); + + // Start the first three servers. + List<ZookeeperServerConfig> configs = getConfigs(0, 0, 3, 0); + for (int i = 0; i < 3; i++) keepers.get(i).config = configs.get(i); + for (int i = 0; i < 3; i++) keepers.get(i).phaser.arriveAndAwaitAdvance(); + + // Wait for all servers to be up and running. + for (int i = 0; i < 3; i++) keepers.get(i).phaser.arriveAndAwaitAdvance(); + + // Write data to verify later. + String path = writeData(configs.get(0)); + + // Let three new servers join, causing the three older ones to retire and leave the ensemble. + configs = getConfigs(0, 3, 3, 3); + for (int i = 0; i < 6; i++) keepers.get(i).config = configs.get(i); + // The existing servers can't reconfigure and leave before the joiners are up. + for (int i = 0; i < 6; i++) keepers.get(i).phaser.arriveAndAwaitAdvance(); + + // Wait for new quorum to be established. + for (int i = 0; i < 6; i++) keepers.get(i).phaser.arriveAndAwaitAdvance(); + + // Verify written data is preserved. + verifyData(path, configs.get(3)); + + // Old servers are removed. + configs = getConfigs(3, 0, 3, 0); + for (int i = 0; i < 6; i++) keepers.get(i).config = configs.get(i); + // Old servers shut down, while the newer servers remain. + for (int i = 0; i < 6; i++) keepers.get(i).phaser.arriveAndAwaitAdvance(); + // Ensure old servers shut down properly. + for (int i = 0; i < 3; i++) keepers.get(i).await(); + // Ensure new servers have reconfigured. + for (int i = 3; i < 6; i++) keepers.get(i).phaser.arriveAndAwaitAdvance(); + + // Verify written data is preserved. + verifyData(path, configs.get(3)); + + + // Cluster shrinks to a single server. + configs = getConfigs(5, 0, 1, 0); + for (int i = 3; i < 6; i++) keepers.get(i).config = configs.get(i); + for (int i = 5; i < 6; i++) keepers.get(i).phaser.arriveAndAwaitAdvance(); + for (int i = 5; i < 6; i++) keepers.get(i).phaser.arriveAndAwaitAdvance(); + // We let the remaining server reconfigure the others out before they die. + for (int i = 3; i < 5; i++) keepers.get(i).phaser.arriveAndAwaitAdvance(); + for (int i = 3; i < 5; i++) keepers.get(i).await(); + verifyData(path, configs.get(5)); + + // Cluster grows to 3 servers again. + configs = getConfigs(5, 0, 3, 2); + for (int i = 5; i < 8; i++) keepers.get(i).config = configs.get(i); + for (int i = 5; i < 8; i++) keepers.get(i).phaser.arriveAndAwaitAdvance(); + // Wait for the joiners. + for (int i = 5; i < 8; i++) keepers.get(i).phaser.arriveAndAwaitAdvance(); + verifyData(path, configs.get(7)); + + // Let the remaining servers terminate. + for (int i = 5; i < 8; i++) keepers.get(i).config = null; + for (int i = 5; i < 8; i++) keepers.get(i).phaser.arriveAndAwaitAdvance(); + for (int i = 5; i < 8; i++) keepers.get(i).await(); + } + + static String writeData(ZookeeperServerConfig config) throws IOException, InterruptedException, KeeperException { + try (ZooKeeperAdmin admin = createAdmin(config)) { + List<ACL> acl = ZooDefs.Ids.OPEN_ACL_UNSAFE; + String node = admin.create("/test-node", "hi".getBytes(UTF_8), acl, CreateMode.EPHEMERAL_SEQUENTIAL); + String read = new String(admin.getData(node, false, new Stat()), UTF_8); + assertEquals("hi", read); + return node; + } + } + + static void verifyData(String path, ZookeeperServerConfig config) throws IOException, InterruptedException, KeeperException { + for (int i = 0; i < 10; i++) { + try (ZooKeeperAdmin admin = createAdmin(config)) { + assertEquals("hi", new String(admin.getData(path, false, new Stat()), UTF_8)); + return; + } + catch (KeeperException.ConnectionLossException e) { + e.printStackTrace(); + Thread.sleep(10 << i); + } + } + } + + static ZooKeeperAdmin createAdmin(ZookeeperServerConfig config) throws IOException { + return new ZooKeeperAdmin(HostName.getLocalhost() + ":" + config.clientPort(), + 10_000, + System.err::println, + new ZkClientConfigBuilder().toConfig()); + } + + static class ZooKeeper { + + final ExecutorService executor = Executors.newSingleThreadExecutor(); + final Phaser phaser = new Phaser(2); + final AtomicReference<Future<?>> future = new AtomicReference<>(); + ZookeeperServerConfig config; + + void run() { + future.set(executor.submit(() -> { + Reconfigurer reconfigurer = new Reconfigurer(new VespaZooKeeperAdminImpl()); + phaser.arriveAndAwaitAdvance(); + while (config != null) { + new ReconfigurableVespaZooKeeperServer(reconfigurer, config); + phaser.arriveAndAwaitAdvance(); // server is now up, let test thread sync here + phaser.arriveAndAwaitAdvance(); // wait before reconfig/teardown to let test thread do stuff + } + reconfigurer.deconstruct(); + })); + } + + void await() throws ExecutionException, InterruptedException, TimeoutException { + future.get().get(30, SECONDS); + } + } + + static List<ZookeeperServerConfig> getConfigs(int removed, int retired, int active, int joining) { + return IntStream.rangeClosed(1, removed + retired + active) + .mapToObj(id -> getConfig(removed, retired, active, joining, id)) + .collect(toList()); + } + + // Config for server #id among retired + active servers, of which the last may be joining, and with offset removed. + static ZookeeperServerConfig getConfig(int removed, int retired, int active, int joining, int id) { + if (id <= removed) + return null; + + Path tempDir = tempDirRoot.resolve("zookeeper-" + id); + return new ZookeeperServerConfig.Builder() + .clientPort(getPorts(id).get(0)) + .dataDir(tempDir.toString()) + .zooKeeperConfigFile(tempDir.resolve("zookeeper.cfg").toString()) + .myid(id) + .myidFile(tempDir.resolve("myid").toString()) + .dynamicReconfiguration(true) + .server(IntStream.rangeClosed(removed + 1, removed + retired + active) + .mapToObj(i -> new ZookeeperServerConfig.Server.Builder() + .id(i) + .clientPort(getPorts(i).get(0)) + .electionPort(getPorts(i).get(1)) + .quorumPort(getPorts(i).get(2)) + .hostname("localhost") + .joining(i - removed > retired + active - joining) + .retired(i - removed <= retired)) + .collect(toList())) + .build(); + } + + static List<Integer> getPorts(int id) { + if (ports.size() < id * 3) { + int previousPort; + if (ports.isEmpty()) { + String[] version = System.getProperty("zk-version").split("\\."); + int versionPortOffset = 0; + for (String part : version) + versionPortOffset = 32 * (versionPortOffset + Integer.parseInt(part)); + previousPort = 20000 + versionPortOffset % 30000; + } + else + previousPort = ports.get(ports.size() - 1); + + for (int i = 0; i < 3; i++) + ports.add(previousPort = nextPort(previousPort)); + } + return ports.subList(id * 3 - 3, id * 3); + } + + static int nextPort(int previousPort) { + for (int j = 1; j <= 30000; j++) { + int port = (previousPort + j); + while (port > 50000) + port -= 30000; + + try (ServerSocket socket = new ServerSocket(port)) { + return socket.getLocalPort(); + } + catch (IOException e) { + System.err.println("Could not bind port " + port + ": " + e); + } + } + throw new RuntimeException("No free ports"); + } + + static Path getTmpDir() { + try { + Path tempDir = Files.createTempDirectory(Paths.get(System.getProperty("java.io.tmpdir")), "vespa-zk-test"); + tempDir.toFile().deleteOnExit(); + return tempDir.toAbsolutePath(); + } + catch (IOException e) { + throw new UncheckedIOException(e); + } + } + +} diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java index 39d0312915f..8b22f658a94 100644 --- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java +++ b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java @@ -86,7 +86,7 @@ public class Configurator { sb.append("reconfigEnabled=true").append("\n"); sb.append("skipACL=yes").append("\n"); ensureThisServerIsRepresented(config.myid(), config.server()); - config.server().forEach(server -> addServerToCfg(sb, server, config.clientPort())); + config.server().forEach(server -> sb.append(serverSpec(server, config.clientPort(), server.joining())).append("\n")); sb.append(new TlsQuorumConfig().createConfig(vespaTlsConfig)); sb.append(new TlsClientServerConfig().createConfig(vespaTlsConfig)); return sb.toString(); @@ -111,7 +111,8 @@ public class Configurator { } } - private void addServerToCfg(StringBuilder sb, ZookeeperServerConfig.Server server, int clientPort) { + static String serverSpec(ZookeeperServerConfig.Server server, int clientPort, boolean joining) { + StringBuilder sb = new StringBuilder(); sb.append("server.") .append(server.id()) .append("=") @@ -120,7 +121,7 @@ public class Configurator { .append(server.quorumPort()) .append(":") .append(server.electionPort()); - if (server.joining()) { + if (joining) { // Servers that are joining an existing cluster must be marked as observers. Note that this will NOT // actually make the server an observer, but prevent it from forming an ensemble independently of the // existing cluster. @@ -130,8 +131,8 @@ public class Configurator { .append("observer"); } sb.append(";") - .append(clientPort) - .append("\n"); + .append(server.clientPort()); + return sb.toString(); } static List<String> zookeeperServerHostnames(ZookeeperServerConfig zookeeperServerConfig) { diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java index d4223e4d815..604419c063d 100644 --- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java +++ b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java @@ -10,14 +10,14 @@ import com.yahoo.yolean.Exceptions; import java.time.Duration; import java.time.Instant; -import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.function.Consumer; import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Collectors; + +import static com.yahoo.vespa.zookeeper.Configurator.serverSpec; +import static java.util.stream.Collectors.toList; /** * Starts zookeeper server and supports reconfiguring zookeeper cluster. Keep this as a component @@ -50,17 +50,22 @@ public class Reconfigurer extends AbstractComponent { this.sleeper = Objects.requireNonNull(sleeper); } - void startOrReconfigure(ZookeeperServerConfig newConfig, VespaZooKeeperServer server, - Supplier<QuorumPeer> quorumPeerGetter, Consumer<QuorumPeer> quorumPeerSetter) { + @Override + public void deconstruct() { + shutdown(); + } + + QuorumPeer startOrReconfigure(ZookeeperServerConfig newConfig, VespaZooKeeperServer server, + Supplier<QuorumPeer> quorumPeerCreator) { if (zooKeeperRunner == null) { - peer = quorumPeerGetter.get(); // Obtain the peer from the server. This will be shared with later servers. + peer = quorumPeerCreator.get(); // Obtain the peer from the server. This will be shared with later servers. zooKeeperRunner = startServer(newConfig, server); } - quorumPeerSetter.accept(peer); - if (shouldReconfigure(newConfig)) { + if (newConfig.dynamicReconfiguration()) { reconfigure(newConfig); } + return peer; } ZookeeperServerConfig activeConfig() { @@ -73,42 +78,30 @@ public class Reconfigurer extends AbstractComponent { } } - private boolean shouldReconfigure(ZookeeperServerConfig newConfig) { - if (!newConfig.dynamicReconfiguration()) return false; - if (activeConfig == null) return false; - return !newConfig.equals(activeConfig()); - } - private ZooKeeperRunner startServer(ZookeeperServerConfig zookeeperServerConfig, VespaZooKeeperServer server) { ZooKeeperRunner runner = new ZooKeeperRunner(zookeeperServerConfig, server); activeConfig = zookeeperServerConfig; return runner; } + // TODO jonmv: read dynamic file, discard if old quorum impossible (config file + .dynamic.<id>) + // TODO jonmv: if dynamic file, all unlisted servers are observers; otherwise joiners are observers + // TODO jonmv: wrap Curator in Provider, for Curator shutdown private void reconfigure(ZookeeperServerConfig newConfig) { Instant reconfigTriggered = Instant.now(); - // No point in trying to reconfigure if there is only one server in the new ensemble, - // the others will be shutdown or are about to be shutdown - if (newConfig.server().size() == 1) shutdownAndDie(Duration.ZERO); - - List<String> newServers = difference(servers(newConfig), servers(activeConfig)); - String leavingServerIds = String.join(",", serverIdsDifference(activeConfig, newConfig)); - String joiningServersSpec = String.join(",", newServers); - leavingServerIds = leavingServerIds.isEmpty() ? null : leavingServerIds; - joiningServersSpec = joiningServersSpec.isEmpty() ? null : joiningServersSpec; - log.log(Level.INFO, "Will reconfigure ZooKeeper cluster. \nJoining servers: " + joiningServersSpec + - "\nleaving servers: " + leavingServerIds + + String newServers = String.join(",", servers(newConfig)); + log.log(Level.INFO, "Will reconfigure ZooKeeper cluster." + "\nServers in active config:" + servers(activeConfig) + "\nServers in new config:" + servers(newConfig)); String connectionSpec = localConnectionSpec(activeConfig); Instant now = Instant.now(); - Duration reconfigTimeout = reconfigTimeout(newServers.size()); + Duration reconfigTimeout = reconfigTimeout(newConfig.server().size()); Instant end = now.plus(reconfigTimeout); // Loop reconfiguring since we might need to wait until another reconfiguration is finished before we can succeed for (int attempt = 1; now.isBefore(end); attempt++) { try { Instant reconfigStarted = Instant.now(); - vespaZooKeeperAdmin.reconfigure(connectionSpec, joiningServersSpec, leavingServerIds); + vespaZooKeeperAdmin.reconfigure(connectionSpec, newServers); Instant reconfigEnded = Instant.now(); log.log(Level.INFO, "Reconfiguration completed in " + Duration.between(reconfigTriggered, reconfigEnded) + @@ -147,24 +140,11 @@ public class Reconfigurer extends AbstractComponent { return HostName.getLocalhost() + ":" + config.clientPort(); } - private static List<String> serverIdsDifference(ZookeeperServerConfig oldConfig, ZookeeperServerConfig newConfig) { - return difference(servers(oldConfig), servers(newConfig)).stream() - .map(server -> server.substring(0, server.indexOf('='))) - .collect(Collectors.toList()); - } - private static List<String> servers(ZookeeperServerConfig config) { - // See https://zookeeper.apache.org/doc/r3.6.3/zookeeperReconfig.html#sc_reconfig_clientport for format return config.server().stream() - .map(server -> server.id() + "=" + server.hostname() + ":" + server.quorumPort() + ":" + - server.electionPort() + ";" + config.clientPort()) - .collect(Collectors.toList()); - } - - private static <T> List<T> difference(List<T> list1, List<T> list2) { - List<T> copy = new ArrayList<>(list1); - copy.removeAll(list2); - return copy; + .filter(server -> ! server.retired()) + .map(server -> serverSpec(server, config.clientPort(), false)) + .collect(toList()); } } diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdmin.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdmin.java index 8809dca0def..59c9628bcab 100644 --- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdmin.java +++ b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdmin.java @@ -10,7 +10,7 @@ import java.time.Duration; */ public interface VespaZooKeeperAdmin { - void reconfigure(String connectionSpec, String joiningServers, String leavingServers) throws ReconfigException; + void reconfigure(String connectionSpec, String servers) throws ReconfigException; /* Timeout for connecting to ZooKeeper */ default Duration sessionTimeout() { return Duration.ofSeconds(30); } diff --git a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java index 1211624e3d6..760c326cf5d 100644 --- a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java +++ b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java @@ -17,7 +17,6 @@ import java.util.Arrays; import java.util.concurrent.Phaser; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; /** @@ -51,31 +50,26 @@ public class ReconfigurerTest { ZookeeperServerConfig nextConfig = createConfig(5, true); reconfigurer.startOrReconfigure(nextConfig); assertEquals("node1:2181", reconfigurer.connectionSpec()); - assertEquals("3=node3:2182:2183;2181,4=node4:2182:2183;2181", reconfigurer.joiningServers()); - assertNull("No servers are leaving", reconfigurer.leavingServers()); - assertEquals(1, reconfigurer.reconfigurations()); - assertSame(nextConfig, reconfigurer.activeConfig()); - - // No reconfiguration happens with same config - reconfigurer.startOrReconfigure(nextConfig); - assertEquals(1, reconfigurer.reconfigurations()); + assertEquals("server.0=node0:2182:2183;2181,server.1=node1:2182:2183;2181,server.2=node2:2182:2183;2181,server.3=node3:2182:2183;2181,server.4=node4:2182:2183;2181", + reconfigurer.servers()); + assertEquals(2, reconfigurer.reconfigurations()); assertSame(nextConfig, reconfigurer.activeConfig()); // Cluster shrinks nextConfig = createConfig(3, true); reconfigurer.startOrReconfigure(nextConfig); - assertEquals(2, reconfigurer.reconfigurations()); + assertEquals(3, reconfigurer.reconfigurations()); assertEquals("node1:2181", reconfigurer.connectionSpec()); - assertNull("No servers are joining", reconfigurer.joiningServers()); - assertEquals("3,4", reconfigurer.leavingServers()); + assertEquals("server.0=node0:2182:2183;2181,server.1=node1:2182:2183;2181,server.2=node2:2182:2183;2181", + reconfigurer.servers()); assertSame(nextConfig, reconfigurer.activeConfig()); // Cluster loses node1, but node3 joins. Indices are shuffled. nextConfig = createConfig(3, true, 1); reconfigurer.startOrReconfigure(nextConfig); - assertEquals(3, reconfigurer.reconfigurations()); - assertEquals("1=node2:2182:2183;2181,2=node3:2182:2183;2181", reconfigurer.joiningServers()); - assertEquals("1,2", reconfigurer.leavingServers()); + assertEquals(4, reconfigurer.reconfigurations()); + assertEquals("server.0=node0:2182:2183;2181,server.1=node2:2182:2183;2181,server.2=node3:2182:2183;2181", + reconfigurer.servers()); assertSame(nextConfig, reconfigurer.activeConfig()); } @@ -89,9 +83,9 @@ public class ReconfigurerTest { ZookeeperServerConfig nextConfig = createConfig(5, true); reconfigurer.startOrReconfigure(nextConfig); assertEquals("node1:2181", reconfigurer.connectionSpec()); - assertEquals("3=node3:2182:2183;2181,4=node4:2182:2183;2181", reconfigurer.joiningServers()); - assertNull("No servers are leaving", reconfigurer.leavingServers()); - assertEquals(1, reconfigurer.reconfigurations()); + assertEquals("server.0=node0:2182:2183;2181,server.1=node1:2182:2183;2181,server.2=node2:2182:2183;2181,server.3=node3:2182:2183;2181,server.4=node4:2182:2183;2181", + reconfigurer.servers()); + assertEquals(2, reconfigurer.reconfigurations()); assertSame(nextConfig, reconfigurer.activeConfig()); } @@ -112,24 +106,27 @@ public class ReconfigurerTest { reconfigurer.shutdown(); } - private ZookeeperServerConfig createConfig(int numberOfServers, boolean dynamicReconfiguration, int... skipIndices) { - Arrays.sort(skipIndices); + private ZookeeperServerConfig createConfig(int numberOfServers, boolean dynamicReconfiguration, int... retiredIndices) { + Arrays.sort(retiredIndices); ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder(); builder.zooKeeperConfigFile(cfgFile.getAbsolutePath()); builder.myidFile(idFile.getAbsolutePath()); for (int i = 0, index = 0; i < numberOfServers; i++, index++) { - while (Arrays.binarySearch(skipIndices, index) >= 0) index++; - builder.server(newServer(i, "node" + index)); + boolean retired = Arrays.binarySearch(retiredIndices, index) >= 0; + if (retired) i--; + builder.server(newServer(i, "node" + index, retired)); } + builder.myid(0); builder.dynamicReconfiguration(dynamicReconfiguration); return builder.build(); } - private ZookeeperServerConfig.Server.Builder newServer(int id, String hostName) { + private ZookeeperServerConfig.Server.Builder newServer(int id, String hostName, boolean retired) { ZookeeperServerConfig.Server.Builder builder = new ZookeeperServerConfig.Server.Builder(); builder.id(id); builder.hostname(hostName); + builder.retired(retired); return builder; } @@ -142,6 +139,7 @@ public class ReconfigurerTest { private static class TestableReconfigurer extends Reconfigurer implements VespaZooKeeperServer { private final TestableVespaZooKeeperAdmin zooKeeperAdmin; + private final Phaser phaser = new Phaser(2); private QuorumPeer serverPeer; TestableReconfigurer(TestableVespaZooKeeperAdmin zooKeeperAdmin) { @@ -156,19 +154,16 @@ public class ReconfigurerTest { } void startOrReconfigure(ZookeeperServerConfig newConfig) { - startOrReconfigure(newConfig, this, MockQuorumPeer::new, peer -> serverPeer = peer); + serverPeer = startOrReconfigure(newConfig, this, MockQuorumPeer::new); + phaser.arriveAndDeregister(); } String connectionSpec() { return zooKeeperAdmin.connectionSpec; } - String joiningServers() { - return zooKeeperAdmin.joiningServers; - } - - String leavingServers() { - return zooKeeperAdmin.leavingServers; + String servers() { + return zooKeeperAdmin.servers; } int reconfigurations() { @@ -177,10 +172,14 @@ public class ReconfigurerTest { @Override public void shutdown() { + phaser.arriveAndAwaitAdvance(); serverPeer.shutdown(Duration.ofSeconds(1)); } @Override - public void start(Path configFilePath) { serverPeer.start(configFilePath); } + public void start(Path configFilePath) { + phaser.arriveAndAwaitAdvance(); + serverPeer.start(configFilePath); + } @Override public boolean reconfigurable() { @@ -192,8 +191,7 @@ public class ReconfigurerTest { private static class TestableVespaZooKeeperAdmin implements VespaZooKeeperAdmin { String connectionSpec; - String joiningServers; - String leavingServers; + String servers; int reconfigurations = 0; private int failures = 0; @@ -205,12 +203,11 @@ public class ReconfigurerTest { } @Override - public void reconfigure(String connectionSpec, String joiningServers, String leavingServers) throws ReconfigException { + public void reconfigure(String connectionSpec, String servers) throws ReconfigException { if (++attempts < failures) throw new ReconfigException("Reconfig failed"); this.connectionSpec = connectionSpec; - this.joiningServers = joiningServers; - this.leavingServers = leavingServers; + this.servers = servers; this.reconfigurations++; } |