diff options
89 files changed, 1158 insertions, 863 deletions
diff --git a/README.md b/README.md index 09a3f9a7124..194d24885f1 100644 --- a/README.md +++ b/README.md @@ -63,17 +63,13 @@ You do not need to build Vespa to use it, but if you want to contribute you need This section explains how to build and test Vespa. To understand where to make changes, see [Code-map.md](Code-map.md). Some suggested improvements with pointers to code are in [TODO.md](TODO.md). -### Set up the build environment +### Development environment -C++ and Java building is supported on CentOS 7. The Java source can also be built on any platform having Java 11 and Maven installed. -We recommend using the following environment: [Create C++ / Java dev environment on CentOS using VirtualBox and Vagrant](vagrant/README.md). -You can also setup CentOS 7 natively and install the following build dependencies: - - sudo yum-config-manager --add-repo https://copr.fedorainfracloud.org/coprs/g/vespa/vespa/repo/epel-7/group_vespa-vespa-epel-7.repo - sudo yum -y install epel-release centos-release-scl yum-utils - sudo yum -y install ccache \ - rpm-build - yum-builddep -y <vespa-source>/dist/vespa.spec +C++ and Java building is supported on CentOS 7. +The Java source can also be built on any platform having Java 11 and Maven installed. +Use the following guide to setup a complete development environment using Docker +for building Vespa, running unit tests and running system tests: +[Vespa development on CentOS 7](https://github.com/vespa-engine/docker-image-dev#vespa-development-on-centos-7). ### Build Java modules @@ -82,20 +78,13 @@ You can also setup CentOS 7 natively and install the following build dependencie bash bootstrap.sh java mvn -T <num-threads> install -### Build C++ modules +Use this if you only need to build the Java modules, otherwise follow the complete development guide above. -Replace `<build-dir>` with the name of the directory in which you'd like to build Vespa. -Replace `<source-dir>` with the directory in which you've cloned/unpacked the source tree. +### Build RPM packages - bash bootstrap-cpp.sh <source-dir> <build-dir> - cd <build-dir> - make -j <num-threads> - ctest3 -j <num-threads> +See [Building Vespa RPM](docker/README.md#building-vespa-rpm) for details. -### Create RPM packages - sh dist.sh VERSION && rpmbuild -ba ~/rpmbuild/SPECS/vespa-VERSION.spec - ## License Code licensed under the Apache 2.0 license. See [LICENSE](LICENSE) for terms. diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java index 4b882023e08..4604eea62c1 100644 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java @@ -54,7 +54,7 @@ public class IdentityDocumentGeneratorTest { ApplicationId appid = ApplicationId.from( TenantName.from("tenant"), ApplicationName.from("application"), InstanceName.from("default")); Allocation allocation = new Allocation(appid, - ClusterMembership.from("container/default/0/0", Version.fromString("1.2.3")), + ClusterMembership.from("container/default/0/0", Version.fromString("1.2.3"), Optional.empty()), new NodeResources(1, 1, 1, 1), Generation.initial(), false); diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java index fdf2bbfccff..19ad9df2d4d 100644 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java @@ -240,7 +240,7 @@ public class InstanceValidatorTest { private List<Node> allocateNode(List<Node> nodeList, Node node, ApplicationId applicationId) { nodeList.removeIf(n -> n.id().equals(node.id())); nodeList.add(node.allocate(applicationId, - ClusterMembership.from("container/default/0/0", Version.fromString("6.123.4")), + ClusterMembership.from("container/default/0/0", Version.fromString("6.123.4"), Optional.empty()), new NodeResources(1, 1, 1, 1), Instant.now())); return nodeList; diff --git a/bootstrap-cmake.sh b/bootstrap-cmake.sh index 27c7ed6c21b..dbe27f5b8bf 100755 --- a/bootstrap-cmake.sh +++ b/bootstrap-cmake.sh @@ -39,7 +39,8 @@ else fi if [ -z "$VESPA_LLVM_VERSION" ]; then - VESPA_LLVM_VERSION=5.0 + VESPA_LLVM_VERSION_PATH=7.0 + VESPA_LLVM_VERSION=7 fi if $UNPRIVILEGED; then @@ -54,8 +55,8 @@ cmake3 \ -DCMAKE_INSTALL_PREFIX=${VESPA_INSTALL_PREFIX} \ -DJAVA_HOME=${JAVA_HOME:-/usr/lib/jvm/java-openjdk} \ -DCMAKE_PREFIX_PATH="/opt/vespa-deps" \ - -DEXTRA_LINK_DIRECTORY="/opt/vespa-deps/lib64;/usr/lib64/llvm$VESPA_LLVM_VERSION/lib" \ - -DEXTRA_INCLUDE_DIRECTORY="/opt/vespa-deps/include;/usr/include/llvm$VESPA_LLVM_VERSION;/usr/include/openblas" \ + -DEXTRA_LINK_DIRECTORY="/opt/vespa-deps/lib64;/usr/lib64/llvm$VESPA_LLVM_VERSION_PATH/lib" \ + -DEXTRA_INCLUDE_DIRECTORY="/opt/vespa-deps/include;/usr/include/llvm$VESPA_LLVM_VERSION_PATH;/usr/include/openblas" \ -DCMAKE_INSTALL_RPATH="${VESPA_INSTALL_PREFIX}/lib64;/opt/vespa-deps/lib64;/usr/lib/jvm/java-1.8.0/jre/lib/amd64/server;/usr/lib64/llvm$VESPA_LLVM_VERSION/lib" \ ${UNPRIVILEGED_ARGS} \ ${EXTRA_CMAKE_ARGS} \ diff --git a/bootstrap-cpp.sh b/bootstrap-cpp.sh index 5fe4723ea90..5753d4cfcfd 100755 --- a/bootstrap-cpp.sh +++ b/bootstrap-cpp.sh @@ -48,7 +48,7 @@ mkdir -p "${BUILD_DIR}" || { BUILD_DIR=$(realpath "${BUILD_DIR}") # Build it -source /opt/rh/devtoolset-8/enable || true +source /opt/rh/devtoolset-9/enable || true cd "${SOURCE_DIR}" bash ./bootstrap.sh full cd "${BUILD_DIR}" diff --git a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java index bad1e526fba..c95d814eb99 100644 --- a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java +++ b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java @@ -76,7 +76,6 @@ public class ClusterControllerClusterConfigurer { options.clusterHasGlobalDocumentTypes = config.cluster_has_global_document_types(); options.minMergeCompletionRatio = config.min_merge_completion_ratio(); options.enableTwoPhaseClusterStateActivation = config.enable_two_phase_cluster_state_transitions(); - options.determineBucketsFromBucketSpaceMetric = config.determine_buckets_from_bucket_space_metric(); } private void configure(SlobroksConfig config) { diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java index ae12a6dabb1..5e775116bf3 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentCluster.java @@ -22,7 +22,6 @@ public class ContentCluster { private final ClusterInfo clusterInfo = new ClusterInfo(); private final Map<Node, Long> nodeStartTimestamps = new TreeMap<>(); - private final boolean determineBucketsFromBucketSpaceMetric; private int slobrokGenerationCount = 0; @@ -33,9 +32,7 @@ public class ContentCluster { private double minRatioOfStorageNodesUp; public ContentCluster(String clusterName, Collection<ConfiguredNode> configuredNodes, Distribution distribution, - int minStorageNodesUp, double minRatioOfStorageNodesUp, - boolean determineBucketsFromBucketSpaceMetric) { - this.determineBucketsFromBucketSpaceMetric = determineBucketsFromBucketSpaceMetric; + int minStorageNodesUp, double minRatioOfStorageNodesUp) { if (configuredNodes == null) throw new IllegalArgumentException("Nodes must be set"); this.clusterName = clusterName; this.distribution = distribution; @@ -186,8 +183,8 @@ public class ContentCluster { minStorageNodesUp, minRatioOfStorageNodesUp, distribution.getRedundancy(), - clusterInfo, - determineBucketsFromBucketSpaceMetric); + clusterInfo + ); return nodeStateChangeChecker.evaluateTransition(node, clusterState, condition, oldState, newState); } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java index 02f52d5f0c7..d943cf27f9c 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java @@ -193,7 +193,7 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd options.nodes, options.storageDistribution, options.minStorageNodesUp, - options.minRatioOfStorageNodesUp, true); + options.minRatioOfStorageNodesUp); NodeStateGatherer stateGatherer = new NodeStateGatherer(timer, timer, log); Communicator communicator = new RPCCommunicator( RPCCommunicator.createRealSupervisor(), diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerOptions.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerOptions.java index 7ad6765cc47..553b3332ee8 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerOptions.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerOptions.java @@ -128,9 +128,6 @@ public class FleetControllerOptions implements Cloneable { public int maxDivergentNodesPrintedInTaskErrorMessages = 10; - // TODO: May be removed once rolled out everywhere. - public boolean determineBucketsFromBucketSpaceMetric = true; - // TODO: Replace usage of this by usage where the nodes are explicitly passed (below) public FleetControllerOptions(String clusterName) { this.clusterName = clusterName; diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java index 6bcb5b07f28..4ae45701344 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java @@ -21,7 +21,6 @@ import java.util.Optional; * @author Haakon Dybdahl */ public class NodeStateChangeChecker { - public static final String LEGACY_BUCKETS_METRIC_NAME = "vds.datastored.alldisks.buckets"; public static final String BUCKETS_METRIC_NAME = "vds.datastored.bucket_space.buckets_total"; public static final Map<String, String> BUCKETS_METRIC_DIMENSIONS = Map.of("bucketSpace", "default"); @@ -29,19 +28,16 @@ public class NodeStateChangeChecker { private double minRatioOfStorageNodesUp; private final int requiredRedundancy; private final ClusterInfo clusterInfo; - private final boolean determineBucketsFromBucketSpaceMetric; public NodeStateChangeChecker( int minStorageNodesUp, double minRatioOfStorageNodesUp, int requiredRedundancy, - ClusterInfo clusterInfo, - boolean determineBucketsFromBucketSpaceMetric) { + ClusterInfo clusterInfo) { this.minStorageNodesUp = minStorageNodesUp; this.minRatioOfStorageNodesUp = minRatioOfStorageNodesUp; this.requiredRedundancy = requiredRedundancy; this.clusterInfo = clusterInfo; - this.determineBucketsFromBucketSpaceMetric = determineBucketsFromBucketSpaceMetric; } public static class Result { @@ -159,18 +155,10 @@ public class NodeStateChangeChecker { } Optional<Metrics.Value> bucketsMetric; - if (determineBucketsFromBucketSpaceMetric) { - bucketsMetric = hostInfo.getMetrics().getValueAt(BUCKETS_METRIC_NAME, BUCKETS_METRIC_DIMENSIONS); - if (!bucketsMetric.isPresent() || bucketsMetric.get().getLast() == null) { - return Result.createDisallowed("Missing last value of the " + BUCKETS_METRIC_NAME + - " metric for storage node " + nodeInfo.getNodeIndex()); - } - } else { - bucketsMetric = hostInfo.getMetrics().getValue(LEGACY_BUCKETS_METRIC_NAME); - if (!bucketsMetric.isPresent() || bucketsMetric.get().getLast() == null) { - return Result.createDisallowed("Missing last value of the " + LEGACY_BUCKETS_METRIC_NAME + - " metric for storage node " + nodeInfo.getNodeIndex()); - } + bucketsMetric = hostInfo.getMetrics().getValueAt(BUCKETS_METRIC_NAME, BUCKETS_METRIC_DIMENSIONS); + if (!bucketsMetric.isPresent() || bucketsMetric.get().getLast() == null) { + return Result.createDisallowed("Missing last value of the " + BUCKETS_METRIC_NAME + + " metric for storage node " + nodeInfo.getNodeIndex()); } long lastBuckets = bucketsMetric.get().getLast(); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java index 36c49fdf5e2..2df9279e450 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterFixture.java @@ -198,7 +198,7 @@ public class ClusterFixture { Collection<ConfiguredNode> nodes = DistributionBuilder.buildConfiguredNodes(nodeCount); Distribution distribution = DistributionBuilder.forFlatCluster(nodeCount); - ContentCluster cluster = new ContentCluster("foo", nodes, distribution, 0, 0.0, true); + ContentCluster cluster = new ContentCluster("foo", nodes, distribution, 0, 0.0); return new ClusterFixture(cluster, distribution); } @@ -206,7 +206,7 @@ public class ClusterFixture { static ClusterFixture forHierarchicCluster(DistributionBuilder.GroupBuilder root) { List<ConfiguredNode> nodes = DistributionBuilder.buildConfiguredNodes(root.totalNodeCount()); Distribution distribution = DistributionBuilder.forHierarchicCluster(root); - ContentCluster cluster = new ContentCluster("foo", nodes, distribution, 0, 0.0, true); + ContentCluster cluster = new ContentCluster("foo", nodes, distribution, 0, 0.0); return new ClusterFixture(cluster, distribution); } diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java index d569feb6f14..059d7d68754 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/FleetControllerTest.java @@ -157,7 +157,7 @@ public abstract class FleetControllerTest implements Waiter { options.nodes, options.storageDistribution, options.minStorageNodesUp, - options.minRatioOfStorageNodesUp, true); + options.minRatioOfStorageNodesUp); NodeStateGatherer stateGatherer = new NodeStateGatherer(timer, timer, log); Communicator communicator = new RPCCommunicator( RPCCommunicator.createRealSupervisor(), diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java index 153d570adaf..e918f94abbf 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java @@ -61,14 +61,14 @@ public class NodeStateChangeCheckerTest { } private NodeStateChangeChecker createChangeChecker(ContentCluster cluster) { - return new NodeStateChangeChecker(minStorageNodesUp, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo(), true); + return new NodeStateChangeChecker(minStorageNodesUp, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo()); } private ContentCluster createCluster(Collection<ConfiguredNode> nodes) { Distribution distribution = mock(Distribution.class); Group group = new Group(2, "to"); when(distribution.getRootGroup()).thenReturn(group); - return new ContentCluster("Clustername", nodes, distribution, minStorageNodesUp, 0.0, true); + return new ContentCluster("Clustername", nodes, distribution, minStorageNodesUp, 0.0); } private StorageNodeInfo createStorageNodeInfo(int index, State state) { @@ -78,7 +78,7 @@ public class NodeStateChangeCheckerTest { String clusterName = "Clustername"; Set<ConfiguredNode> configuredNodeIndexes = new HashSet<>(); - ContentCluster cluster = new ContentCluster(clusterName, configuredNodeIndexes, distribution, minStorageNodesUp, 0.0, true); + ContentCluster cluster = new ContentCluster(clusterName, configuredNodeIndexes, distribution, minStorageNodesUp, 0.0); String rpcAddress = ""; StorageNodeInfo storageNodeInfo = new StorageNodeInfo(cluster, index, false, rpcAddress, distribution); @@ -136,7 +136,7 @@ public class NodeStateChangeCheckerTest { public void testUnknownStorageNode() { ContentCluster cluster = createCluster(createNodes(4)); NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( - 5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo(), true); + 5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo()); NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( new Node(NodeType.STORAGE, 10), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); @@ -161,7 +161,7 @@ public class NodeStateChangeCheckerTest { ContentCluster cluster = createCluster(createNodes(4)); setAllNodesUp(cluster, HostInfo.createHostInfo(createDistributorHostInfo(4, 5, 6))); NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker( - 5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo(), true); + 5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo()); NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition( nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java index dc76b381f51..68926b4d10d 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeHandlerTest.java @@ -80,7 +80,7 @@ public class StateChangeHandlerTest { Distribution distribution = new Distribution(Distribution.getDefaultDistributionConfig(2, 100)); this.config = config; for (int i=0; i<config.nodeCount; ++i) configuredNodes.add(new ConfiguredNode(i, false)); - cluster = new ContentCluster("testcluster", configuredNodes, distribution, 0, 0.0, true); + cluster = new ContentCluster("testcluster", configuredNodes, distribution, 0, 0.0); nodeStateChangeHandler = new StateChangeHandler(clock, eventLog, null); params.minStorageNodesUp(1).minDistributorNodesUp(1) .minRatioOfStorageNodesUp(0.0).minRatioOfDistributorNodesUp(0.0) diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java index 8b7c50cda56..5f9e0d56cfa 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateChangeTest.java @@ -53,7 +53,7 @@ public class StateChangeTest extends FleetControllerTest { MetricUpdater metricUpdater = new MetricUpdater(new NoMetricReporter(), options.fleetControllerIndex); eventLog = new EventLog(timer, metricUpdater); ContentCluster cluster = new ContentCluster(options.clusterName, options.nodes, options.storageDistribution, - options.minStorageNodesUp, options.minRatioOfStorageNodesUp, true); + options.minStorageNodesUp, options.minRatioOfStorageNodesUp); NodeStateGatherer stateGatherer = new NodeStateGatherer(timer, timer, eventLog); DatabaseHandler database = new DatabaseHandler(new ZooKeeperDatabaseFactory(), timer, options.zooKeeperServerAddress, options.fleetControllerIndex, timer); StateChangeHandler stateGenerator = new StateChangeHandler(timer, eventLog, metricUpdater); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/NodeTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/NodeTest.java index 368f64352d1..6bd7f086249 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/NodeTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/NodeTest.java @@ -121,7 +121,7 @@ public class NodeTest extends StateRestApiTest { public void testNodeNotSeenInSlobrok() throws Exception { setUp(true); ContentCluster old = music.context.cluster; - music.context.cluster = new ContentCluster(old.getName(), old.getConfiguredNodes().values(), old.getDistribution(), 0, 0.0, true); + music.context.cluster = new ContentCluster(old.getName(), old.getConfiguredNodes().values(), old.getDistribution(), 0, 0.0); NodeState currentState = new NodeState(NodeType.STORAGE, State.DOWN); currentState.setDescription("Not seen"); music.context.currentConsolidatedState.setNodeState(new Node(NodeType.STORAGE, 1), currentState); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java index 1370763a7c8..faebbf8755d 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/StateRestApiTest.java @@ -42,8 +42,7 @@ public abstract class StateRestApiTest { { Set<ConfiguredNode> nodes = FleetControllerTest.toNodes(0, 1, 2, 3); ContentCluster cluster = new ContentCluster( - "books", nodes, distribution, 6 /* minStorageNodesUp*/, 0.9 /* minRatioOfStorageNodesUp */, - true /* determineBucketsFromBucketSpaceMetric */); + "books", nodes, distribution, 6 /* minStorageNodesUp*/, 0.9 /* minRatioOfStorageNodesUp */); initializeCluster(cluster, nodes); AnnotatedClusterState baselineState = AnnotatedClusterState.withoutAnnotations(ClusterState.stateFromString("distributor:4 storage:4")); Map<String, AnnotatedClusterState> bucketSpaceStates = new HashMap<>(); @@ -57,8 +56,7 @@ public abstract class StateRestApiTest { Set<ConfiguredNode> nodesInSlobrok = FleetControllerTest.toNodes(1, 3, 5, 7); ContentCluster cluster = new ContentCluster( - "music", nodes, distribution, 4 /* minStorageNodesUp*/, 0.0 /* minRatioOfStorageNodesUp */, - true /* determineBucketsFromBucketSpaceMetric */); + "music", nodes, distribution, 4 /* minStorageNodesUp*/, 0.0 /* minRatioOfStorageNodesUp */); if (dontInitializeNode2) { // TODO: this skips initialization of node 2 to fake that it is not answering // which really leaves us in an illegal state diff --git a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java index 15b775a4543..23273f9f1bf 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java @@ -47,11 +47,6 @@ class OverrideProcessor implements PreProcessor { private static final String ENVIRONMENT_ATTRIBUTE = "environment"; private static final String REGION_ATTRIBUTE = "region"; - // TODO: Remove after September 2019 - public OverrideProcessor(Environment environment, RegionName region) { - this(InstanceName.from("default"), environment, region); - } - public OverrideProcessor(InstanceName instance, Environment environment, RegionName region) { this.instance = instance; this.environment = environment; diff --git a/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java index 73f97cf516f..c49a799f2b7 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java @@ -38,20 +38,10 @@ public class XmlPreProcessor { private final RegionName region; private final List<PreProcessor> chain; - // TODO: Remove after September 2019 - public XmlPreProcessor(File applicationDir, File xmlInput, Environment environment, RegionName region) throws IOException { - this(applicationDir, new FileReader(xmlInput), InstanceName.from("default"), environment, region); - } - public XmlPreProcessor(File applicationDir, File xmlInput, InstanceName instance, Environment environment, RegionName region) throws IOException { this(applicationDir, new FileReader(xmlInput), instance, environment, region); } - // TODO: Remove after September 2019 - public XmlPreProcessor(File applicationDir, Reader xmlInput, Environment environment, RegionName region) throws IOException { - this(applicationDir, xmlInput, InstanceName.from("default"), environment, region); - } - public XmlPreProcessor(File applicationDir, Reader xmlInput, InstanceName instance, Environment environment, RegionName region) { this.applicationDir = applicationDir; this.xmlInput = xmlInput; diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/DeployData.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/DeployData.java index 344035745f8..82da1e2c6ea 100644 --- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/DeployData.java +++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/DeployData.java @@ -2,9 +2,6 @@ package com.yahoo.config.model.application.provider; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.ApplicationName; -import com.yahoo.config.provision.InstanceName; -import com.yahoo.config.provision.TenantName; /** * Data generated or computed during deployment @@ -31,22 +28,6 @@ public class DeployData { private final long generation; private final long currentlyActiveGeneration; - // TODO: Remove after September 2019 - public DeployData(String deployedByUser, - String deployedFromDir, - String applicationName, - Long deployTimestamp, - boolean internalRedeploy, - Long generation, - long currentlyActiveGeneration) { - this(deployedByUser, - deployedFromDir, - ApplicationId.from(TenantName.defaultName(), ApplicationName.from(applicationName), InstanceName.from("default")), - deployTimestamp, - internalRedeploy, - generation, currentlyActiveGeneration); - } - public DeployData(String deployedByUser, String deployedFromDir, ApplicationId applicationId, @@ -77,7 +58,4 @@ public class DeployData { public ApplicationId getApplicationId() { return applicationId; } - // TODO: remove after September 2019 - public String getApplicationName() { return applicationId.application().toString(); } - } diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index a074c4af3dc..8ebb449ceda 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -77,7 +77,9 @@ public interface ModelContext { // TODO Revisit in May or June 2020 double defaultTopKProbability(); + // TODO: Remove once there are no Vespa versions below 7.170 boolean useBucketSpaceMetric(); + default boolean useNewAthenzFilter() { return true; } // TODO bjorncs: Remove after end of April // TODO: Remove after April 2020 diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java index ad6eebe1ca5..5418ba1a6ee 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java @@ -9,6 +9,7 @@ import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.NodeResources; import com.yahoo.text.XML; import com.yahoo.vespa.model.HostResource; @@ -21,7 +22,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.function.Function; -import java.util.regex.Pattern; /** * A common utility class to represent a requirement for nodes during model building. @@ -209,7 +209,7 @@ public class NodesSpecification { .vespaVersion(version) .exclusive(exclusive) .combinedId(combinedId.map(ClusterSpec.Id::from)) - .dockerImageRepo(dockerImageRepo) + .dockerImageRepository(dockerImageRepo.map(DockerImage::fromString)) .build(); return hostSystem.allocateHosts(cluster, Capacity.from(min, max, required, canFail), logger); } @@ -217,7 +217,7 @@ public class NodesSpecification { private static Pair<NodeResources, NodeResources> nodeResources(ModelElement nodesElement) { ModelElement resources = nodesElement.child("resources"); if (resources != null) { - return nodeResourcesFromResorcesElement(resources); + return nodeResourcesFromResourcesElement(resources); } else if (nodesElement.stringAttribute("flavor") != null) { // legacy fallback var flavorResources = NodeResources.fromLegacyName(nodesElement.stringAttribute("flavor")); @@ -228,7 +228,7 @@ public class NodesSpecification { } } - private static Pair<NodeResources, NodeResources> nodeResourcesFromResorcesElement(ModelElement element) { + private static Pair<NodeResources, NodeResources> nodeResourcesFromResourcesElement(ModelElement element) { Pair<Double, Double> vcpu = toRange(element.requiredStringAttribute("vcpu"), .0, Double::parseDouble); Pair<Double, Double> memory = toRange(element.requiredStringAttribute("memory"), .0, s -> parseGbAmount(s, "B")); Pair<Double, Double> disk = toRange(element.requiredStringAttribute("disk"), .0, s -> parseGbAmount(s, "B")); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 4bd9f5fa8b0..8c8e8be5a30 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -25,6 +25,7 @@ import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeResources; @@ -659,7 +660,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { ClusterSpec clusterSpec = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(cluster.getName())) .vespaVersion(deployState.getWantedNodeVespaVersion()) - .dockerImageRepo(deployState.getWantedDockerImageRepo()) + .dockerImageRepository(deployState.getWantedDockerImageRepo().map(DockerImage::fromString)) .build(); int nodeCount = deployState.zone().environment().isProduction() ? 2 : 1; Capacity capacity = Capacity.from(new ClusterResources(nodeCount, 1, NodeResources.unspecified), @@ -691,7 +692,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { NodeType type = NodeType.valueOf(nodesElement.getAttribute("type")); ClusterSpec clusterSpec = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(cluster.getName())) .vespaVersion(context.getDeployState().getWantedNodeVespaVersion()) - .dockerImageRepo(context.getDeployState().getWantedDockerImageRepo()) + .dockerImageRepository(context.getDeployState().getWantedDockerImageRepo().map(DockerImage::fromString)) .build(); Map<HostResource, ClusterMembership> hosts = cluster.getRoot().hostSystem().allocateHosts(clusterSpec, diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterControllerConfig.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterControllerConfig.java index adff10ae580..43ef37d311d 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterControllerConfig.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterControllerConfig.java @@ -40,8 +40,6 @@ public class ClusterControllerConfig extends AbstractConfigProducer implements F bucketSplittingMinimumBits = clusterTuning.childAsInteger("bucket-splitting.minimum-bits"); } - boolean useBucketSpaceMetric = deployState.getProperties().useBucketSpaceMetric(); - if (tuning != null) { return new ClusterControllerConfig(ancestor, clusterName, tuning.childAsDuration("init-progress-time"), @@ -51,11 +49,11 @@ public class ClusterControllerConfig extends AbstractConfigProducer implements F tuning.childAsDouble("min-distributor-up-ratio"), tuning.childAsDouble("min-storage-up-ratio"), bucketSplittingMinimumBits, - minNodeRatioPerGroup, - useBucketSpaceMetric); + minNodeRatioPerGroup + ); } else { return new ClusterControllerConfig(ancestor, clusterName, null, null, null, null, null, null, - bucketSplittingMinimumBits, minNodeRatioPerGroup, useBucketSpaceMetric); + bucketSplittingMinimumBits, minNodeRatioPerGroup); } } } @@ -69,7 +67,6 @@ public class ClusterControllerConfig extends AbstractConfigProducer implements F Double minStorageUpRatio; Integer minSplitBits; private Double minNodeRatioPerGroup; - private boolean useBucketSpaceMetric; // TODO refactor; too many args private ClusterControllerConfig(AbstractConfigProducer parent, @@ -81,8 +78,7 @@ public class ClusterControllerConfig extends AbstractConfigProducer implements F Double minDistributorUpRatio, Double minStorageUpRatio, Integer minSplitBits, - Double minNodeRatioPerGroup, - boolean useBucketSpaceMetric) { + Double minNodeRatioPerGroup) { super(parent, "fleetcontroller"); this.clusterName = clusterName; @@ -94,7 +90,6 @@ public class ClusterControllerConfig extends AbstractConfigProducer implements F this.minStorageUpRatio = minStorageUpRatio; this.minSplitBits = minSplitBits; this.minNodeRatioPerGroup = minNodeRatioPerGroup; - this.useBucketSpaceMetric = useBucketSpaceMetric; } @Override @@ -111,7 +106,6 @@ public class ClusterControllerConfig extends AbstractConfigProducer implements F builder.index(0); builder.cluster_name(clusterName); builder.fleet_controller_count(getChildren().size()); - builder.determine_buckets_from_bucket_space_metric(useBucketSpaceMetric); if (initProgressTime != null) { builder.init_progress_time((int) initProgressTime.getMilliSeconds()); diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java index 178bbac9e64..779b6854e21 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java @@ -20,7 +20,7 @@ public class ClusterMembership { protected ClusterMembership() {} - private ClusterMembership(String stringValue, Version vespaVersion, Optional<String> dockerImageRepo) { + private ClusterMembership(String stringValue, Version vespaVersion, Optional<DockerImage> dockerImageRepo) { String[] components = stringValue.split("/"); if (components.length < 4) throw new RuntimeException("Could not parse '" + stringValue + "' to a cluster membership. " + @@ -44,7 +44,7 @@ public class ClusterMembership { .vespaVersion(vespaVersion) .exclusive(exclusive) .combinedId(combinedId.map(ClusterSpec.Id::from)) - .dockerImageRepo(dockerImageRepo) + .dockerImageRepository(dockerImageRepo) .build(); this.index = Integer.parseInt(components[3]); this.stringValue = toStringValue(); @@ -110,11 +110,12 @@ public class ClusterMembership { @Override public String toString() { return stringValue(); } + // TODO: Remove when 7.208 is the latest model in use public static ClusterMembership from(String stringValue, Version vespaVersion) { return new ClusterMembership(stringValue, vespaVersion, Optional.empty()); } - public static ClusterMembership from(String stringValue, Version vespaVersion, Optional<String> dockerImageRepo) { + public static ClusterMembership from(String stringValue, Version vespaVersion, Optional<DockerImage> dockerImageRepo) { return new ClusterMembership(stringValue, vespaVersion, dockerImageRepo); } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java index f7aacbc757b..97549e851ad 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java @@ -22,10 +22,10 @@ public final class ClusterSpec { private final Version vespaVersion; private boolean exclusive; private final Optional<Id> combinedId; - private final Optional<String> dockerImageRepo; + private final Optional<DockerImage> dockerImageRepo; private ClusterSpec(Type type, Id id, Optional<Group> groupId, Version vespaVersion, boolean exclusive, - Optional<Id> combinedId, Optional<String> dockerImageRepo) { + Optional<Id> combinedId, Optional<DockerImage> dockerImageRepo) { this.type = type; this.id = id; this.groupId = groupId; @@ -36,6 +36,8 @@ public final class ClusterSpec { throw new IllegalArgumentException("combinedId must be empty for cluster of type " + type); } this.combinedId = combinedId; + if (dockerImageRepo.isPresent() && dockerImageRepo.get().tag().isPresent()) + throw new IllegalArgumentException("dockerimageRepo is not allowed to have a tag"); this.dockerImageRepo = dockerImageRepo; } @@ -46,7 +48,7 @@ public final class ClusterSpec { public Id id() { return id; } /** Returns the docker image repository part of a docker image we want this cluster to run */ - public Optional<String> dockerImageRepo() { return dockerImageRepo; } + public Optional<DockerImage> dockerImageRepo() { return dockerImageRepo; } /** Returns the docker image (repository + vespa version) we want this cluster to run */ public Optional<String> dockerImage() { return dockerImageRepo.map(repo -> repo + ":" + vespaVersion.toFullString()); } @@ -94,7 +96,7 @@ public final class ClusterSpec { private final boolean specification; private Optional<Group> groupId = Optional.empty(); - private Optional<String> dockerImageRepo = Optional.empty(); + private Optional<DockerImage> dockerImageRepo = Optional.empty(); private Version vespaVersion; private boolean exclusive = false; private Optional<Id> combinedId = Optional.empty(); @@ -139,7 +141,14 @@ public final class ClusterSpec { return this; } + @Deprecated + // TODO: Remove after 7.208 is oldest version in use public Builder dockerImageRepo(Optional<String> dockerImageRepo) { + this.dockerImageRepo = dockerImageRepo.map(DockerImage::fromString); + return this; + } + + public Builder dockerImageRepository(Optional<DockerImage> dockerImageRepo) { this.dockerImageRepo = dockerImageRepo; return this; } @@ -163,7 +172,7 @@ public final class ClusterSpec { if ( ! other.id.equals(this.id)) return false; if ( ! other.groupId.equals(this.groupId)) return false; if ( ! other.vespaVersion.equals(this.vespaVersion)) return false; - if ( ! other.dockerImageRepo.orElse("").equals(this.dockerImageRepo.orElse(""))) return false; + if ( ! other.dockerImageRepo.equals(this.dockerImageRepo)) return false; return true; } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java b/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java index 56c5544f6d7..9ba26be072c 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java @@ -87,7 +87,7 @@ public class AllocatedHostsSerializer { object.setString(hostSpecMembershipKey, membership.stringValue()); object.setString(hostSpecVespaVersionKey, membership.cluster().vespaVersion().toFullString()); membership.cluster().dockerImageRepo().ifPresent(repo -> { - object.setString(hostSpecDockerImageRepoKey, repo); + object.setString(hostSpecDockerImageRepoKey, repo.repository()); }); }); host.flavor().ifPresent(flavor -> toSlime(flavor, object)); @@ -208,7 +208,7 @@ public class AllocatedHostsSerializer { return ClusterMembership.from(object.field(hostSpecMembershipKey).asString(), com.yahoo.component.Version.fromString(object.field(hostSpecVespaVersionKey).asString()), object.field(hostSpecDockerImageRepoKey).valid() - ? Optional.of(object.field(hostSpecDockerImageRepoKey).asString()) + ? Optional.of(DockerImage.fromString(object.field(hostSpecDockerImageRepoKey).asString())) : Optional.empty()); } diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java index c38d67ddc81..71e039f6e8e 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java @@ -42,7 +42,7 @@ public class ClusterMembershipTest { assertTrue(instance.cluster().dockerImageRepo().isEmpty()); } { - Optional<String> dockerImageRepo = Optional.of("docker.foo.com:4443/vespa/bar"); + Optional<DockerImage> dockerImageRepo = Optional.of(DockerImage.fromString("docker.foo.com:4443/vespa/bar")); ClusterMembership instance = ClusterMembership.from("combined/id1/4/37/exclusive/containerId1", Vtag.currentVersion, dockerImageRepo); ClusterMembership serialized = ClusterMembership.from(instance.stringValue(), Vtag.currentVersion, dockerImageRepo); assertEquals(instance, serialized); diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java index f2163608050..33157a01c07 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java @@ -41,7 +41,7 @@ public class AllocatedHostsSerializerTest { List.of(), Optional.empty(), Optional.of(ClusterMembership.from("container/test/0/0", Version.fromString("6.73.1"), - Optional.of("docker.foo.com:4443/vespa/bar"))), + Optional.of(DockerImage.fromString("docker.foo.com:4443/vespa/bar")))), Optional.empty(), Optional.empty(), Optional.empty(), Optional.of(DockerImage.fromString("docker.foo.com:4443/vespa/bar")))); hosts.add(new HostSpec("flavor-from-resources-1", diff --git a/configdefinitions/src/vespa/fleetcontroller.def b/configdefinitions/src/vespa/fleetcontroller.def index 3f2a5637a53..96b43a15c5e 100644 --- a/configdefinitions/src/vespa/fleetcontroller.def +++ b/configdefinitions/src/vespa/fleetcontroller.def @@ -179,10 +179,5 @@ min_merge_completion_ratio double default=1.0 ## transition logic aims to minimize the window of time where active states diverge. enable_two_phase_cluster_state_transitions bool default=false -## Determines which metric will be used to decide whether a content node manages -## zero buckets, when deciding whether it can be set permanently down (typically -## to be removed from the application). -## If true, use vds.datastored.bucket_space.buckets_total (new), otherwise use -## vds.datastored.alldisks.buckets (legacy). -## This setting is intended to be used to safely roll out the new metric. +## Deprecated - not used determine_buckets_from_bucket_space_metric bool default=true diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index aaa671d5ad3..400460d2ce6 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -150,7 +150,6 @@ public class ModelContextImpl implements ModelContext { private final Optional<EndpointCertificateSecrets> endpointCertificateSecrets; private final double defaultTermwiseLimit; private final double defaultSoftStartSeconds; - private final boolean useBucketSpaceMetric; private final String proxyProtocol; private final Optional<AthenzDomain> athenzDomain; @@ -188,8 +187,6 @@ public class ModelContextImpl implements ModelContext { .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); defaultTopKprobability = Flags.DEFAULT_TOP_K_PROBABILITY.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); - this.useBucketSpaceMetric = Flags.USE_BUCKET_SPACE_METRIC.bindTo(flagSource) - .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); this.proxyProtocol = Flags.PROXY_PROTOCOL.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); this.athenzDomain = athenzDomain; @@ -244,7 +241,6 @@ public class ModelContextImpl implements ModelContext { @Override public double defaultTermwiseLimit() { return defaultTermwiseLimit; } - @Override public double defaultSoftStartSeconds() { return 0; } @@ -254,8 +250,9 @@ public class ModelContextImpl implements ModelContext { return defaultTopKprobability; } + // TODO: Remove @Override - public boolean useBucketSpaceMetric() { return useBucketSpaceMetric; } + public boolean useBucketSpaceMetric() { return true; } @Override public boolean useNewAthenzFilter() { return true; } diff --git a/container-core/src/main/java/com/yahoo/container/core/config/BundleInstaller.java b/container-core/src/main/java/com/yahoo/container/core/config/BundleInstaller.java new file mode 100644 index 00000000000..fc919571b6c --- /dev/null +++ b/container-core/src/main/java/com/yahoo/container/core/config/BundleInstaller.java @@ -0,0 +1,21 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import com.yahoo.osgi.Osgi; +import org.osgi.framework.Bundle; + +import java.util.List; + +/** + * @author gjoranv + */ +public interface BundleInstaller { + + /** + * Installs the bundle with the given file reference, plus all bundles in its X-JDisc-Preinstall-Bundle directive. + * Returns all bundles installed to the given OSGi framework as a result of this call. + */ + List<Bundle> installBundles(FileReference reference, Osgi osgi) throws InterruptedException; + +} diff --git a/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java b/container-core/src/main/java/com/yahoo/container/core/config/BundleManager.java index ca11ad387ee..5e9b42a5fda 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/BundleManager.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.core.config; import com.yahoo.collections.PredicateSplit; @@ -9,13 +9,12 @@ import com.yahoo.osgi.Osgi; import org.osgi.framework.Bundle; import org.osgi.framework.wiring.BundleRevision; -import java.io.File; +import java.util.ArrayList; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.TimeUnit; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -23,12 +22,12 @@ import static com.yahoo.collections.PredicateSplit.partition; import static com.yahoo.container.core.BundleLoaderProperties.DISK_BUNDLE_PREFIX; /** - * Manages the set of installed 3rd-party component bundles. + * Manages the set of installed and active/inactive bundles. * - * @author Tony Vaagenes * @author gjoranv + * @author Tony Vaagenes */ -public class BundleLoader { +public class BundleManager { /* Map of file refs of active bundles (not scheduled for uninstall) to a list of all bundles that were installed * (pre-install directive) by the bundle pointed to by the file ref (including itself). @@ -40,20 +39,96 @@ public class BundleLoader { */ private final Map<FileReference, List<Bundle>> reference2Bundles = new LinkedHashMap<>(); - private final Logger log = Logger.getLogger(BundleLoader.class.getName()); + private final Logger log = Logger.getLogger(BundleManager.class.getName()); private final Osgi osgi; - public BundleLoader(Osgi osgi) { + // A custom bundle installer for non-disk bundles, to be used for testing + private BundleInstaller customBundleInstaller = null; + + public BundleManager(Osgi osgi) { this.osgi = osgi; } - private void install(List<FileReference> references) { + /** + * Installs the given set of bundles and returns the set of bundles that is no longer used + * by the application, and should therefore be scheduled for uninstall. + */ + public synchronized Set<Bundle> use(List<FileReference> newFileReferences) { + // Must be done before allowing duplicates because allowed duplicates affect osgi.getCurrentBundles + Set<Bundle> bundlesToUninstall = getObsoleteBundles(newFileReferences); + + Set<FileReference> obsoleteReferences = getObsoleteFileReferences(newFileReferences); + allowDuplicateBundles(obsoleteReferences); + removeInactiveFileReferences(obsoleteReferences); + + installBundles(newFileReferences); + startBundles(); + + bundlesToUninstall.removeAll(allActiveBundles()); + log.info("Bundles to schedule for uninstall: " + bundlesToUninstall); + + log.info(installedBundlesMessage()); + return bundlesToUninstall; + } + + /** + * Returns the bundles that are not assumed to be retained by the new application generation. + * Note that at this point we don't yet know the full set of new bundles, because of the potential + * pre-install directives in the new bundles. However, only "disk bundles" (file:) can be listed + * in the pre-install directive, so we know about all the obsolete application bundles. + */ + private Set<Bundle> getObsoleteBundles(List<FileReference> newReferences) { + Set<Bundle> bundlesToRemove = new HashSet<>(osgi.getCurrentBundles()); + + for (FileReference fileReferenceToKeep : newReferences) { + if (reference2Bundles.containsKey(fileReferenceToKeep)) { + bundlesToRemove.removeAll(reference2Bundles.get(fileReferenceToKeep)); + } + } + bundlesToRemove.removeAll(osgi.getInitialBundles()); + return bundlesToRemove; + } + + + private Set<FileReference> getObsoleteFileReferences(List<FileReference> newReferences) { + Set<FileReference> obsoleteReferences = new HashSet<>(reference2Bundles.keySet()); + obsoleteReferences.removeAll(newReferences); + return obsoleteReferences; + } + + /** + * Allow duplicates (bsn+version) for each bundle that corresponds to obsolete file references, + * and avoid allowing duplicates for bundles that were installed via the + * X-JDisc-Preinstall-Bundle directive. These bundles are always "disk bundles" (library + * bundles installed on the node, and not transferred via file distribution). + * Such bundles will never have duplicates because they always have the same location id. + */ + private void allowDuplicateBundles(Set<FileReference> obsoleteReferences) { + // The bundle at index 0 for each file reference always corresponds to the bundle at the file reference location + Set<Bundle> allowedDuplicates = obsoleteReferences.stream() + .filter(reference -> ! isDiskBundle(reference)) + .map(reference -> reference2Bundles.get(reference).get(0)) + .collect(Collectors.toSet()); + + log.info(() -> allowedDuplicates.isEmpty() ? "" : "Adding bundles to allowed duplicates: " + allowedDuplicates); + osgi.allowDuplicateBundles(allowedDuplicates); + } + + /** + * Cleans up the map of active file references + */ + private void removeInactiveFileReferences(Set<FileReference> fileReferencesToRemove) { + // Clean up the map of active bundles + fileReferencesToRemove.forEach(reference2Bundles::remove); + } + + private void installBundles(List<FileReference> references) { Set<FileReference> bundlesToInstall = new HashSet<>(references); // This is just an optimization, as installing a bundle with the same location id returns the already installed bundle. bundlesToInstall.removeAll(reference2Bundles.keySet()); - PredicateSplit<FileReference> bundlesToInstall_isDisk = partition(bundlesToInstall, BundleLoader::isDiskBundle); + PredicateSplit<FileReference> bundlesToInstall_isDisk = partition(bundlesToInstall, BundleManager::isDiskBundle); installBundlesFromDisk(bundlesToInstall_isDisk.trueValues); installBundlesFromFileDistribution(bundlesToInstall_isDisk.falseValues); @@ -81,7 +156,9 @@ public class BundleLoader { FileAcquirer fileAcquirer = Container.get().getFileAcquirer(); boolean hasFileDistribution = (fileAcquirer != null); if (hasFileDistribution) { - installWithFileDistribution(bundlesToInstall, fileAcquirer); + installWithFileDistribution(bundlesToInstall, new FileAcquirerBundleInstaller(fileAcquirer)); + } else if (customBundleInstaller != null) { + installWithFileDistribution(bundlesToInstall, customBundleInstaller); } else { log.warning("Can't retrieve bundles since file distribution is disabled."); } @@ -89,25 +166,18 @@ public class BundleLoader { } private void installBundleFromDisk(FileReference reference) { - assert(reference.value().startsWith(DISK_BUNDLE_PREFIX)); - String referenceFileName = reference.value().substring(DISK_BUNDLE_PREFIX.length()); log.info("Installing bundle from disk with reference '" + reference.value() + "'"); - File file = new File(referenceFileName); - if ( ! file.exists()) { - throw new IllegalArgumentException("Reference '" + reference.value() + "' not found on disk."); - } - - List<Bundle> bundles = osgi.install(file.getAbsolutePath()); - + var bundleInstaller = new DiskBundleInstaller(); + List<Bundle> bundles = bundleInstaller.installBundles(reference, osgi); reference2Bundles.put(reference, bundles); } - private void installWithFileDistribution(List<FileReference> bundlesToInstall, FileAcquirer fileAcquirer) { + private void installWithFileDistribution(List<FileReference> bundlesToInstall, BundleInstaller bundleInstaller) { for (FileReference reference : bundlesToInstall) { try { log.info("Installing bundle with reference '" + reference.value() + "'"); - List<Bundle> bundles = obtainBundles(reference, fileAcquirer); + List<Bundle> bundles = bundleInstaller.installBundles(reference, osgi); reference2Bundles.put(reference, bundles); } catch(Exception e) { @@ -116,11 +186,6 @@ public class BundleLoader { } } - private List<Bundle> obtainBundles(FileReference reference, FileAcquirer fileAcquirer) throws InterruptedException { - File file = fileAcquirer.waitFor(reference, 7, TimeUnit.DAYS); - return osgi.install(file.getAbsolutePath()); - } - /** * Resolves and starts (calls the Bundles BundleActivator) all bundles. Bundle resolution must take place * after all bundles are installed to ensure that the framework can resolve dependencies between bundles. @@ -146,87 +211,12 @@ public class BundleLoader { return (bundleRevision.getTypes() & BundleRevision.TYPE_FRAGMENT) != 0; } - /** - * Returns the bundles that are not assumed to be retained by the new application generation. - * Note that at this point we don't yet know the full set of new bundles, because of the potential - * pre-install directives in the new bundles. However, only "disk bundles" (file:) can be listed - * in the pre-install directive, so we know about all the obsolete application bundles. - */ - private Set<Bundle> getObsoleteBundles(List<FileReference> newReferences) { - Set<Bundle> bundlesToRemove = new HashSet<>(osgi.getCurrentBundles()); - - for (FileReference fileReferenceToKeep : newReferences) { - if (reference2Bundles.containsKey(fileReferenceToKeep)) { - bundlesToRemove.removeAll(reference2Bundles.get(fileReferenceToKeep)); - } - } - bundlesToRemove.removeAll(osgi.getInitialBundles()); - return bundlesToRemove; - } - - /** - * Cleans up the map of active file references - */ - private void removeInactiveFileReferences(List<FileReference> newReferences) { - // Clean up the map of active bundles - Set<FileReference> fileReferencesToRemove = getObsoleteFileReferences(newReferences); - fileReferencesToRemove.forEach(reference2Bundles::remove); - } - - - /** - * Allow duplicates (bsn+version) for each bundle that corresponds to obsolete file references, - * and avoid allowing duplicates for bundles that were installed via the - * X-JDisc-Preinstall-Bundle directive. These bundles are always "disk bundles" (library - * bundles installed on the node, and not transferred via file distribution). - * Such bundles will never have duplicates because they always have the same location id. - */ - private void allowDuplicateBundles(List<FileReference> newReferences) { - Set<FileReference> obsoleteReferences = getObsoleteFileReferences(newReferences); - - // The bundle at index 0 for each file reference always corresponds to the bundle at the file reference location - Set<Bundle> allowedDuplicates = obsoleteReferences.stream() - .filter(reference -> ! isDiskBundle(reference)) - .map(reference -> reference2Bundles.get(reference).get(0)) - .collect(Collectors.toSet()); - - log.info(() -> allowedDuplicates.isEmpty() ? "" : "Adding bundles to allowed duplicates: " + allowedDuplicates); - osgi.allowDuplicateBundles(allowedDuplicates); - } - - private Set<FileReference> getObsoleteFileReferences(List<FileReference> newReferences) { - Set<FileReference> obsoleteReferences = new HashSet<>(reference2Bundles.keySet()); - obsoleteReferences.removeAll(newReferences); - return obsoleteReferences; - } - private Set<Bundle> allActiveBundles() { return reference2Bundles.keySet().stream() .flatMap(reference -> reference2Bundles.get(reference).stream()) .collect(Collectors.toSet()); } - /** - * Installs the given set of bundles and returns the set of bundles that is no longer used - * by the application, and should therefore be scheduled for uninstall. - */ - public synchronized Set<Bundle> use(List<FileReference> newBundles) { - // Must be done before allowing duplicates because allowed duplicates affect osgi.getCurrentBundles - Set<Bundle> bundlesToUninstall = getObsoleteBundles(newBundles); - - allowDuplicateBundles(newBundles); - removeInactiveFileReferences(newBundles); - - install(newBundles); - startBundles(); - - bundlesToUninstall.removeAll(allActiveBundles()); - log.info("Bundles to schedule for uninstall: " + bundlesToUninstall); - - log.info(installedBundlesMessage()); - return bundlesToUninstall; - } - private String installedBundlesMessage() { StringBuilder sb = new StringBuilder("Installed bundles: {" ); for (Bundle b : osgi.getBundles()) @@ -236,4 +226,14 @@ public class BundleLoader { return sb.toString(); } + // Only for testing + void useCustomBundleInstaller(BundleInstaller bundleInstaller) { + customBundleInstaller = bundleInstaller; + } + + // Only for testing + List<FileReference> getActiveFileReferences() { + return new ArrayList<>(reference2Bundles.keySet()); + } + } diff --git a/container-core/src/main/java/com/yahoo/container/core/config/DiskBundleInstaller.java b/container-core/src/main/java/com/yahoo/container/core/config/DiskBundleInstaller.java new file mode 100644 index 00000000000..3edabe9f861 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/container/core/config/DiskBundleInstaller.java @@ -0,0 +1,31 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import com.yahoo.osgi.Osgi; +import org.osgi.framework.Bundle; + +import java.io.File; +import java.util.List; + +import static com.yahoo.container.core.BundleLoaderProperties.DISK_BUNDLE_PREFIX; + +/** + * @author gjoranv + */ +public class DiskBundleInstaller implements BundleInstaller { + + @Override + public List<Bundle> installBundles(FileReference reference, Osgi osgi) { + assert(reference.value().startsWith(DISK_BUNDLE_PREFIX)); + String referenceFileName = reference.value().substring(DISK_BUNDLE_PREFIX.length()); + + File file = new File(referenceFileName); + if ( ! file.exists()) { + throw new IllegalArgumentException("Reference '" + reference.value() + "' not found on disk."); + } + + return osgi.install(file.getAbsolutePath()); + } + +} diff --git a/container-core/src/main/java/com/yahoo/container/core/config/FileAcquirerBundleInstaller.java b/container-core/src/main/java/com/yahoo/container/core/config/FileAcquirerBundleInstaller.java new file mode 100644 index 00000000000..72951e67b4e --- /dev/null +++ b/container-core/src/main/java/com/yahoo/container/core/config/FileAcquirerBundleInstaller.java @@ -0,0 +1,53 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import com.yahoo.filedistribution.fileacquirer.FileAcquirer; +import com.yahoo.osgi.Osgi; +import org.osgi.framework.Bundle; + +import java.io.File; +import java.nio.file.Files; +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.logging.Logger; + +/** + * @author gjoranv + */ +public class FileAcquirerBundleInstaller implements BundleInstaller { + private static Logger log = Logger.getLogger(FileAcquirerBundleInstaller.class.getName()); + + private final FileAcquirer fileAcquirer; + + public FileAcquirerBundleInstaller(FileAcquirer fileAcquirer) { + this.fileAcquirer = fileAcquirer; + } + + @Override + public List<Bundle> installBundles(FileReference reference, Osgi osgi) throws InterruptedException { + File file = fileAcquirer.waitFor(reference, 7, TimeUnit.DAYS); + + if (notReadable(file)) { + // Wait a few sec in case FileAcquirer returns right before the file is actually ready. + // This happened on rare occasions due to a (fixed) bug in file distribution. + log.warning("Unable to open bundle file with reference '" + reference + "'. Waiting for up to 5 sec."); + int retries = 0; + while (notReadable(file) && retries < 10) { + Thread.sleep(500); + retries++; + } + if (notReadable(file)) { + com.yahoo.protect.Process.logAndDie("Shutting down - unable to read bundle file with reference '" + reference + + "' and path " + file.getAbsolutePath()); + } + } + + return osgi.install(file.getAbsolutePath()); + } + + private static boolean notReadable(File file) { + return ! Files.isReadable(file.toPath()); + } + +} diff --git a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java index ef132694e10..d87b38e8b18 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java @@ -9,7 +9,6 @@ import com.yahoo.component.ComponentSpecification; import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.concurrent.ThreadFactoryFactory; import com.yahoo.config.FileReference; -import com.yahoo.container.core.config.testutil.MockOsgiWrapper; import com.yahoo.container.di.ComponentDeconstructor; import com.yahoo.container.di.Container; import com.yahoo.container.di.componentgraph.core.ComponentGraph; @@ -82,7 +81,7 @@ public class HandlersConfigurerDi { OsgiFramework osgiFramework) { this(subscriberFactory, vespaContainer, configId, deconstructor, discInjector, - new ContainerAndDiOsgi(osgiFramework, new BundleLoader(new OsgiImpl(osgiFramework)))); + new ContainerAndDiOsgi(osgiFramework, new BundleManager(new OsgiImpl(osgiFramework)))); } // Only public for testing @@ -102,9 +101,9 @@ public class HandlersConfigurerDi { private static class ContainerAndDiOsgi extends OsgiImpl implements OsgiWrapper { private final OsgiFramework osgiFramework; - private final BundleLoader bundleLoader; + private final BundleManager bundleLoader; - public ContainerAndDiOsgi(OsgiFramework osgiFramework, BundleLoader bundleLoader) { + public ContainerAndDiOsgi(OsgiFramework osgiFramework, BundleManager bundleLoader) { super(osgiFramework); this.osgiFramework = osgiFramework; this.bundleLoader = bundleLoader; diff --git a/container-core/src/test/java/com/yahoo/container/core/config/BundleManagerTest.java b/container-core/src/test/java/com/yahoo/container/core/config/BundleManagerTest.java new file mode 100644 index 00000000000..414e6b05128 --- /dev/null +++ b/container-core/src/test/java/com/yahoo/container/core/config/BundleManagerTest.java @@ -0,0 +1,106 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import org.junit.Before; +import org.junit.Test; +import org.osgi.framework.Bundle; + +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * @author gjoranv + */ +public class BundleManagerTest { + + private static final FileReference BUNDLE_1_REF = new FileReference("bundle-1"); + private static final Bundle BUNDLE_1 = new TestBundle(BUNDLE_1_REF.value()); + private static final FileReference BUNDLE_2_REF = new FileReference("bundle-2"); + private static final Bundle BUNDLE_2 = new TestBundle(BUNDLE_2_REF.value()); + + private BundleManager bundleLoader; + private TestOsgi osgi; + + @Before + public void setup() { + osgi = new TestOsgi(testBundles()); + var bundleInstaller = new TestBundleInstaller(); + bundleLoader = new BundleManager(osgi); + bundleLoader.useCustomBundleInstaller(bundleInstaller); + } + + @Test + public void bundles_are_installed_and_started() { + bundleLoader.use(List.of(BUNDLE_1_REF)); + assertEquals(1, osgi.getInstalledBundles().size()); + + // The bundle is installed and started + TestBundle installedBundle = (TestBundle)osgi.getInstalledBundles().get(0); + assertEquals(BUNDLE_1.getSymbolicName(), installedBundle.getSymbolicName()); + assertTrue(installedBundle.started); + + // The file reference is active + assertEquals(1, bundleLoader.getActiveFileReferences().size()); + assertEquals(BUNDLE_1_REF, bundleLoader.getActiveFileReferences().get(0)); + } + + @Test + public void new_bundle_can_be_installed_in_reconfig() { + bundleLoader.use(List.of(BUNDLE_1_REF)); + Set<Bundle> obsoleteBundles = bundleLoader.use(List.of(BUNDLE_1_REF, BUNDLE_2_REF)); + + // No bundles are obsolete + assertTrue(obsoleteBundles.isEmpty()); + + // Both bundles are installed + assertEquals(2, osgi.getInstalledBundles().size()); + assertEquals(BUNDLE_1.getSymbolicName(), osgi.getInstalledBundles().get(0).getSymbolicName()); + assertEquals(BUNDLE_2.getSymbolicName(), osgi.getInstalledBundles().get(1).getSymbolicName()); + + // Both bundles are current + assertEquals(2, osgi.getCurrentBundles().size()); + assertEquals(BUNDLE_1.getSymbolicName(), osgi.getCurrentBundles().get(0).getSymbolicName()); + assertEquals(BUNDLE_2.getSymbolicName(), osgi.getCurrentBundles().get(1).getSymbolicName()); + + + // Both file references are active + assertEquals(2, bundleLoader.getActiveFileReferences().size()); + assertEquals(BUNDLE_1_REF, bundleLoader.getActiveFileReferences().get(0)); + assertEquals(BUNDLE_2_REF, bundleLoader.getActiveFileReferences().get(1)); + } + + @Test + public void unused_bundle_is_marked_obsolete_after_reconfig() { + bundleLoader.use(List.of(BUNDLE_1_REF)); + Set<Bundle> obsoleteBundles = bundleLoader.use(List.of(BUNDLE_2_REF)); + + // The returned set of obsolete bundles contains bundle-1 + assertEquals(1, obsoleteBundles.size()); + assertEquals(BUNDLE_1.getSymbolicName(), obsoleteBundles.iterator().next().getSymbolicName()); + + // Both bundles are installed + assertEquals(2, osgi.getInstalledBundles().size()); + assertEquals(BUNDLE_1.getSymbolicName(), osgi.getInstalledBundles().get(0).getSymbolicName()); + assertEquals(BUNDLE_2.getSymbolicName(), osgi.getInstalledBundles().get(1).getSymbolicName()); + + // Only bundle-2 is current + assertEquals(1, osgi.getCurrentBundles().size()); + assertEquals(BUNDLE_2.getSymbolicName(), osgi.getCurrentBundles().get(0).getSymbolicName()); + + // Only the bundle-2 file reference is active + assertEquals(1, bundleLoader.getActiveFileReferences().size()); + assertEquals(BUNDLE_2_REF, bundleLoader.getActiveFileReferences().get(0)); + } + + + private static Map<String, Bundle> testBundles() { + return Map.of(BUNDLE_1_REF.value(), BUNDLE_1, + BUNDLE_2_REF.value(), BUNDLE_2); + } + +} diff --git a/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java b/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java new file mode 100644 index 00000000000..421f4302c27 --- /dev/null +++ b/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java @@ -0,0 +1,102 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.container.bundle.MockBundle; +import org.osgi.framework.Bundle; +import org.osgi.framework.Version; +import org.osgi.framework.wiring.BundleCapability; +import org.osgi.framework.wiring.BundleRequirement; +import org.osgi.framework.wiring.BundleRevision; +import org.osgi.framework.wiring.BundleWiring; +import org.osgi.resource.Capability; +import org.osgi.resource.Requirement; + +import java.util.List; + +/** + * @author gjoranv + */ +class TestBundle extends MockBundle { + + private static final BundleRevision revision = new TestBundleRevision(); + + private final String symbolicName; + + boolean started = false; + + TestBundle(String symbolicName) { + this.symbolicName = symbolicName; + } + + @Override + public void start() { + started = true; + } + + @Override + public String getSymbolicName() { + return symbolicName; + } + + + @SuppressWarnings("unchecked") + @Override + public <T> T adapt(Class<T> type) { + if (type.equals(BundleRevision.class)) { + return (T) revision; + } else { + throw new UnsupportedOperationException(); + } + } + + + static class TestBundleRevision implements BundleRevision { + + // Ensure this is not seen as a fragment bundle. + @Override + public int getTypes() { + return 0; + } + + @Override + public String getSymbolicName() { + throw new UnsupportedOperationException(); + } + + @Override + public Version getVersion() { + throw new UnsupportedOperationException(); + } + + @Override + public List<BundleCapability> getDeclaredCapabilities(String namespace) { + throw new UnsupportedOperationException(); + } + + @Override + public List<BundleRequirement> getDeclaredRequirements(String namespace) { + throw new UnsupportedOperationException(); + } + + @Override + public BundleWiring getWiring() { + throw new UnsupportedOperationException(); + } + + @Override + public List<Capability> getCapabilities(String namespace) { + throw new UnsupportedOperationException(); + } + + @Override + public List<Requirement> getRequirements(String namespace) { + throw new UnsupportedOperationException(); + } + + @Override + public Bundle getBundle() { + throw new UnsupportedOperationException(); + } + } + +} diff --git a/container-core/src/test/java/com/yahoo/container/core/config/TestBundleInstaller.java b/container-core/src/test/java/com/yahoo/container/core/config/TestBundleInstaller.java new file mode 100644 index 00000000000..43a5268eabf --- /dev/null +++ b/container-core/src/test/java/com/yahoo/container/core/config/TestBundleInstaller.java @@ -0,0 +1,20 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import com.yahoo.osgi.Osgi; +import org.osgi.framework.Bundle; + +import java.util.List; + +/** + * @author gjoranv + */ +class TestBundleInstaller implements BundleInstaller { + + @Override + public List<Bundle> installBundles(FileReference reference, Osgi osgi) { + return osgi.install(reference.value()); + } + +} diff --git a/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java b/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java new file mode 100644 index 00000000000..54a3159239c --- /dev/null +++ b/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java @@ -0,0 +1,57 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.osgi.MockOsgi; +import org.osgi.framework.Bundle; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Map; + +/** + * @author gjoranv + */ +class TestOsgi extends MockOsgi { + + private final Map<String, Bundle> availableBundles; + + private final List<Bundle> installedBundles = new ArrayList<>(); + private final List<Bundle> allowedDuplicates = new ArrayList<>(); + + TestOsgi(Map<String, Bundle> availableBundles) { + this.availableBundles = availableBundles; + } + + @Override + public List<Bundle> install(String fileReferenceValue) { + if (! availableBundles.containsKey(fileReferenceValue)) + throw new IllegalArgumentException("No such bundle: " + fileReferenceValue); + + Bundle bundle = availableBundles.get(fileReferenceValue); + installedBundles.add(bundle); + return List.of(bundle); + } + + @Override + public Bundle[] getBundles() { + return installedBundles.toArray(new Bundle[0]); + } + + public List<Bundle> getInstalledBundles() { + return installedBundles; + } + + @Override + public List<Bundle> getCurrentBundles() { + var currentBundles = new ArrayList<>(installedBundles); + currentBundles.removeAll(allowedDuplicates); + return currentBundles; + } + + @Override + public void allowDuplicateBundles(Collection<Bundle> bundles) { + allowedDuplicates.addAll(bundles); + } + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java index dd0c143ae28..ab39d8e5660 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java @@ -8,23 +8,28 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Optional; +import java.util.function.Supplier; /** * @author olaa */ public class MockContactRetriever implements ContactRetriever{ - private final Map<PropertyId, Contact> contacts = new HashMap<>(); + private final Map<PropertyId, Supplier<Contact>> contacts = new HashMap<>(); @Override public Contact getContact(Optional<PropertyId> propertyId) { - return contacts.getOrDefault(propertyId.get(), contact()); + return contacts.getOrDefault(propertyId.get(), this::contact).get(); } - public void addContact(PropertyId propertyId, Contact contact) { + public void addContact(PropertyId propertyId, Supplier<Contact> contact) { contacts.put(propertyId, contact); } + public void addContact(PropertyId propertyId, Contact contact) { + contacts.put(propertyId, () -> contact); + } + public Contact contact() { return new Contact(URI.create("contacts.tld"), URI.create("properties.tld"), URI.create("issues.tld"), Collections.emptyList(), "queue", Optional.of("component")); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java index 05c5acfda06..1e42efdd256 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java @@ -4,9 +4,13 @@ package com.yahoo.vespa.hosted.controller.api.systemflags.v1; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.SystemName; import com.yahoo.text.JSON; +import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.json.DimensionHelper; import com.yahoo.vespa.flags.json.FlagData; import java.io.BufferedInputStream; @@ -22,8 +26,11 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.TreeMap; +import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.zip.ZipEntry; @@ -153,7 +160,8 @@ public class SystemFlagsDataArchive { if (rawData.isBlank()) { flagData = new FlagData(directoryDeducedFlagId); } else { - flagData = FlagData.deserialize(rawData); + String normalizedRawData = normalizeJson(rawData); + flagData = FlagData.deserialize(normalizedRawData); if (!directoryDeducedFlagId.equals(flagData.id())) { throw new IllegalArgumentException( String.format("Flag data file with flag id '%s' in directory for '%s'", @@ -161,7 +169,6 @@ public class SystemFlagsDataArchive { } String serializedData = flagData.serializeToJson(); - String normalizedRawData = removeCommentsFromJson(rawData); if (!JSON.equals(serializedData, normalizedRawData)) { throw new IllegalArgumentException(filePath + " contains unknown non-comment fields: " + "after removing any comment fields the JSON is:\n " + @@ -174,19 +181,42 @@ public class SystemFlagsDataArchive { builder.addFile(filename, flagData); } - static String removeCommentsFromJson(String json) { - JsonNode jsonNode = uncheck(() -> mapper.readTree(json)); - removeComments(jsonNode); - return jsonNode.toString(); + static String normalizeJson(String json) { + JsonNode root = uncheck(() -> mapper.readTree(json)); + removeCommentsRecursively(root); + verifyValues(root); + return root.toString(); } - private static void removeComments(JsonNode node) { + private static void verifyValues(JsonNode root) { + var cursor = new JsonAccessor(root); + cursor.get("rules").forEachArrayElement(rule -> rule.get("conditions").forEachArrayElement(condition -> { + var dimension = condition.get("dimension"); + if (dimension.isEqualTo(DimensionHelper.toWire(FetchVector.Dimension.APPLICATION_ID))) { + condition.get("values").forEachArrayElement(conditionValue -> { + String applicationIdString = conditionValue.asString() + .orElseThrow(() -> new IllegalArgumentException("Non-string application ID: " + conditionValue)); + // Throws exception if not recognized + ApplicationId.fromSerializedForm(applicationIdString); + }); + } else if (dimension.isEqualTo(DimensionHelper.toWire(FetchVector.Dimension.NODE_TYPE))) { + condition.get("values").forEachArrayElement(conditionValue -> { + String nodeTypeString = conditionValue.asString() + .orElseThrow(() -> new IllegalArgumentException("Non-string node type: " + conditionValue)); + // Throws exception if not recognized + NodeType.valueOf(nodeTypeString); + }); + } + })); + } + + private static void removeCommentsRecursively(JsonNode node) { if (node instanceof ObjectNode) { ObjectNode objectNode = (ObjectNode) node; objectNode.remove("comment"); } - node.forEach(SystemFlagsDataArchive::removeComments); + node.forEach(SystemFlagsDataArchive::removeCommentsRecursively); } private static String toFilePath(FlagId flagId, String filename) { @@ -210,4 +240,40 @@ public class SystemFlagsDataArchive { } } + + private static class JsonAccessor { + private final JsonNode jsonNode; + + public JsonAccessor(JsonNode jsonNode) { + this.jsonNode = jsonNode; + } + + public JsonAccessor get(String fieldName) { + if (jsonNode == null) { + return this; + } else { + return new JsonAccessor(jsonNode.get(fieldName)); + } + } + + public Optional<String> asString() { + return jsonNode != null && jsonNode.isTextual() ? Optional.of(jsonNode.textValue()) : Optional.empty(); + } + + public void forEachArrayElement(Consumer<JsonAccessor> consumer) { + if (jsonNode != null && jsonNode.isArray()) { + jsonNode.forEach(jsonNodeElement -> consumer.accept(new JsonAccessor(jsonNodeElement))); + } + } + + /** Returns true if this (JsonNode) is a string and equal to value. */ + public boolean isEqualTo(String value) { + return jsonNode != null && jsonNode.isTextual() && Objects.equals(jsonNode.textValue(), value); + } + + @Override + public String toString() { + return jsonNode == null ? "undefined" : jsonNode.toString(); + } + } } diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java index 509de30fd44..35f6794e28d 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java @@ -33,7 +33,9 @@ import java.util.Set; import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author bjorncs @@ -132,7 +134,7 @@ public class SystemFlagsDataArchiveTest { " }\n" + " ]\n" + "}", - SystemFlagsDataArchive.removeCommentsFromJson("{\n" + + SystemFlagsDataArchive.normalizeJson("{\n" + " \"comment\": \"comment a\",\n" + " \"a\": {\n" + " \"comment\": \"comment b\",\n" + @@ -150,6 +152,54 @@ public class SystemFlagsDataArchiveTest { "}"))); } + @Test + public void normalize_json_fail_on_invalid_application() { + try { + SystemFlagsDataArchive.normalizeJson("{\n" + + " \"id\": \"foo\",\n" + + " \"rules\": [\n" + + " {\n" + + " \"conditions\": [\n" + + " {\n" + + " \"type\": \"whitelist\",\n" + + " \"dimension\": \"application\",\n" + + " \"values\": [ \"a.b.c\" ]\n" + + " }\n" + + " ],\n" + + " \"value\": true\n" + + " }\n" + + " ]\n" + + "}\n"); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Application ids must be on the form tenant:application:instance, but was a.b.c", e.getMessage()); + } + } + + @Test + public void normalize_json_fail_on_invalid_node_type() { + try { + SystemFlagsDataArchive.normalizeJson("{\n" + + " \"id\": \"foo\",\n" + + " \"rules\": [\n" + + " {\n" + + " \"conditions\": [\n" + + " {\n" + + " \"type\": \"whitelist\",\n" + + " \"dimension\": \"node-type\",\n" + + " \"values\": [ \"footype\" ]\n" + + " }\n" + + " ],\n" + + " \"value\": true\n" + + " }\n" + + " ]\n" + + "}\n"); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("No enum constant com.yahoo.config.provision.NodeType.footype", e.getMessage()); + } + } + private static void assertArchiveReturnsCorrectTestFlagDataForTarget(SystemFlagsDataArchive archive) { assertFlagDataHasValue(archive, MY_TEST_FLAG, mainControllerTarget, "main.controller"); assertFlagDataHasValue(archive, MY_TEST_FLAG, prodUsWestCfgTarget, "main.prod.us-west-1"); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java index 7e354a02832..d1f2087ce12 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java @@ -6,14 +6,18 @@ import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.LockedTenant; import com.yahoo.vespa.hosted.controller.TenantController; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.organization.ContactRetriever; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.yolean.Exceptions; import java.time.Duration; +import java.util.Optional; import java.util.function.Predicate; import java.util.logging.Logger; +import static java.util.logging.Level.INFO; + /** * Periodically fetch and store contact information for tenants. * @@ -34,13 +38,21 @@ public class ContactInformationMaintainer extends Maintainer { protected void maintain() { TenantController tenants = controller().tenants(); for (Tenant tenant : tenants.asList()) { + log.log(INFO, "Updating contact information for " + tenant); try { switch (tenant.type()) { - case athenz: tenants.lockIfPresent(tenant.name(), LockedTenant.Athenz.class, lockedTenant -> - tenants.store(lockedTenant.with(contactRetriever.getContact(lockedTenant.get().propertyId())))); - return; - case cloud: return; - default: throw new IllegalArgumentException("Unexpected tenant type '" + tenant.type() + "'."); + case athenz: + tenants.lockIfPresent(tenant.name(), LockedTenant.Athenz.class, lockedTenant -> { + Contact contact = contactRetriever.getContact(lockedTenant.get().propertyId()); + log.log(INFO, "Contact found for " + tenant + " was " + + (Optional.of(contact).equals(tenant.contact()) ? "un" : "") + "changed"); + tenants.store(lockedTenant.with(contact)); + }); + break; + case cloud: + break; + default: + throw new IllegalArgumentException("Unexpected tenant type '" + tenant.type() + "'."); } } catch (Exception e) { log.log(LogLevel.WARNING, "Failed to update contact information for " + tenant + ": " + diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java index cec44c5d3a0..3e283d6c54a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java @@ -52,7 +52,7 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { this.name = name; this.activeSystems = Set.copyOf(activeSystems); - service = new ScheduledThreadPoolExecutor(1); + service = new ScheduledThreadPoolExecutor(1, r -> new Thread(r, name() + "-worker")); long delay = staggeredDelay(controller.curator().cluster(), controller.hostname(), controller.clock().instant(), interval); service.scheduleAtFixedRate(this, delay, interval.toMillis(), TimeUnit.MILLISECONDS); jobControl.started(name()); @@ -115,8 +115,7 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { return interval.toMillis(); long offset = cluster.indexOf(host) * interval.toMillis() / cluster.size(); - long timeUntilNextRun = Math.floorMod(offset - now.toEpochMilli(), interval.toMillis()); - return timeUntilNextRun + interval.toMillis() / cluster.size(); + return Math.floorMod(offset - now.toEpochMilli(), interval.toMillis()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java index 0db718080c2..32d2e2f35f4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java @@ -37,22 +37,22 @@ public class ContactInformationMaintainerTest { @Test public void updates_contact_information() { - long propertyId = 1; - TenantName name = tester.createTenant("tenant1", "domain1", propertyId); - Supplier<AthenzTenant> tenant = () -> (AthenzTenant) tester.controller().tenants().require(name); - assertFalse("No contact information initially", tenant.get().contact().isPresent()); + PropertyId propertyId1 = new PropertyId("1"); + PropertyId propertyId2 = new PropertyId("2"); + TenantName name1 = tester.createTenant("tenant1", "domain1", 1L); + TenantName name2 = tester.createTenant("zenant1", "domain2", 2L); + Supplier<AthenzTenant> tenant1 = () -> (AthenzTenant) tester.controller().tenants().require(name1); + Supplier<AthenzTenant> tenant2 = () -> (AthenzTenant) tester.controller().tenants().require(name2); + assertFalse("No contact information initially", tenant1.get().contact().isPresent()); + assertFalse("No contact information initially", tenant2.get().contact().isPresent()); Contact contact = testContact(); - registerContact(propertyId, contact); - maintainer.run(); + tester.serviceRegistry().contactRetriever().addContact(propertyId1, () -> { throw new RuntimeException("ERROR"); }); + tester.serviceRegistry().contactRetriever().addContact(propertyId2, () -> contact); + maintainer.maintain(); - assertTrue("Contact information added", tenant.get().contact().isPresent()); - assertEquals(contact, tenant.get().contact().get()); - } - - private void registerContact(long propertyId, Contact contact) { - PropertyId p = new PropertyId(String.valueOf(propertyId)); - tester.serviceRegistry().contactRetrieverMock().addContact(p, contact); + assertEquals("No contact information added due to error", Optional.empty(), tenant1.get().contact()); + assertEquals("Contact information added", Optional.of(contact), tenant2.get().contact()); } private static Contact testContact() { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java index 3aa1f2b5af2..efd5d61ce56 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java @@ -38,17 +38,22 @@ public class MaintainerTest { @Test public void staggering() { List<HostName> cluster = List.of(HostName.from("cfg1"), HostName.from("cfg2"), HostName.from("cfg3")); - Instant now = Instant.ofEpochMilli(1001); Duration interval = Duration.ofMillis(300); - assertEquals(299, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); - assertEquals(399, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); - assertEquals(199, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + Instant now = Instant.ofEpochMilli(1000); + assertEquals(200, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); + assertEquals( 0, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); + assertEquals(100, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); - now = Instant.ofEpochMilli(1101); + now = Instant.ofEpochMilli(1001); assertEquals(199, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); assertEquals(299, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); - assertEquals(399, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + assertEquals( 99, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + + now = Instant.ofEpochMilli(1101); + assertEquals( 99, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); + assertEquals(199, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); + assertEquals(299, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); assertEquals(300, Maintainer.staggeredDelay(cluster, HostName.from("cfg0"), now, interval)); } diff --git a/default_build_settings.cmake b/default_build_settings.cmake index 622755495c2..6ed2e06e060 100644 --- a/default_build_settings.cmake +++ b/default_build_settings.cmake @@ -9,9 +9,9 @@ endfunction() function(setup_vespa_default_build_settings_rhel_7_7) message("-- Setting up default build settings for rhel 7.7") - set(DEFAULT_EXTRA_LINK_DIRECTORY "${VESPA_DEPS}/lib64" "/usr/lib64/llvm5.0/lib" PARENT_SCOPE) - set(DEFAULT_EXTRA_INCLUDE_DIRECTORY "${VESPA_DEPS}/include" "/usr/include/llvm5.0" "/usr/include/openblas" PARENT_SCOPE) - set(DEFAULT_VESPA_LLVM_VERSION "5.0" PARENT_SCOPE) + set(DEFAULT_EXTRA_LINK_DIRECTORY "${VESPA_DEPS}/lib64" "/usr/lib64/llvm7.0/lib" PARENT_SCOPE) + set(DEFAULT_EXTRA_INCLUDE_DIRECTORY "${VESPA_DEPS}/include" "/usr/include/llvm7.0" "/usr/include/openblas" PARENT_SCOPE) + set(DEFAULT_VESPA_LLVM_VERSION "7" PARENT_SCOPE) endfunction() function(setup_vespa_default_build_settings_rhel_8_1) @@ -21,9 +21,9 @@ endfunction() function(setup_vespa_default_build_settings_centos_7) message("-- Setting up default build settings for centos 7") - set(DEFAULT_EXTRA_LINK_DIRECTORY "${VESPA_DEPS}/lib64" "/usr/lib64/llvm5.0/lib" PARENT_SCOPE) - set(DEFAULT_EXTRA_INCLUDE_DIRECTORY "${VESPA_DEPS}/include" "/usr/include/llvm5.0" "/usr/include/openblas" PARENT_SCOPE) - set(DEFAULT_VESPA_LLVM_VERSION "5.0" PARENT_SCOPE) + set(DEFAULT_EXTRA_LINK_DIRECTORY "${VESPA_DEPS}/lib64" "/usr/lib64/llvm7.0/lib" PARENT_SCOPE) + set(DEFAULT_EXTRA_INCLUDE_DIRECTORY "${VESPA_DEPS}/include" "/usr/include/llvm7.0" "/usr/include/openblas" PARENT_SCOPE) + set(DEFAULT_VESPA_LLVM_VERSION "7" PARENT_SCOPE) endfunction() function(setup_vespa_default_build_settings_centos_8) @@ -37,9 +37,9 @@ function(setup_vespa_default_build_settings_darwin) if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang") set(DEFAULT_LLVM_INCLUDE_DIRECTORY "/usr/local/opt/llvm/include") set(DEFAULT_LLVM_LINK_DIRECTORY "/usr/local/opt/llvm/lib") - set(DEFAULT_VESPA_LLVM_VERSION "9" PARENT_SCOPE) + set(DEFAULT_VESPA_LLVM_VERSION "10" PARENT_SCOPE) else() - set(DEFAULT_VESPA_LLVM_VERSION "8" PARENT_SCOPE) + set(DEFAULT_VESPA_LLVM_VERSION "10" PARENT_SCOPE) endif() set(DEFAULT_CMAKE_PREFIX_PATH "${VESPA_DEPS}" "/usr/local/opt/bison" "/usr/local/opt/flex" "/usr/local/opt/openssl@1.1" "/usr/local/opt/openblas" PARENT_SCOPE) set(DEFAULT_EXTRA_LINK_DIRECTORY "${VESPA_DEPS}/lib" "/usr/local/opt/bison/lib" "/usr/local/opt/flex/lib" "/usr/local/opt/icu4c/lib" "/usr/local/opt/openssl@1.1/lib" "/usr/local/opt/openblas/lib") diff --git a/dist/vespa.spec b/dist/vespa.spec index 16222ed3852..b2f3d1d5df4 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -30,11 +30,11 @@ BuildRequires: centos-release-scl %endif %endif %if 0%{?el7} -BuildRequires: devtoolset-8-gcc-c++ -BuildRequires: devtoolset-8-libatomic-devel -BuildRequires: devtoolset-8-binutils +BuildRequires: devtoolset-9-gcc-c++ +BuildRequires: devtoolset-9-libatomic-devel +BuildRequires: devtoolset-9-binutils BuildRequires: rh-maven35 -%define _devtoolset_enable /opt/rh/devtoolset-8/enable +%define _devtoolset_enable /opt/rh/devtoolset-9/enable %define _rhmaven35_enable /opt/rh/rh-maven35/enable %endif %if 0%{?el8} @@ -49,7 +49,7 @@ BuildRequires: libatomic BuildRequires: Judy-devel %if 0%{?el7} BuildRequires: cmake3 -BuildRequires: llvm5.0-devel +BuildRequires: llvm7.0-devel BuildRequires: vespa-boost-devel >= 1.59.0-6 BuildRequires: vespa-gtest >= 1.8.1-1 BuildRequires: vespa-protobuf-devel >= 3.7.0-4 @@ -154,14 +154,14 @@ Requires: perf Requires: gdb Requires: net-tools %if 0%{?el7} -Requires: llvm5.0 +Requires: llvm7.0 Requires: vespa-openssl >= 1.1.1c-1 Requires: vespa-icu >= 65.1.0-1 Requires: vespa-protobuf >= 3.7.0-4 Requires: vespa-telegraf >= 1.1.1-1 -%define _vespa_llvm_version 5.0 -%define _extra_link_directory /usr/lib64/llvm5.0/lib;%{_vespa_deps_prefix}/lib64 -%define _extra_include_directory /usr/include/llvm5.0;%{_vespa_deps_prefix}/include;/usr/include/openblas +%define _vespa_llvm_version 7 +%define _extra_link_directory /usr/lib64/llvm7.0/lib;%{_vespa_deps_prefix}/lib64 +%define _extra_include_directory /usr/include/llvm7.0;%{_vespa_deps_prefix}/include;/usr/include/openblas %endif %if 0%{?el8} Requires: llvm-libs >= 8.0.1 diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadManager.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadManager.java deleted file mode 100644 index 6fd4beac056..00000000000 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadManager.java +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.docproc.jdisc; - -import com.yahoo.document.DocumentUtil; -import com.yahoo.log.LogLevel; - -import java.util.concurrent.atomic.AtomicLong; -import java.util.logging.Logger; - -/** - * @author Einar M R Rosenvinge - */ -class DocprocThreadManager { - - private static Logger log = Logger.getLogger(DocprocThreadManager.class.getName()); - - private final long maxConcurrentByteSize; - private final AtomicLong bytesStarted = new AtomicLong(0); - private final AtomicLong bytesFinished = new AtomicLong(0); - - DocprocThreadManager(double maxConcurrentFactor, double documentExpansionFactor, int containerCoreMemoryMb) { - this((long) (((double) DocumentUtil.calculateMaxPendingSize(maxConcurrentFactor, documentExpansionFactor, - containerCoreMemoryMb)) * maxConcurrentFactor)); - } - - DocprocThreadManager(long maxConcurrentByteSize) { - final int MINCONCURRENTBYTES=256*1024*1024; //256M - if (maxConcurrentByteSize < MINCONCURRENTBYTES) { - maxConcurrentByteSize = MINCONCURRENTBYTES; - } - - this.maxConcurrentByteSize = maxConcurrentByteSize; - log.log(LogLevel.CONFIG, "Docproc service allowed to concurrently process " - + (((double) maxConcurrentByteSize) / 1024.0d / 1024.0d) + " megabytes of input data."); - } - - boolean isAboveLimit() { - return (bytesFinished.get() - bytesStarted.get() > maxConcurrentByteSize); - } - void beforeExecute(DocumentProcessingTask task) { - bytesStarted.getAndAdd(task.getApproxSize()); - } - - void afterExecute(DocumentProcessingTask task) { - bytesFinished.getAndAdd(task.getApproxSize()); - } - void shutdown() { - } - -} diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutor.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutor.java deleted file mode 100644 index e1a902c8d5c..00000000000 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutor.java +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.docproc.jdisc; - -import com.yahoo.concurrent.DaemonThreadFactory; -import com.yahoo.log.LogLevel; - -import java.util.List; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; -import java.util.logging.Logger; - -/** - * @author Einar M R Rosenvinge - */ -public class DocprocThreadPoolExecutor extends ThreadPoolExecutor { - - private static Logger log = Logger.getLogger(DocprocThreadPoolExecutor.class.getName()); - private DocprocThreadManager threadManager; - - public DocprocThreadPoolExecutor(int maxNumThreads, BlockingQueue<Runnable> queue, DocprocThreadManager threadMgr) { - super((maxNumThreads > 0) ? maxNumThreads : Runtime.getRuntime().availableProcessors(), - (maxNumThreads > 0) ? maxNumThreads : 2048, - 1, TimeUnit.SECONDS, - queue, - new DaemonThreadFactory("docproc-")); - this.threadManager = threadMgr; - allowCoreThreadTimeOut(false); - log.log(LogLevel.DEBUG, "Created docproc thread pool with " + super.getCorePoolSize() + " worker threads."); - } - - @Override - protected void beforeExecute(Thread thread, Runnable runnable) { - threadManager.beforeExecute((DocumentProcessingTask) runnable); - } - - @Override - protected void afterExecute(Runnable runnable, Throwable throwable) { - threadManager.afterExecute((DocumentProcessingTask) runnable); - } - - @Override - public void shutdown() { - super.shutdown(); - threadManager.shutdown(); - } - - @Override - public List<Runnable> shutdownNow() { - List<Runnable> list = super.shutdownNow(); - threadManager.shutdown(); - return list; - } - - boolean isAboveLimit() { - return threadManager.isAboveLimit(); - } - -} diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandler.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandler.java index 5b7b9d85a91..d34c300b8d0 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandler.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandler.java @@ -35,6 +35,7 @@ import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; @@ -52,7 +53,7 @@ public class DocumentProcessingHandler extends AbstractRequestHandler { private final ComponentRegistry<DocprocService> docprocServiceRegistry; private final ComponentRegistry<AbstractConcreteDocumentFactory> docFactoryRegistry; private final ChainRegistry<DocumentProcessor> chainRegistry = new ChainRegistry<>(); - private DocprocThreadPoolExecutor threadPool; + private ThreadPoolExecutor threadPool; private final ScheduledThreadPoolExecutor laterExecutor = new ScheduledThreadPoolExecutor(2, new DaemonThreadFactory("docproc-later-")); private ContainerDocumentConfig containerDocConfig; @@ -61,7 +62,7 @@ public class DocumentProcessingHandler extends AbstractRequestHandler { public DocumentProcessingHandler(ComponentRegistry<DocprocService> docprocServiceRegistry, ComponentRegistry<DocumentProcessor> documentProcessorComponentRegistry, ComponentRegistry<AbstractConcreteDocumentFactory> docFactoryRegistry, - DocprocThreadPoolExecutor threadPool, DocumentTypeManager documentTypeManager, + ThreadPoolExecutor threadPool, DocumentTypeManager documentTypeManager, ChainsModel chainsModel, SchemaMap schemaMap, Statistics statistics, Metric metric, ContainerDocumentConfig containerDocConfig) { @@ -88,28 +89,27 @@ public class DocumentProcessingHandler extends AbstractRequestHandler { } } + private static int computeNumThreads(int maxThreads) { + return (maxThreads > 0) ? maxThreads : Runtime.getRuntime().availableProcessors(); + } + public DocumentProcessingHandler(ComponentRegistry<DocprocService> docprocServiceRegistry, ComponentRegistry<DocumentProcessor> documentProcessorComponentRegistry, ComponentRegistry<AbstractConcreteDocumentFactory> docFactoryRegistry, DocumentProcessingHandlerParameters params) { this(docprocServiceRegistry, documentProcessorComponentRegistry, docFactoryRegistry, - new DocprocThreadPoolExecutor(params.getMaxNumThreads(), - chooseQueueType(params.getMaxNumThreads()), - new DocprocThreadManager(params.getMaxConcurrentFactor(), - params.getDocumentExpansionFactor(), - params.getContainerCoreMemoryMb())), + new ThreadPoolExecutor(computeNumThreads(params.getMaxNumThreads()), + computeNumThreads(params.getMaxNumThreads()), + 0,TimeUnit.SECONDS, + new LinkedBlockingQueue<>(), + new DaemonThreadFactory("docproc-") + ), params.getDocumentTypeManager(), params.getChainsModel(), params.getSchemaMap(), params.getStatisticsManager(), params.getMetric(), params.getContainerDocConfig()); } - private static BlockingQueue<Runnable> chooseQueueType(int maxNumThreads) { - return (maxNumThreads > 0) - ? new LinkedBlockingQueue<>() - : new SynchronousQueue<>(); - } - @Inject public DocumentProcessingHandler(ComponentRegistry<DocumentProcessor> documentProcessorComponentRegistry, ComponentRegistry<AbstractConcreteDocumentFactory> docFactoryRegistry, @@ -124,9 +124,6 @@ public class DocumentProcessingHandler extends AbstractRequestHandler { this(new ComponentRegistry<>(), documentProcessorComponentRegistry, docFactoryRegistry, new DocumentProcessingHandlerParameters().setMaxNumThreads (docprocConfig.numthreads()) - .setMaxConcurrentFactor(containerMbusConfig.maxConcurrentFactor()) - .setDocumentExpansionFactor(containerMbusConfig.documentExpansionFactor()) - .setContainerCoreMemoryMb(containerMbusConfig.containerCoreMemory()) .setDocumentTypeManager(new DocumentTypeManager(docManConfig)) .setChainsModel(buildFromConfig(chainsConfig)).setSchemaMap(configureMapping(mappingConfig)) .setStatisticsManager(manager) @@ -198,14 +195,10 @@ public class DocumentProcessingHandler extends AbstractRequestHandler { } private void submit(DocumentProcessingTask task) { - if (threadPool.isAboveLimit()) { + try { + threadPool.execute(task); + } catch (RejectedExecutionException ree) { task.queueFull(); - } else { - try { - threadPool.execute(task); - } catch (RejectedExecutionException ree) { - task.queueFull(); - } } } diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandlerParameters.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandlerParameters.java index bf308e39218..b8a6aa9c105 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandlerParameters.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/DocumentProcessingHandlerParameters.java @@ -18,9 +18,6 @@ import com.yahoo.statistics.Statistics; public class DocumentProcessingHandlerParameters { private int maxNumThreads = 0; - private double maxConcurrentFactor = 0.2; - private double documentExpansionFactor = 20.0; - private int containerCoreMemoryMb = 50; private DocumentTypeManager documentTypeManager = null; private ChainsModel chainsModel = null; private SchemaMap schemaMap = null; @@ -28,22 +25,7 @@ public class DocumentProcessingHandlerParameters { private Metric metric = new NullMetric(); private ContainerDocumentConfig containerDocConfig; - public DocumentProcessingHandlerParameters() { - } - - /** - * Returns the number of megabytes of memory reserved for container core classes and data. - * - * @return the number of megabytes of memory reserved for container core classes and data. - */ - public int getContainerCoreMemoryMb() { - return containerCoreMemoryMb; - } - public DocumentProcessingHandlerParameters setContainerCoreMemoryMb(int containerCoreMemoryMb) { - this.containerCoreMemoryMb = containerCoreMemoryMb; - return this; - } public Metric getMetric() { return metric; @@ -55,36 +37,6 @@ public class DocumentProcessingHandlerParameters { } /** - * Returns the document expansion factor, i.e. by what factor a serialized and possibly compressed - * input document is expected to expand during deserialization, including any temporary memory needed - * when processing it. - * - * @return the document expansion factor. - */ - public double getDocumentExpansionFactor() { - return documentExpansionFactor; - } - - public DocumentProcessingHandlerParameters setDocumentExpansionFactor(double documentExpansionFactor) { - this.documentExpansionFactor = documentExpansionFactor; - return this; - } - - /** - * Returns the max concurrent factor. - * - * @return the max concurrent factor. - */ - public double getMaxConcurrentFactor() { - return maxConcurrentFactor; - } - - public DocumentProcessingHandlerParameters setMaxConcurrentFactor(double maxConcurrentFactor) { - this.maxConcurrentFactor = maxConcurrentFactor; - return this; - } - - /** * Returns the maximum number of thread that the thread pool will ever attempt to run simultaneously. * * @return the maximum number of thread that the thread pool will ever attempt to run simultaneously. diff --git a/docproc/src/test/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutorTestCase.java b/docproc/src/test/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutorTestCase.java deleted file mode 100644 index a74fb9a3edf..00000000000 --- a/docproc/src/test/java/com/yahoo/docproc/jdisc/DocprocThreadPoolExecutorTestCase.java +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.docproc.jdisc; - -import org.junit.Test; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.TimeUnit; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -/** - * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a> - */ -public class DocprocThreadPoolExecutorTestCase { - private final Set<Long> threadIds = Collections.synchronizedSet(new HashSet<Long>()); - - @Test - public void threadPool() throws InterruptedException { - int numThreads = 8; - int numTasks = 200; - - LinkedBlockingQueue<Runnable> q = new LinkedBlockingQueue<>(); - DocprocThreadManager mgr = new DocprocThreadManager(1000l); - DocprocThreadPoolExecutor pool = new DocprocThreadPoolExecutor(numThreads, q, mgr); - - List<MockedDocumentProcessingTask> tasks = new ArrayList<>(numTasks); - for (int i = 0; i < numTasks; i++) { - tasks.add(new MockedDocumentProcessingTask()); - } - - for (int i = 0; i < numTasks; i++) { - pool.execute(tasks.get(i)); - } - pool.shutdown(); - pool.awaitTermination(120L, TimeUnit.SECONDS); - - for (int i = 0; i < numTasks; i++) { - assertTrue(tasks.get(i).hasBeenRun()); - } - - System.err.println(threadIds); - assertEquals(numThreads, threadIds.size()); - } - - private class MockedDocumentProcessingTask extends DocumentProcessingTask { - private boolean hasBeenRun = false; - - public MockedDocumentProcessingTask() { - super(null, null, null); - } - - @Override - public void run() { - threadIds.add(Thread.currentThread().getId()); - System.err.println(System.currentTimeMillis() + " MOCK Thread " + Thread.currentThread().getId() + " running task " + this); - for (int i = 0; i < 100000; i++) { - Math.sin((double) (System.currentTimeMillis() / 10000L)); - } - System.err.println(System.currentTimeMillis() + " MOCK Thread " + Thread.currentThread().getId() + " DONE task " + this); - hasBeenRun = true; - } - - @Override - public int getApproxSize() { - return 333; - } - - @Override - public String toString() { - return "seqNum " + getSeqNum(); - } - - public boolean hasBeenRun() { - return hasBeenRun; - } - } -} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index a4e34098025..40faefdd052 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -102,13 +102,6 @@ public class Flags { "Takes effect on next node agent tick. Change is orchestrated, but does NOT require container restart", HOSTNAME, APPLICATION_ID); - public static final UnboundBooleanFlag USE_BUCKET_SPACE_METRIC = defineFeatureFlag( - "use-bucket-space-metric", true, - "Whether to use vds.datastored.bucket_space.buckets_total (true) instead of " + - "vds.datastored.alldisks.buckets (false, legacy).", - "Takes effect on the next deployment of the application", - APPLICATION_ID); - public static final UnboundStringFlag TLS_INSECURE_AUTHORIZATION_MODE = defineStringFlag( "tls-insecure-authorization-mode", "log_only", "TLS insecure authorization mode. Allowed values: ['disable', 'log_only', 'enforce']", diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java index cd856c275ea..6aa5a8d9fc1 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java @@ -228,20 +228,27 @@ public class CoredumpHandler { private Dimensions generateDimensions(NodeAgentContext context) { NodeSpec node = context.node(); - ApplicationId owner = node.owner().get(); - NodeMembership membership = node.membership().get(); Dimensions.Builder dimensionsBuilder = new Dimensions.Builder() .add("host", node.hostname()) .add("flavor", node.flavor()) .add("state", node.state().toString()) - .add("zone", context.zone().getId().value()) - .add("tenantName", owner.tenant().value()) - .add("applicationName", owner.application().value()) - .add("instanceName", owner.instance().value()) - .add("app", String.join(".", owner.application().value(), owner.instance().value())) - .add("applicationId", owner.toFullString()) - .add("clustertype", membership.clusterType()) - .add("clusterid", membership.clusterId()); + .add("zone", context.zone().getId().value()); + + node.owner().ifPresent(owner -> + dimensionsBuilder + .add("tenantName", owner.tenant().value()) + .add("applicationName", owner.application().value()) + .add("instanceName", owner.instance().value()) + .add("app", String.join(".", owner.application().value(), owner.instance().value())) + .add("applicationId", owner.toFullString()) + ); + + node.membership().ifPresent(membership -> + dimensionsBuilder + .add("clustertype", membership.clusterType()) + .add("clusterid", membership.clusterId()) + ); + node.parentHostname().ifPresent(parent -> dimensionsBuilder.add("parentHostname", parent)); node.allowedToBeDown().ifPresent(allowed -> dimensionsBuilder.add("orchestratorState", allowed ? "ALLOWED_TO_BE_DOWN" : "NO_REMARKS")); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumCommand.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumCommand.java index 12029c4fe7e..7b543304e6e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumCommand.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumCommand.java @@ -5,6 +5,7 @@ import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandLine; import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal; +import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.regex.Matcher; @@ -41,6 +42,7 @@ public abstract class YumCommand<T extends YumCommand<T>> { private final String yumCommand; private final Pattern commandOutputNoopPattern; private final List<YumPackageName> packages; + private final List<String> options = new ArrayList<>(); GenericYumCommand(Terminal terminal, String yumCommand, List<YumPackageName> packages, Pattern commandOutputNoopPattern) { this.terminal = terminal; @@ -48,12 +50,31 @@ public abstract class YumCommand<T extends YumCommand<T>> { this.packages = packages; this.commandOutputNoopPattern = commandOutputNoopPattern; + switch (yumCommand) { + case "install": { + if (packages.size() > 1) options.add("skip_missing_names_on_install=False"); + break; + } + case "upgrade": { + if (packages.size() > 1) options.add("skip_missing_names_on_update=False"); + break; + } + case "remove": break; + default: throw new IllegalArgumentException("Unknown yum command: " + yumCommand); + } + if (packages.isEmpty() && ! "upgrade".equals(yumCommand)) { throw new IllegalArgumentException("No packages specified"); } } @Override + protected void addParametersToCommandLine(CommandLine commandLine) { + super.addParametersToCommandLine(commandLine); + options.forEach(option -> commandLine.add("--setopt", option)); + } + + @Override public boolean converge(TaskContext context) { if (packages.isEmpty() && ! "upgrade".equals(yumCommand)) { throw new IllegalArgumentException("No packages specified"); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTester.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTester.java index 4d04e1199c5..c5b46473218 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTester.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTester.java @@ -73,6 +73,10 @@ public class YumTester extends Yum { StringBuilder cmd = new StringBuilder(); cmd.append("yum ").append(command).append(" --assumeyes"); enableRepos.forEach(repo -> cmd.append(" --enablerepo=").append(repo)); + if ("install".equals(command) && packages.size() > 1) + cmd.append(" --setopt skip_missing_names_on_install=False"); + if ("upgrade".equals(command) && packages.size() > 1) + cmd.append(" --setopt skip_missing_names_on_update=False"); packages.forEach(pkg -> cmd.append(" ").append(pkg.toName())); cmd.append(" 2>&1"); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java index a0887e74e47..dc9c036b96b 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java @@ -75,7 +75,7 @@ public class YumTest { @Test public void testAlreadyInstalled() { terminal.expectCommand( - "yum install --assumeyes --enablerepo=repo1 --enablerepo=repo2 package-1 package-2 2>&1", + "yum install --assumeyes --enablerepo=repo1 --enablerepo=repo2 --setopt skip_missing_names_on_install=False package-1 package-2 2>&1", 0, "foobar\nNothing to do\n"); @@ -88,7 +88,7 @@ public class YumTest { @Test public void testAlreadyUpgraded() { terminal.expectCommand( - "yum upgrade --assumeyes package-1 package-2 2>&1", + "yum upgrade --assumeyes --setopt skip_missing_names_on_update=False package-1 package-2 2>&1", 0, "foobar\nNo packages marked for update\n"); @@ -112,7 +112,7 @@ public class YumTest { @Test public void testInstall() { terminal.expectCommand( - "yum install --assumeyes package-1 package-2 2>&1", + "yum install --assumeyes --setopt skip_missing_names_on_install=False package-1 package-2 2>&1", 0, "installing, installing"); @@ -124,7 +124,7 @@ public class YumTest { @Test public void testInstallWithEnablerepo() { terminal.expectCommand( - "yum install --assumeyes --enablerepo=repo-name package-1 package-2 2>&1", + "yum install --assumeyes --enablerepo=repo-name --setopt skip_missing_names_on_install=False package-1 package-2 2>&1", 0, "installing, installing"); @@ -210,7 +210,7 @@ public class YumTest { @Test(expected = ChildProcessFailureException.class) public void testFailedInstall() { terminal.expectCommand( - "yum install --assumeyes --enablerepo=repo-name package-1 package-2 2>&1", + "yum install --assumeyes --enablerepo=repo-name --setopt skip_missing_names_on_install=False package-1 package-2 2>&1", 1, "error"); @@ -224,7 +224,7 @@ public class YumTest { @Test public void testUnknownPackages() { terminal.expectCommand( - "yum install --assumeyes package-1 package-2 package-3 2>&1", + "yum install --assumeyes --setopt skip_missing_names_on_install=False package-1 package-2 package-3 2>&1", 0, "Loaded plugins: fastestmirror, langpacks\n" + "Loading mirror speeds from cached hostfile\n" + diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java index e01f7ea7bf5..17c73282b4b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Maintainer.java @@ -42,7 +42,7 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { HostName hostname = HostName.from(com.yahoo.net.HostName.getLocalhost()); long delay = staggeredDelay(nodeRepository.database().cluster(), hostname, nodeRepository.clock().instant(), interval); - service = new ScheduledThreadPoolExecutor(1); + service = new ScheduledThreadPoolExecutor(1, r -> new Thread(r, name() + "-worker")); service.scheduleAtFixedRate(this, delay, interval.toMillis(), TimeUnit.MILLISECONDS); jobControl.started(name(), this); } @@ -113,8 +113,7 @@ public abstract class Maintainer extends AbstractComponent implements Runnable { return interval.toMillis(); long offset = cluster.indexOf(host) * interval.toMillis() / cluster.size(); - long timeUntilNextRun = Math.floorMod(offset - now.toEpochMilli(), interval.toMillis()); - return timeUntilNextRun + interval.toMillis() / cluster.size(); + return Math.floorMod(offset - now.toEpochMilli(), interval.toMillis()); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java index 94f4dab1245..2f834ab289f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java @@ -187,7 +187,7 @@ public class NodeSerializer { object.setLong(currentRestartGenerationKey, allocation.restartGeneration().current()); object.setBool(removableKey, allocation.isRemovable()); object.setString(wantedVespaVersionKey, allocation.membership().cluster().vespaVersion().toString()); - allocation.membership().cluster().dockerImageRepo().ifPresent(repo -> object.setString(wantedDockerImageRepoKey, repo)); + allocation.membership().cluster().dockerImageRepo().ifPresent(repo -> object.setString(wantedDockerImageRepoKey, repo.repository())); allocation.networkPorts().ifPresent(ports -> NetworkPortsSerializer.toSlime(ports, object.setArray(networkPortsKey))); } @@ -317,9 +317,9 @@ public class NodeSerializer { return Optional.of(Version.fromString(object.asString())); } - private Optional<String> dockerImageRepoFromSlime(Inspector object) { + private Optional<DockerImage> dockerImageRepoFromSlime(Inspector object) { if ( ! object.valid() || object.asString().isEmpty()) return Optional.empty(); - return Optional.of(object.asString()); + return Optional.of(DockerImage.fromString(object.asString())); } private Optional<DockerImage> dockerImageFromSlime(Inspector object) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index d96282f1722..652fd5c6861 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -55,6 +55,9 @@ class NodeAllocation { /** The number of nodes in the accepted nodes which are of the requested flavor */ private int acceptedOfRequestedFlavor = 0; + /** The number of nodes in the accepted nodes which are of the requested flavor and not already retired */ + private int acceptedNonretiredOfRequestedFlavor = 0; + /** The number of nodes rejected because of clashing parentHostname */ private int rejectedWithClashingParentHost = 0; @@ -115,7 +118,7 @@ class NodeAllocation { if (offered.status().wantToRetire()) wantToRetireNode = true; if (requestedNodes.isExclusive() && ! hostsOnly(application.tenant(), offered.parentHostname())) wantToRetireNode = true; - if (( ! saturated() && hasCompatibleFlavor(node)) || acceptToRetire(node)) + if (( ! saturatedByNonretired() && hasCompatibleFlavor(node)) || acceptToRetire(node)) accepted.add(acceptNode(node, wantToRetireNode, node.isResizable)); } else { @@ -213,6 +216,7 @@ class NodeAllocation { private boolean acceptToRetire(PrioritizableNode node) { if (node.node.state() != Node.State.active) return false; if (! node.node.allocation().get().membership().cluster().group().equals(cluster.group())) return false; + if (node.node.allocation().get().membership().retired()) return true; // don't second-guess if already retired return cluster.type().isContent() || (cluster.type() == ClusterSpec.Type.container && !hasCompatibleFlavor(node)); @@ -229,17 +233,15 @@ class NodeAllocation { node = node.with(node.allocation().get().withRequestedResources(requestedNodes.resources().orElse(node.flavor().resources()))); if (! wantToRetire) { - if (resize) { - NodeResources hostResources = allNodes.parentOf(node).get().flavor().resources(); - node = node.with(new Flavor(requestedNodes.resources().get() - .with(hostResources.diskSpeed()) - .with(hostResources.storageType()))); - } + if (resize && ! ( node.allocation().isPresent() && node.allocation().get().membership().retired())) + node = resize(node); if (node.state() != Node.State.active) // reactivated node - make sure its not retired node = node.unretire(); acceptedOfRequestedFlavor++; + if ( ! (node.allocation().isPresent() && node.allocation().get().membership().retired())) + acceptedNonretiredOfRequestedFlavor++; } else { ++wasRetiredJustNow; // Retire nodes which are of an unwanted flavor, retired flavor or have an overlapping parent host @@ -256,6 +258,13 @@ class NodeAllocation { return node; } + private Node resize(Node node) { + NodeResources hostResources = allNodes.parentOf(node).get().flavor().resources(); + return node.with(new Flavor(requestedNodes.resources().get() + .with(hostResources.diskSpeed()) + .with(hostResources.storageType()))); + } + private Node setCluster(ClusterSpec cluster, Node node) { ClusterMembership membership = node.allocation().get().membership().with(cluster); return node.with(node.allocation().get().with(membership)); @@ -266,6 +275,11 @@ class NodeAllocation { return requestedNodes.saturatedBy(acceptedOfRequestedFlavor); } + /** Returns true if no more nodes are needed in this list to not make changes to the retired set */ + private boolean saturatedByNonretired() { + return requestedNodes.saturatedBy(acceptedNonretiredOfRequestedFlavor); + } + /** Returns true if the content of this list is sufficient to meet the request */ boolean fulfilled() { return requestedNodes.fulfilledBy(acceptedOfRequestedFlavor); @@ -320,8 +334,10 @@ class NodeAllocation { } } else if (deltaRetiredCount < 0) { // unretire until deltaRetiredCount is 0 - for (PrioritizableNode node : byIncreasingIndex(nodes)) { - if ( node.node.allocation().get().membership().retired() && hasCompatibleFlavor(node)) { + for (PrioritizableNode node : byUnretiringPriority(nodes)) { + if ( node.node.allocation().get().membership().retired() && hasCompatibleFlavor(node) ) { + if (node.isResizable) + node.node = resize(node.node); node.node = node.node.unretire(); if (++deltaRetiredCount == 0) break; } @@ -332,7 +348,7 @@ class NodeAllocation { // Set whether the node is exclusive Allocation allocation = node.node.allocation().get(); node.node = node.node.with(allocation.with(allocation.membership() - .with(allocation.membership().cluster().exclusive(requestedNodes.isExclusive())))); + .with(allocation.membership().cluster().exclusive(requestedNodes.isExclusive())))); } return nodes.stream().map(n -> n.node).collect(Collectors.toList()); @@ -363,8 +379,12 @@ class NodeAllocation { return nodes.stream().sorted(nodeIndexComparator().reversed()).collect(Collectors.toList()); } - private List<PrioritizableNode> byIncreasingIndex(Set<PrioritizableNode> nodes) { - return nodes.stream().sorted(nodeIndexComparator()).collect(Collectors.toList()); + /** Prefer to unretire nodes we don't want to retire, and otherwise those with lower index */ + private List<PrioritizableNode> byUnretiringPriority(Set<PrioritizableNode> nodes) { + return nodes.stream() + .sorted(Comparator.comparing((PrioritizableNode n) -> n.node.status().wantToRetire()) + .thenComparing(n -> n.node.allocation().get().membership().index())) + .collect(Collectors.toList()); } private Comparator<PrioritizableNode> nodeIndexComparator() { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java index 516bb2e84ea..8cfcbcb3797 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java @@ -145,6 +145,18 @@ class PrioritizableNode implements Comparable<PrioritizableNode> { return node.id(); } + @Override + public int hashCode() { + return node.hashCode(); + } + + @Override + public boolean equals(Object other) { + if (other == this) return true; + if ( ! (other instanceof PrioritizableNode)) return false; + return this.node.equals(((PrioritizableNode)other).node); + } + static class Builder { public final Node node; diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintainerTest.java index c19b54a8cab..7ce64093491 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintainerTest.java @@ -19,15 +19,22 @@ public class MaintainerTest { @Test public void staggering() { List<HostName> cluster = Arrays.asList(HostName.from("cfg1"), HostName.from("cfg2"), HostName.from("cfg3")); - Instant now = Instant.ofEpochMilli(1001); Duration interval = Duration.ofMillis(300); - assertEquals(299, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); - assertEquals(399, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); - assertEquals(199, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); - now = Instant.ofEpochMilli(1101); + Instant now = Instant.ofEpochMilli(1000); + assertEquals(200, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); + assertEquals( 0, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); + assertEquals(100, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + + now = Instant.ofEpochMilli(1001); assertEquals(199, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); assertEquals(299, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); - assertEquals(399, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + assertEquals( 99, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + + now = Instant.ofEpochMilli(1101); + assertEquals( 99, Maintainer.staggeredDelay(cluster, HostName.from("cfg1"), now, interval)); + assertEquals(199, Maintainer.staggeredDelay(cluster, HostName.from("cfg2"), now, interval)); + assertEquals(299, Maintainer.staggeredDelay(cluster, HostName.from("cfg3"), now, interval)); + assertEquals(300, Maintainer.staggeredDelay(cluster, HostName.from("cfg0"), now, interval)); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java index 50399e9c87f..7e9c3ef09dc 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java @@ -52,7 +52,7 @@ public class InPlaceResizeProvisionTest { private static final ClusterSpec container1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("container1")).vespaVersion("7.157.9").build(); private static final ClusterSpec container2 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("container2")).vespaVersion("7.157.9").build(); - private static final ClusterSpec content1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("content1")).vespaVersion("7.157.9").build(); + private static final ClusterSpec content1 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("content1")).vespaVersion("7.157.9").build(); private final InMemoryFlagSource flagSource = new InMemoryFlagSource(); private final ProvisioningTester tester = new ProvisioningTester.Builder() @@ -66,10 +66,10 @@ public class InPlaceResizeProvisionTest { addParentHosts(4, largeResources.with(fast).with(local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, largeResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(8, 16, 32, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(8, 16, 32, 1, fast, local)); assertEquals("No nodes are retired", 0, tester.getNodes(app, Node.State.active).retired().size()); } @@ -78,10 +78,10 @@ public class InPlaceResizeProvisionTest { addParentHosts(4, mediumResources.with(fast).with(local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, smallResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(2, 4, 8, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(2, 4, 8, 1, fast, local)); assertEquals("No nodes are retired", 0, tester.getNodes(app, Node.State.active).retired().size()); } @@ -94,16 +94,16 @@ public class InPlaceResizeProvisionTest { .prepare(container2, 4, 1, mediumResources) .activate(); Set<String> container1Hostnames = listCluster(container1).stream().map(Node::hostname).collect(Collectors.toSet()); - assertClusterSizeAndResources(container1, 4, new NodeResources(2, 4, 8, 1, fast, local)); - assertClusterSizeAndResources(container2, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(2, 4, 8, 1, fast, local)); + assertSizeAndResources(container2, 4, new NodeResources(4, 8, 16, 1, fast, local)); new PrepareHelper(tester, app) .prepare(container1, 4, 1, mediumResources) .prepare(container2, 4, 1, smallResources) .activate(); assertEquals(container1Hostnames, listCluster(container1).stream().map(Node::hostname).collect(Collectors.toSet())); - assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); - assertClusterSizeAndResources(container2, 4, new NodeResources(2, 4, 8, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container2, 4, new NodeResources(2, 4, 8, 1, fast, local)); assertEquals("No nodes are retired", 0, tester.getNodes(app, Node.State.active).retired().size()); } @@ -117,7 +117,7 @@ public class InPlaceResizeProvisionTest { new PrepareHelper(tester, app) .prepare(container1, 6, 1, largeResources).activate(); assertTrue(listCluster(container1).stream().map(Node::hostname).collect(Collectors.toSet()).containsAll(initialHostnames)); - assertClusterSizeAndResources(container1, 6, new NodeResources(8, 16, 32, 1, fast, local)); + assertSizeAndResources(container1, 6, new NodeResources(8, 16, 32, 1, fast, local)); assertEquals("No nodes are retired", 0, tester.getNodes(app, Node.State.active).retired().size()); } @@ -156,13 +156,34 @@ public class InPlaceResizeProvisionTest { assertTrue("All initial nodes should still be allocated to the application", initialHostnames.isEmpty()); } + /** In this scenario there should be no resizing */ + @Test + public void increase_size_decrease_resources() { + addParentHosts(12, largeResources.with(fast)); + + NodeResources resources = new NodeResources(4, 8, 16, 1); + NodeResources halvedResources = new NodeResources(2, 4, 8, 1); + + new PrepareHelper(tester, app).prepare(container1, 4, 1, resources).activate(); + assertSizeAndResources(container1, 4, resources); + + // No resizing since it would initially (before redistribution) lead to too few resources: + new PrepareHelper(tester, app).prepare(container1, 8, 1, halvedResources).activate(); + assertSizeAndResources(listCluster(container1).retired(), 4, resources); + assertSizeAndResources(listCluster(container1).not().retired(), 8, halvedResources); + + // Redeploying the same capacity should also not lead to any resizing + new PrepareHelper(tester, app).prepare(container1, 8, 1, halvedResources).activate(); + assertSizeAndResources(listCluster(container1).retired(), 4, resources); + assertSizeAndResources(listCluster(container1).not().retired(), 8, halvedResources); + } @Test(expected = OutOfCapacityException.class) public void cannot_inplace_decrease_resources_while_increasing_cluster_size() { addParentHosts(6, mediumResources.with(fast).with(local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); new PrepareHelper(tester, app).prepare(container1, 6, 1, smallResources); } @@ -172,7 +193,7 @@ public class InPlaceResizeProvisionTest { addParentHosts(4, largeResources.with(fast).with(local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); new PrepareHelper(tester, app).prepare(container1, 2, 1, smallResources); } @@ -182,7 +203,7 @@ public class InPlaceResizeProvisionTest { addParentHosts(4, largeResources.with(fast).with(local)); new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate(); - assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); + assertSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local)); new PrepareHelper(tester, app).prepare(container1, 4, 2, smallResources); } @@ -192,10 +213,13 @@ public class InPlaceResizeProvisionTest { tester.prepareAndActivateInfraApplication(infraApp, NodeType.host); } - private void assertClusterSizeAndResources(ClusterSpec cluster, int clusterSize, NodeResources resources) { - NodeList nodes = listCluster(cluster); - nodes.forEach(node -> assertEquals(node.toString(), node.flavor().resources(), resources)); - assertEquals(clusterSize, nodes.size()); + private void assertSizeAndResources(ClusterSpec cluster, int clusterSize, NodeResources resources) { + assertSizeAndResources(listCluster(cluster), clusterSize, resources); + } + + private void assertSizeAndResources(NodeList nodes, int size, NodeResources resources) { + assertEquals(size, nodes.size()); + nodes.forEach(n -> assertEquals(resources, n.flavor().resources())); } private NodeList listCluster(ClusterSpec cluster) { diff --git a/searchlib/src/tests/attribute/attribute_test.cpp b/searchlib/src/tests/attribute/attribute_test.cpp index f402f536ae2..4715292f86a 100644 --- a/searchlib/src/tests/attribute/attribute_test.cpp +++ b/searchlib/src/tests/attribute/attribute_test.cpp @@ -3,6 +3,7 @@ #include <vespa/document/fieldvalue/intfieldvalue.h> #include <vespa/document/fieldvalue/stringfieldvalue.h> #include <vespa/document/update/arithmeticvalueupdate.h> +#include <vespa/document/update/assignvalueupdate.h> #include <vespa/document/update/mapvalueupdate.h> #include <vespa/fastlib/io/bufferedfile.h> #include <vespa/searchlib/attribute/attribute.h> @@ -1493,20 +1494,21 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue typedef ArithmeticValueUpdate ArithVU; auto & vec = static_cast<VectorType &>(*ptr.get()); - addDocs(ptr, 6); - for (uint32_t doc = 0; doc < 6; ++doc) { + addDocs(ptr, 7); + for (uint32_t doc = 0; doc < 7; ++doc) { ASSERT_TRUE(vec.append(doc, initValue.getValue(), 100)); } - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 6u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 7u); EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 0u); EXPECT_TRUE(ptr->apply(0, MapVU(initFieldValue, ArithVU(ArithVU::Add, 10)))); EXPECT_TRUE(ptr->apply(1, MapVU(initFieldValue, ArithVU(ArithVU::Sub, 10)))); EXPECT_TRUE(ptr->apply(2, MapVU(initFieldValue, ArithVU(ArithVU::Mul, 10)))); EXPECT_TRUE(ptr->apply(3, MapVU(initFieldValue, ArithVU(ArithVU::Div, 10)))); + EXPECT_TRUE(ptr->apply(6, MapVU(initFieldValue, AssignValueUpdate(IntFieldValue(70))))); ptr->commit(); - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 10u); - EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 4u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 12u); + EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 5u); std::vector<BufferType> buf(2); ptr->get(0, &buf[0], 2); @@ -1517,6 +1519,8 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(buf[0].getWeight(), 1000); ptr->get(3, &buf[0], 2); EXPECT_EQUAL(buf[0].getWeight(), 10); + ptr->get(6, &buf[0], 2); + EXPECT_EQUAL(buf[0].getWeight(), 70); // removeifzero EXPECT_TRUE(ptr->apply(4, MapVU(initFieldValue, ArithVU(ArithVU::Sub, 100)))); @@ -1527,8 +1531,8 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(ptr->get(4, &buf[0], 2), uint32_t(1)); EXPECT_EQUAL(buf[0].getWeight(), 0); } - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 11u); - EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 5u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 13u); + EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 6u); // createifnonexistant EXPECT_TRUE(ptr->apply(5, MapVU(nonExistant, ArithVU(ArithVU::Add, 10)))); @@ -1542,18 +1546,18 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(ptr->get(5, &buf[0], 2), uint32_t(1)); EXPECT_EQUAL(buf[0].getWeight(), 100); } - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 12u); - EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 6u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 14u); + EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 7u); // try divide by zero (should be ignored) vec.clearDoc(0); - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 13u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 15u); ASSERT_TRUE(vec.append(0, initValue.getValue(), 12345)); - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 14u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 16u); EXPECT_TRUE(ptr->apply(0, MapVU(initFieldValue, ArithVU(ArithVU::Div, 0)))); - EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 14u); - EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 6u); + EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 16u); + EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 7u); ptr->commit(); ptr->get(0, &buf[0], 1); EXPECT_EQUAL(buf[0].getWeight(), 12345); diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp index 1f002ce612c..08c7186e8c7 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp @@ -11,6 +11,7 @@ #include "ipostinglistattributebase.h" #include "ipostinglistsearchcontext.h" #include "stringbase.h" +#include <vespa/document/update/assignvalueupdate.h> #include <vespa/document/update/mapvalueupdate.h> #include <vespa/fastlib/io/bufferedfile.h> #include <vespa/searchlib/common/tunefileinfo.h> @@ -28,6 +29,7 @@ LOG_SETUP(".searchlib.attribute.attributevector"); using vespalib::getLastErrorString; using document::ValueUpdate; +using document::AssignValueUpdate; using vespalib::make_string; using vespalib::Array; using vespalib::IllegalStateException; @@ -467,6 +469,9 @@ AttributeVector::apply(DocId doc, const MapValueUpdate &map) { if (vu.inherits(ArithmeticValueUpdate::classId)) { const ArithmeticValueUpdate &au(static_cast<const ArithmeticValueUpdate &>(vu)); retval = applyWeight(doc, map.getKey(), au); + } else if (vu.inherits(AssignValueUpdate::classId)) { + const AssignValueUpdate &au(static_cast<const AssignValueUpdate &>(vu)); + retval = applyWeight(doc, map.getKey(), au); } else { retval = false; } @@ -477,6 +482,7 @@ AttributeVector::apply(DocId doc, const MapValueUpdate &map) { bool AttributeVector::applyWeight(DocId, const FieldValue &, const ArithmeticValueUpdate &) { return false; } +bool AttributeVector::applyWeight(DocId, const FieldValue&, const AssignValueUpdate&) { return false; } void AttributeVector::removeAllOldGenerations() { diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.h b/searchlib/src/vespa/searchlib/attribute/attributevector.h index 4a53f2dd5a2..75699868691 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.h +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.h @@ -30,6 +30,7 @@ class FastOS_FileInterface; namespace document { class ArithmeticValueUpdate; + class AssignValueUpdate; class MapValueUpdate; class FieldValue; } @@ -332,6 +333,9 @@ protected: template<typename T> bool adjustWeight(ChangeVectorT< ChangeTemplate<T> > &changes, DocId doc, const T &v, const ArithmeticValueUpdate &wd); + template<typename T> + bool adjustWeight(ChangeVectorT< ChangeTemplate<T> > &changes, DocId doc, const T &v, const document::AssignValueUpdate &wu); + template <typename T> static int32_t applyWeightChange(int32_t weight, const ChangeTemplate<T> &weightChange) { @@ -341,6 +345,8 @@ protected: return weight * weightChange._weight; } else if (weightChange._type == ChangeBase::DIVWEIGHT) { return weight / weightChange._weight; + } else if (weightChange._type == ChangeBase::SETWEIGHT) { + return weightChange._weight; } return weight; } @@ -561,6 +567,7 @@ private: virtual void onAddDocs(DocId docIdLimit) = 0; void divideByZeroWarning(); virtual bool applyWeight(DocId doc, const FieldValue &fv, const ArithmeticValueUpdate &wAdjust); + virtual bool applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust); virtual void onSave(IAttributeSaveTarget & saveTarget); virtual bool onLoad(); diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.hpp b/searchlib/src/vespa/searchlib/attribute/attributevector.hpp index 66c2678497a..efc96bc57c2 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.hpp +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.hpp @@ -3,7 +3,9 @@ #include "attributevector.h" #include "integerbase.h" +#include <vespa/document/fieldvalue/intfieldvalue.h> #include <vespa/document/update/arithmeticvalueupdate.h> +#include <vespa/document/update/assignvalueupdate.h> #include <cmath> namespace search { @@ -61,6 +63,32 @@ AttributeVector::adjustWeight(ChangeVectorT< ChangeTemplate<T> > & changes, DocI template<typename T> bool +AttributeVector::adjustWeight(ChangeVectorT< ChangeTemplate<T> >& changes, DocId doc, const T& v, const document::AssignValueUpdate& wu) +{ + bool retval(hasWeightedSetType() && (doc < getNumDocs())); + if (retval) { + size_t oldSz(changes.size()); + if (wu.hasValue()) { + const FieldValue &wv = wu.getValue(); + if (wv.inherits(document::IntFieldValue::classId)) { + changes.push_back(ChangeTemplate<T>(ChangeBase::SETWEIGHT, doc, v, wv.getAsInt())); + } else { + retval = false; + } + } else { + retval = false; + } + if (retval) { + const size_t diff = changes.size() - oldSz; + _status.incNonIdempotentUpdates(diff); + _status.incUpdates(diff); + } + } + return retval; +} + +template<typename T> +bool AttributeVector::applyArithmetic(ChangeVectorT< ChangeTemplate<T> > & changes, DocId doc, const T & v, const ArithmeticValueUpdate & arithm) { diff --git a/searchlib/src/vespa/searchlib/attribute/changevector.h b/searchlib/src/vespa/searchlib/attribute/changevector.h index 75eac911f6c..af4a4fd6618 100644 --- a/searchlib/src/vespa/searchlib/attribute/changevector.h +++ b/searchlib/src/vespa/searchlib/attribute/changevector.h @@ -18,6 +18,7 @@ struct ChangeBase { INCREASEWEIGHT, MULWEIGHT, DIVWEIGHT, + SETWEIGHT, ADD, SUB, MUL, diff --git a/searchlib/src/vespa/searchlib/attribute/floatbase.cpp b/searchlib/src/vespa/searchlib/attribute/floatbase.cpp index 813180bc5d7..4ffc6e333b6 100644 --- a/searchlib/src/vespa/searchlib/attribute/floatbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/floatbase.cpp @@ -76,6 +76,12 @@ bool FloatingPointAttribute::applyWeight(DocId doc, const FieldValue & fv, const return AttributeVector::adjustWeight(_changes, doc, NumericChangeData<double>(v), wAdjust); } +bool FloatingPointAttribute::applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) +{ + double v = fv.getAsDouble(); + return AttributeVector::adjustWeight(_changes, doc, NumericChangeData<double>(v), wAdjust); +} + bool FloatingPointAttribute::apply(DocId doc, const ArithmeticValueUpdate & op) { bool retval(doc < getNumDocs()); diff --git a/searchlib/src/vespa/searchlib/attribute/floatbase.h b/searchlib/src/vespa/searchlib/attribute/floatbase.h index ce8d22c2706..65a146d7ba7 100644 --- a/searchlib/src/vespa/searchlib/attribute/floatbase.h +++ b/searchlib/src/vespa/searchlib/attribute/floatbase.h @@ -28,6 +28,7 @@ public: } bool apply(DocId doc, const ArithmeticValueUpdate & op); bool applyWeight(DocId doc, const FieldValue & fv, const ArithmeticValueUpdate & wAdjust) override; + bool applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) override; uint32_t clearDoc(DocId doc) override; protected: const char * getString(DocId doc, char * s, size_t sz) const override; diff --git a/searchlib/src/vespa/searchlib/attribute/integerbase.cpp b/searchlib/src/vespa/searchlib/attribute/integerbase.cpp index f065bc63648..a150b5e1699 100644 --- a/searchlib/src/vespa/searchlib/attribute/integerbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/integerbase.cpp @@ -80,6 +80,12 @@ bool IntegerAttribute::applyWeight(DocId doc, const FieldValue & fv, const Arith return AttributeVector::adjustWeight(_changes, doc, NumericChangeData<largeint_t>(v), wAdjust); } +bool IntegerAttribute::applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) +{ + largeint_t v = fv.getAsLong(); + return AttributeVector::adjustWeight(_changes, doc, NumericChangeData<largeint_t>(v), wAdjust); +} + bool IntegerAttribute::apply(DocId doc, const ArithmeticValueUpdate & op) { bool retval(doc < getNumDocs()); diff --git a/searchlib/src/vespa/searchlib/attribute/integerbase.h b/searchlib/src/vespa/searchlib/attribute/integerbase.h index 46774f9cebf..4e5fe8fec19 100644 --- a/searchlib/src/vespa/searchlib/attribute/integerbase.h +++ b/searchlib/src/vespa/searchlib/attribute/integerbase.h @@ -28,6 +28,7 @@ public: } bool apply(DocId doc, const ArithmeticValueUpdate & op); bool applyWeight(DocId doc, const FieldValue & fv, const ArithmeticValueUpdate & wAdjust) override; + bool applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) override; uint32_t clearDoc(DocId doc) override; protected: IntegerAttribute(const vespalib::string & name, const Config & c); diff --git a/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp index 219b3dd2473..40f211c2621 100644 --- a/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp @@ -37,7 +37,7 @@ MultiValueEnumAttribute<B, M>::considerAttributeChange(const Change & c, UniqueS { if (c._type == ChangeBase::APPEND || (this->getInternalCollectionType().createIfNonExistant() && - (c._type >= ChangeBase::INCREASEWEIGHT && c._type <= ChangeBase::DIVWEIGHT))) + (c._type >= ChangeBase::INCREASEWEIGHT && c._type <= ChangeBase::SETWEIGHT))) { EnumIndex idx; if (!this->_enumStore.find_index(c._data.raw(), idx)) { diff --git a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp index b23860ebd31..0ae536aee23 100644 --- a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp @@ -172,7 +172,7 @@ MultiValueAttribute<B, M>::apply_attribute_changes_to_wset(DocumentValues& docVa wset_inserted[data] = current->_weight; } else if (current->_type == ChangeBase::REMOVE) { wset_inserted.erase(data); - } else if ((current->_type >= ChangeBase::INCREASEWEIGHT) && (current->_type <= ChangeBase::DIVWEIGHT)) { + } else if ((current->_type >= ChangeBase::INCREASEWEIGHT) && (current->_type <= ChangeBase::SETWEIGHT)) { auto existing = wset_inserted.find(data); if (existing != wset_inserted.end()) { existing->second = this->applyWeightChange(existing->second, *current); diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp index 40e706e924d..d888ead21bf 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp @@ -309,6 +309,12 @@ bool StringAttribute::applyWeight(DocId doc, const FieldValue & fv, const Arithm return AttributeVector::adjustWeight(_changes, doc, StringChangeData(v), wAdjust); } +bool StringAttribute::applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) +{ + vespalib::string v = fv.getAsString(); + return AttributeVector::adjustWeight(_changes, doc, StringChangeData(v), wAdjust); +} + bool StringAttribute::apply(DocId, const ArithmeticValueUpdate & ) { return false; diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.h b/searchlib/src/vespa/searchlib/attribute/stringbase.h index 3518544cbdc..e6238cc0f94 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.h +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.h @@ -44,6 +44,7 @@ public: } bool apply(DocId doc, const ArithmeticValueUpdate & op); bool applyWeight(DocId doc, const FieldValue & fv, const ArithmeticValueUpdate & wAdjust) override; + bool applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) override; bool findEnum(const char * value, EnumHandle & e) const override = 0; std::vector<EnumHandle> findFoldedEnums(const char *value) const override = 0; uint32_t get(DocId doc, largeint_t * v, uint32_t sz) const override; diff --git a/travis/travis-build-full.sh b/travis/travis-build-full.sh index 62821e0e06d..f44e5e55d7a 100755 --- a/travis/travis-build-full.sh +++ b/travis/travis-build-full.sh @@ -6,7 +6,7 @@ export SOURCE_DIR=/source export NUM_THREADS=6 export MALLOC_ARENA_MAX=1 export MAVEN_OPTS="-Xss1m -Xms128m -Xmx2g" -source /etc/profile.d/enable-devtoolset-8.sh +source /etc/profile.d/enable-devtoolset-9.sh source /etc/profile.d/enable-rh-maven35.sh ccache --max-size=1250M diff --git a/vagrant/.gitignore b/vagrant/.gitignore deleted file mode 100644 index d16c9dbeeb5..00000000000 --- a/vagrant/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -.vagrant/ -*.box
\ No newline at end of file diff --git a/vagrant/README.md b/vagrant/README.md deleted file mode 100644 index 71fa071cb46..00000000000 --- a/vagrant/README.md +++ /dev/null @@ -1,107 +0,0 @@ -<!-- Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> - -# Create C++ / Java dev environment on CentOS using VirtualBox and Vagrant - -## Prerequisites -* [Install VirtualBox](https://www.virtualbox.org/wiki/Downloads) -* [Install Vagrant](https://www.vagrantup.com/downloads.html) - -## Create dev environment - -#### 1. Change working directory to <vespa-source>/vagrant - - cd <vespa-source>/vagrant - -#### 2. Choose dev environment - -##### a. For a dev environment with plain centos/7 and no GUI: - - export VESPA_VAGRANT_VM_BOX=centos/7 - export VESPA_VAGRANT_DISABLE_GUI=true - -##### b. For a dev environment with GUI and CLion: - -Create centos7-desktop box: - -* Install packer by following guide at [packer.io](https://www.packer.io/intro/getting-started/install.html) - -* Clone boxcutter centos repo and build the box: -``` -git clone https://github.com/boxcutter/centos.git -./bin/box build centos7-desktop.json virtualbox -``` - -Example exports: - - export VESPA_VAGRANT_VM_BOX="centos7-desktop" - export VESPA_VAGRANT_VM_BOX_URL="$HOME/git/boxcutter/centos/box/virtualbox/centos7-desktop-xx.yyyy.z.box" - - -#### 3. Install Vagrant VirtualBox Guest Additions plugin -This is required for mounting shared folders and get mouse pointer integration and seamless windows in the virtual CentOS desktop. - - vagrant plugin install vagrant-vbguest - -#### 4. Start and provision the environment - - vagrant up - -#### 5. Connect to machine via SSH -SSH agent forwarding is enabled to ensure easy interaction with GitHub inside the machine. - - vagrant ssh - -#### 6. Checkout vespa source inside virtual machine -This is needed in order to compile and run tests fast on the local file system inside the virtual machine. - - git clone git@github.com:vespa-engine/vespa.git - -## Build Java modules -Please follow the build instructions described [here](../README.md#build-java-modules). - - -## Build C++ modules -Please follow the build instructions described [here](../README.md#build-c-modules). -Skip these steps if doing development with CLion. - - -## Build and Develop using CLion -CLion is installed as part of the environment and is recommended for C++ development. - -#### 1. Bootstrap C++ building -cd to the vespa/ directory created by git clone and execute: - - ./bootstrap.sh java - ./bootstrap-cpp.sh . . - -#### 2. Start CLion -Open a terminal inside the virtual CentOS desktop (password is "vagrant") and run: - - clion - -When prompted, configure toolchains as follows: - - CMake: /usr/bin/cmake3 - Make: /usr/bin/make - C Compiler: /opt/rh/devtoolset-8/root/usr/bin/cc - C++ Compiler: /opt/rh/devtoolset-8/root/usr/bin/c++ - -#### 3. Open the Vespa Project -Go to *File* -> *Open* and choose <vespa-source>>/CMakeLists.txt. - -#### 4. Set compiler threads -Go to *File* -> *Settings* -> *Build, Execution, Deployment* -> *CMake*. -Under *Build Options* specify "-j 4" and click *Apply*. - -#### 5. Run bootstrap again - - ./bootstrap-cpp.sh . . - -(Some of the changes made by it are undone by clion on the first startup.) - -#### 6. Build all modules -Choose target **all_modules** from the set of build targets at the top right and click build. - -## Starting and stopping the Vagrant machine -Use `vagrant suspend` to suspend the machine and then `vagrant resume` to resume it later on. -Alternatively use `vagrant halt` + `vagrant up` to shutdown and reboot. Latter approach is slower but requires less disk space since RAM content is not persisted to host. diff --git a/vagrant/Vagrantfile b/vagrant/Vagrantfile deleted file mode 100644 index be0824dbd55..00000000000 --- a/vagrant/Vagrantfile +++ /dev/null @@ -1,93 +0,0 @@ -# -*- mode: ruby -*- -# vi: set ft=ruby : - -disable_gui = ENV['VESPA_VAGRANT_DISABLE_GUI'] - -def get_mandatory_env_value(name) - opt = ENV[name] - if opt.nil? or opt.empty? - raise Vagrant::Errors::VagrantError.new, "Environment variable #{name} must be set to a valid value before running vagrant" - end - return opt -end - -def get_env_value(name, fallback) - opt = ENV[name] - if opt.nil? or opt.empty? - return fallback - end - return opt -end - -vm_box = get_mandatory_env_value('VESPA_VAGRANT_VM_BOX') -vm_memory = get_env_value('VESPA_VAGRANT_VM_MEMORY', "8192") -vm_cpus = get_env_value('VESPA_VAGRANT_VM_CPUS', 4) - -unless disable_gui - vm_box_url = get_mandatory_env_value('VESPA_VAGRANT_VM_BOX_URL') -end - -# For a complete reference, please see the online documentation at https://docs.vagrantup.com. -Vagrant.configure("2") do |config| - - config.vm.box = vm_box - config.vm.box_url = vm_box_url unless disable_gui - - config.ssh.forward_agent = true - - config.vm.synced_folder "../dist", "/vagrant/dist" - - config.vm.provider "virtualbox" do |vb| - # Display the VirtualBox GUI when booting the machine - vb.gui = true unless disable_gui - vb.name = "vespa-dev" - - vb.memory = vm_memory - vb.cpus = vm_cpus - end - - # Install required and nice-to-have packages - config.vm.provision "shell", inline: <<-SHELL - yum-config-manager --add-repo https://copr.fedorainfracloud.org/coprs/g/vespa/vespa/repo/epel-7/group_vespa-vespa-epel-7.repo - yum -y install epel-release - yum -y install centos-release-scl - yum -y install yum-utils - yum -y install git \ - ccache \ - maven \ - rpm-build \ - valgrind \ - sudo \ - firefox \ - vim \ - emacs - sed -e '/^BuildRequires:/d' -e 's/^Requires:/BuildRequires:/' /vagrant/dist/vespa.spec > /tmp/vesparun.spec - yum-builddep -y /vagrant/dist/vespa.spec /tmp/vesparun.spec - rm /tmp/vesparun.spec - echo -e "* soft nproc 409600\n* hard nproc 409600" > /etc/security/limits.d/99-nproc.conf - echo -e "* soft nofile 262144\n* hard nofile 262144" > /etc/security/limits.d/99-nofile.conf - - unless disable_gui - echo -e "fs.inotify.max_user_watches = 524288" > /etc/sysctl.d/clion.conf - wget -q -O - https://download.jetbrains.com/cpp/CLion-2018.1.6.tar.gz | tar -C /opt -zx - ln -sf /opt/clion-2018.1.6/bin/clion.sh /usr/bin/clion - end - - yum update -y - hostname localhost - SHELL - - # Add settings for Vespa and dev tools as the default user, usually 'vagrant' (privileged: false) - # NOTE: adding these settings to .bashrc would break vagrant suspend/resume/provision - # due to env vars modified by /opt/rh/devtoolset-8/enable. - config.vm.provision "shell", privileged: false, inline: <<-SCRIPT - grep -l VESPA_HOME ~/.bash_profile >/dev/null || (\ - printf "%s\n" \ - 'export VESPA_HOME=$HOME/vespa' \ - 'export PATH=$PATH:$VESPA_HOME/bin' \ - 'source /opt/rh/rh-maven35/enable' \ - 'source /opt/rh/devtoolset-8/enable' \ - >> ~/.bash_profile ) - SCRIPT - -end diff --git a/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperation.java b/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperation.java index 94176bbb658..b6fe19f8cde 100644 --- a/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperation.java +++ b/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperation.java @@ -67,6 +67,7 @@ public class VespaDocumentOperation extends EvalFunc<String> { private static final String PROPERTY_CREATE_IF_NON_EXISTENT = "create-if-non-existent"; private static final String PROPERTY_ID_TEMPLATE = "docid"; private static final String PROPERTY_OPERATION = "operation"; + private static final String PROPERTY_VERBOSE = "verbose"; private static final String BAG_AS_MAP_FIELDS = "bag-as-map-fields"; private static final String SIMPLE_ARRAY_FIELDS = "simple-array-fields"; private static final String SIMPLE_OBJECT_FIELDS = "simple-object-fields"; @@ -99,6 +100,7 @@ public class VespaDocumentOperation extends EvalFunc<String> { partialOperationMap.put(UPDATE_MAP_FIELDS, PARTIAL_UPDATE_ASSIGN); } + private final boolean verbose; private final String template; private final Operation operation; private final Properties properties; @@ -113,6 +115,7 @@ public class VespaDocumentOperation extends EvalFunc<String> { properties = VespaConfiguration.loadProperties(params); template = properties.getProperty(PROPERTY_ID_TEMPLATE); operation = Operation.fromString(properties.getProperty(PROPERTY_OPERATION, "put")); + verbose = Boolean.parseBoolean(properties.getProperty(PROPERTY_VERBOSE, "false")); } @Override @@ -145,6 +148,9 @@ public class VespaDocumentOperation extends EvalFunc<String> { Schema inputSchema = getInputSchema(); Map<String, Object> fields = TupleTools.tupleMap(inputSchema, tuple); String docId = TupleTools.toString(fields, template); + if (verbose) { + System.out.println("Processing docId: "+ docId); + } // create json json = create(operation, docId, fields, properties, inputSchema); if (json == null || json.length() == 0) { diff --git a/vespa-hadoop/src/test/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperationTest.java b/vespa-hadoop/src/test/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperationTest.java index 67003273cac..e278026b00d 100644 --- a/vespa-hadoop/src/test/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperationTest.java +++ b/vespa-hadoop/src/test/java/com/yahoo/vespa/hadoop/pig/VespaDocumentOperationTest.java @@ -6,9 +6,14 @@ import org.apache.pig.impl.logicalLayer.FrontendException; import org.apache.pig.impl.logicalLayer.schema.Schema; import org.codehaus.jackson.JsonNode; import org.codehaus.jackson.map.ObjectMapper; +import org.hamcrest.CoreMatchers; +import org.junit.After; +import org.junit.Before; import org.junit.Test; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.PrintStream; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -16,9 +21,22 @@ import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; @SuppressWarnings("serial") public class VespaDocumentOperationTest { + private final ByteArrayOutputStream outContent = new ByteArrayOutputStream(); + private final PrintStream originalOut = System.out; + + @Before + public void setUpStreams() { + System.setOut(new PrintStream(outContent)); + } + + @After + public void restoreStreams() { + System.setOut(originalOut); + } @Test public void requireThatUDFReturnsCorrectJson() throws Exception { String json = getDocumentOperationJson("docid=id:<application>:metrics::<name>-<date>"); @@ -539,6 +557,59 @@ public class VespaDocumentOperationTest { assertEquals(234567, bagNode.get("234567").asInt()); } + @Test + public void requireThatUDFPrintIdWhenVerbose() throws IOException { + DataBag bag = BagFactory.getInstance().newDefaultBag(); + + Schema objectSchema = new Schema(); + Tuple objectTuple = TupleFactory.getInstance().newTuple(); + addToTuple("key", DataType.CHARARRAY, "123456", objectSchema, objectTuple); + addToTuple("value", DataType.INTEGER, 123456, objectSchema, objectTuple); + bag.add(objectTuple); + + objectSchema = new Schema(); + objectTuple = TupleFactory.getInstance().newTuple(); + addToTuple("key", DataType.CHARARRAY, "234567", objectSchema, objectTuple); + addToTuple("value", DataType.INTEGER, 234567, objectSchema, objectTuple); + bag.add(objectTuple); + + Schema schema = new Schema(); + Tuple tuple = TupleFactory.getInstance().newTuple(); + addToTuple("bag", DataType.BAG, bag, objectSchema, schema, tuple); + + VespaDocumentOperation docOp = new VespaDocumentOperation("docid=7654321", "bag-as-map-fields=bag","verbose=true"); + docOp.setInputSchema(schema); + String json = docOp.exec(tuple); + + assertThat(outContent.toString(), CoreMatchers.containsString("Processing docId: 7654321")); + } + + @Test + public void requireThatUDFVerboseSetToFalseByDefault() throws IOException { + DataBag bag = BagFactory.getInstance().newDefaultBag(); + + Schema objectSchema = new Schema(); + Tuple objectTuple = TupleFactory.getInstance().newTuple(); + addToTuple("key", DataType.CHARARRAY, "123456", objectSchema, objectTuple); + addToTuple("value", DataType.INTEGER, 123456, objectSchema, objectTuple); + bag.add(objectTuple); + + objectSchema = new Schema(); + objectTuple = TupleFactory.getInstance().newTuple(); + addToTuple("key", DataType.CHARARRAY, "234567", objectSchema, objectTuple); + addToTuple("value", DataType.INTEGER, 234567, objectSchema, objectTuple); + bag.add(objectTuple); + + Schema schema = new Schema(); + Tuple tuple = TupleFactory.getInstance().newTuple(); + addToTuple("bag", DataType.BAG, bag, objectSchema, schema, tuple); + + VespaDocumentOperation docOp = new VespaDocumentOperation("docid=7654321", "bag-as-map-fields=bag"); + docOp.setInputSchema(schema); + String json = docOp.exec(tuple); + + assertEquals("", outContent.toString()); + } private void addToTuple(String alias, byte type, Object value, Schema schema, Tuple tuple) { schema.add(new Schema.FieldSchema(alias, type)); diff --git a/vespalib/src/tests/stllike/asciistream_test.cpp b/vespalib/src/tests/stllike/asciistream_test.cpp index fd362d9c49a..c9435edea45 100644 --- a/vespalib/src/tests/stllike/asciistream_test.cpp +++ b/vespalib/src/tests/stllike/asciistream_test.cpp @@ -387,8 +387,10 @@ AsciistreamTest::testCreateFromFile() EXPECT_EQUAL("", s); EXPECT_TRUE(is.eof()); +#ifdef __linux__ is = asciistream::createFromDevice("/proc/stat"); EXPECT_FALSE(is.eof()); +#endif } void diff --git a/vespalib/src/vespa/vespalib/util/alloc.cpp b/vespalib/src/vespa/vespalib/util/alloc.cpp index 6aa96992254..4c0675198bb 100644 --- a/vespalib/src/vespa/vespalib/util/alloc.cpp +++ b/vespalib/src/vespa/vespalib/util/alloc.cpp @@ -66,7 +66,11 @@ readOptionalEnvironmentVar(const char * name, size_t defaultValue) { void initializeEnvironment() { +#ifdef __linux__ _G_HugeFlags = (getenv("VESPA_USE_HUGEPAGES") != nullptr) ? MAP_HUGETLB : 0; +#else + _G_HugeFlags = 0; +#endif _G_SilenceCoreOnOOM = (getenv("VESPA_SILENCE_CORE_ON_OOM") != nullptr) ? true : false; _G_MMapLogLimit = readOptionalEnvironmentVar("VESPA_MMAP_LOG_LIMIT", std::numeric_limits<size_t>::max()); _G_MMapNoCoreLimit = readOptionalEnvironmentVar("VESPA_MMAP_NOCORE_LIMIT", std::numeric_limits<size_t>::max()); @@ -350,11 +354,13 @@ MMapAllocator::salloc(size_t sz, void * wantedAddress) _G_hasHugePageFailureJustHappened = false; } } +#ifdef __linux__ if (sz >= _G_MMapNoCoreLimit) { if (madvise(buf, sz, MADV_DONTDUMP) != 0) { LOG(warning, "Failed madvise(%p, %ld, MADV_DONTDUMP) = '%s'", buf, sz, FastOS_FileInterface::getLastErrorString().c_str()); } } +#endif if (sz >= _G_MMapLogLimit) { LockGuard guard(_G_lock); _G_HugeMappings[buf] = MMapInfo(mmapId, sz, stackTrace); diff --git a/vespalog/src/vespa/log/control-file.cpp b/vespalog/src/vespa/log/control-file.cpp index 2f070bf088e..aead7c456ef 100644 --- a/vespalog/src/vespa/log/control-file.cpp +++ b/vespalog/src/vespa/log/control-file.cpp @@ -264,7 +264,12 @@ ControlFile::getLevels(const char *name) strcat(appendedString, &padSpaces[3 - padding]); int prefix_len = strlen(appendedString); +#pragma GCC diagnostic push +#ifndef __clang__ +#pragma GCC diagnostic ignored "-Wstringop-truncation" +#endif strncat(appendedString, inheritLevels, Logger::NUM_LOGLEVELS*sizeof(int)); +#pragma GCC diagnostic pop strcat(appendedString, "\n"); int len = strlen(appendedString); diff --git a/vespamalloc/src/vespamalloc/malloc/allocchunk.cpp b/vespamalloc/src/vespamalloc/malloc/allocchunk.cpp index 41165244e5c..fb95018e94e 100644 --- a/vespamalloc/src/vespamalloc/malloc/allocchunk.cpp +++ b/vespamalloc/src/vespamalloc/malloc/allocchunk.cpp @@ -7,7 +7,7 @@ namespace vespamalloc { void AFListBase::linkInList(AtomicHeadPtr & head, AFListBase * list) { AFListBase * tail; - for (tail = list; tail->_next != NULL ;tail = tail->_next) { } + for (tail = list; tail->_next != nullptr ;tail = tail->_next) { } linkIn(head, list, tail); } @@ -16,7 +16,7 @@ void AFListBase::linkIn(AtomicHeadPtr & head, AFListBase * csl, AFListBase * tai HeadPtr oldHead = head.load(std::memory_order_relaxed); HeadPtr newHead(csl, oldHead._tag + 1); tail->_next = static_cast<AFListBase *>(oldHead._ptr); - while ( ! head.compare_exchange_weak(oldHead, newHead, std::memory_order_release, std::memory_order_relaxed) ) { + while ( __builtin_expect(! head.compare_exchange_weak(oldHead, newHead, std::memory_order_release, std::memory_order_relaxed), false) ) { newHead._tag = oldHead._tag + 1; tail->_next = static_cast<AFListBase *>(oldHead._ptr); } @@ -26,19 +26,19 @@ AFListBase * AFListBase::linkOut(AtomicHeadPtr & head) { HeadPtr oldHead = head.load(std::memory_order_relaxed); AFListBase *csl = static_cast<AFListBase *>(oldHead._ptr); - if (csl == NULL) { - return NULL; + if (csl == nullptr) { + return nullptr; } HeadPtr newHead(csl->_next, oldHead._tag + 1); - while ( ! head.compare_exchange_weak(oldHead, newHead, std::memory_order_acquire, std::memory_order_relaxed) ) { + while ( __builtin_expect(! head.compare_exchange_weak(oldHead, newHead, std::memory_order_acquire, std::memory_order_relaxed), false) ) { csl = static_cast<AFListBase *>(oldHead._ptr); - if (csl == NULL) { - return NULL; + if (csl == nullptr) { + return nullptr; } newHead._ptr = csl->_next; newHead._tag = oldHead._tag + 1; } - csl->_next = NULL; + csl->_next = nullptr; return csl; } diff --git a/vespamalloc/src/vespamalloc/malloc/allocchunk.h b/vespamalloc/src/vespamalloc/malloc/allocchunk.h index 8b6a2ce3fc8..ed0b52c7b73 100644 --- a/vespamalloc/src/vespamalloc/malloc/allocchunk.h +++ b/vespamalloc/src/vespamalloc/malloc/allocchunk.h @@ -24,12 +24,55 @@ struct TaggedPtr { size_t _tag; }; +#if defined(__x86_64__) +struct AtomicTaggedPtr { + AtomicTaggedPtr() noexcept : _ptr(nullptr), _tag(0) { } + AtomicTaggedPtr(void *h, size_t t) noexcept : _ptr(h), _tag(t) {} + + AtomicTaggedPtr load(std::memory_order = std::memory_order_seq_cst) { + // Note that this is NOT an atomic load. The current use as the initial load + // in a compare_exchange loop is safe as a teared load will just give a retry. + return *this; + } + void store(AtomicTaggedPtr ptr) { + // Note that this is NOT an atomic store. The current use is in a unit test as an initial + // store before any threads are started. Just done so to keep api compatible with std::atomic as + // that is the preferred implementation.. + *this = ptr; + } + bool + compare_exchange_weak(AtomicTaggedPtr & oldPtr, AtomicTaggedPtr newPtr, std::memory_order, std::memory_order) { + char result; + __asm__ volatile ( + "lock ;" + "cmpxchg16b %6;" + "setz %1;" + : "+m" (*this), + "=q" (result), + "+a" (oldPtr._ptr), + "+d" (oldPtr._tag) + : "b" (newPtr._ptr), + "c" (newPtr._tag) + : "cc", "memory" + ); + return result; + } + + void *_ptr; + size_t _tag; +} __attribute__ ((aligned (16))); +#else + using AtomicTaggedPtr = TaggedPtr; +#endif + + class AFListBase { public: - using HeadPtr = TaggedPtr; - using AtomicHeadPtr = std::atomic<HeadPtr>; - AFListBase() : _next(NULL) { } + using HeadPtr = std::conditional<std::atomic<TaggedPtr>::is_always_lock_free, TaggedPtr, AtomicTaggedPtr>::type; + using AtomicHeadPtr = std::conditional<std::atomic<TaggedPtr>::is_always_lock_free, std::atomic<TaggedPtr>, AtomicTaggedPtr>::type; + + AFListBase() : _next(nullptr) { } void setNext(AFListBase * csl) { _next = csl; } static void init(); static void linkInList(AtomicHeadPtr & head, AFListBase * list); |