diff options
137 files changed, 1566 insertions, 1122 deletions
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java index 23389de3fad..d2fe304a72c 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MasterElectionTest.java @@ -39,6 +39,8 @@ public class MasterElectionTest extends FleetControllerTest { @Rule public TestRule cleanupZookeeperLogsOnSuccess = new CleanupZookeeperLogsOnSuccess(); + private static int defaultZkSessionTimeoutInMillis() { return 10_000; } + protected void setUpFleetController(int count, boolean useFakeTimer, FleetControllerOptions options) throws Exception { if (zooKeeperServer == null) { zooKeeperServer = new ZooKeeperTestServer(); @@ -46,7 +48,7 @@ public class MasterElectionTest extends FleetControllerTest { slobrok = new Slobrok(); usingFakeTimer = useFakeTimer; this.options = options; - this.options.zooKeeperSessionTimeout = 10 * timeoutMS; + this.options.zooKeeperSessionTimeout = defaultZkSessionTimeoutInMillis(); this.options.zooKeeperServerAddress = zooKeeperServer.getAddress(); this.options.slobrokConnectionSpecs = new String[1]; this.options.slobrokConnectionSpecs[0] = "tcp/localhost:" + slobrok.port(); @@ -62,7 +64,7 @@ public class MasterElectionTest extends FleetControllerTest { int fleetControllerIndex, int fleetControllerCount) throws Exception { FleetControllerOptions options = o.clone(); - options.zooKeeperSessionTimeout = 10 * timeoutMS; + options.zooKeeperSessionTimeout = defaultZkSessionTimeoutInMillis(); options.zooKeeperServerAddress = zooKeeperServer.getAddress(); options.slobrokConnectionSpecs = new String[1]; options.slobrokConnectionSpecs[0] = "tcp/localhost:" + slobrok.port(); // Spec.fromLocalHostName(slobrok.port()).toString(); @@ -251,7 +253,6 @@ public class MasterElectionTest extends FleetControllerTest { FleetControllerOptions options = defaultOptions("mycluster"); // "Magic" port value is in range allocated to module for testing. zooKeeperServer = ZooKeeperTestServer.createWithFixedPort(18342); - options.zooKeeperSessionTimeout = 100; options.masterZooKeeperCooldownPeriod = 100; setUpFleetController(2, true, options); waitForMaster(0); @@ -273,7 +274,6 @@ public class MasterElectionTest extends FleetControllerTest { public void testZooKeeperUnavailable() throws Exception { startingTest("MasterElectionTest::testZooKeeperUnavailable"); FleetControllerOptions options = defaultOptions("mycluster"); - options.zooKeeperSessionTimeout = 100; options.masterZooKeeperCooldownPeriod = 100; options.zooKeeperServerAddress = "localhost"; setUpFleetController(5, true, options); 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 55373f425e0..514ca2a00f5 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 @@ -56,7 +56,8 @@ public interface ModelContext { boolean useFdispatchByDefault(); boolean dispatchWithProtobuf(); boolean useAdaptiveDispatch(); - boolean enableMetricsProxyContainer(); + // TODO: Remove when 7.61 is the oldest model in use + default boolean enableMetricsProxyContainer() { return false; } } } diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index 4b35af53154..0a54dd6790d 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -37,7 +37,6 @@ public class TestProperties implements ModelContext.Properties { private boolean useFdispatchByDefault = true; private boolean dispatchWithProtobuf = true; private boolean useAdaptiveDispatch = false; - private boolean enableMetricsProxyContainer = false; @Override public boolean multitenant() { return multitenant; } @@ -55,7 +54,6 @@ public class TestProperties implements ModelContext.Properties { @Override public boolean useDedicatedNodeForLogserver() { return useDedicatedNodeForLogserver; } @Override public boolean useFdispatchByDefault() { return useFdispatchByDefault; } @Override public boolean dispatchWithProtobuf() { return dispatchWithProtobuf; } - @Override public boolean enableMetricsProxyContainer() { return enableMetricsProxyContainer; } public TestProperties setApplicationId(ApplicationId applicationId) { this.applicationId = applicationId; @@ -87,10 +85,6 @@ public class TestProperties implements ModelContext.Properties { return this; } - public TestProperties setEnableMetricsProxyContainer(boolean enableMetricsProxyContainer) { - this.enableMetricsProxyContainer = enableMetricsProxyContainer; - return this; - } public static class Spec implements ConfigServerSpec { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java index 3bc38cad1d1..2a698233713 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java @@ -19,10 +19,8 @@ import java.util.LinkedHashMap; import java.util.Map; import static com.yahoo.config.model.api.container.ContainerServiceType.METRICS_PROXY_CONTAINER; -import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainer.NodeDimensionNames.CANONICAL_FLAVOR; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainer.NodeDimensionNames.CLUSTER_ID; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainer.NodeDimensionNames.CLUSTER_TYPE; -import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainer.NodeDimensionNames.FLAVOR; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.METRICS_PROXY_BUNDLE_NAME; /** @@ -37,8 +35,6 @@ public class MetricsProxyContainer extends Container implements { static final class NodeDimensionNames { - static final String FLAVOR = "flavor"; - static final String CANONICAL_FLAVOR = "canonicalFlavor"; static final String CLUSTER_TYPE = "clustertype"; static final String CLUSTER_ID = "clusterid"; } @@ -118,11 +114,6 @@ public class MetricsProxyContainer extends Container implements public void getConfig(NodeDimensionsConfig.Builder builder) { Map<String, String> dimensions = new LinkedHashMap<>(); if (isHostedVespa) { - getHostResource().getFlavor().ifPresent(flavor -> { - dimensions.put(FLAVOR, flavor.name()); - dimensions.put(CANONICAL_FLAVOR, flavor.canonicalName()); - }); - getHostResource().primaryClusterMembership().map(ClusterMembership::cluster).ifPresent(cluster -> { dimensions.put(CLUSTER_TYPE, cluster.type().name()); dimensions.put(CLUSTER_ID, cluster.id().value()); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java index a8f0f5941b0..7a6b1064b24 100755 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java @@ -209,7 +209,7 @@ public abstract class ContainerCluster<CONTAINER extends Container> private void addTestrunnerComponentsIfTester(DeployState deployState) { if (deployState.isHosted() && deployState.getProperties().applicationId().instance().isTester()) - addPlatformBundle(Paths.get(Defaults.getDefaults().underVespaHome("vespa-testrunner-components-jar-with-dependencies.jar"))); + addPlatformBundle(Paths.get(Defaults.getDefaults().underVespaHome("lib/jars/vespa-testrunner-components-jar-with-dependencies.jar"))); } public final void addDefaultHandlersExceptStatus() { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java index d2bf4b601a6..f755871ac4b 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java @@ -11,7 +11,6 @@ import org.junit.Test; import static com.yahoo.config.model.api.container.ContainerServiceType.METRICS_PROXY_CONTAINER; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.CLUSTER_CONFIG_ID; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.CONTAINER_CONFIG_ID; -import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.MY_FLAVOR; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.getHostedModel; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.getModel; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.getNodeDimensionsConfig; @@ -33,7 +32,6 @@ public class MetricsProxyContainerTest { public void one_metrics_proxy_container_is_added_to_every_node() { var numberOfHosts = 4; var tester = new VespaModelTester(); - tester.enableMetricsProxyContainer(true); tester.addHosts(numberOfHosts); VespaModel model = tester.createModel(servicesWithManyNodes(), true); @@ -108,8 +106,6 @@ public class MetricsProxyContainerTest { assertEquals("content", config.dimensions(NodeDimensionNames.CLUSTER_TYPE)); assertEquals("my-content", config.dimensions(NodeDimensionNames.CLUSTER_ID)); - assertEquals(MY_FLAVOR, config.dimensions(NodeDimensionNames.FLAVOR)); - assertEquals(MY_FLAVOR, config.dimensions(NodeDimensionNames.CANONICAL_FLAVOR)); } @@ -162,7 +158,7 @@ public class MetricsProxyContainerTest { " </admin>", " <content version='1.0' id='my-content'>", " <documents />", - " <nodes count='1' flavor='" + MY_FLAVOR + "' />", + " <nodes count='1' />", " </content>", "</services>" ); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java index 13589c763e2..bac9d674460 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java @@ -27,7 +27,6 @@ class MetricsProxyModelTester { static final String MY_TENANT = "mytenant"; static final String MY_APPLICATION = "myapp"; static final String MY_INSTANCE = "myinstance"; - static final String MY_FLAVOR = "myflavor"; static final String CLUSTER_CONFIG_ID = "admin/metrics"; @@ -37,7 +36,6 @@ class MetricsProxyModelTester { static VespaModel getModel(String servicesXml) { var numberOfHosts = 1; var tester = new VespaModelTester(); - tester.enableMetricsProxyContainer(true); tester.addHosts(numberOfHosts); tester.setHosted(false); return tester.createModel(servicesXml, true); @@ -46,8 +44,7 @@ class MetricsProxyModelTester { static VespaModel getHostedModel(String servicesXml) { var numberOfHosts = 2; var tester = new VespaModelTester(); - tester.enableMetricsProxyContainer(true); - tester.addHosts(flavorFromString(MY_FLAVOR), numberOfHosts); + tester.addHosts(numberOfHosts); tester.setHosted(true); tester.setApplicationId(MY_TENANT, MY_APPLICATION, MY_INSTANCE); return tester.createModel(servicesXml, true); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java index 866c4027711..2e5acb9025d 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java @@ -48,7 +48,6 @@ public class VespaModelTester { private Map<NodeResources, Collection<Host>> hostsByResources = new HashMap<>(); private ApplicationId applicationId = ApplicationId.defaultId(); private boolean useDedicatedNodeForLogserver = false; - private boolean enableMetricsProxyContainer = false; public VespaModelTester() { this(new NullConfigModelRegistry()); @@ -105,10 +104,6 @@ public class VespaModelTester { this.useDedicatedNodeForLogserver = useDedicatedNodeForLogserver; } - public void enableMetricsProxyContainer(boolean enableMetricsProxyContainer) { - this.enableMetricsProxyContainer = enableMetricsProxyContainer; - } - /** Creates a model which uses 0 as start index and fails on out of capacity */ public VespaModel createModel(String services, String ... retiredHostNames) { return createModel(Zone.defaultZone(), services, true, retiredHostNames); @@ -149,8 +144,7 @@ public class VespaModelTester { .setMultitenant(true) .setHostedVespa(hosted) .setApplicationId(applicationId) - .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver) - .setEnableMetricsProxyContainer(enableMetricsProxyContainer); + .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver); DeployState deployState = new DeployState.Builder() .applicationPackage(appPkg) diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/AllocatedHosts.java b/config-provisioning/src/main/java/com/yahoo/config/provision/AllocatedHosts.java index de4f3a555bd..96942c53a12 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/AllocatedHosts.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/AllocatedHosts.java @@ -26,6 +26,13 @@ import java.util.Set; */ public class AllocatedHosts { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String mappingKey = "mapping"; private static final String hostSpecKey = "hostSpec"; private static final String hostSpecHostNameKey = "hostName"; diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneApi.java b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneApi.java index 9f6ba29d8de..fd76dc10bdb 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneApi.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneApi.java @@ -17,8 +17,4 @@ public interface ZoneApi { default RegionName getRegionName() { return getId().region(); } CloudName getCloudName(); - - default ZoneId toDeprecatedId() { - return ZoneId.from(getEnvironment(), getRegionName(), getCloudName(), getSystemName()); - } } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneId.java b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneId.java index 5e664e00b4c..b0ac59718aa 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneId.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneId.java @@ -20,30 +20,16 @@ public class ZoneId { private final Environment environment; private final RegionName region; - private final SystemName system; - private ZoneId(Environment environment, RegionName region, CloudName cloud, SystemName system) { + private ZoneId(Environment environment, RegionName region) { this.environment = Objects.requireNonNull(environment, "environment must be non-null"); this.region = Objects.requireNonNull(region, "region must be non-null"); - this.system = Objects.requireNonNull(system, "system must be non-null"); - } - - private ZoneId(Environment environment, RegionName region) { - this(environment, region, CloudName.defaultName(), SystemName.defaultSystem()); } public static ZoneId from(Environment environment, RegionName region) { return new ZoneId(environment, region); } - public static ZoneId from(SystemName system, Environment environment, RegionName region) { - return new ZoneId(environment, region, CloudName.defaultName(), system); - } - - public static ZoneId from(Environment environment, RegionName region, CloudName cloud, SystemName system) { - return new ZoneId(environment, region, cloud, system); - } - public static ZoneId from(String environment, String region) { return from(Environment.from(environment), RegionName.from(region)); } @@ -55,20 +41,14 @@ public class ZoneId { case 2: return from(parts[0], parts[1]); case 4: - return from(parts[2], parts[3], parts[0], parts[1]); + // Deprecated: parts[0] == cloud, parts[1] == system + // TODO: Figure out whether this can be removed + return from(parts[2], parts[3]); default: throw new IllegalArgumentException("Cannot deserialize zone id '" + value + "'"); } } - public static ZoneId from(Environment environment, RegionName region, CloudName cloud) { - return new ZoneId(environment, region, cloud, SystemName.defaultSystem()); - } - - public static ZoneId from(String environment, String region, String cloud, String system) { - return new ZoneId(Environment.from(environment), RegionName.from(region), CloudName.from(cloud), SystemName.from(system)); - } - public static ZoneId defaultId() { return new ZoneId(Environment.defaultEnvironment(), RegionName.defaultName()); } @@ -81,10 +61,6 @@ public class ZoneId { return region; } - public SystemName system() { - return system; - } - /** Returns the serialised value of this. Inverse of {@code ZoneId.from(String value)}. */ public String value() { return environment + "." + region; diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/ZoneIdTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/ZoneIdTest.java index 27d45ba7d7d..434badbe9bf 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/ZoneIdTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/ZoneIdTest.java @@ -26,17 +26,6 @@ public class ZoneIdTest { ZoneId zoneId = ZoneId.from(environment, region); assertEquals(region, zoneId.region()); assertEquals(environment, zoneId.environment()); - assertEquals(SystemName.defaultSystem(), zoneId.system()); - - ZoneId zoneIdWithSystem = ZoneId.from(system, environment, region); - assertEquals(region, zoneIdWithSystem.region()); - assertEquals(environment, zoneIdWithSystem.environment()); - assertEquals(system, zoneIdWithSystem.system()); - - ZoneId zoneIdWithCloudAndSystem = ZoneId.from(environment, region, cloud, system); - assertEquals(region, zoneIdWithCloudAndSystem.region()); - assertEquals(environment, zoneIdWithCloudAndSystem.environment()); - assertEquals(system, zoneIdWithCloudAndSystem.system()); } @Test @@ -45,12 +34,6 @@ public class ZoneIdTest { assertEquals(environment.value() + "." + region.value(), zoneId.value()); assertEquals(ZoneId.from(zoneId.value()), zoneId); - ZoneId zoneIdWithCloudAndSystem = ZoneId.from(environment, region, cloud, system); - assertEquals(environment.value() + "." + region.value(), zoneIdWithCloudAndSystem.value()); - assertEquals(ZoneId.from(zoneIdWithCloudAndSystem.value()), zoneIdWithCloudAndSystem); - // TODO: Expect cloud and system to be part of deserialized value when the new format is supported everywhere - //assertEquals(cloud.value() + "." + system.name() + "." + environment.value() + "." + region.value() , zoneId.value()); - String serializedZoneId = "some.illegal.value"; expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Cannot deserialize zone id '" + serializedZoneId + "'"); 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 0279d175488..fc6667087c6 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 @@ -132,7 +132,6 @@ public class ModelContextImpl implements ModelContext { private final boolean useFdispatchByDefault; private final boolean useAdaptiveDispatch; private final boolean dispatchWithProtobuf; - private final boolean enableMetricsProxyContainer; public Properties(ApplicationId applicationId, boolean multitenantFromConfig, @@ -165,8 +164,6 @@ public class ModelContextImpl implements ModelContext { .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); this.useAdaptiveDispatch = Flags.USE_ADAPTIVE_DISPATCH.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); - this.enableMetricsProxyContainer = Flags.ENABLE_METRICS_PROXY_CONTAINER.bindTo(flagSource) - .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); } @Override @@ -218,8 +215,6 @@ public class ModelContextImpl implements ModelContext { @Override public boolean useAdaptiveDispatch() { return useAdaptiveDispatch; } - @Override - public boolean enableMetricsProxyContainer() { return enableMetricsProxyContainer; } } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializer.java index 379af7f71ea..91f9e3c8eed 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ContainerEndpointSerializer.java @@ -18,6 +18,13 @@ import java.util.List; */ public class ContainerEndpointSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String clusterIdField = "clusterId"; private static final String namesField = "names"; diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 9474c9f9160..06713d14d88 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -7148,7 +7148,10 @@ "public com.yahoo.data.access.Inspector inspect()", "public java.lang.String toString()", "public java.lang.String toJson()", - "public java.lang.StringBuilder writeJson(java.lang.StringBuilder)" + "public java.lang.StringBuilder writeJson(java.lang.StringBuilder)", + "public java.lang.Double getDouble(java.lang.String)", + "public com.yahoo.tensor.Tensor getTensor(java.lang.String)", + "public java.util.Set featureNames()" ], "fields": [] }, diff --git a/container-search/src/main/java/com/yahoo/data/JsonProducer.java b/container-search/src/main/java/com/yahoo/data/JsonProducer.java index 6d925b41379..c9dc0946a3e 100644 --- a/container-search/src/main/java/com/yahoo/data/JsonProducer.java +++ b/container-search/src/main/java/com/yahoo/data/JsonProducer.java @@ -12,6 +12,7 @@ public interface JsonProducer { * be human-readable and containing embedded newlines; also the * exact indentation etc may change, so use compact=true for a * canonical format. + * * @param target the StringBuilder to append to. * @return the target passed in is also returned (to allow chaining). */ @@ -20,7 +21,8 @@ public interface JsonProducer { /** * Convenience method equivalent to: * writeJson(new StringBuilder()).toString() - * @return String containing JSON representation of this object's data. + * + * @return a String containing JSON representation of this object's data. */ default String toJson() { return writeJson(new StringBuilder()).toString(); diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FeatureDataField.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FeatureDataField.java index 1f60dd3d1cf..b0003f4321e 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FeatureDataField.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FeatureDataField.java @@ -6,9 +6,8 @@ import com.yahoo.data.access.Type; import com.yahoo.search.result.FeatureData; /** - * Class representing a "feature data" field. This was historically - * just a string containing JSON; now it's a structure of - * data (that will be rendered as JSON by default). + * Class representing a "feature data" field: A map of values which are + * either floats or tensors. */ public class FeatureDataField extends LongstringField { @@ -23,12 +22,8 @@ public class FeatureDataField extends LongstringField { @Override public Object convert(Inspector value) { - if (! value.valid()) { - return null; - } - if (value.type() == Type.STRING) { - return value.asString(); - } + if ( ! value.valid()) return null; + if (value.type() == Type.STRING) return value.asString(); return new FeatureData(value); } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/LongstringField.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/LongstringField.java index 2f9c6d5b325..5de38e43c96 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/LongstringField.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/LongstringField.java @@ -5,10 +5,6 @@ */ package com.yahoo.prelude.fastsearch; -import java.nio.ByteBuffer; - -import com.yahoo.io.SlowInflate; -import com.yahoo.text.Utf8; import com.yahoo.data.access.Inspector; /** diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/JSONDebugSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/JSONDebugSearcher.java index 2330ca2382a..5f921b67702 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/JSONDebugSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/JSONDebugSearcher.java @@ -3,6 +3,7 @@ package com.yahoo.prelude.searcher; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.hitfield.JSONString; +import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; import com.yahoo.processing.request.CompoundName; @@ -27,7 +28,7 @@ public class JSONDebugSearcher extends Searcher { private static CompoundName PROPERTYNAME = new CompoundName("dumpjson"); @Override - public Result search(com.yahoo.search.Query query, Execution execution) { + public Result search(Query query, Execution execution) { Result r = execution.search(query); String propertyName = query.properties().getString(PROPERTYNAME); if (propertyName != null) { diff --git a/container-search/src/main/java/com/yahoo/search/result/FeatureData.java b/container-search/src/main/java/com/yahoo/search/result/FeatureData.java index 53e77631ff9..7e5d6b12f30 100644 --- a/container-search/src/main/java/com/yahoo/search/result/FeatureData.java +++ b/container-search/src/main/java/com/yahoo/search/result/FeatureData.java @@ -6,29 +6,42 @@ import com.yahoo.data.access.Inspectable; import com.yahoo.data.access.Type; import com.yahoo.data.JsonProducer; import com.yahoo.data.access.simple.JsonRender; +import com.yahoo.io.GrowableByteBuffer; +import com.yahoo.tensor.Tensor; +import com.yahoo.tensor.serialization.JsonFormat; +import com.yahoo.tensor.serialization.TypedBinaryFormat; + +import java.nio.charset.StandardCharsets; +import java.util.HashSet; +import java.util.Optional; +import java.util.Set; /** - * A wrapper for structured data representing feature values. + * A wrapper for structured data representing feature values: A map of floats and tensors. + * This class is not thread safe even when it is only consumed. */ public class FeatureData implements Inspectable, JsonProducer { private final Inspector value; + private Set<String> featureNames = null; + public FeatureData(Inspector value) { this.value = value; } + /** + * Returns the fields of this as an inspector, where tensors are represented as binary data + * which can be decoded using + * <code>com.yahoo.tensor.serialization.TypedBinaryFormat.decode(Optional.empty(), GrowableByteBuffer.wrap(featureValue.asData()))</code> + */ @Override - public Inspector inspect() { - return value; - } + public Inspector inspect() { return value; } + @Override public String toString() { - if (value.type() == Type.EMPTY) { - return ""; - } else { - return toJson(); - } + if (value.type() == Type.EMPTY) return ""; + return toJson(); } @Override @@ -38,7 +51,64 @@ public class FeatureData implements Inspectable, JsonProducer { @Override public StringBuilder writeJson(StringBuilder target) { - return JsonRender.render(value, target, true); + return JsonRender.render(value, new Encoder(target, true)); + } + + /** + * Returns the value of a scalar feature, or null if it is not present. + * + * @throws IllegalArgumentException if the value exists but isn't a scalar + * (that is, if it is a tensor with nonzero rank) + */ + public Double getDouble(String featureName) { + Inspector featureValue = value.field(featureName); + if ( ! featureValue.valid()) return null; + + switch (featureValue.type()) { + case DOUBLE: return featureValue.asDouble(); + case DATA: throw new IllegalArgumentException("Feature '" + featureName + "' is a tensor, not a double"); + default: throw new IllegalStateException("Unexpected feature value type " + featureValue.type()); + } + } + + /** + * Returns the value of a tensor feature, or null if it is not present. + * This will return any feature value: Scalars are returned as a rank 0 tensor. + */ + public Tensor getTensor(String featureName) { + Inspector featureValue = value.field(featureName); + if ( ! featureValue.valid()) return null; + + switch (featureValue.type()) { + case DOUBLE: return Tensor.from(featureValue.asDouble()); + case DATA: return TypedBinaryFormat.decode(Optional.empty(), GrowableByteBuffer.wrap(featureValue.asData())); + default: throw new IllegalStateException("Unexpected feature value type " + featureValue.type()); + } + } + + /** Returns the names of the features available in this */ + public Set<String> featureNames() { + if (featureNames != null) return featureNames; + + featureNames = new HashSet<>(); + value.fields().forEach(field -> featureNames.add(field.getKey())); + return featureNames; + } + + /** A JSON encoder which encodes DATA as a tensor */ + private static class Encoder extends JsonRender.StringEncoder { + + Encoder(StringBuilder out, boolean compact) { + super(out, compact); + } + + @Override + public void encodeDATA(byte[] value) { + // This could be done more efficiently ... + target().append(new String(JsonFormat.encodeWithType(TypedBinaryFormat.decode(Optional.empty(), GrowableByteBuffer.wrap(value))), + StandardCharsets.UTF_8)); + } + } } diff --git a/container-search/src/main/java/com/yahoo/search/result/PositionsData.java b/container-search/src/main/java/com/yahoo/search/result/PositionsData.java index 483849a5435..203e0206f1e 100644 --- a/container-search/src/main/java/com/yahoo/search/result/PositionsData.java +++ b/container-search/src/main/java/com/yahoo/search/result/PositionsData.java @@ -10,7 +10,7 @@ import com.yahoo.data.access.simple.JsonRender; /** * A wrapper for structured data representing an array of position values. - **/ + */ public class PositionsData implements Inspectable, JsonProducer, XmlProducer { private final Inspector value; diff --git a/container-search/src/test/java/com/yahoo/search/result/FeatureDataTestCase.java b/container-search/src/test/java/com/yahoo/search/result/FeatureDataTestCase.java new file mode 100644 index 00000000000..9cc7cc743fc --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/result/FeatureDataTestCase.java @@ -0,0 +1,52 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.result; + +import com.yahoo.data.access.slime.SlimeAdapter; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Slime; +import com.yahoo.tensor.Tensor; +import com.yahoo.tensor.serialization.TypedBinaryFormat; +import org.junit.Test; + +import java.util.stream.Collectors; + +import static org.junit.Assert.assertEquals; + +/** + * @author bratseth + */ +public class FeatureDataTestCase { + + private static final double delta = 0.00000001; + + @Test + public void testFeatureData() { + Cursor features = new Slime().setObject(); + features.setDouble("scalar1", 1.5); + features.setDouble("scalar2", 2.5); + Tensor tensor1 = Tensor.from("tensor(x[3]):[1.5, 2, 2.5]"); + features.setData("tensor1", TypedBinaryFormat.encode(tensor1)); + Tensor tensor2 = Tensor.from(0.5); + features.setData("tensor2", TypedBinaryFormat.encode(tensor2)); + + FeatureData featureData = new FeatureData(new SlimeAdapter(features)); + assertEquals("scalar1,scalar2,tensor1,tensor2", + featureData.featureNames().stream().sorted().collect(Collectors.joining(","))); + assertEquals(1.5, featureData.getDouble("scalar1"), delta); + assertEquals(2.5, featureData.getDouble("scalar2"), delta); + assertEquals(Tensor.from(1.5), featureData.getTensor("scalar1")); + assertEquals(Tensor.from(2.5), featureData.getTensor("scalar2")); + assertEquals(tensor1, featureData.getTensor("tensor1")); + assertEquals(tensor2, featureData.getTensor("tensor2")); + + String expectedJson = + "{" + + "\"scalar1\":1.5," + + "\"scalar2\":2.5," + + "\"tensor1\":{\"type\":\"tensor(x[3])\",\"cells\":[{\"address\":{\"x\":\"0\"},\"value\":1.5},{\"address\":{\"x\":\"1\"},\"value\":2.0},{\"address\":{\"x\":\"2\"},\"value\":2.5}]}," + + "\"tensor2\":{\"type\":\"tensor()\",\"cells\":[{\"address\":{},\"value\":0.5}]}" + + "}"; + assertEquals(expectedJson, featureData.toJson()); + } + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java index 94e111455ac..a1beef23dbb 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java @@ -154,7 +154,7 @@ public enum JobType { case test: return Optional.of(systemTest); case staging: return Optional.of(stagingTest); } - return from(system, ZoneId.from(system, environment, region)); + return from(system, ZoneId.from(environment, region)); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java index 159eb234aa7..b8bb9a7ef79 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java @@ -3,11 +3,10 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.zone.UpgradePolicy; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; -import com.yahoo.config.provision.zone.UpgradePolicy; -import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.yolean.Exceptions; @@ -68,7 +67,9 @@ public abstract class InfrastructureUpgrader extends Maintainer { for (SystemApplication application : applications) { if (convergedOn(target, application.dependencies(), zone)) { boolean currentAppConverged = convergedOn(target, application, zone); - if (!currentAppConverged) { + // In dynamically provisioned zones there may be no tenant hosts at the time of upgrade, so we + // should always set the target version. + if (application == SystemApplication.tenantHost || !currentAppConverged) { upgrade(target, application, zone); } converged &= currentAppConverged; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java index 5a40dd591fd..156e8d0d242 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java @@ -34,7 +34,7 @@ public class SystemUpgrader extends InfrastructureUpgrader { if (minVersion(zone, application, Node::wantedVersion).map(target::isAfter) .orElse(true)) { log.info(String.format("Deploying %s version %s in %s", application.id(), target, zone.getId())); - controller().applications().deploy(application, zone.toDeprecatedId(), target); + controller().applications().deploy(application, zone.getId(), target); } } @@ -45,7 +45,7 @@ public class SystemUpgrader extends InfrastructureUpgrader { if (minVersion.isEmpty()) return true; return minVersion.get().equals(target) - && application.configConvergedIn(zone.toDeprecatedId(), controller(), Optional.of(target)); + && application.configConvergedIn(zone.getId(), controller(), Optional.of(target)); } @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 47c7b54264e..1f20bdf5533 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -54,6 +54,13 @@ import java.util.TreeMap; */ public class ApplicationSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + // Application fields private final String idField = "id"; private final String createdAtField = "createdAt"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/AuditLogSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/AuditLogSerializer.java index 7fee9a7f9b4..d18e561ce5d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/AuditLogSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/AuditLogSerializer.java @@ -19,6 +19,13 @@ import java.util.function.Function; */ public class AuditLogSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String entriesField = "entries"; private static final String atField = "at"; private static final String principalField = "principal"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ConfidenceOverrideSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ConfidenceOverrideSerializer.java index a87875da104..2cb981aac03 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ConfidenceOverrideSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ConfidenceOverrideSerializer.java @@ -18,6 +18,13 @@ import java.util.Map; */ public class ConfidenceOverrideSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private final static String overridesField = "overrides"; public Slime toSlime(Map<Version, Confidence> overrides) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java index 40781ac6e92..418038d4f1e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/LogSerializer.java @@ -27,6 +27,13 @@ import java.util.stream.Collectors; */ class LogSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String idField = "id"; private static final String typeField = "type"; private static final String timestampField = "at"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializer.java index 3dfb5ffe5f8..e3dedd65e68 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializer.java @@ -24,6 +24,13 @@ import java.util.ArrayList; */ public class NameServiceQueueSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String requestsField = "requests"; private static final String requestType = "requestType"; private static final String recordsField = "records"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java index 21f8b1bcb80..d68e24a27ea 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionSerializer.java @@ -20,6 +20,13 @@ import java.util.TreeSet; */ public class OsVersionSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String versionsField = "versions"; private static final String versionField = "version"; private static final String cloudField = "cloud"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java index 3e3c0df1673..88805f54d65 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java @@ -26,6 +26,13 @@ import java.util.TreeMap; */ public class OsVersionStatusSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String versionsField = "versions"; private static final String versionField = "version"; private static final String nodesField = "nodes"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java index a9c6c54a44a..9cfce8dc16a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java @@ -24,6 +24,13 @@ import java.util.function.Function; */ public class RoutingPolicySerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String routingPoliciesField = "routingPolicies"; private static final String clusterField = "cluster"; private static final String canonicalNameField = "canonicalName"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java index f29af1055d0..1c95c9766f5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java @@ -56,6 +56,13 @@ import static java.util.Comparator.comparing; */ class RunSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String stepsField = "steps"; private static final String applicationField = "id"; private static final String jobTypeField = "type"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java index 56e80068908..3a4e6c3954c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java @@ -29,6 +29,13 @@ import java.util.Optional; */ public class TenantSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String nameField = "name"; private static final String typeField = "type"; private static final String athenzDomainField = "athenzDomain"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionSerializer.java index 5edae803fdb..e5897963254 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionSerializer.java @@ -13,6 +13,13 @@ import com.yahoo.slime.Slime; */ public class VersionSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String versionField = "version"; public Slime toSlime(Version version) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java index 72d38bbee5f..405a2e452d0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java @@ -27,6 +27,13 @@ import java.util.Set; */ public class VersionStatusSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + // VersionStatus fields private static final String versionsField = "versions"; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java index 700e6e9cb42..57f29fb72af 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java @@ -83,7 +83,7 @@ public class ZoneFilterMock implements ZoneList { @Override public List<ZoneId> ids() { - return List.copyOf(zones.stream().map(ZoneApi::toDeprecatedId).collect(Collectors.toList())); + return List.copyOf(zones.stream().map(ZoneApi::getId).collect(Collectors.toList())); } @Override diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java index 39ff29f4ae0..38aa4af4756 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java @@ -57,16 +57,16 @@ public class OsUpgraderTest { ); // Bootstrap system - tester.configServer().bootstrap(List.of(zone1.toDeprecatedId(), zone2.toDeprecatedId(), zone3.toDeprecatedId(), zone4.toDeprecatedId(), zone5.toDeprecatedId()), + tester.configServer().bootstrap(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId(), zone5.getId()), List.of(SystemApplication.tenantHost)); // Add system applications that exist in a real system, but are currently not upgraded - tester.configServer().addNodes(List.of(zone1.toDeprecatedId(), zone2.toDeprecatedId(), zone3.toDeprecatedId(), zone4.toDeprecatedId(), zone5.toDeprecatedId()), + tester.configServer().addNodes(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId(), zone5.getId()), List.of(SystemApplication.configServer)); // Fail a few nodes. Failed nodes should not affect versions - failNodeIn(zone1.toDeprecatedId(), SystemApplication.tenantHost); - failNodeIn(zone3.toDeprecatedId(), SystemApplication.tenantHost); + failNodeIn(zone1.getId(), SystemApplication.tenantHost); + failNodeIn(zone3.getId(), SystemApplication.tenantHost); // New OS version released Version version1 = Version.fromString("7.1"); @@ -78,37 +78,37 @@ public class OsUpgraderTest { // zone 1: begins upgrading osUpgrader.maintain(); - assertWanted(version1, SystemApplication.tenantHost, zone1.toDeprecatedId()); + assertWanted(version1, SystemApplication.tenantHost, zone1.getId()); // Other zones remain on previous version (none) - assertWanted(Version.emptyVersion, SystemApplication.proxy, zone2.toDeprecatedId(), zone3.toDeprecatedId(), zone4.toDeprecatedId()); + assertWanted(Version.emptyVersion, SystemApplication.proxy, zone2.getId(), zone3.getId(), zone4.getId()); // zone 1: completes upgrade - completeUpgrade(version1, SystemApplication.tenantHost, zone1.toDeprecatedId()); + completeUpgrade(version1, SystemApplication.tenantHost, zone1.getId()); statusUpdater.maintain(); assertEquals(2, nodesOn(version1).size()); assertEquals(11, nodesOn(Version.emptyVersion).size()); // zone 2 and 3: begins upgrading osUpgrader.maintain(); - assertWanted(version1, SystemApplication.proxy, zone2.toDeprecatedId(), zone3.toDeprecatedId()); + assertWanted(version1, SystemApplication.proxy, zone2.getId(), zone3.getId()); // zone 4: still on previous version - assertWanted(Version.emptyVersion, SystemApplication.tenantHost, zone4.toDeprecatedId()); + assertWanted(Version.emptyVersion, SystemApplication.tenantHost, zone4.getId()); // zone 2 and 3: completes upgrade - completeUpgrade(version1, SystemApplication.tenantHost, zone2.toDeprecatedId(), zone3.toDeprecatedId()); + completeUpgrade(version1, SystemApplication.tenantHost, zone2.getId(), zone3.getId()); // zone 4: begins upgrading osUpgrader.maintain(); - assertWanted(version1, SystemApplication.tenantHost, zone4.toDeprecatedId()); + assertWanted(version1, SystemApplication.tenantHost, zone4.getId()); // zone 4: completes upgrade - completeUpgrade(version1, SystemApplication.tenantHost, zone4.toDeprecatedId()); + completeUpgrade(version1, SystemApplication.tenantHost, zone4.getId()); // Next run does nothing as all zones are upgraded osUpgrader.maintain(); - assertWanted(version1, SystemApplication.tenantHost, zone1.toDeprecatedId(), zone2.toDeprecatedId(), zone3.toDeprecatedId(), zone4.toDeprecatedId()); + assertWanted(version1, SystemApplication.tenantHost, zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId()); statusUpdater.maintain(); assertTrue("All nodes on target version", tester.controller().osVersionStatus().nodesIn(cloud).stream() .allMatch(node -> node.version().equals(version1))); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java index cb5e7cc90a1..7b817c175b8 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java @@ -50,7 +50,7 @@ public class SystemUpgraderTest { Version version1 = Version.fromString("6.5"); // Bootstrap a system without host applications - tester.configServer().bootstrap(List.of(zone1.toDeprecatedId(), zone2.toDeprecatedId(), zone3.toDeprecatedId(), zone4.toDeprecatedId()), + tester.configServer().bootstrap(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId()), SystemApplication.configServer, SystemApplication.proxy); // Fail a few nodes. Failed nodes should not affect versions failNodeIn(zone1, SystemApplication.configServer); @@ -144,7 +144,7 @@ public class SystemUpgraderTest { SystemUpgrader systemUpgrader = systemUpgrader(UpgradePolicy.create().upgrade(zone1)); // Bootstrap system - tester.configServer().bootstrap(List.of(zone1.toDeprecatedId()), SystemApplication.configServer, + tester.configServer().bootstrap(List.of(zone1.getId()), SystemApplication.configServer, SystemApplication.proxy); Version version1 = Version.fromString("6.5"); tester.upgradeSystem(version1); @@ -184,7 +184,7 @@ public class SystemUpgraderTest { ); Version version1 = Version.fromString("6.5"); - tester.configServer().bootstrap(List.of(zone1.toDeprecatedId(), zone2.toDeprecatedId(), zone3.toDeprecatedId(), zone4.toDeprecatedId()), SystemApplication.all()); + tester.configServer().bootstrap(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId()), SystemApplication.all()); tester.upgradeSystem(version1); systemUpgrader.maintain(); assertCurrentVersion(SystemApplication.all(), version1, zone1, zone2, zone3, zone4); @@ -282,7 +282,7 @@ public class SystemUpgraderTest { public void does_not_deploy_proxy_app_in_zones_without_proxy() { List<SystemApplication> applications = List.of( SystemApplication.configServerHost, SystemApplication.configServer, SystemApplication.tenantHost); - tester.configServer().bootstrap(List.of(zone1.toDeprecatedId()), applications); + tester.configServer().bootstrap(List.of(zone1.getId()), applications); tester.configServer().disallowConvergenceCheck(SystemApplication.proxy.id()); SystemUpgrader systemUpgrader = systemUpgrader(UpgradePolicy.create().upgrade(zone1)); @@ -309,7 +309,7 @@ public class SystemUpgraderTest { private void convergeServices(SystemApplication application, ZoneApi... zones) { for (ZoneApi zone : zones) { - tester.controllerTester().configServer().convergeServices(application.id(), zone.toDeprecatedId()); + tester.controllerTester().configServer().convergeServices(application.id(), zone.getId()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java index b2dfd7b4cb6..745a7af203b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java @@ -81,12 +81,12 @@ public class OsApiTest extends ControllerContainerTest { // Status is updated after some zones are upgraded upgradeAndUpdateStatus(); - completeUpgrade(zone1.toDeprecatedId()); + completeUpgrade(zone1.getId()); assertFile(new Request("http://localhost:8080/os/v1/"), "versions-partially-upgraded.json"); // All zones are upgraded upgradeAndUpdateStatus(); - completeUpgrade(zone2.toDeprecatedId(), zone3.toDeprecatedId()); + completeUpgrade(zone2.getId(), zone3.getId()); assertFile(new Request("http://localhost:8080/os/v1/"), "versions-all-upgraded.json"); // Downgrade with force is permitted diff --git a/dist/vespa.spec b/dist/vespa.spec index f7360419ce6..99b24c2ff8e 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -42,17 +42,6 @@ BuildRequires: vespa-protobuf-devel >= 3.7.0-4 BuildRequires: cmake >= 3.9.1 BuildRequires: maven BuildRequires: vespa-protobuf-devel >= 3.7.0-4 -%if 0%{?fc27} -BuildRequires: llvm-devel >= 5.0.2 -BuildRequires: boost-devel >= 1.64 -BuildRequires: vespa-gtest >= 1.8.1-1 -%endif -%if 0%{?fc28} -BuildRequires: llvm-devel >= 6.0.1 -BuildRequires: boost-devel >= 1.66 -BuildRequires: gtest-devel -BuildRequires: gmock-devel -%endif %if 0%{?fc29} BuildRequires: llvm-devel >= 7.0.0 BuildRequires: boost-devel >= 1.66 @@ -125,14 +114,6 @@ Requires: vespa-protobuf >= 3.7.0-4 %endif %if 0%{?fedora} Requires: vespa-protobuf >= 3.7.0-4 -%if 0%{?fc27} -Requires: llvm-libs >= 5.0.2 -%define _vespa_llvm_version 5.0 -%endif -%if 0%{?fc28} -Requires: llvm-libs >= 6.0.1 -%define _vespa_llvm_version 6.0 -%endif %if 0%{?fc29} Requires: llvm-libs >= 7.0.0 %define _vespa_llvm_version 7 diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java index 8cdb0bee7c2..43313392cdb 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java @@ -26,9 +26,8 @@ import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.dockerapi.exception.ContainerNotFoundException; import com.yahoo.vespa.hosted.dockerapi.exception.DockerException; import com.yahoo.vespa.hosted.dockerapi.exception.DockerExecTimeoutException; -import com.yahoo.vespa.hosted.dockerapi.metrics.CounterWrapper; -import com.yahoo.vespa.hosted.dockerapi.metrics.Dimensions; -import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; +import com.yahoo.vespa.hosted.dockerapi.metrics.Counter; +import com.yahoo.vespa.hosted.dockerapi.metrics.Metrics; import java.io.ByteArrayOutputStream; import java.time.Duration; @@ -56,19 +55,18 @@ public class DockerImpl implements Docker { private final DockerClient dockerClient; private final DockerImageGarbageCollector dockerImageGC; - private final CounterWrapper numberOfDockerDaemonFails; + private final Counter numberOfDockerApiFails; @Inject - public DockerImpl(MetricReceiverWrapper metricReceiverWrapper) { - this(createDockerClient(), metricReceiverWrapper); + public DockerImpl(Metrics metrics) { + this(createDockerClient(), metrics); } - DockerImpl(DockerClient dockerClient, MetricReceiverWrapper metricReceiver) { + DockerImpl(DockerClient dockerClient, Metrics metrics) { this.dockerClient = dockerClient; this.dockerImageGC = new DockerImageGarbageCollector(this); - Dimensions dimensions = new Dimensions.Builder().add("role", "docker").build(); - numberOfDockerDaemonFails = metricReceiver.declareCounter(MetricReceiverWrapper.APPLICATION_DOCKER, dimensions, "daemon.api_fails"); + numberOfDockerApiFails = metrics.declareCounter("docker.api_fails"); } @Override @@ -86,7 +84,7 @@ public class DockerImpl implements Docker { return true; } } catch (RuntimeException e) { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerException("Failed to pull image '" + image.asString() + "'", e); } } @@ -110,7 +108,7 @@ public class DockerImpl implements Docker { } catch (NotFoundException e) { return Optional.empty(); } catch (RuntimeException e) { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerException("Failed to inspect image '" + dockerImage.asString() + "'", e); } } @@ -146,7 +144,7 @@ public class DockerImpl implements Docker { return new ProcessResult(state.getExitCode(), new String(output.toByteArray()), new String(errors.toByteArray())); } catch (RuntimeException | InterruptedException e) { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerException("Container '" + containerName.asString() + "' failed to execute " + Arrays.toString(command), e); } @@ -171,7 +169,7 @@ public class DockerImpl implements Docker { } catch (NotFoundException ignored) { return Optional.empty(); } catch (RuntimeException e) { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerException("Failed to get info for container '" + container + "'", e); } } @@ -186,7 +184,7 @@ public class DockerImpl implements Docker { } catch (NotFoundException ignored) { return Optional.empty(); } catch (RuntimeException | InterruptedException e) { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerException("Failed to get stats for container '" + containerName.asString() + "'", e); } } @@ -200,7 +198,7 @@ public class DockerImpl implements Docker { } catch (NotModifiedException ignored) { // If is already started, ignore } catch (RuntimeException e) { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerException("Failed to start container '" + containerName.asString() + "'", e); } } @@ -214,7 +212,7 @@ public class DockerImpl implements Docker { } catch (NotModifiedException ignored) { // If is already stopped, ignore } catch (RuntimeException e) { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerException("Failed to stop container '" + containerName.asString() + "'", e); } } @@ -226,7 +224,7 @@ public class DockerImpl implements Docker { } catch (NotFoundException e) { throw new ContainerNotFoundException(containerName); } catch (RuntimeException e) { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerException("Failed to delete container '" + containerName.asString() + "'", e); } } @@ -253,7 +251,7 @@ public class DockerImpl implements Docker { } catch (NotFoundException e) { throw new ContainerNotFoundException(containerName); } catch (RuntimeException e) { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerException("Failed to update container '" + containerName.asString() + "' to " + resources, e); } } @@ -307,7 +305,7 @@ public class DockerImpl implements Docker { try { return dockerClient.listContainersCmd().withShowAll(true).exec(); } catch (RuntimeException e) { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerException("Failed to list all containers", e); } } @@ -316,7 +314,7 @@ public class DockerImpl implements Docker { try { return dockerClient.listImagesCmd().withShowAll(true).exec(); } catch (RuntimeException e) { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerException("Failed to list all images", e); } } @@ -327,7 +325,7 @@ public class DockerImpl implements Docker { } catch (NotFoundException ignored) { // Image was already deleted, ignore } catch (RuntimeException e) { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerException("Failed to delete docker image " + dockerImage.asString(), e); } } @@ -357,7 +355,7 @@ public class DockerImpl implements Docker { logger.log(LogLevel.INFO, "Download completed: " + dockerImage.asString()); removeScheduledPoll(dockerImage); } else { - numberOfDockerDaemonFails.add(); + numberOfDockerApiFails.increment(); throw new DockerClientException("Could not download image: " + dockerImage); } } diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Counter.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Counter.java new file mode 100644 index 00000000000..3a0b820c846 --- /dev/null +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Counter.java @@ -0,0 +1,28 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.dockerapi.metrics; + +/** + * @author freva + */ +public class Counter implements MetricValue { + private final Object lock = new Object(); + + private long value = 0; + + public void increment() { + add(1L); + } + + public void add(long n) { + synchronized (lock) { + value += n; + } + } + + @Override + public Number getValue() { + synchronized (lock) { + return value; + } + } +} diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/CounterWrapper.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/CounterWrapper.java deleted file mode 100644 index 55c42271674..00000000000 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/CounterWrapper.java +++ /dev/null @@ -1,39 +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.vespa.hosted.dockerapi.metrics; - -import com.yahoo.metrics.simple.Counter; - -/** - * Forwards sample to {@link com.yahoo.metrics.simple.Counter} to be displayed in /state/v1/metrics, - * while also saving the value so it can be accessed programatically later. - * - * @author valerijf - */ -public class CounterWrapper implements MetricValue { - private final Object lock = new Object(); - - private final Counter counter; - private long value = 0; - - CounterWrapper(Counter counter) { - this.counter = counter; - } - - public void add() { - add(1L); - } - - public void add(long n) { - synchronized (lock) { - counter.add(n); - value += n; - } - } - - @Override - public Number getValue() { - synchronized (lock) { - return value; - } - } -} diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/DimensionMetrics.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/DimensionMetrics.java index 770ff5e2216..ef59c4b17d6 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/DimensionMetrics.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/DimensionMetrics.java @@ -4,47 +4,67 @@ package com.yahoo.vespa.hosted.dockerapi.metrics; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.TreeMap; /** * @author freva */ public class DimensionMetrics { - private final static ObjectMapper objectMapper = new ObjectMapper(); + private static final ObjectMapper objectMapper = new ObjectMapper(); + private static final Map<String, Object> routing = Map.of("yamas", Map.of("namespaces", List.of("Vespa"))); private final String application; private final Dimensions dimensions; private final Map<String, Number> metrics; DimensionMetrics(String application, Dimensions dimensions, Map<String, Number> metrics) { - this.application = application; - this.dimensions = dimensions; - this.metrics = metrics; + this.application = Objects.requireNonNull(application); + this.dimensions = Objects.requireNonNull(dimensions); + this.metrics = Objects.requireNonNull(metrics); } - Map<String, Object> getMetrics() { - final Map<String, Object> routing = new HashMap<>(); - final Map<String, Object> routingMonitoring = new HashMap<>(); - routing.put("yamas", routingMonitoring); - routingMonitoring.put("namespaces", Collections.singletonList("Vespa")); - - Map<String, Object> report = new HashMap<>(); + public String toSecretAgentReport() throws JsonProcessingException { + Map<String, Object> report = new TreeMap<>(); report.put("application", application); - report.put("dimensions", dimensions.dimensionsMap); - report.put("metrics", metrics); + report.put("dimensions", new TreeMap<>(dimensions.asMap())); + report.put("metrics", new TreeMap<>(metrics)); report.put("routing", routing); - return report; - } - - public String toSecretAgentReport() throws JsonProcessingException { - Map<String, Object> report = getMetrics(); report.put("timestamp", System.currentTimeMillis() / 1000); return objectMapper.writeValueAsString(report); } + public String getApplication() { + return application; + } + + public Dimensions getDimensions() { + return dimensions; + } + + public Map<String, Number> getMetrics() { + return metrics; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + DimensionMetrics that = (DimensionMetrics) o; + return application.equals(that.application) && + dimensions.equals(that.dimensions) && + metrics.equals(that.metrics); + } + + @Override + public int hashCode() { + return Objects.hash(application, dimensions, metrics); + } + public static class Builder { private final String application; private final Dimensions dimensions; diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Dimensions.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Dimensions.java index 586622100fb..63b92e06505 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Dimensions.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Dimensions.java @@ -1,20 +1,24 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.dockerapi.metrics; -import java.util.Collections; import java.util.HashMap; import java.util.Map; /** - * Each metric reported to secret agent has dimensions. - * - * @author valerijf + * @author freva */ public class Dimensions { - final Map<String, Object> dimensionsMap; - private Dimensions(Map<String, Object> dimensionsMap) { - this.dimensionsMap = dimensionsMap; + public static final Dimensions NONE = new Dimensions(Map.of()); + + private final Map<String, String> dimensionsMap; + + public Dimensions(Map<String, String> dimensionsMap) { + this.dimensionsMap = Map.copyOf(dimensionsMap); + } + + public Map<String, String> asMap() { + return dimensionsMap; } @Override @@ -45,7 +49,7 @@ public class Dimensions { } public Dimensions build() { - return new Dimensions(Collections.unmodifiableMap(new HashMap<>(dimensionsMap))); + return new Dimensions(dimensionsMap); } } } diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Gauge.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Gauge.java new file mode 100644 index 00000000000..b413475fc2b --- /dev/null +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Gauge.java @@ -0,0 +1,24 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.dockerapi.metrics; + +/** + * @author freva + */ +public class Gauge implements MetricValue { + private final Object lock = new Object(); + + private double value; + + public void sample(double x) { + synchronized (lock) { + this.value = x; + } + } + + @Override + public Number getValue() { + synchronized (lock) { + return value; + } + } +} diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/GaugeWrapper.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/GaugeWrapper.java deleted file mode 100644 index 02e1f15a94f..00000000000 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/GaugeWrapper.java +++ /dev/null @@ -1,35 +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.vespa.hosted.dockerapi.metrics; - -import com.yahoo.metrics.simple.Gauge; - -/** - * Forwards sample to {@link com.yahoo.metrics.simple.Gauge} to be displayed in /state/v1/metrics, - * while also saving the value so it can be accessed programatically later. - * - * @author valerijf - */ -public class GaugeWrapper implements MetricValue { - private final Object lock = new Object(); - - private final Gauge gauge; - private double value; - - GaugeWrapper(Gauge gauge) { - this.gauge = gauge; - } - - public void sample(double x) { - synchronized (lock) { - gauge.sample(x); - this.value = x; - } - } - - @Override - public Number getValue() { - synchronized (lock) { - return value; - } - } -} diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapper.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapper.java deleted file mode 100644 index 58126a59cbb..00000000000 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapper.java +++ /dev/null @@ -1,151 +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.vespa.hosted.dockerapi.metrics; - -import com.google.inject.Inject; -import com.yahoo.metrics.simple.MetricReceiver; -import com.yahoo.metrics.simple.Point; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; - -/** - * Export metrics to both /state/v1/metrics and makes them available programmatically. - * Each metric belongs to a monitoring application - * - * @author freva - */ -public class MetricReceiverWrapper { - // Application names used - public static final String APPLICATION_DOCKER = "docker"; - public static final String APPLICATION_HOST = "vespa.host"; - public static final String APPLICATION_NODE = "vespa.node"; - - private final Object monitor = new Object(); - private final Map<DimensionType, Map<String, ApplicationMetrics>> metrics = new HashMap<>(); - private final MetricReceiver metricReceiver; - - @Inject - public MetricReceiverWrapper(MetricReceiver metricReceiver) { - this.metricReceiver = metricReceiver; - } - - /** - * Declaring the same dimensions and name results in the same CounterWrapper instance (idempotent). - */ - public CounterWrapper declareCounter(String application, Dimensions dimensions, String name) { - return declareCounter(application, dimensions, name, DimensionType.DEFAULT); - } - - public CounterWrapper declareCounter(String application, Dimensions dimensions, String name, DimensionType type) { - synchronized (monitor) { - Map<Dimensions, Map<String, MetricValue>> metricsByDimensions = getOrCreateApplicationMetrics(application, type); - if (!metricsByDimensions.containsKey(dimensions)) metricsByDimensions.put(dimensions, new HashMap<>()); - if (!metricsByDimensions.get(dimensions).containsKey(name)) { - CounterWrapper counter = new CounterWrapper(metricReceiver.declareCounter(name, new Point(dimensions.dimensionsMap))); - metricsByDimensions.get(dimensions).put(name, counter); - } - - return (CounterWrapper) metricsByDimensions.get(dimensions).get(name); - } - } - - /** - * Declaring the same dimensions and name results in the same GaugeWrapper instance (idempotent). - */ - public GaugeWrapper declareGauge(String application, Dimensions dimensions, String name) { - return declareGauge(application, dimensions, name, DimensionType.DEFAULT); - } - - public GaugeWrapper declareGauge(String application, Dimensions dimensions, String name, DimensionType type) { - synchronized (monitor) { - Map<Dimensions, Map<String, MetricValue>> metricsByDimensions = getOrCreateApplicationMetrics(application, type); - if (!metricsByDimensions.containsKey(dimensions)) - metricsByDimensions.put(dimensions, new HashMap<>()); - if (!metricsByDimensions.get(dimensions).containsKey(name)) { - GaugeWrapper gauge = new GaugeWrapper(metricReceiver.declareGauge(name, new Point(dimensions.dimensionsMap))); - metricsByDimensions.get(dimensions).put(name, gauge); - } - - return (GaugeWrapper) metricsByDimensions.get(dimensions).get(name); - } - } - - public List<DimensionMetrics> getDefaultMetrics() { - return getMetricsByType(DimensionType.DEFAULT); - } - - // For testing, returns same as getDefaultMetrics(), but without "timestamp" - public Set<Map<String, Object>> getDefaultMetricsRaw() { - synchronized (monitor) { - Set<Map<String, Object>> dimensionMetrics = new HashSet<>(); - metrics.getOrDefault(DimensionType.DEFAULT, new HashMap<>()) - .forEach((application, applicationMetrics) -> applicationMetrics.metricsByDimensions().entrySet().stream() - .map(entry -> new DimensionMetrics(application, entry.getKey(), - entry.getValue().entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, value -> value.getValue().getValue())))) - .map(DimensionMetrics::getMetrics) - .forEach(dimensionMetrics::add)); - return dimensionMetrics; - } - } - - public List<DimensionMetrics> getMetricsByType(DimensionType type) { - synchronized (monitor) { - List<DimensionMetrics> dimensionMetrics = new ArrayList<>(); - metrics.getOrDefault(type, new HashMap<>()) - .forEach((application, applicationMetrics) -> applicationMetrics.metricsByDimensions().entrySet().stream() - .map(entry -> new DimensionMetrics(application, entry.getKey(), - entry.getValue().entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, value -> value.getValue().getValue())))) - .forEach(dimensionMetrics::add)); - return dimensionMetrics; - } - } - - public void deleteMetricByDimension(String name, Dimensions dimensionsToRemove, DimensionType type) { - synchronized (monitor) { - Optional.ofNullable(metrics.get(type)) - .map(m -> m.get(name)) - .map(ApplicationMetrics::metricsByDimensions) - .ifPresent(m -> m.remove(dimensionsToRemove)); - } - } - - // For testing - Map<String, Number> getMetricsForDimension(String application, Dimensions dimensions) { - synchronized (monitor) { - Map<Dimensions, Map<String, MetricValue>> metricsByDimensions = getOrCreateApplicationMetrics(application, DimensionType.DEFAULT); - return metricsByDimensions.getOrDefault(dimensions, Collections.emptyMap()) - .entrySet() - .stream() - .collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().getValue())); - } - } - - private Map<Dimensions, Map<String, MetricValue>> getOrCreateApplicationMetrics(String application, DimensionType type) { - Map<String, ApplicationMetrics> applicationMetrics = metrics.computeIfAbsent(type, m -> new HashMap<>()); - if (! applicationMetrics.containsKey(application)) { - ApplicationMetrics metrics = new ApplicationMetrics(); - applicationMetrics.put(application, metrics); - } - return applicationMetrics.get(application).metricsByDimensions(); - } - - // "Application" is the monitoring application, not Vespa application - private static class ApplicationMetrics { - private final Map<Dimensions, Map<String, MetricValue>> metricsByDimensions = new LinkedHashMap<>(); - - Map<Dimensions, Map<String, MetricValue>> metricsByDimensions() { - return metricsByDimensions; - } - } - - // Used to distinguish whether metrics have been populated with all tag vaules - public enum DimensionType {DEFAULT, PRETAGGED} -} diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricValue.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricValue.java index 7bd4968747f..b20aa1b11ff 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricValue.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricValue.java @@ -1,8 +1,8 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.dockerapi.metrics; /** - * @author valerijf + * @author freva */ public interface MetricValue { Number getValue(); diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Metrics.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Metrics.java new file mode 100644 index 00000000000..f9b169f0a93 --- /dev/null +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Metrics.java @@ -0,0 +1,128 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.dockerapi.metrics; + +import com.google.inject.Inject; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; + +/** + * Stores the latest metric for the given application, name, dimension triplet in memory + * + * @author freva + */ +public class Metrics { + // Application names used + public static final String APPLICATION_HOST = "vespa.host"; + public static final String APPLICATION_NODE = "vespa.node"; + + private final Object monitor = new Object(); + private final Map<DimensionType, Map<String, ApplicationMetrics>> metrics = new HashMap<>(); + + @Inject + public Metrics() { } + + /** + * Creates a counter metric under vespa.host application, with no dimensions and default dimension type + * See {@link #declareCounter(String, String, Dimensions, DimensionType)} + */ + public Counter declareCounter(String name) { + return declareCounter(name, Dimensions.NONE); + } + + /** + * Creates a counter metric under vespa.host application, with the given dimensions and default dimension type + * See {@link #declareCounter(String, String, Dimensions, DimensionType)} + */ + public Counter declareCounter(String name, Dimensions dimensions) { + return declareCounter(APPLICATION_HOST, name, dimensions, DimensionType.DEFAULT); + } + + /** Creates a counter metric. This method is idempotent. */ + public Counter declareCounter(String application, String name, Dimensions dimensions, DimensionType type) { + synchronized (monitor) { + return (Counter) getOrCreateApplicationMetrics(application, type) + .computeIfAbsent(dimensions, d -> new HashMap<>()) + .computeIfAbsent(name, n -> new Counter()); + } + } + + /** + * Creates a gauge metric under vespa.host application, with no dimensions and default dimension type + * See {@link #declareGauge(String, String, Dimensions, DimensionType)} + */ + public Gauge declareGauge(String name) { + return declareGauge(name, Dimensions.NONE); + } + + /** + * Creates a gauge metric under vespa.host application, with the given dimensions and default dimension type + * See {@link #declareGauge(String, String, Dimensions, DimensionType)} + */ + public Gauge declareGauge(String name, Dimensions dimensions) { + return declareGauge(APPLICATION_HOST, name, dimensions, DimensionType.DEFAULT); + } + + /** Creates a gauge metric. This method is idempotent */ + public Gauge declareGauge(String application, String name, Dimensions dimensions, DimensionType type) { + synchronized (monitor) { + return (Gauge) getOrCreateApplicationMetrics(application, type) + .computeIfAbsent(dimensions, d -> new HashMap<>()) + .computeIfAbsent(name, n -> new Gauge()); + } + } + + public List<DimensionMetrics> getDefaultMetrics() { + return getMetricsByType(DimensionType.DEFAULT); + } + + public List<DimensionMetrics> getMetricsByType(DimensionType type) { + synchronized (monitor) { + List<DimensionMetrics> dimensionMetrics = new ArrayList<>(); + metrics.getOrDefault(type, Map.of()) + .forEach((application, applicationMetrics) -> applicationMetrics.metricsByDimensions().entrySet().stream() + .map(entry -> new DimensionMetrics(application, entry.getKey(), + entry.getValue().entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, value -> value.getValue().getValue())))) + .forEach(dimensionMetrics::add)); + return dimensionMetrics; + } + } + + public void deleteMetricByDimension(String name, Dimensions dimensionsToRemove, DimensionType type) { + synchronized (monitor) { + Optional.ofNullable(metrics.get(type)) + .map(m -> m.get(name)) + .map(ApplicationMetrics::metricsByDimensions) + .ifPresent(m -> m.remove(dimensionsToRemove)); + } + } + + Map<Dimensions, Map<String, MetricValue>> getOrCreateApplicationMetrics(String application, DimensionType type) { + return metrics.computeIfAbsent(type, m -> new HashMap<>()) + .computeIfAbsent(application, app -> new ApplicationMetrics()) + .metricsByDimensions(); + } + + // "Application" is the monitoring application, not Vespa application + private static class ApplicationMetrics { + private final Map<Dimensions, Map<String, MetricValue>> metricsByDimensions = new LinkedHashMap<>(); + + Map<Dimensions, Map<String, MetricValue>> metricsByDimensions() { + return metricsByDimensions; + } + } + + // Used to distinguish whether metrics have been populated with all tag vaules + public enum DimensionType { + /** Default metrics get added default dimensions set in check config */ + DEFAULT, + + /** Pretagged metrics will only get the dimensions explicitly set when creating the counter/gauge */ + PRETAGGED + } +} diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java index df221302575..4843d8f9685 100644 --- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java +++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java @@ -14,8 +14,7 @@ import com.github.dockerjava.api.command.PullImageCmd; import com.github.dockerjava.api.exception.NotFoundException; import com.github.dockerjava.core.command.ExecStartResultCallback; import com.yahoo.config.provision.DockerImage; -import com.yahoo.metrics.simple.MetricReceiver; -import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; +import com.yahoo.vespa.hosted.dockerapi.metrics.Metrics; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.Matchers; @@ -37,8 +36,8 @@ import static org.mockito.Mockito.when; public class DockerImplTest { private final DockerClient dockerClient = mock(DockerClient.class); - private final MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation); - private final DockerImpl docker = new DockerImpl(dockerClient, metricReceiver); + private final Metrics metrics = new Metrics(); + private final DockerImpl docker = new DockerImpl(dockerClient, metrics); @Test public void testExecuteCompletes() { diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapperTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapperTest.java deleted file mode 100644 index c20e64d906e..00000000000 --- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapperTest.java +++ /dev/null @@ -1,96 +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.vespa.hosted.dockerapi.metrics; - -import com.yahoo.metrics.simple.MetricReceiver; -import org.junit.Test; - -import java.util.Map; - -import static org.junit.Assert.assertEquals; - -/** - * @author freva - */ -public class MetricReceiverWrapperTest { - private static final Dimensions hostDimension = new Dimensions.Builder().add("host", "abc.yahoo.com").build(); - private static final String applicationDocker = MetricReceiverWrapper.APPLICATION_DOCKER; - - @Test - public void testDefaultValue() { - MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation); - - metricReceiver.declareCounter(applicationDocker, hostDimension, "some.name"); - - assertEquals(metricReceiver.getMetricsForDimension(applicationDocker, hostDimension).get("some.name"), 0L); - } - - @Test - public void testSimpleIncrementMetric() { - MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation); - CounterWrapper counter = metricReceiver.declareCounter(applicationDocker, hostDimension, "a_counter.value"); - - counter.add(5); - counter.add(8); - - Map<String, Number> latestMetrics = metricReceiver.getMetricsForDimension(applicationDocker, hostDimension); - assertEquals("Expected only 1 metric value to be set", 1, latestMetrics.size()); - assertEquals(latestMetrics.get("a_counter.value"), 13L); // 5 + 8 - } - - @Test - public void testSimpleGauge() { - MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation); - GaugeWrapper gauge = metricReceiver.declareGauge(applicationDocker, hostDimension, "test.gauge"); - - gauge.sample(42); - gauge.sample(-342.23); - - Map<String, Number> latestMetrics = metricReceiver.getMetricsForDimension(applicationDocker, hostDimension); - assertEquals("Expected only 1 metric value to be set", 1, latestMetrics.size()); - assertEquals(latestMetrics.get("test.gauge"), -342.23); - } - - @Test - public void testRedeclaringSameGauge() { - MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation); - GaugeWrapper gauge = metricReceiver.declareGauge(applicationDocker, hostDimension, "test.gauge"); - gauge.sample(42); - - // Same as hostDimension, but new instance. - Dimensions newDimension = new Dimensions.Builder().add("host", "abc.yahoo.com").build(); - GaugeWrapper newGauge = metricReceiver.declareGauge(applicationDocker, newDimension, "test.gauge"); - newGauge.sample(56); - - assertEquals(metricReceiver.getMetricsForDimension(applicationDocker, hostDimension).get("test.gauge"), 56.); - } - - @Test - public void testSameMetricNameButDifferentDimensions() { - MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation); - GaugeWrapper gauge = metricReceiver.declareGauge(applicationDocker, hostDimension, "test.gauge"); - gauge.sample(42); - - // Not the same as hostDimension. - Dimensions newDimension = new Dimensions.Builder().add("host", "abcd.yahoo.com").build(); - GaugeWrapper newGauge = metricReceiver.declareGauge(applicationDocker, newDimension, "test.gauge"); - newGauge.sample(56); - - assertEquals(metricReceiver.getMetricsForDimension(applicationDocker, hostDimension).get("test.gauge"), 42.); - assertEquals(metricReceiver.getMetricsForDimension(applicationDocker, newDimension).get("test.gauge"), 56.); - } - - @Test - public void testDeletingMetric() { - MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation); - metricReceiver.declareGauge(applicationDocker, hostDimension, "test.gauge"); - - Dimensions differentDimension = new Dimensions.Builder().add("host", "abcd.yahoo.com").build(); - metricReceiver.declareGauge(applicationDocker, differentDimension, "test.gauge"); - - assertEquals(2, metricReceiver.getDefaultMetricsRaw().size()); - metricReceiver.deleteMetricByDimension(applicationDocker, differentDimension, MetricReceiverWrapper.DimensionType.DEFAULT); - assertEquals(1, metricReceiver.getDefaultMetricsRaw().size()); - assertEquals(metricReceiver.getMetricsForDimension(applicationDocker, hostDimension).size(), 1); - assertEquals(metricReceiver.getMetricsForDimension(applicationDocker, differentDimension).size(), 0); - } -} diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricsTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricsTest.java new file mode 100644 index 00000000000..fc153ee0562 --- /dev/null +++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricsTest.java @@ -0,0 +1,99 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.dockerapi.metrics; + +import org.junit.Test; + +import java.util.Map; +import java.util.stream.Collectors; + +import static com.yahoo.vespa.hosted.dockerapi.metrics.Metrics.APPLICATION_HOST; +import static com.yahoo.vespa.hosted.dockerapi.metrics.Metrics.DimensionType.DEFAULT; +import static org.junit.Assert.assertEquals; + +/** + * @author freva + */ +public class MetricsTest { + private static final Dimensions hostDimension = new Dimensions.Builder().add("host", "abc.yahoo.com").build(); + private final Metrics metrics = new Metrics(); + + @Test + public void testDefaultValue() { + metrics.declareCounter("some.name", hostDimension); + + assertEquals(getMetricsForDimension(hostDimension).get("some.name"), 0L); + } + + @Test + public void testSimpleIncrementMetric() { + Counter counter = metrics.declareCounter("a_counter.value", hostDimension); + + counter.add(5); + counter.add(8); + + Map<String, Number> latestMetrics = getMetricsForDimension(hostDimension); + assertEquals("Expected only 1 metric value to be set", 1, latestMetrics.size()); + assertEquals(latestMetrics.get("a_counter.value"), 13L); // 5 + 8 + } + + @Test + public void testSimpleGauge() { + Gauge gauge = metrics.declareGauge("test.gauge", hostDimension); + + gauge.sample(42); + gauge.sample(-342.23); + + Map<String, Number> latestMetrics = getMetricsForDimension(hostDimension); + assertEquals("Expected only 1 metric value to be set", 1, latestMetrics.size()); + assertEquals(latestMetrics.get("test.gauge"), -342.23); + } + + @Test + public void testRedeclaringSameGauge() { + Gauge gauge = metrics.declareGauge("test.gauge", hostDimension); + gauge.sample(42); + + // Same as hostDimension, but new instance. + Dimensions newDimension = new Dimensions.Builder().add("host", "abc.yahoo.com").build(); + Gauge newGauge = metrics.declareGauge("test.gauge", newDimension); + newGauge.sample(56); + + assertEquals(getMetricsForDimension(hostDimension).get("test.gauge"), 56.); + } + + @Test + public void testSameMetricNameButDifferentDimensions() { + Gauge gauge = metrics.declareGauge("test.gauge", hostDimension); + gauge.sample(42); + + // Not the same as hostDimension. + Dimensions newDimension = new Dimensions.Builder().add("host", "abcd.yahoo.com").build(); + Gauge newGauge = metrics.declareGauge("test.gauge", newDimension); + newGauge.sample(56); + + assertEquals(getMetricsForDimension(hostDimension).get("test.gauge"), 42.); + assertEquals(getMetricsForDimension(newDimension).get("test.gauge"), 56.); + } + + @Test + public void testDeletingMetric() { + metrics.declareGauge("test.gauge", hostDimension); + + Dimensions differentDimension = new Dimensions.Builder().add("host", "abcd.yahoo.com").build(); + metrics.declareGauge("test.gauge", differentDimension); + + assertEquals(2, metrics.getMetricsByType(DEFAULT).size()); + metrics.deleteMetricByDimension(APPLICATION_HOST, differentDimension, DEFAULT); + assertEquals(1, metrics.getMetricsByType(DEFAULT).size()); + assertEquals(getMetricsForDimension(hostDimension).size(), 1); + assertEquals(getMetricsForDimension(differentDimension).size(), 0); + } + + private Map<String, Number> getMetricsForDimension(Dimensions dimensions) { + return metrics.getOrCreateApplicationMetrics(APPLICATION_HOST, DEFAULT) + .getOrDefault(dimensions, Map.of()) + .entrySet() + .stream() + .collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().getValue())); + } +} 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 906a56d3f34..66c8da86403 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -105,9 +105,9 @@ public class Flags { "Takes effect on next deployment", APPLICATION_ID); - public static final UnboundListFlag<String> DYNAMIC_PROVISIONING_FLAVORS = defineListFlag( - "dynamic-provisioning-flavors", List.of(), - "List of additional Vespa flavor names that can be used for dynamic provisioning", + public static final UnboundListFlag<String> DISABLED_DYNAMIC_PROVISIONING_FLAVORS = defineListFlag( + "disabled-dynamic-provisioning-flavors", List.of(), + "List of disabled Vespa flavor names that cannot be used for dynamic provisioning", "Takes effect on next provisioning"); public static final UnboundBooleanFlag ENABLE_DISK_WRITE_TEST = defineFeatureFlag( diff --git a/logserver/src/main/java/com/yahoo/logserver/handlers/archive/LogWriter.java b/logserver/src/main/java/com/yahoo/logserver/handlers/archive/LogWriter.java index 47a9b04291d..83d6a4a0def 100644 --- a/logserver/src/main/java/com/yahoo/logserver/handlers/archive/LogWriter.java +++ b/logserver/src/main/java/com/yahoo/logserver/handlers/archive/LogWriter.java @@ -46,11 +46,7 @@ public class LogWriter { * </UL> */ private Writer nextWriter() throws IOException { - - if (writer != null) { - writer.close(); - } - + close(); int maxAttempts = 1000; while (maxAttempts-- > 0) { String name = prefix + "-" + generation++; @@ -119,15 +115,15 @@ public class LogWriter { } - public void flush() throws IOException { + public synchronized void flush() throws IOException { if (writer != null) { writer.flush(); } } - public void close() throws IOException { - flush(); + public synchronized void close() throws IOException { if (writer != null) { + writer.flush(); writer.close(); writer = null; } diff --git a/logserver/src/main/java/com/yahoo/logserver/handlers/archive/LogWriterLRUCache.java b/logserver/src/main/java/com/yahoo/logserver/handlers/archive/LogWriterLRUCache.java index 3d692297f1c..5c1da722f57 100644 --- a/logserver/src/main/java/com/yahoo/logserver/handlers/archive/LogWriterLRUCache.java +++ b/logserver/src/main/java/com/yahoo/logserver/handlers/archive/LogWriterLRUCache.java @@ -14,7 +14,7 @@ import java.util.Map; public class LogWriterLRUCache extends LinkedHashMap<Integer, LogWriter> { private static final Logger log = Logger.getLogger(LogWriterLRUCache.class.getName()); - final int maxEntries = 100; + final int maxEntries = 5; public LogWriterLRUCache(int initialCapacity, float loadFactor) { super(initialCapacity, loadFactor, true); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/MetricsManager.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/MetricsManager.java index fe823c72127..14d1203824b 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/MetricsManager.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/MetricsManager.java @@ -81,6 +81,8 @@ public class MetricsManager { */ public List<MetricsPacket> getMetrics(List<VespaService> services, Instant startTime) { if (services.isEmpty()) return Collections.emptyList(); + + log.log(DEBUG, () -> "Updating services prior to fetching metrics, number of services= " + services.size()); vespaServices.updateServices(services); List<MetricsPacket.Builder> result = vespaMetrics.getMetrics(services); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java index 054fa704ecb..2ca24dad1e2 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java @@ -14,7 +14,6 @@ import ai.vespa.metricsproxy.metric.model.ConsumerId; import ai.vespa.metricsproxy.metric.model.DimensionId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; import ai.vespa.metricsproxy.service.VespaService; -import ai.vespa.metricsproxy.service.VespaServices; import java.util.ArrayList; import java.util.Collections; @@ -32,7 +31,6 @@ import static ai.vespa.metricsproxy.metric.model.ConsumerId.toConsumerId; import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId; import static com.google.common.base.Strings.isNullOrEmpty; -import static com.yahoo.log.LogLevel.DEBUG; /** * @author Unknown @@ -77,8 +75,6 @@ public class VespaMetrics { public List<MetricsPacket.Builder> getMetrics(List<VespaService> services) { List<MetricsPacket.Builder> metricsPackets = new ArrayList<>(); - log.log(DEBUG, () -> "Updating services prior to fetching metrics, number of services= " + services.size()); - Map<ConsumersConfig.Consumer.Metric, List<ConsumerId>> consumersByMetric = metricsConsumers.getConsumersByMetric(); for (VespaService service : services) { @@ -86,42 +82,58 @@ public class VespaMetrics { Optional<MetricsPacket.Builder> systemCheck = getSystemMetrics(service); systemCheck.ifPresent(metricsPackets::add); - // One metrics packet per set of metrics that share the same dimensions+consumers - // TODO: Move aggregation into MetricsPacket itself? - Metrics serviceMetrics = getServiceMetrics(service, consumersByMetric); - Map<AggregationKey, List<Metric>> aggregatedMetrics = - aggregateMetrics(service.getDimensions(), serviceMetrics); - - aggregatedMetrics.forEach((aggregationKey, metrics) -> { - MetricsPacket.Builder builder = new MetricsPacket.Builder(toServiceId(service.getMonitoringName())) - .putMetrics(metrics) - .putDimension(METRIC_TYPE_DIMENSION_ID, "standard") - .putDimension(INSTANCE_DIMENSION_ID, service.getInstanceName()) - .putDimensions(aggregationKey.getDimensions()); - setMetaInfo(builder, serviceMetrics.getTimeStamp()); - builder.addConsumers(aggregationKey.getConsumers()); - metricsPackets.add(builder); - }); + Metrics allServiceMetrics = service.getMetrics(); + + if (! allServiceMetrics.getMetrics().isEmpty()) { + Metrics serviceMetrics = getServiceMetrics(allServiceMetrics, consumersByMetric); + + // One metrics packet per set of metrics that share the same dimensions+consumers + // TODO: Move aggregation into MetricsPacket itself? + Map<AggregationKey, List<Metric>> aggregatedMetrics = aggregateMetrics(service.getDimensions(), serviceMetrics); + + aggregatedMetrics.forEach((aggregationKey, metrics) -> { + MetricsPacket.Builder builder = new MetricsPacket.Builder(toServiceId(service.getMonitoringName())) + .putMetrics(metrics) + .putDimension(METRIC_TYPE_DIMENSION_ID, "standard") + .putDimension(INSTANCE_DIMENSION_ID, service.getInstanceName()) + .putDimensions(aggregationKey.getDimensions()); + setMetaInfo(builder, serviceMetrics.getTimeStamp()); + builder.addConsumers(aggregationKey.getConsumers()); + metricsPackets.add(builder); + }); + } else { + // Service did not return any metrics, so add metrics packet based on service health. + // TODO: Make VespaService.getMetrics return MetricsPacket and handle health on its own. + metricsPackets.add(getHealth(service)); + } } - return metricsPackets; } + private MetricsPacket.Builder getHealth(VespaService service) { + HealthMetric health = service.getHealth(); + return new MetricsPacket.Builder(toServiceId(service.getMonitoringName())) + .timestamp(System.currentTimeMillis() / 1000) + .statusCode(health.getStatus().ordinal()) // TODO: MetricsPacket should use StatusCode instead of int + .statusMessage(health.getMessage()) + .putDimensions(service.getDimensions()) + .putDimension(INSTANCE_DIMENSION_ID, service.getInstanceName()); + } + /** * Returns the metrics to output for the given service, with updated timestamp * In order to include a metric, it must exist in the given map of metric to consumers. * Each returned metric will contain a collection of consumers that it should be routed to. */ - private Metrics getServiceMetrics(VespaService service, Map<ConsumersConfig.Consumer.Metric, List<ConsumerId>> consumersByMetric) { - Metrics serviceMetrics = new Metrics(); - Metrics allServiceMetrics = service.getMetrics(); - serviceMetrics.setTimeStamp(getMostRecentTimestamp(allServiceMetrics)); + private Metrics getServiceMetrics(Metrics allServiceMetrics, Map<ConsumersConfig.Consumer.Metric, List<ConsumerId>> consumersByMetric) { + Metrics configuredServiceMetrics = new Metrics(); + configuredServiceMetrics.setTimeStamp(getMostRecentTimestamp(allServiceMetrics)); for (Metric candidate : allServiceMetrics.getMetrics()) { getConfiguredMetrics(candidate.getName(), consumersByMetric.keySet()).forEach( - configuredMetric -> serviceMetrics.add( + configuredMetric -> configuredServiceMetrics.add( metricWithConfigProperties(candidate, configuredMetric, consumersByMetric))); } - return serviceMetrics; + return configuredServiceMetrics; } private Map<DimensionId, String> extractDimensions(Map<DimensionId, String> dimensions, List<ConsumersConfig.Consumer.Metric.Dimension> configuredDimensions) { diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/HealthMetric.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/HealthMetric.java index 41a8c3d414e..4961cc8b2a6 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/HealthMetric.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/HealthMetric.java @@ -4,49 +4,51 @@ package ai.vespa.metricsproxy.metric; +import ai.vespa.metricsproxy.metric.model.StatusCode; + +import static ai.vespa.metricsproxy.metric.model.StatusCode.DOWN; +import static ai.vespa.metricsproxy.metric.model.StatusCode.UNKNOWN; +import static ai.vespa.metricsproxy.metric.model.StatusCode.UP; + /** + * TODO: Use MetricsPacket instead of this class. + * * @author Jo Kristian Bergum */ public class HealthMetric { private final String message; - private final String status; + private final StatusCode status; private final boolean isAlive; - private HealthMetric(String status, String message, boolean isAlive) { + private HealthMetric(StatusCode status, String message, boolean isAlive) { this.message = message; this.status = status; this.isAlive = isAlive; } public static HealthMetric get(String status, String message) { - if (status == null) { - status = ""; - } - if (message == null) { - message = ""; - } - status = status.toLowerCase(); + if (message == null) message = ""; + var statusCode = StatusCode.fromString(status); + return new HealthMetric(statusCode, message, statusCode == UP); + } - if (status.equals("up") || status.equals("ok")) { - return new HealthMetric(status, message, true); - } else { - return new HealthMetric(status, message, false); - } + public static HealthMetric getDown(String message) { + return new HealthMetric(DOWN, message, false); } - public static HealthMetric getFailed(String message) { - return new HealthMetric("down", message, false); + public static HealthMetric getUnknown(String message) { + return new HealthMetric(UNKNOWN, message, false); } public static HealthMetric getOk(String message) { - return new HealthMetric("up", message, true); + return new HealthMetric(UP, message, true); } public String getMessage() { return this.message; } - public String getStatus() { + public StatusCode getStatus() { return this.status; } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/Metrics.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/Metrics.java index ca611368730..f1d029d8746 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/Metrics.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/Metrics.java @@ -30,7 +30,6 @@ public class Metrics { private void ensureNotFrozen() { if (isFrozen) throw new IllegalStateException("Frozen Metrics cannot be modified!"); - } public long getTimeStamp() { @@ -84,22 +83,6 @@ public class Metrics { } - /** - * Get a single metric based on the metric name - * TODO: Remove, might be multiple metrics with same name, but different - * - * @param key metric name - * @return The value or null if metric was not found or expired - */ - public Number get(String key) { - isFrozen = true; - Metric m = getMetric(key); - if (m != null) { - return m.getValue(); - } - return null; - } - public String toString() { StringBuilder sb = new StringBuilder(); for (Metric m : metrics) { diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java index fa45c6251f6..098fd48c8b3 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java @@ -14,7 +14,6 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.function.Function; @@ -88,7 +87,7 @@ public class MetricsPacket { // Except for 'service' for which we require an explicit non-null value. private ServiceId service; private int statusCode = 0; - private String statusMessage = "<null>"; + private String statusMessage = ""; private long timestamp = 0L; private Map<MetricId, Number> metrics = new LinkedHashMap<>(); private final Map<DimensionId, String> dimensions = new LinkedHashMap<>(); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/StatusCode.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/StatusCode.java new file mode 100644 index 00000000000..7f5a7d0e64b --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/StatusCode.java @@ -0,0 +1,35 @@ +/* + * Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + */ + +package ai.vespa.metricsproxy.metric.model; + +/** + * Status code for a Vespa service. + * + * @author gjoranv + */ +public enum StatusCode { + + UP(0, "up"), + DOWN(1, "down"), + UNKNOWN(2, "unknown"); + + public final int code; + public final String status; + + StatusCode(int code, String status) { + this.code = code; + this.status = status; + } + + public static StatusCode fromString(String statusString) { + if ("ok".equalsIgnoreCase(statusString)) return UP; + try { + return valueOf(statusString.trim().toUpperCase()); + } catch (IllegalArgumentException | NullPointerException e) { + return UNKNOWN; + } + } + +} diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java index 495e3ec1f7d..aadcc1418af 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java @@ -6,6 +6,7 @@ package ai.vespa.metricsproxy.metric.model.json; import ai.vespa.metricsproxy.metric.model.MetricsPacket; import ai.vespa.metricsproxy.metric.model.ServiceId; +import ai.vespa.metricsproxy.metric.model.StatusCode; import java.util.ArrayList; import java.util.List; @@ -34,9 +35,13 @@ public class GenericJsonUtil { var genericMetricsList = packets.stream() .map(packet -> new GenericMetrics(packet.metrics(), packet.dimensions())) .collect(toList()); - var genericService = new GenericService(serviceId.id, - packets.get(0).timestamp, - genericMetricsList); + var genericService = packets.stream().findFirst() + .map(firstPacket -> new GenericService(serviceId.id, + firstPacket.timestamp, + StatusCode.values()[firstPacket.statusCode], + firstPacket.statusMessage, + genericMetricsList)) + .get(); if (VESPA_NODE_SERVICE_ID.equals(serviceId)) { jsonModel.node = new GenericNode(genericService.timestamp, genericService.metrics); } else { diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericService.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericService.java index bd3dbf935ed..f348bd4beca 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericService.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericService.java @@ -4,12 +4,12 @@ package ai.vespa.metricsproxy.metric.model.json; +import ai.vespa.metricsproxy.metric.model.StatusCode; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonPropertyOrder; -import java.util.ArrayList; import java.util.List; import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_ABSENT; @@ -37,10 +37,11 @@ public class GenericService { public GenericService() { } - GenericService(String name, Long timestamp, List<GenericMetrics> metrics) { + // TODO: take StatusCode instead of int + GenericService(String name, Long timestamp, StatusCode statusCode, String message, List<GenericMetrics> metrics) { this.name = name; this.timestamp = timestamp; - status = new Status("up"); + status = new Status(statusCode, message); this.metrics = metrics; } @@ -50,8 +51,9 @@ public class GenericService { public static class Status { public Status() { } - Status(String code) { - this.code = code; + Status(StatusCode statusCode, String description) { + code = statusCode.status; + this.description = description; } @JsonProperty("code") diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/DummyHealthMetricFetcher.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/DummyHealthMetricFetcher.java index f87171a42dc..c9bfc8b365c 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/DummyHealthMetricFetcher.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/DummyHealthMetricFetcher.java @@ -28,7 +28,7 @@ public class DummyHealthMetricFetcher extends RemoteHealthMetricFetcher { if (service.isAlive()) { return HealthMetric.getOk("Service is running - pid check only"); } else { - return HealthMetric.getFailed("Service is not running - pid check only"); + return HealthMetric.getDown("Service is not running - pid check only"); } } } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/HttpMetricFetcher.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/HttpMetricFetcher.java index 9094ef22c20..81358041502 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/HttpMetricFetcher.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/HttpMetricFetcher.java @@ -25,8 +25,6 @@ import java.util.logging.Logger; public abstract class HttpMetricFetcher { private final static Logger log = Logger.getLogger(HttpMetricFetcher.class.getPackage().getName()); public final static String STATE_PATH = "/state/v1/"; - final static String METRICS_PATH = STATE_PATH + "metrics"; - final static String HEALTH_PATH = STATE_PATH + "health"; // The call to apache will do 3 retries. As long as we check the services in series, we can't have this too high. public static int CONNECTION_TIMEOUT = 5000; private final static int SOCKET_TIMEOUT = 60000; diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteHealthMetricFetcher.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteHealthMetricFetcher.java index 503f582a827..068a8faade8 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteHealthMetricFetcher.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteHealthMetricFetcher.java @@ -18,13 +18,10 @@ import java.util.logging.Logger; * @author Jo Kristian Bergum */ public class RemoteHealthMetricFetcher extends HttpMetricFetcher { - private final static Logger log = Logger.getLogger(RemoteHealthMetricFetcher.class.getPackage().getName()); - /** - * @param service The service to fetch metrics from - * @param port The port to use - */ + private final static String HEALTH_PATH = STATE_PATH + "health"; + public RemoteHealthMetricFetcher(VespaService service, int port) { super(service, port, HEALTH_PATH); } @@ -45,8 +42,8 @@ public class RemoteHealthMetricFetcher extends HttpMetricFetcher { /** * Connect to remote service over http and fetch metrics */ - HealthMetric createHealthMetrics(String data, int fetchCount) { - HealthMetric healthMetric = HealthMetric.getFailed("Failed fetching status page for service"); + private HealthMetric createHealthMetrics(String data, int fetchCount) { + HealthMetric healthMetric = HealthMetric.getDown("Failed fetching status page for service"); try { healthMetric = parse(data); } catch (Exception e) { @@ -57,7 +54,7 @@ public class RemoteHealthMetricFetcher extends HttpMetricFetcher { private HealthMetric parse(String data) { if (data == null || data.isEmpty()) { - return HealthMetric.getFailed("Empty response from status page"); + return HealthMetric.getUnknown("Empty response from status page"); } try { JSONObject o = new JSONObject(data); @@ -71,7 +68,7 @@ public class RemoteHealthMetricFetcher extends HttpMetricFetcher { } catch (JSONException e) { log.log(LogLevel.DEBUG, "Failed to parse json response from metrics page:" + e + ":" + data); - return HealthMetric.getFailed("Not able to parse json from status page"); + return HealthMetric.getUnknown("Not able to parse json from status page"); } } } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteMetricsFetcher.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteMetricsFetcher.java index a606ec7d8cd..552b4dc4010 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteMetricsFetcher.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteMetricsFetcher.java @@ -25,10 +25,9 @@ import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; * @author Jo Kristian Bergum */ public class RemoteMetricsFetcher extends HttpMetricFetcher { - /** - * @param service The service to fetch metrics from - * @param port The port to use - */ + + final static String METRICS_PATH = STATE_PATH + "metrics"; + RemoteMetricsFetcher(VespaService service, int port) { super(service, port, METRICS_PATH); } @@ -50,7 +49,7 @@ public class RemoteMetricsFetcher extends HttpMetricFetcher { /** * Connect to remote service over http and fetch metrics */ - public Metrics createMetrics(String data, int fetchCount) { + Metrics createMetrics(String data, int fetchCount) { Metrics remoteMetrics = new Metrics(); try { remoteMetrics = parse(data); @@ -61,7 +60,7 @@ public class RemoteMetricsFetcher extends HttpMetricFetcher { return remoteMetrics; } - Metrics parse(String data) throws JSONException { + private Metrics parse(String data) throws JSONException { JSONObject o = new JSONObject(data); if (!(o.has("metrics"))) { return new Metrics(); //empty diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/core/MetricsManagerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/core/MetricsManagerTest.java index 1635ccab197..eb620fd37be 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/core/MetricsManagerTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/core/MetricsManagerTest.java @@ -6,7 +6,7 @@ package ai.vespa.metricsproxy.core; import ai.vespa.metricsproxy.TestUtil; import ai.vespa.metricsproxy.core.ConsumersConfig.Consumer; -import ai.vespa.metricsproxy.metric.ExternalMetrics; +import ai.vespa.metricsproxy.metric.HealthMetric; import ai.vespa.metricsproxy.metric.Metric; import ai.vespa.metricsproxy.metric.Metrics; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensions; @@ -15,6 +15,7 @@ import ai.vespa.metricsproxy.metric.dimensions.NodeDimensions; import ai.vespa.metricsproxy.metric.dimensions.NodeDimensionsConfig; import ai.vespa.metricsproxy.metric.model.DimensionId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; +import ai.vespa.metricsproxy.service.DownService; import ai.vespa.metricsproxy.service.DummyService; import ai.vespa.metricsproxy.service.VespaService; import ai.vespa.metricsproxy.service.VespaServices; @@ -23,6 +24,7 @@ import org.junit.Before; import org.junit.Test; import java.time.Instant; +import java.util.Collections; import java.util.List; import java.util.Map; @@ -62,6 +64,21 @@ public class MetricsManagerTest { } @Test + public void service_that_is_down_has_a_separate_metrics_packet() { + // Reset to use only the service that is down + var downService = new DownService(HealthMetric.getDown("No response")); + List<VespaService> testServices = Collections.singletonList(downService); + MetricsManager metricsManager = TestUtil.createMetricsManager(new VespaServices(testServices), + getMetricsConsumers(),getApplicationDimensions(), getNodeDimensions()); + + List<MetricsPacket> packets = metricsManager.getMetrics(testServices, Instant.EPOCH); + assertThat(packets.size(), is(1)); + assertTrue(packets.get(0).metrics().isEmpty()); + assertThat(packets.get(0).dimensions().get(toDimensionId("instance")), is(DownService.NAME)); + assertThat(packets.get(0).dimensions().get(toDimensionId("global")), is("value")); + } + + @Test public void each_service_gets_separate_metrics_packets() { List<MetricsPacket> packets = metricsManager.getMetrics(testServices, Instant.EPOCH); assertThat(packets.size(), is(2)); @@ -77,11 +94,11 @@ public class MetricsManagerTest { @Test public void verify_expected_output_from_getMetricsById() { - String dummy0Metrics = metricsManager.getMetricsByConfigId("dummy/id/0"); + String dummy0Metrics = metricsManager.getMetricsByConfigId(SERVICE_0_ID); assertThat(dummy0Metrics, containsString("'dummy.id.0'.val=1.050")); assertThat(dummy0Metrics, containsString("'dummy.id.0'.c_test=1")); - String dummy1Metrics = metricsManager.getMetricsByConfigId("dummy/id/1"); + String dummy1Metrics = metricsManager.getMetricsByConfigId(SERVICE_1_ID); assertThat(dummy1Metrics, containsString("'dummy.id.1'.val=2.350")); assertThat(dummy1Metrics, containsString("'dummy.id.1'.c_test=6")); } diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/GenericMetricsHandlerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/GenericMetricsHandlerTest.java index 744c96d7744..301dbf56c3f 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/GenericMetricsHandlerTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/GenericMetricsHandlerTest.java @@ -8,6 +8,7 @@ import ai.vespa.metricsproxy.TestUtil; import ai.vespa.metricsproxy.core.ConsumersConfig; import ai.vespa.metricsproxy.core.MetricsConsumers; import ai.vespa.metricsproxy.core.MetricsManager; +import ai.vespa.metricsproxy.metric.HealthMetric; import ai.vespa.metricsproxy.metric.Metric; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensions; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensionsConfig; @@ -17,6 +18,7 @@ import ai.vespa.metricsproxy.metric.model.MetricsPacket; import ai.vespa.metricsproxy.metric.model.json.GenericJsonModel; import ai.vespa.metricsproxy.metric.model.json.GenericMetrics; import ai.vespa.metricsproxy.metric.model.json.GenericService; +import ai.vespa.metricsproxy.service.DownService; import ai.vespa.metricsproxy.service.DummyService; import ai.vespa.metricsproxy.service.VespaService; import ai.vespa.metricsproxy.service.VespaServices; @@ -34,9 +36,11 @@ import java.util.concurrent.Executors; import static ai.vespa.metricsproxy.core.VespaMetrics.INSTANCE_DIMENSION_ID; import static ai.vespa.metricsproxy.core.VespaMetrics.VESPA_CONSUMER_ID; import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId; +import static ai.vespa.metricsproxy.metric.model.StatusCode.DOWN; import static ai.vespa.metricsproxy.metric.model.json.JacksonUtil.createObjectMapper; import static ai.vespa.metricsproxy.service.DummyService.METRIC_1; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; @@ -47,8 +51,9 @@ import static org.junit.Assert.assertNotNull; public class GenericMetricsHandlerTest { private static final List<VespaService> testServices = ImmutableList.of( - new DummyService(0, "dummy/id/0"), - new DummyService(1, "dummy/id/0")); + new DummyService(0, ""), + new DummyService(1, ""), + new DownService(HealthMetric.getDown("No response"))); private static final String CPU_METRIC = "cpu"; @@ -93,7 +98,7 @@ public class GenericMetricsHandlerTest { String response = testDriver.sendRequest(URI).readAll(); var jsonModel = createObjectMapper().readValue(response, GenericJsonModel.class); - assertEquals(1, jsonModel.services.size()); + assertEquals(2, jsonModel.services.size()); GenericService dummyService = jsonModel.services.get(0); assertEquals(2, dummyService.metrics.size()); @@ -107,6 +112,22 @@ public class GenericMetricsHandlerTest { } @Test + public void response_contains_health_from_service_that_is_down() throws Exception { + String response = testDriver.sendRequest(URI).readAll(); + var jsonModel = createObjectMapper().readValue(response, GenericJsonModel.class); + + GenericService downService = jsonModel.services.get(1); + assertEquals(DOWN.status, downService.status.code); + assertEquals("No response", downService.status.description); + + // Service should output metric dimensions, even without metrics, because they contain important info about the service. + assertEquals(1, downService.metrics.size()); + assertEquals(0, downService.metrics.get(0).values.size()); + assertFalse(downService.metrics.get(0).dimensions.isEmpty()); + assertEquals(DownService.NAME, downService.metrics.get(0).dimensions.get(INSTANCE_DIMENSION_ID.id)); + } + + @Test public void all_timestamps_are_equal_and_non_zero() throws Exception { String response = testDriver.sendRequest(URI).readAll(); var jsonModel = createObjectMapper().readValue(response, GenericJsonModel.class); diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/MetricsTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/MetricsTest.java index b9e6377c27b..91eda8f744c 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/MetricsTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/MetricsTest.java @@ -4,6 +4,7 @@ package ai.vespa.metricsproxy.metric; +import ai.vespa.metricsproxy.metric.model.StatusCode; import ai.vespa.metricsproxy.service.DummyService; import ai.vespa.metricsproxy.service.VespaService; import org.junit.Test; @@ -46,7 +47,8 @@ public class MetricsTest { public void testBasicMetric() { Metrics m = new Metrics(); m.add(new Metric("count", 1, System.currentTimeMillis() / 1000)); - assertThat(m.get("count").intValue(), is(1)); + assertThat(m.getMetrics().size(), is(1)); + assertThat(m.getMetrics().get(0).getName(), is("count")); } @Test @@ -62,7 +64,7 @@ public class MetricsTest { m = HealthMetric.get("bad", "test message"); assertThat(m.isOk(), is(false)); - assertThat(m.getStatus(), is("bad")); + assertThat(m.getStatus(), is(StatusCode.UNKNOWN)); } @Test diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/StatusCodeTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/StatusCodeTest.java new file mode 100644 index 00000000000..8a66f98f42a --- /dev/null +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/StatusCodeTest.java @@ -0,0 +1,34 @@ +/* + * Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + */ + +package ai.vespa.metricsproxy.metric.model; + +import org.junit.Test; + +import static ai.vespa.metricsproxy.metric.model.StatusCode.DOWN; +import static ai.vespa.metricsproxy.metric.model.StatusCode.UNKNOWN; +import static ai.vespa.metricsproxy.metric.model.StatusCode.UP; +import static org.junit.Assert.assertEquals; + +/** + * @author gjoranv + */ +public class StatusCodeTest { + + @Test + public void strings_are_converted_to_correct_status_code() { + assertEquals(UP, StatusCode.fromString("up")); + assertEquals(UP, StatusCode.fromString("UP")); + assertEquals(UP, StatusCode.fromString("ok")); + assertEquals(UP, StatusCode.fromString("OK")); + + assertEquals(DOWN, StatusCode.fromString("down")); + assertEquals(DOWN, StatusCode.fromString("DOWN")); + + assertEquals(UNKNOWN, StatusCode.fromString("unknown")); + assertEquals(UNKNOWN, StatusCode.fromString("UNKNOWN")); + assertEquals(UNKNOWN, StatusCode.fromString("anything else is unknown")); + } + +} diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModelTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModelTest.java index dc3eb12ff2c..5d248db8b18 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModelTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModelTest.java @@ -5,6 +5,7 @@ package ai.vespa.metricsproxy.metric.model.json; import ai.vespa.metricsproxy.metric.model.MetricsPacket; +import ai.vespa.metricsproxy.metric.model.StatusCode; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.Test; @@ -19,6 +20,7 @@ import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId; import static ai.vespa.metricsproxy.metric.model.json.JacksonUtil.createObjectMapper; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; /** * @author gjoranv @@ -53,6 +55,7 @@ public class GenericJsonModelTest { var servicePacket = new MetricsPacket.Builder(toServiceId("my-service")) .timestamp(123456L) + .statusCode(0) .putMetric(toMetricId("service-metric"), 1234) .putDimension(toDimensionId("service-dim"), "service-dim-value") .build(); @@ -69,6 +72,9 @@ public class GenericJsonModelTest { assertEquals(1, jsonModel.services.size()); GenericService service = jsonModel.services.get(0); + assertEquals(StatusCode.UP.status, service.status.code); + assertEquals("", service.status.description); + assertEquals(1, service.metrics.size()); GenericMetrics serviceMetrics = service.metrics.get(0); assertEquals(1234L, serviceMetrics.values.get("service-metric").longValue()); diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ContainerServiceTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ContainerServiceTest.java index 0a84c03acec..725501aacaa 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ContainerServiceTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ContainerServiceTest.java @@ -13,6 +13,7 @@ import org.junit.Test; import static ai.vespa.metricsproxy.TestUtil.getFileContents; import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; +import static ai.vespa.metricsproxy.service.RemoteMetricsFetcher.METRICS_PATH; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat; @@ -34,7 +35,7 @@ public class ContainerServiceTest { csPort = 18637; // see factory/doc/port-ranges.txt try { String response = getFileContents("metrics-container-state-multi-chain.json"); - service = new MockHttpServer(csPort, response, HttpMetricFetcher.METRICS_PATH); + service = new MockHttpServer(csPort, response, METRICS_PATH); } catch (Exception e) { e.printStackTrace(); } diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/DownService.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/DownService.java new file mode 100644 index 00000000000..f78c496fcd1 --- /dev/null +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/DownService.java @@ -0,0 +1,32 @@ +/* + * Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + */ + +package ai.vespa.metricsproxy.service; + +import ai.vespa.metricsproxy.metric.HealthMetric; +import ai.vespa.metricsproxy.metric.Metrics; + +/** + * @author gjoranv + */ +public class DownService extends VespaService { + public static final String NAME = "down-service"; + + private final HealthMetric healthMetric; + + public DownService(HealthMetric healthMetric) { + super(NAME, ""); + this.healthMetric = healthMetric; + } + + @Override + public Metrics getMetrics() { + return new Metrics(); + } + + @Override + public HealthMetric getHealth() { + return healthMetric; + } +} diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsFetcherTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsFetcherTest.java index 278b2a3143a..ce33ce9f7bf 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsFetcherTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsFetcherTest.java @@ -22,10 +22,10 @@ public class MetricsFetcherTest { String jsonData = TestUtil.getFileContents("metrics-state.json"); RemoteMetricsFetcher fetcher = new RemoteMetricsFetcher(new DummyService(0, "dummy/id/0"), port); Metrics metrics = fetcher.createMetrics(jsonData, 0); - assertThat("Wrong number of metrics", metrics.size(), is(10)); - assertThat("Wrong value for metric", metrics.get("query_hits.count").intValue(), is(28)); - assertThat("Wrong value for metric ", metrics.get("queries.rate").doubleValue(), is(0.4667)); - assertThat("Wrong timestamp", metrics.getTimeStamp(), is(1334134700L)); + assertThat(metrics.size(), is(10)); + assertThat(metrics.getMetric("query_hits.count").getValue().intValue(), is(28)); + assertThat(metrics.getMetric("queries.rate").getValue().doubleValue(), is(0.4667)); + assertThat(metrics.getTimeStamp(), is(1334134700L)); } @Test diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServiceTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServiceTest.java index 702f741c41f..a0c6b5333cc 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServiceTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServiceTest.java @@ -10,6 +10,7 @@ import org.junit.Before; import org.junit.Test; import static ai.vespa.metricsproxy.TestUtil.getFileContents; +import static ai.vespa.metricsproxy.service.RemoteMetricsFetcher.METRICS_PATH; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat; @@ -30,7 +31,7 @@ public class VespaServiceTest { public void setupHTTPServer() { csPort = 18632; // see factory/doc/port-ranges.txt try { - service = new MockHttpServer(csPort, response, HttpMetricFetcher.METRICS_PATH); + service = new MockHttpServer(csPort, response, METRICS_PATH); } catch (Exception e) { e.printStackTrace(); } diff --git a/node-admin/src/main/application/services.xml b/node-admin/src/main/application/services.xml index db00c686c99..d5a4dce7c5a 100644 --- a/node-admin/src/main/application/services.xml +++ b/node-admin/src/main/application/services.xml @@ -5,7 +5,7 @@ <!-- Please update container test when changing this file --> <accesslog type="vespa" fileNamePattern="logs/vespa/node-admin/access.log.%Y%m%d%H%M%S" symlinkName="access.log" /> <component id="docker-api" class="com.yahoo.vespa.hosted.dockerapi.DockerImpl" bundle="docker-api"/> - <component id="metrics-wrapper" class="com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper" bundle="docker-api"/> + <component id="metrics" class="com.yahoo.vespa.hosted.dockerapi.metrics.Metrics" bundle="docker-api"/> <preprocess:include file="variant.xml" required="false"/> </container> diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java index 0e6000c651b..73c86fc8de1 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java @@ -7,7 +7,6 @@ import com.yahoo.config.provision.HostName; import com.yahoo.vespa.athenz.identity.ServiceIdentitySslSocketFactory; import com.yahoo.vespa.athenz.identity.SiaIdentityProvider; import com.yahoo.vespa.hosted.node.admin.component.ConfigServerInfo; -import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; import org.apache.http.HttpHeaders; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; @@ -38,6 +37,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.logging.Logger; /** * Retries request on config server a few times before giving up. Assumes that all requests should be sent with @@ -47,7 +47,7 @@ import java.util.Optional; * @author bjorncs */ public class ConfigServerApiImpl implements ConfigServerApi { - private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(ConfigServerApiImpl.class); + private static final Logger logger = Logger.getLogger(ConfigServerApiImpl.class.getName()); private final ObjectMapper mapper = new ObjectMapper(); @@ -106,7 +106,7 @@ public class ConfigServerApiImpl implements ConfigServerApi { try { return mapper.readValue(response.getEntity().getContent(), wantedReturnType); } catch (IOException e) { - throw new RuntimeException("Failed parse response from config server", e); + throw new UncheckedIOException("Failed parse response from config server", e); } } catch (HttpException e) { if (!e.isRetryable()) throw e; @@ -117,15 +117,15 @@ public class ConfigServerApiImpl implements ConfigServerApi { // Failure to communicate with a config server is not abnormal during upgrades if (e.getMessage().contains("(Connection refused)")) { - NODE_ADMIN_LOGGER.info("Connection refused to " + configServer + " (upgrading?), will try next"); + logger.info("Connection refused to " + configServer + " (upgrading?), will try next"); } else { - NODE_ADMIN_LOGGER.warning("Failed to communicate with " + configServer + ", will try next: " + e.getMessage()); + logger.warning("Failed to communicate with " + configServer + ", will try next: " + e.getMessage()); } } } - throw new RuntimeException("All requests against the config servers (" - + configServers + ") failed, last as follows:", lastException); + throw HttpException.handleException( + "All requests against the config servers (" + configServers + ") failed, last as follows:", lastException); } @Override diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java index 256fe38ec68..3825107bfa6 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java @@ -1,13 +1,19 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.configserver; +import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; +import org.apache.http.NoHttpResponseException; + import javax.ws.rs.core.Response; +import java.io.EOFException; +import java.net.SocketException; +import java.net.SocketTimeoutException; /** * @author hakonhall */ @SuppressWarnings("serial") -public class HttpException extends RuntimeException { +public class HttpException extends ConvergenceException { private final boolean isRetryable; @@ -21,7 +27,12 @@ public class HttpException extends RuntimeException { this.isRetryable = isRetryable; } - public boolean isRetryable() { + private HttpException(String message) { + super(message); + this.isRetryable = false; + } + + boolean isRetryable() { return isRetryable; } @@ -55,6 +66,22 @@ public class HttpException extends RuntimeException { throw new HttpException(status, message, true); } + /** + * Returns {@link HttpException} if the given Throwable is of a known and well understood error or + * a RuntimeException with the given exception as cause otherwise. + */ + public static RuntimeException handleException(String prefix, Throwable t) { + for (; t != null; t = t.getCause()) { + if (t instanceof SocketException || + t instanceof SocketTimeoutException || + t instanceof NoHttpResponseException || + t instanceof EOFException) + return new HttpException(prefix + t.getMessage()); + } + + return new RuntimeException(prefix, t); + } + public static class NotFoundException extends HttpException { public NotFoundException(String message) { super(Response.Status.NOT_FOUND, message, false); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepositoryException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepositoryException.java index 6094518c3fc..10e61d373d2 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepositoryException.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepositoryException.java @@ -1,7 +1,9 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; -public class NodeRepositoryException extends RuntimeException { +import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; + +public class NodeRepositoryException extends ConvergenceException { public NodeRepositoryException(String message) { super(message); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java index d402e75ff7b..52d6f16dd78 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java @@ -91,6 +91,13 @@ public class NodeSpec { Set<String> additionalIpAddresses, NodeReports reports, Optional<String> parentHostname) { + if (state == NodeState.active) { + Objects.requireNonNull(wantedVespaVersion, "Unknown vespa version for active node"); + Objects.requireNonNull(wantedDockerImage, "Unknown docker image for active node"); + Objects.requireNonNull(wantedRestartGeneration, "Unknown restartGeneration for active node"); + Objects.requireNonNull(currentRestartGeneration, "Unknown currentRestartGeneration for active node"); + } + this.hostname = Objects.requireNonNull(hostname); this.wantedDockerImage = Objects.requireNonNull(wantedDockerImage); this.currentDockerImage = Objects.requireNonNull(currentDockerImage); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index ca52eca13d2..fe19b81614d 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -12,10 +12,8 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.Ge import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.GetNodesResponse; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.NodeMessageResponse; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.NodeRepositoryNode; -import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; import java.time.Instant; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; @@ -23,6 +21,7 @@ import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.function.Function; +import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -30,7 +29,7 @@ import java.util.stream.Stream; * @author stiankri, dybis */ public class RealNodeRepository implements NodeRepository { - private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(RealNodeRepository.class); + private static final Logger logger = Logger.getLogger(RealNodeRepository.class.getName()); private final ConfigServerApi configServerApi; @@ -46,7 +45,7 @@ public class RealNodeRepository implements NodeRepository { NodeMessageResponse response = configServerApi.post("/nodes/v2/node", nodesToPost, NodeMessageResponse.class); if (Strings.isNullOrEmpty(response.errorCode)) return; - throw new NodeRepositoryException("Failed to add nodes to node-repo: " + response.message + " " + response.errorCode); + throw new NodeRepositoryException("Failed to add nodes: " + response.message + " " + response.errorCode); } @Override @@ -80,43 +79,37 @@ public class RealNodeRepository implements NodeRepository { */ @Override public Map<String, Acl> getAcls(String hostName) { - try { - String path = String.format("/nodes/v2/acl/%s?children=true", hostName); - GetAclResponse response = configServerApi.get(path, GetAclResponse.class); - - // Group ports by container hostname that trusts them - Map<String, Set<Integer>> trustedPorts = response.trustedPorts.stream() - .collect(Collectors.groupingBy( - GetAclResponse.Port::getTrustedBy, - Collectors.mapping(port -> port.port, Collectors.toSet()))); - - // Group node ip-addresses by container hostname that trusts them - Map<String, Set<Acl.Node>> trustedNodes = response.trustedNodes.stream() - .collect(Collectors.groupingBy( - GetAclResponse.Node::getTrustedBy, - Collectors.mapping( - node -> new Acl.Node(node.hostname, node.ipAddress), - Collectors.toSet()))); - - // Group trusted networks by container hostname that trusts them - Map<String, Set<String>> trustedNetworks = response.trustedNetworks.stream() - .collect(Collectors.groupingBy(GetAclResponse.Network::getTrustedBy, - Collectors.mapping(node -> node.network, Collectors.toSet()))); - - - // For each hostname create an ACL - return Stream.of(trustedNodes.keySet(), trustedPorts.keySet(), trustedNetworks.keySet()) - .flatMap(Set::stream) - .distinct() - .collect(Collectors.toMap( - Function.identity(), - hostname -> new Acl(trustedPorts.get(hostname), trustedNodes.get(hostname), - trustedNetworks.get(hostname)))); - } catch (HttpException.NotFoundException e) { - NODE_ADMIN_LOGGER.warning("Failed to fetch ACLs for " + hostName + " No ACL will be applied"); - } - - return Collections.emptyMap(); + String path = String.format("/nodes/v2/acl/%s?children=true", hostName); + GetAclResponse response = configServerApi.get(path, GetAclResponse.class); + + // Group ports by container hostname that trusts them + Map<String, Set<Integer>> trustedPorts = response.trustedPorts.stream() + .collect(Collectors.groupingBy( + GetAclResponse.Port::getTrustedBy, + Collectors.mapping(port -> port.port, Collectors.toSet()))); + + // Group node ip-addresses by container hostname that trusts them + Map<String, Set<Acl.Node>> trustedNodes = response.trustedNodes.stream() + .collect(Collectors.groupingBy( + GetAclResponse.Node::getTrustedBy, + Collectors.mapping( + node -> new Acl.Node(node.hostname, node.ipAddress), + Collectors.toSet()))); + + // Group trusted networks by container hostname that trusts them + Map<String, Set<String>> trustedNetworks = response.trustedNetworks.stream() + .collect(Collectors.groupingBy(GetAclResponse.Network::getTrustedBy, + Collectors.mapping(node -> node.network, Collectors.toSet()))); + + + // For each hostname create an ACL + return Stream.of(trustedNodes.keySet(), trustedPorts.keySet(), trustedNetworks.keySet()) + .flatMap(Set::stream) + .distinct() + .collect(Collectors.toMap( + Function.identity(), + hostname -> new Acl(trustedPorts.get(hostname), trustedNodes.get(hostname), + trustedNetworks.get(hostname)))); } @Override @@ -127,7 +120,7 @@ public class RealNodeRepository implements NodeRepository { NodeMessageResponse.class); if (Strings.isNullOrEmpty(response.errorCode)) return; - throw new NodeRepositoryException("Unexpected message " + response.message + " " + response.errorCode); + throw new NodeRepositoryException("Failed to update node attributes: " + response.message + " " + response.errorCode); } @Override @@ -137,10 +130,10 @@ public class RealNodeRepository implements NodeRepository { "/nodes/v2/state/" + state + "/" + hostName, Optional.empty(), /* body */ NodeMessageResponse.class); - NODE_ADMIN_LOGGER.info(response.message); + logger.info(response.message); if (Strings.isNullOrEmpty(response.errorCode)) return; - throw new NodeRepositoryException("Unexpected message " + response.message + " " + response.errorCode); + throw new NodeRepositoryException("Failed to set node state: " + response.message + " " + response.errorCode); } private static NodeSpec createNodeSpec(NodeRepositoryNode node) { @@ -149,30 +142,13 @@ public class RealNodeRepository implements NodeRepository { Objects.requireNonNull(node.state, "Unknown node state"); NodeState nodeState = NodeState.valueOf(node.state); - if (nodeState == NodeState.active) { - Objects.requireNonNull(node.wantedVespaVersion, "Unknown vespa version for active node"); - Objects.requireNonNull(node.wantedDockerImage, "Unknown docker image for active node"); - Objects.requireNonNull(node.restartGeneration, "Unknown restartGeneration for active node"); - Objects.requireNonNull(node.currentRestartGeneration, "Unknown currentRestartGeneration for active node"); - } - - String hostName = Objects.requireNonNull(node.hostname, "hostname is null"); - - NodeOwner owner = null; - if (node.owner != null) { - owner = new NodeOwner(node.owner.tenant, node.owner.application, node.owner.instance); - } - - NodeMembership membership = null; - if (node.membership != null) { - membership = new NodeMembership(node.membership.clusterType, node.membership.clusterId, - node.membership.group, node.membership.index, node.membership.retired); - } - NodeReports reports = NodeReports.fromMap(node.reports == null ? Collections.emptyMap() : node.reports); + Optional<NodeMembership> membership = Optional.ofNullable(node.membership) + .map(m -> new NodeMembership(m.clusterType, m.clusterId, m.group, m.index, m.retired)); + NodeReports reports = NodeReports.fromMap(Optional.ofNullable(node.reports).orElseGet(Map::of)); return new NodeSpec( - hostName, + node.hostname, Optional.ofNullable(node.wantedDockerImage).map(DockerImage::fromString), Optional.ofNullable(node.currentDockerImage).map(DockerImage::fromString), nodeState, @@ -185,8 +161,8 @@ public class RealNodeRepository implements NodeRepository { Optional.ofNullable(node.currentOsVersion).map(Version::fromString), Optional.ofNullable(node.allowedToBeDown), Optional.ofNullable(node.wantToDeprovision), - Optional.ofNullable(owner), - Optional.ofNullable(membership), + Optional.ofNullable(node.owner).map(o -> new NodeOwner(o.tenant, o.application, o.instance)), + membership, Optional.ofNullable(node.restartGeneration), Optional.ofNullable(node.currentRestartGeneration), node.rebootGeneration, diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorException.java index fe19da0c41c..8575bf7f655 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorException.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorException.java @@ -1,8 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.configserver.orchestrator; +import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; + @SuppressWarnings("serial") -public class OrchestratorException extends RuntimeException { +public class OrchestratorException extends ConvergenceException { public OrchestratorException(String message) { super(message); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImpl.java index 64a67aa612a..6124e1bdc0e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImpl.java @@ -40,8 +40,7 @@ public class OrchestratorImpl implements Orchestrator { } catch (HttpException.NotFoundException n) { throw new OrchestratorNotFoundException("Failed to suspend " + hostName + ", host not found"); } catch (HttpException e) { - throw new OrchestratorException("Failed to suspend " + hostName + ": " + - e.toString()); + throw new OrchestratorException("Failed to suspend " + hostName + ": " + e.toString()); } catch (RuntimeException e) { throw new RuntimeException("Got error on suspend", e); } @@ -60,9 +59,8 @@ public class OrchestratorImpl implements Orchestrator { parentHostName, params); batchOperationResult = configServerApi.put(url, Optional.empty(), BatchOperationResult.class); } catch (HttpException e) { - throw new OrchestratorException("Failed to batch suspend for " + - parentHostName + ": " + e.toString()); - } catch (Exception e) { + throw new OrchestratorException("Failed to batch suspend for " + parentHostName + ": " + e.toString()); + } catch (RuntimeException e) { throw new RuntimeException("Got error on batch suspend for " + parentHostName + ", with nodes " + hostNames, e); } @@ -80,9 +78,8 @@ public class OrchestratorImpl implements Orchestrator { } catch (HttpException.NotFoundException n) { throw new OrchestratorNotFoundException("Failed to resume " + hostName + ", host not found"); } catch (HttpException e) { - throw new OrchestratorException("Failed to suspend " + hostName + ": " + - e.toString()); - } catch (Exception e) { + throw new OrchestratorException("Failed to suspend " + hostName + ": " + e.toString()); + } catch (RuntimeException e) { throw new RuntimeException("Got error on resume", e); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java index efeb3039379..2fe8d4b4792 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.configserver.state; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; +import com.yahoo.vespa.hosted.node.admin.configserver.HttpException; import com.yahoo.vespa.hosted.node.admin.configserver.state.bindings.HealthResponse; /** @@ -16,26 +17,12 @@ public class StateImpl implements State { @Override public HealthCode getHealth() { - HealthResponse response; try { - response = configServerApi.get("/state/v1/health", HealthResponse.class); - } catch (RuntimeException e) { - if (causedByConnectionRefused(e)) { - return HealthCode.DOWN; - } - - throw e; + HealthResponse response = configServerApi.get("/state/v1/health", HealthResponse.class); + return HealthCode.fromString(response.status.code); + } catch (HttpException e) { + return HealthCode.DOWN; } - return HealthCode.fromString(response.status.code); } - private static boolean causedByConnectionRefused(Throwable throwable) { - for (Throwable cause = throwable; cause != null; cause = cause.getCause()) { - if (cause instanceof java.net.ConnectException) { - return true; - } - } - - return false; - } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java index 00ec985ba0c..7de2aae77c8 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java @@ -16,11 +16,8 @@ public interface NodeAdmin { /** Start/stop NodeAgents and schedule next NodeAgent ticks with the given NodeAgentContexts */ void refreshContainersToRun(Set<NodeAgentContext> nodeAgentContexts); - /** Gather node agent and its docker container metrics and forward them to the {@code MetricReceiverWrapper} */ - void updateNodeAgentMetrics(); - - /** Gather node admin metrics and forward them to the {@code MetricReceiverWrapper} */ - void updateNodeAdminMetrics(); + /** Update node admin metrics */ + void updateMetrics(); /** * Attempts to freeze/unfreeze all NodeAgents and itself. To freeze a NodeAgent means that diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java index 0d520241ac8..cb10eac9e6c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java @@ -1,10 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.nodeadmin; -import com.yahoo.vespa.hosted.dockerapi.metrics.CounterWrapper; +import com.yahoo.vespa.hosted.dockerapi.metrics.Counter; import com.yahoo.vespa.hosted.dockerapi.metrics.Dimensions; -import com.yahoo.vespa.hosted.dockerapi.metrics.GaugeWrapper; -import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; +import com.yahoo.vespa.hosted.dockerapi.metrics.Gauge; +import com.yahoo.vespa.hosted.dockerapi.metrics.Metrics; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgent; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextManager; @@ -39,28 +39,26 @@ public class NodeAdminImpl implements NodeAdmin { private boolean previousWantFrozen; private boolean isFrozen; private Instant startOfFreezeConvergence; - private final Map<String, NodeAgentWithScheduler> nodeAgentWithSchedulerByHostname = new ConcurrentHashMap<>(); - private final GaugeWrapper numberOfContainersInLoadImageState; - private final GaugeWrapper jvmHeapUsed; - private final GaugeWrapper jvmHeapFree; - private final GaugeWrapper jvmHeapTotal; - private final CounterWrapper numberOfUnhandledExceptionsInNodeAgent; + private final Gauge jvmHeapUsed; + private final Gauge jvmHeapFree; + private final Gauge jvmHeapTotal; + private final Counter numberOfUnhandledExceptions; - public NodeAdminImpl(NodeAgentFactory nodeAgentFactory, MetricReceiverWrapper metricReceiver, Clock clock) { + public NodeAdminImpl(NodeAgentFactory nodeAgentFactory, Metrics metrics, Clock clock) { this((NodeAgentWithSchedulerFactory) nodeAgentContext -> create(clock, nodeAgentFactory, nodeAgentContext), - metricReceiver, clock, NODE_AGENT_FREEZE_TIMEOUT, NODE_AGENT_SPREAD); + metrics, clock, NODE_AGENT_FREEZE_TIMEOUT, NODE_AGENT_SPREAD); } - public NodeAdminImpl(NodeAgentFactory nodeAgentFactory, MetricReceiverWrapper metricReceiver, + public NodeAdminImpl(NodeAgentFactory nodeAgentFactory, Metrics metrics, Clock clock, Duration freezeTimeout, Duration spread) { this((NodeAgentWithSchedulerFactory) nodeAgentContext -> create(clock, nodeAgentFactory, nodeAgentContext), - metricReceiver, clock, freezeTimeout, spread); + metrics, clock, freezeTimeout, spread); } NodeAdminImpl(NodeAgentWithSchedulerFactory nodeAgentWithSchedulerFactory, - MetricReceiverWrapper metricReceiver, Clock clock, Duration freezeTimeout, Duration spread) { + Metrics metrics, Clock clock, Duration freezeTimeout, Duration spread) { this.nodeAgentWithSchedulerFactory = nodeAgentWithSchedulerFactory; this.clock = clock; @@ -70,13 +68,12 @@ public class NodeAdminImpl implements NodeAdmin { this.isFrozen = true; this.startOfFreezeConvergence = clock.instant(); - Dimensions dimensions = new Dimensions.Builder().add("role", "docker").build(); - this.numberOfContainersInLoadImageState = metricReceiver.declareGauge(MetricReceiverWrapper.APPLICATION_DOCKER, dimensions, "nodes.image.loading"); - this.numberOfUnhandledExceptionsInNodeAgent = metricReceiver.declareCounter(MetricReceiverWrapper.APPLICATION_DOCKER, dimensions, "nodes.unhandled_exceptions"); + this.numberOfUnhandledExceptions = metrics.declareCounter("unhandled_exceptions", + new Dimensions(Map.of("src", "node-agents"))); - this.jvmHeapUsed = metricReceiver.declareGauge(MetricReceiverWrapper.APPLICATION_HOST, new Dimensions.Builder().build(), "mem.heap.used"); - this.jvmHeapFree = metricReceiver.declareGauge(MetricReceiverWrapper.APPLICATION_HOST, new Dimensions.Builder().build(), "mem.heap.free"); - this.jvmHeapTotal = metricReceiver.declareGauge(MetricReceiverWrapper.APPLICATION_HOST, new Dimensions.Builder().build(), "mem.heap.total"); + this.jvmHeapUsed = metrics.declareGauge("mem.heap.used"); + this.jvmHeapFree = metrics.declareGauge("mem.heap.free"); + this.jvmHeapTotal = metrics.declareGauge("mem.heap.total"); } @Override @@ -105,22 +102,12 @@ public class NodeAdminImpl implements NodeAdmin { } @Override - public void updateNodeAgentMetrics() { - int numberContainersWaitingImage = 0; - int numberOfNewUnhandledExceptions = 0; - + public void updateMetrics() { for (NodeAgentWithScheduler nodeAgentWithScheduler : nodeAgentWithSchedulerByHostname.values()) { - if (nodeAgentWithScheduler.isDownloadingImage()) numberContainersWaitingImage++; - numberOfNewUnhandledExceptions += nodeAgentWithScheduler.getAndResetNumberOfUnhandledExceptions(); + numberOfUnhandledExceptions.add(nodeAgentWithScheduler.getAndResetNumberOfUnhandledExceptions()); nodeAgentWithScheduler.updateContainerNodeMetrics(); } - numberOfContainersInLoadImageState.sample(numberContainersWaitingImage); - numberOfUnhandledExceptionsInNodeAgent.add(numberOfNewUnhandledExceptions); - } - - @Override - public void updateNodeAdminMetrics() { Runtime runtime = Runtime.getRuntime(); long freeMemory = runtime.freeMemory(); long totalMemory = runtime.totalMemory(); @@ -208,7 +195,6 @@ public class NodeAdminImpl implements NodeAdmin { @Override public void stopForHostSuspension() { nodeAgent.stopForHostSuspension(); } @Override public void stopForRemoval() { nodeAgent.stopForRemoval(); } @Override public void updateContainerNodeMetrics() { nodeAgent.updateContainerNodeMetrics(); } - @Override public boolean isDownloadingImage() { return nodeAgent.isDownloadingImage(); } @Override public int getAndResetNumberOfUnhandledExceptions() { return nodeAgent.getAndResetNumberOfUnhandledExceptions(); } @Override public void scheduleTickWith(NodeAgentContext context, Instant at) { nodeAgentScheduler.scheduleTickWith(context, at); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java index 2cd15a3ebe4..f3302bd2359 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java @@ -80,8 +80,7 @@ public class NodeAdminStateUpdater { metricsScheduler.scheduleAtFixedRate(() -> { try { if (suspendedStates.contains(currentState)) return; - nodeAdmin.updateNodeAgentMetrics(); - nodeAdmin.updateNodeAdminMetrics(); + nodeAdmin.updateMetrics(); } catch (Throwable e) { log.log(Level.WARNING, "Metric fetcher scheduler failed", e); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgent.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgent.java index d62cb8e45d9..de5ee1b69a4 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgent.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgent.java @@ -33,11 +33,6 @@ public interface NodeAgent { void updateContainerNodeMetrics(); /** - * Returns true if NodeAgent is waiting for an image download to finish - */ - boolean isDownloadingImage(); - - /** * Returns and resets number of unhandled exceptions */ int getAndResetNumberOfUnhandledExceptions(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 8c38b1bbd84..2716cc8cc59 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -18,7 +18,7 @@ import com.yahoo.vespa.hosted.dockerapi.exception.DockerException; import com.yahoo.vespa.hosted.dockerapi.exception.DockerExecTimeoutException; import com.yahoo.vespa.hosted.dockerapi.metrics.DimensionMetrics; import com.yahoo.vespa.hosted.dockerapi.metrics.Dimensions; -import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; +import com.yahoo.vespa.hosted.dockerapi.metrics.Metrics; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeOwner; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; @@ -40,7 +40,6 @@ import java.util.Objects; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; -import java.util.logging.Level; import java.util.logging.Logger; import static com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentImpl.ContainerState.ABSENT; @@ -73,8 +72,6 @@ public class NodeAgentImpl implements NodeAgent { private final DoubleFlag containerCpuCap; private int numberOfUnhandledException = 0; - private DockerImage imageBeingDownloaded = null; - private long currentRebootGeneration = 0; private Optional<Long> currentRestartGeneration = Optional.empty(); @@ -378,20 +375,16 @@ public class NodeAgentImpl implements NodeAgent { || (zone.getSystemName().isCd() && zone.getEnvironment() != Environment.prod); } - private void scheduleDownLoadIfNeeded(NodeSpec node, Optional<Container> container) { - if (node.getWantedDockerImage().equals(container.map(c -> c.image))) return; + private boolean downloadImageIfNeeded(NodeSpec node, Optional<Container> container) { + if (node.getWantedDockerImage().equals(container.map(c -> c.image))) return false; - if (dockerOperations.pullImageAsyncIfNeeded(node.getWantedDockerImage().get())) { - imageBeingDownloaded = node.getWantedDockerImage().get(); - } else if (imageBeingDownloaded != null) { // Image was downloading, but now it's ready - imageBeingDownloaded = null; - } + return node.getWantedDockerImage().map(dockerOperations::pullImageAsyncIfNeeded).orElse(false); } public void converge(NodeAgentContext context) { try { doConverge(context); - } catch (OrchestratorException | ConvergenceException e) { + } catch (ConvergenceException e) { context.log(logger, e.getMessage()); } catch (ContainerNotFoundException e) { containerState = ABSENT; @@ -436,6 +429,7 @@ public class NodeAgentImpl implements NodeAgent { case reserved: case parked: case failed: + case inactive: removeContainerIfNeededUpdateContainerState(context, container); updateNodeRepoWithCurrentAttributes(context); break; @@ -447,9 +441,8 @@ public class NodeAgentImpl implements NodeAgent { .filter(diskUtil -> diskUtil >= 0.8) .ifPresent(diskUtil -> storageMaintainer.removeOldFilesFromNode(context)); - scheduleDownLoadIfNeeded(node, container); - if (isDownloadingImage()) { - context.log(logger, "Waiting for image to download " + imageBeingDownloaded.asString()); + if (downloadImageIfNeeded(node, container)) { + context.log(logger, "Waiting for image to download " + context.node().getWantedDockerImage().get().asString()); return; } container = removeContainerIfNeededUpdateContainerState(context, container); @@ -481,10 +474,6 @@ public class NodeAgentImpl implements NodeAgent { context.log(logger, "Call resume against Orchestrator"); orchestrator.resume(context.hostname().value()); break; - case inactive: - removeContainerIfNeededUpdateContainerState(context, container); - updateNodeRepoWithCurrentAttributes(context); - break; case provisioned: nodeRepository.setNodeState(context.hostname().value(), NodeState.dirty); break; @@ -497,7 +486,7 @@ public class NodeAgentImpl implements NodeAgent { nodeRepository.setNodeState(context.hostname().value(), NodeState.ready); break; default: - throw new RuntimeException("UNKNOWN STATE " + node.getState().name()); + throw new ConvergenceException("UNKNOWN STATE " + node.getState().name()); } } @@ -543,7 +532,7 @@ public class NodeAgentImpl implements NodeAgent { Dimensions dimensions = dimensionsBuilder.build(); ContainerStats stats = containerStats.get(); - final String APP = MetricReceiverWrapper.APPLICATION_NODE; + final String APP = Metrics.APPLICATION_NODE; final int totalNumCpuCores = stats.getCpuStats().getOnlineCpus(); final long cpuContainerKernelTime = stats.getCpuStats().getUsageInKernelMode(); final long cpuContainerTotalTime = stats.getCpuStats().getTotalUsage(); @@ -604,27 +593,15 @@ public class NodeAgentImpl implements NodeAgent { for (DimensionMetrics dimensionMetrics : metrics) { params.append(dimensionMetrics.toSecretAgentReport()); } - } catch (JsonProcessingException e) { - // TODO: wrap everything into one try-block (to avoid 'return') when old metrics proxy is discontinued - context.log(logger, LogLevel.WARNING, "Failed to wrap metrics in secret agent report", e); - return; - } - String wrappedMetrics = "s:" + params.toString(); - - // Push metrics to the metrics proxy in each container - runPushMetricsCommand(context, wrappedMetrics, true); - runPushMetricsCommand(context, wrappedMetrics, false); - } + String wrappedMetrics = "s:" + params.toString(); - // TODO: Clean up and inline method when old metrics proxy has been discontinued. - private void runPushMetricsCommand(NodeAgentContext context, String wrappedMetrics, boolean newMetricsProxy) { - int port = newMetricsProxy ? 19095 : 19091; - String[] command = {"vespa-rpc-invoke", "-t", "2", "tcp/localhost:" + port, "setExtraMetrics", wrappedMetrics}; - try { + // Push metrics to the metrics proxy in each container. + // TODO Remove port selection logic when all hosted apps have upgraded to Vespa 7. + int port = context.node().getVespaVersion().map(version -> version.getMajor() == 6).orElse(false) ? 19091 : 19095; + String[] command = {"vespa-rpc-invoke", "-t", "2", "tcp/localhost:" + port, "setExtraMetrics", wrappedMetrics}; dockerOperations.executeCommandInContainerAsRoot(context, 5L, command); - } catch (DockerExecTimeoutException e) { - Level level = newMetricsProxy ? LogLevel.DEBUG : LogLevel.WARNING; - context.log(logger, level, "Failed to push metrics to container", e); + } catch (JsonProcessingException | DockerExecTimeoutException e) { + context.log(logger, LogLevel.WARNING, "Failed to push metrics to container", e); } } @@ -637,11 +614,6 @@ public class NodeAgentImpl implements NodeAgent { } @Override - public boolean isDownloadingImage() { - return imageBeingDownloaded != null; - } - - @Override public int getAndResetNumberOfUnhandledExceptions() { int temp = numberOfUnhandledException; numberOfUnhandledException = 0; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/StoredBoolean.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/StoredBoolean.java new file mode 100644 index 00000000000..5400c19d63e --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/StoredBoolean.java @@ -0,0 +1,53 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.file; + +import com.yahoo.vespa.hosted.node.admin.component.TaskContext; + +import java.nio.file.Path; +import java.util.logging.Logger; + +/** + * Class wrapping a boolean stored on disk. + * + * <p>The implementation is compatible with {@link StoredInteger} when absence or 0 means false. + * + * @author hakonhall + */ +public class StoredBoolean { + private static Logger logger = Logger.getLogger(StoredBoolean.class.getName()); + + private final UnixPath path; + + /** The parent directory must exist. Value is false by default. */ + public StoredBoolean(Path path) { + this.path = new UnixPath(path); + } + + public boolean value() { + return path.readUtf8FileIfExists().map(String::trim).map(s -> !"0".equals(s)).orElse(false); + } + + /** Sets value to true. */ + public void set(TaskContext context) { + if (!value()) { + context.log(logger, "Writes " + path); + path.writeUtf8File("1"); + } + } + + public void set(TaskContext context, boolean value) { + if (value) { + set(context); + } else { + clear(context); + } + } + + /** Sets value to false. */ + public void clear(TaskContext context) { + if (value()) { + context.log(logger, "Deletes " + path); + path.deleteIfExists(); + } + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java index 1d0299ae272..376fda1d2dc 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java @@ -50,10 +50,24 @@ public class UnixPath { return path; } + public boolean exists() { + return Files.exists(path); + } + public String readUtf8File() { return new String(readBytes(), StandardCharsets.UTF_8); } + public Optional<String> readUtf8FileIfExists() { + try { + return Optional.of(new String(Files.readAllBytes(path), StandardCharsets.UTF_8)); + } catch (NoSuchFileException ignored) { + return Optional.empty(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + public byte[] readBytes() { return uncheck(() -> Files.readAllBytes(path)); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/PrefixLogger.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/PrefixLogger.java deleted file mode 100644 index f4d85a19f6d..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/PrefixLogger.java +++ /dev/null @@ -1,66 +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.vespa.hosted.node.admin.util; - -import com.yahoo.log.LogLevel; -import com.yahoo.vespa.hosted.dockerapi.ContainerName; - -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * @author freva - */ -public class PrefixLogger { - private final String prefix; - private final Logger logger; - - private <T> PrefixLogger(Class<T> clazz, String prefix) { - this.logger = Logger.getLogger(clazz.getName()); - this.prefix = prefix + ": "; - } - - public static <T> PrefixLogger getNodeAdminLogger(Class<T> clazz) { - return new PrefixLogger(clazz, "NodeAdmin"); - } - - public static <T> PrefixLogger getNodeAgentLogger(Class<T> clazz, ContainerName containerName) { - return new PrefixLogger(clazz, "NodeAgent-" + containerName.asString()); - } - - private void log(Level level, String message, Throwable thrown) { - logger.log(level, prefix + message, thrown); - } - - private void log(Level level, String message) { - logger.log(level, prefix + message); - } - - - public void debug(String message) { - log(LogLevel.DEBUG, message); - } - - public void info(String message) { - log(LogLevel.INFO, message); - } - - public void info(String message, Throwable thrown) { - log(LogLevel.INFO, message, thrown); - } - - public void error(String message) { - log(LogLevel.ERROR, message); - } - - public void error(String message, Throwable thrown) { - log(LogLevel.ERROR, message, thrown); - } - - public void warning(String message) { - log(LogLevel.WARNING, message); - } - - public void warning(String message, Throwable thrown) { - log(LogLevel.WARNING, message, thrown); - } -} diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java index 2604aa05367..14755ebf9cc 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.configserver.state; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; +import com.yahoo.vespa.hosted.node.admin.configserver.HttpException; import com.yahoo.vespa.hosted.node.admin.configserver.state.bindings.HealthResponse; import org.junit.Test; @@ -28,7 +29,7 @@ public class StateImplTest { @Test public void connectException() { - RuntimeException exception = new RuntimeException(new ConnectException("connection refused")); + RuntimeException exception = HttpException.handleException("Error: ", new ConnectException("connection refused")); when(api.get(any(), any())).thenThrow(exception); HealthCode code = state.getHealth(); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java index f9524d32c81..7f0f3fd37f6 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java @@ -4,11 +4,10 @@ package com.yahoo.vespa.hosted.node.admin.integrationTests; import com.yahoo.collections.Pair; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeType; -import com.yahoo.metrics.simple.MetricReceiver; import com.yahoo.system.ProcessExecuter; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.dockerapi.Docker; -import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; +import com.yahoo.vespa.hosted.dockerapi.metrics.Metrics; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeState; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; @@ -92,11 +91,11 @@ public class DockerTester implements AutoCloseable { FileSystem fileSystem = TestFileSystem.create(); DockerOperations dockerOperations = new DockerOperationsImpl(docker, processExecuter, ipAddresses); - MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation); + Metrics metrics = new Metrics(); NodeAgentFactory nodeAgentFactory = contextSupplier -> new NodeAgentImpl( contextSupplier, nodeRepository, orchestrator, dockerOperations, storageMaintainer, flagSource, Optional.empty(), Optional.empty(), Optional.empty()); - nodeAdmin = new NodeAdminImpl(nodeAgentFactory, mr, Clock.systemUTC(), Duration.ofMillis(10), Duration.ZERO); + nodeAdmin = new NodeAdminImpl(nodeAgentFactory, metrics, Clock.systemUTC(), Duration.ofMillis(10), Duration.ZERO); NodeAgentContextFactory nodeAgentContextFactory = (nodeSpec, acl) -> new NodeAgentContextImpl.Builder(nodeSpec).acl(acl).fileSystem(fileSystem).build(); nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeAgentContextFactory, nodeRepository, orchestrator, diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java index ce2e9f6d7a2..6e645e6c70f 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java @@ -2,13 +2,11 @@ package com.yahoo.vespa.hosted.node.admin.nodeadmin; import com.yahoo.config.provision.NodeType; -import com.yahoo.metrics.simple.MetricReceiver; import com.yahoo.test.ManualClock; -import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; +import com.yahoo.vespa.hosted.dockerapi.metrics.Metrics; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeState; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextFactory; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; import org.junit.Test; import org.mockito.InOrder; @@ -40,11 +38,10 @@ import static org.mockito.Mockito.when; public class NodeAdminImplTest { private final NodeAgentWithSchedulerFactory nodeAgentWithSchedulerFactory = mock(NodeAgentWithSchedulerFactory.class); - private final NodeAgentContextFactory nodeAgentContextFactory = mock(NodeAgentContextFactory.class); private final ManualClock clock = new ManualClock(); private final NodeAdminImpl nodeAdmin = new NodeAdminImpl(nodeAgentWithSchedulerFactory, - new MetricReceiverWrapper(MetricReceiver.nullImplementation), clock, Duration.ZERO, Duration.ZERO); + new Metrics(), clock, Duration.ZERO, Duration.ZERO); @Test public void nodeAgentsAreProperlyLifeCycleManaged() { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index c43750684f6..f754d1798ec 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -5,7 +5,6 @@ import com.yahoo.component.Version; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.NodeType; import com.yahoo.io.IOUtils; -import com.yahoo.metrics.simple.MetricReceiver; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.dockerapi.Container; @@ -13,7 +12,7 @@ import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.ContainerResources; import com.yahoo.vespa.hosted.dockerapi.ContainerStats; import com.yahoo.vespa.hosted.dockerapi.exception.DockerException; -import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; +import com.yahoo.vespa.hosted.dockerapi.metrics.Metrics; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeMembership; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeOwner; @@ -34,10 +33,8 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Collections; -import java.util.Map; +import java.util.List; import java.util.Optional; -import java.util.Set; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; @@ -79,7 +76,7 @@ public class NodeAgentImplTest { private final NodeRepository nodeRepository = mock(NodeRepository.class); private final Orchestrator orchestrator = mock(Orchestrator.class); private final StorageMaintainer storageMaintainer = mock(StorageMaintainer.class); - private final MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation); + private final Metrics metrics = new Metrics(); private final AclMaintainer aclMaintainer = mock(AclMaintainer.class); private final HealthChecker healthChecker = mock(HealthChecker.class); private final CredentialsMaintainer credentialsMaintainer = mock(CredentialsMaintainer.class); @@ -688,9 +685,6 @@ public class NodeAgentImplTest { .replaceAll("\"timestamp\":\\d+", "\"timestamp\":0") .replaceAll("([0-9]+\\.[0-9]{1,3})([0-9]*)", "$1"); // Only keep the first 3 decimals - // TODO: Remove when old metrics proxy is discontinued. - calledCommand[3] = calledCommand[3].replaceFirst("19091", "19095"); - assertEquals(context, calledContainerName); assertEquals(5L, calledTimeout); assertArrayEquals(expectedCommand, calledCommand); @@ -713,8 +707,7 @@ public class NodeAgentImplTest { nodeAgent.updateContainerNodeMetrics(); - Set<Map<String, Object>> actualMetrics = metricReceiver.getDefaultMetricsRaw(); - assertEquals(Collections.emptySet(), actualMetrics); + assertEquals(List.of(), metrics.getDefaultMetrics()); } @Test diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/StoredBooleanTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/StoredBooleanTest.java new file mode 100644 index 00000000000..bc5487f740c --- /dev/null +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/StoredBooleanTest.java @@ -0,0 +1,52 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.file; + +import com.yahoo.vespa.hosted.node.admin.component.TaskContext; +import com.yahoo.vespa.test.file.TestFileSystem; +import org.junit.Test; + +import java.io.IOException; +import java.nio.file.FileSystem; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +/** + * @author hakonhall + */ +public class StoredBooleanTest { + private final TaskContext context = mock(TaskContext.class); + private final FileSystem fileSystem = TestFileSystem.create(); + private final Path path = fileSystem.getPath("/foo"); + private final StoredBoolean storedBoolean = new StoredBoolean(path); + + @Test + public void storedBoolean() { + assertFalse(storedBoolean.value()); + storedBoolean.set(context); + assertTrue(storedBoolean.value()); + storedBoolean.clear(context); + assertFalse(storedBoolean.value()); + } + + @Test + public void testCompatibility() throws IOException { + StoredInteger storedInteger = new StoredInteger(path); + assertFalse(storedBoolean.value()); + + storedInteger.write(context, 1); + assertTrue(storedBoolean.value()); + + storedInteger.write(context, 2); + assertTrue(storedBoolean.value()); + + storedInteger.write(context, 0); + assertFalse(storedBoolean.value()); + + Files.delete(path); + assertFalse(storedBoolean.value()); + } +}
\ No newline at end of file diff --git a/node-admin/src/test/resources/expected.container.system.metrics.txt b/node-admin/src/test/resources/expected.container.system.metrics.txt index c44d72b395e..ec750798c98 100644 --- a/node-admin/src/test/resources/expected.container.system.metrics.txt +++ b/node-admin/src/test/resources/expected.container.system.metrics.txt @@ -1,83 +1,80 @@ s: { - "routing": { - "yamas": { - "namespaces": - ["Vespa"] - } - }, "application": "vespa.node", + "dimensions": { + "host": "host1.test.yahoo.com", + "orchestratorState":"ALLOWED_TO_BE_DOWN", + "parentHostname": "parent.host.name.yahoo.com", + "role": "tenants", + "state": "active" + }, "metrics": { - "mem.limit": 4294967296, - "mem.used": 1073741824, - "mem_total.util": 40.808, - "mem_total.used": 1752707072, - "disk.used": 39625000000, "cpu.sys.util": 3.402, - "disk.util": 15.85, - "cpu.vcpus": 2.0, "cpu.util": 5.4, + "cpu.vcpus": 2.0, + "disk.limit": 250000000000, + "disk.used": 39625000000, + "disk.util": 15.85, + "mem.limit": 4294967296, + "mem.used": 1073741824, "mem.util": 25.0, - "disk.limit": 250000000000 + "mem_total.used": 1752707072, + "mem_total.util": 40.808 }, - "dimensions": { - "host": "host1.test.yahoo.com", - "orchestratorState":"ALLOWED_TO_BE_DOWN", - "role": "tenants", - "state": "active", - "parentHostname": "parent.host.name.yahoo.com" + "routing": { + "yamas": { + "namespaces": ["Vespa"] + } }, "timestamp": 0 } { - "routing": { - "yamas": { - "namespaces": - ["Vespa"] - } - }, "application": "vespa.node", + "dimensions": { + "host": "host1.test.yahoo.com", + "interface": "eth0", + "orchestratorState":"ALLOWED_TO_BE_DOWN", + "parentHostname": "parent.host.name.yahoo.com", + "role": "tenants", + "state": "active" + }, "metrics": { - "net.out.bytes": 20303455, + "net.in.bytes": 19499270, "net.in.dropped": 4, + "net.in.errors": 55, + "net.out.bytes": 20303455, "net.out.dropped": 13, - "net.in.bytes": 19499270, - "net.out.errors": 3, - "net.in.errors": 55 + "net.out.errors": 3 }, - "dimensions": { - "role": "tenants", - "host": "host1.test.yahoo.com", - "orchestratorState":"ALLOWED_TO_BE_DOWN", - "state": "active", - "interface": "eth0", - "parentHostname": "parent.host.name.yahoo.com" + "routing": { + "yamas": { + "namespaces": ["Vespa"] + } }, "timestamp": 0 } { - "routing": { - "yamas": { - "namespaces": - ["Vespa"] - } - }, "application": "vespa.node", + "dimensions": { + "host": "host1.test.yahoo.com", + "interface": "eth1", + "orchestratorState":"ALLOWED_TO_BE_DOWN", + "parentHostname": "parent.host.name.yahoo.com", + "role": "tenants", + "state": "active" + }, "metrics": { - "net.out.bytes": 54246745, + "net.in.bytes": 3245766, "net.in.dropped": 0, + "net.in.errors": 0, + "net.out.bytes": 54246745, "net.out.dropped": 0, - "net.in.bytes": 3245766, - "net.out.errors": 0, - "net.in.errors": 0 + "net.out.errors": 0 }, - "dimensions": { - "role": "tenants", - "host": "host1.test.yahoo.com", - "orchestratorState":"ALLOWED_TO_BE_DOWN", - "state": "active", - "interface": "eth1", - "parentHostname": "parent.host.name.yahoo.com" + "routing": { + "yamas": { + "namespaces": ["Vespa"] + } }, "timestamp": 0 }
\ No newline at end of file diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java index a4b915a6128..fd2294c1b5d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java @@ -27,6 +27,13 @@ import java.util.function.Function; */ public class LoadBalancerSerializer { + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + private static final String idField = "id"; private static final String hostnameField = "hostname"; private static final String dnsZoneField = "dnsZone"; 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 d38a6e5031c..424889caf72 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 @@ -44,12 +44,12 @@ import java.util.function.UnaryOperator; */ public class NodeSerializer { - // WARNING: Since there are multiple config servers in a cluster and they upgrade one by one - // (and rewrite all nodes on startup), - // changes to the serialized format must be made such that what is serialized on version N+1 - // can be read by version N: + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: // - ADDING FIELDS: Always ok // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. /** The configured node flavors */ private final NodeFlavors flavors; diff --git a/parent/pom.xml b/parent/pom.xml index d8ae9a35261..1855553bc20 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -92,7 +92,7 @@ <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-assembly-plugin</artifactId> - <version>3.1.0</version> + <version>3.1.1</version> </plugin> <plugin> <groupId>org.apache.maven.plugins</groupId> @@ -441,9 +441,14 @@ <version>${asm.version}</version> </dependency> <dependency> - <groupId>com.goldmansachs</groupId> - <artifactId>gs-collections</artifactId> - <version>6.1.0</version> + <groupId>org.eclipse.collections</groupId> + <artifactId>eclipse-collections</artifactId> + <version>9.2.0</version> + </dependency> + <dependency> + <groupId>org.eclipse.collections</groupId> + <artifactId>eclipse-collections-api</artifactId> + <version>9.2.0</version> </dependency> <dependency> <groupId>com.infradna.tool</groupId> diff --git a/predicate-search/pom.xml b/predicate-search/pom.xml index fa1bee2fe28..778ede93366 100644 --- a/predicate-search/pom.xml +++ b/predicate-search/pom.xml @@ -34,8 +34,12 @@ <artifactId>guava</artifactId> </dependency> <dependency> - <groupId>com.goldmansachs</groupId> - <artifactId>gs-collections</artifactId> + <groupId>org.eclipse.collections</groupId> + <artifactId>eclipse-collections</artifactId> + </dependency> + <dependency> + <groupId>org.eclipse.collections</groupId> + <artifactId>eclipse-collections-api</artifactId> </dependency> <dependency> <groupId>io.airlift</groupId> diff --git a/predicate-search/src/main/java/com/yahoo/search/predicate/index/CachedPostingListCounter.java b/predicate-search/src/main/java/com/yahoo/search/predicate/index/CachedPostingListCounter.java index 91599da5483..9356e86aa2f 100644 --- a/predicate-search/src/main/java/com/yahoo/search/predicate/index/CachedPostingListCounter.java +++ b/predicate-search/src/main/java/com/yahoo/search/predicate/index/CachedPostingListCounter.java @@ -2,9 +2,9 @@ package com.yahoo.search.predicate.index; import com.google.common.collect.MinMaxPriorityQueue; -import com.gs.collections.api.tuple.primitive.ObjectLongPair; -import com.gs.collections.impl.map.mutable.primitive.ObjectIntHashMap; -import com.gs.collections.impl.map.mutable.primitive.ObjectLongHashMap; +import org.eclipse.collections.api.tuple.primitive.ObjectLongPair; +import org.eclipse.collections.impl.map.mutable.primitive.ObjectIntHashMap; +import org.eclipse.collections.impl.map.mutable.primitive.ObjectLongHashMap; import java.util.ArrayList; import java.util.Arrays; @@ -119,7 +119,7 @@ public class CachedPostingListCounter { private static class Entry implements Comparable<Entry> { public final int[] docIds; - public final double cost; + final double cost; private Entry(int[] docIds, long frequency) { this.docIds = docIds; diff --git a/predicate-search/src/main/java/com/yahoo/search/predicate/index/SimpleIndex.java b/predicate-search/src/main/java/com/yahoo/search/predicate/index/SimpleIndex.java index 64583273e77..3e1ed7ad9e4 100644 --- a/predicate-search/src/main/java/com/yahoo/search/predicate/index/SimpleIndex.java +++ b/predicate-search/src/main/java/com/yahoo/search/predicate/index/SimpleIndex.java @@ -1,10 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.predicate.index; -import com.gs.collections.api.map.primitive.LongObjectMap; -import com.gs.collections.api.tuple.primitive.LongObjectPair; -import com.gs.collections.impl.map.mutable.primitive.LongObjectHashMap; import com.yahoo.search.predicate.serialization.SerializationHelper; +import org.eclipse.collections.api.map.primitive.LongObjectMap; +import org.eclipse.collections.api.tuple.primitive.LongObjectPair; +import org.eclipse.collections.impl.map.mutable.primitive.LongObjectHashMap; import java.io.DataInputStream; import java.io.DataOutputStream; diff --git a/predicate-search/src/main/java/com/yahoo/search/predicate/index/conjunction/ConjunctionIndex.java b/predicate-search/src/main/java/com/yahoo/search/predicate/index/conjunction/ConjunctionIndex.java index 5a100ea9cf5..d062af43f22 100644 --- a/predicate-search/src/main/java/com/yahoo/search/predicate/index/conjunction/ConjunctionIndex.java +++ b/predicate-search/src/main/java/com/yahoo/search/predicate/index/conjunction/ConjunctionIndex.java @@ -1,17 +1,17 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.predicate.index.conjunction; -import com.gs.collections.api.map.primitive.IntObjectMap; -import com.gs.collections.api.map.primitive.LongObjectMap; -import com.gs.collections.api.tuple.primitive.IntObjectPair; -import com.gs.collections.api.tuple.primitive.LongObjectPair; -import com.gs.collections.impl.map.mutable.primitive.IntObjectHashMap; -import com.gs.collections.impl.map.mutable.primitive.LongObjectHashMap; import com.yahoo.document.predicate.FeatureConjunction; import com.yahoo.search.predicate.PredicateQuery; import com.yahoo.search.predicate.SubqueryBitmap; import com.yahoo.search.predicate.serialization.SerializationHelper; import com.yahoo.search.predicate.utils.PrimitiveArraySorter; +import org.eclipse.collections.api.map.primitive.IntObjectMap; +import org.eclipse.collections.api.map.primitive.LongObjectMap; +import org.eclipse.collections.api.tuple.primitive.IntObjectPair; +import org.eclipse.collections.api.tuple.primitive.LongObjectPair; +import org.eclipse.collections.impl.map.mutable.primitive.IntObjectHashMap; +import org.eclipse.collections.impl.map.mutable.primitive.LongObjectHashMap; import java.io.DataInputStream; import java.io.DataOutputStream; diff --git a/predicate-search/src/main/java/com/yahoo/search/predicate/index/conjunction/ConjunctionIndexBuilder.java b/predicate-search/src/main/java/com/yahoo/search/predicate/index/conjunction/ConjunctionIndexBuilder.java index a6a03177018..8e3261a4cf8 100644 --- a/predicate-search/src/main/java/com/yahoo/search/predicate/index/conjunction/ConjunctionIndexBuilder.java +++ b/predicate-search/src/main/java/com/yahoo/search/predicate/index/conjunction/ConjunctionIndexBuilder.java @@ -3,9 +3,9 @@ package com.yahoo.search.predicate.index.conjunction; import com.google.common.primitives.Ints; import com.google.common.primitives.Longs; -import com.gs.collections.api.map.primitive.IntObjectMap; -import com.gs.collections.impl.map.mutable.primitive.IntObjectHashMap; -import com.gs.collections.impl.map.mutable.primitive.LongObjectHashMap; +import org.eclipse.collections.api.map.primitive.IntObjectMap; +import org.eclipse.collections.impl.map.mutable.primitive.IntObjectHashMap; +import org.eclipse.collections.impl.map.mutable.primitive.LongObjectHashMap; import java.util.ArrayList; import java.util.HashMap; diff --git a/predicate-search/src/test/java/com/yahoo/search/predicate/index/CachedPostingListCounterTest.java b/predicate-search/src/test/java/com/yahoo/search/predicate/index/CachedPostingListCounterTest.java index a3dfd00149c..31777959704 100644 --- a/predicate-search/src/test/java/com/yahoo/search/predicate/index/CachedPostingListCounterTest.java +++ b/predicate-search/src/test/java/com/yahoo/search/predicate/index/CachedPostingListCounterTest.java @@ -1,8 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.predicate.index; -import com.gs.collections.impl.map.mutable.primitive.ObjectIntHashMap; import org.apache.commons.lang.ArrayUtils; +import org.eclipse.collections.impl.map.mutable.primitive.ObjectIntHashMap; import org.junit.Test; import java.util.ArrayList; diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp index d37072f5da2..9b535be19b7 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp @@ -1079,8 +1079,12 @@ DocumentMetaStore::foreach(const search::IGidToLidMapperVisitor &visitor) const } // namespace proton -template class search::btree:: -BTreeIterator<proton::DocumentMetaStore::DocId, - search::btree::BTreeNoLeafData, - search::btree::NoAggregated, - const proton::DocumentMetaStore::KeyComp &>; +namespace search::btree { + +template class BTreeIteratorBase<proton::DocumentMetaStore::DocId, BTreeNoLeafData, NoAggregated, BTreeDefaultTraits::INTERNAL_SLOTS, BTreeDefaultTraits::LEAF_SLOTS, BTreeDefaultTraits::PATH_SIZE>; + +template class BTreeConstIterator<proton::DocumentMetaStore::DocId, BTreeNoLeafData, NoAggregated, const proton::DocumentMetaStore::KeyComp &>; + +template class BTreeIterator<proton::DocumentMetaStore::DocId, BTreeNoLeafData, NoAggregated, const proton::DocumentMetaStore::KeyComp &>; + +} diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h index efac1158cfb..27c1c97556c 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h @@ -268,8 +268,12 @@ public: } -extern template class search::btree:: -BTreeIterator<proton::DocumentMetaStore::DocId, - search::btree::BTreeNoLeafData, - search::btree::NoAggregated, - const proton::DocumentMetaStore::KeyComp &>; +namespace search::btree { + +extern template class BTreeIteratorBase<proton::DocumentMetaStore::DocId, BTreeNoLeafData, NoAggregated, BTreeDefaultTraits::INTERNAL_SLOTS, BTreeDefaultTraits::LEAF_SLOTS, BTreeDefaultTraits::PATH_SIZE>; + +extern template class BTreeConstIterator<proton::DocumentMetaStore::DocId, BTreeNoLeafData, NoAggregated, const proton::DocumentMetaStore::KeyComp &>; + +extern template class BTreeIterator<proton::DocumentMetaStore::DocId, BTreeNoLeafData, NoAggregated, const proton::DocumentMetaStore::KeyComp &>; + +} diff --git a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp index 6b6abd956fc..5476f0f8e66 100644 --- a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp +++ b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp @@ -1,13 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("enumstore_test"); #include <vespa/vespalib/testkit/testapp.h> -//#define LOG_ENUM_STORE #include <vespa/searchlib/attribute/enumstore.hpp> #include <limits> #include <string> #include <iostream> +#include <vespa/log/log.h> +LOG_SETUP("enumstore_test"); + namespace search { size_t enumStoreAlign(size_t size) diff --git a/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp b/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp index c7c3447a4cc..251040ecfa7 100644 --- a/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp +++ b/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp @@ -26,6 +26,8 @@ struct DummyExpression : IntrinsicExpression { DummyExpression(const FeatureType &type_in) : type(type_in) {} vespalib::string describe_self() const override { return "dummy"; } const FeatureType &result_type() const override { return type; } + void prepare_shared_state(const QueryEnv &, IObjectStore &) const override { + } FeatureExecutor &create_executor(const QueryEnv &, vespalib::Stash &stash) const override { return stash.create<DummyExecutor>(); } @@ -81,7 +83,7 @@ SetupResult::SetupResult(const TypeMap &object_inputs, setup_ok = rank.setup(index_env, {}); EXPECT_TRUE(!deps.accept_type_mismatch); } -SetupResult::~SetupResult() {} +SetupResult::~SetupResult() = default; void verify_output_type(const TypeMap &object_inputs, const vespalib::string &expression, const FeatureType &expect) diff --git a/searchlib/src/tests/features/util/util_test.cpp b/searchlib/src/tests/features/util/util_test.cpp index 6b166682346..208c290cff3 100644 --- a/searchlib/src/tests/features/util/util_test.cpp +++ b/searchlib/src/tests/features/util/util_test.cpp @@ -8,6 +8,7 @@ using namespace search; using namespace search::fef; using namespace search::fef::test; using namespace search::features; +using namespace search::features::util; SimpleTermData make_term(uint32_t uid) { SimpleTermData term; @@ -37,4 +38,23 @@ TEST_F("require that label can be mapped to term", TermLabelFixture) { EXPECT_EQUAL((ITermData*)0, util::getTermByLabel(f1.queryEnv, "unknown")); } +template <typename T> +void verifyStrToNum() { + EXPECT_EQUAL(-17, static_cast<long>(strToNum<T>("-17"))); + EXPECT_EQUAL(-1, static_cast<long>(strToNum<T>("-1"))); + EXPECT_EQUAL(0, static_cast<long>(strToNum<T>("0"))); + EXPECT_EQUAL(1, static_cast<long>(strToNum<T>("1"))); + EXPECT_EQUAL(17, static_cast<long>(strToNum<T>("17"))); + EXPECT_EQUAL(0, static_cast<long>(strToNum<T>("0x0"))); + EXPECT_EQUAL(1, static_cast<long>(strToNum<T>("0x1"))); + EXPECT_EQUAL(27, static_cast<long>(strToNum<T>("0x1b"))); +} + +TEST("verify str2Num") { + verifyStrToNum<int8_t>(); + verifyStrToNum<int16_t>(); + verifyStrToNum<int32_t>(); + verifyStrToNum<int64_t>(); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/memoryindex/field_index/field_index_test.cpp b/searchlib/src/tests/memoryindex/field_index/field_index_test.cpp index 05c905cdc32..0f1c966ad5d 100644 --- a/searchlib/src/tests/memoryindex/field_index/field_index_test.cpp +++ b/searchlib/src/tests/memoryindex/field_index/field_index_test.cpp @@ -171,7 +171,7 @@ assertPostingList(const std::string &exp, uint32_t docId = itr.getKey(); ss << docId; if (store != nullptr) { // consider features as well - EntryRef ref(itr.getData()); + EntryRef ref(itr.getData().get_features()); store->setupForField(0, decoder); store->setupForUnpackFeatures(ref, decoder); decoder.unpackFeatures(matchData, docId); diff --git a/searchlib/src/vespa/searchlib/attribute/enumcomparator.h b/searchlib/src/vespa/searchlib/attribute/enumcomparator.h index 4cd446352d0..255d0bead9f 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumcomparator.h +++ b/searchlib/src/vespa/searchlib/attribute/enumcomparator.h @@ -27,13 +27,18 @@ public: /** * Creates a comparator using the given enum store. **/ - EnumStoreComparatorT(const EnumStoreType & enumStore); + EnumStoreComparatorT(const EnumStoreType & enumStore) + : _enumStore(enumStore), + _value() + {} /** * Creates a comparator using the given enum store and that uses the * given value during compare if the enum index is invalid. **/ - EnumStoreComparatorT(const EnumStoreType & enumStore, - EntryValue value); + EnumStoreComparatorT(const EnumStoreType & enumStore, EntryValue value) + : _enumStore(enumStore), + _value(value) + {} static int compare(EntryValue lhs, EntryValue rhs) { if (lhs < rhs) { @@ -60,7 +65,7 @@ private: typedef typename ParentType::EnumIndex EnumIndex; typedef typename ParentType::EntryValue EntryValue; using ParentType::getValue; - bool _prefix; + bool _prefix; size_t _prefixLen; public: /** @@ -90,22 +95,6 @@ public: } }; - -template <typename EntryType> -EnumStoreComparatorT<EntryType>::EnumStoreComparatorT(const EnumStoreType & enumStore) : - _enumStore(enumStore), - _value() -{ -} - -template <typename EntryType> -EnumStoreComparatorT<EntryType>::EnumStoreComparatorT(const EnumStoreType & enumStore, - EntryValue value) : - _enumStore(enumStore), - _value(value) -{ -} - template <> int EnumStoreComparatorT<NumericEntryType<float> >::compare(EntryValue lhs, EntryValue rhs); diff --git a/searchlib/src/vespa/searchlib/attribute/enumstorebase.cpp b/searchlib/src/vespa/searchlib/attribute/enumstorebase.cpp index 94c431368cb..decb4152d8d 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstorebase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/enumstorebase.cpp @@ -704,6 +704,10 @@ template class btree::BTreeIteratorBase<EnumStoreBase::Index, datastore::EntryRef, btree::NoAggregated, EnumTreeTraits::INTERNAL_SLOTS, EnumTreeTraits::LEAF_SLOTS, EnumTreeTraits::PATH_SIZE>; +template class btree::BTreeConstIterator<EnumStoreBase::Index, btree::BTreeNoLeafData, btree::NoAggregated, const EnumStoreComparatorWrapper, EnumTreeTraits>; + +template class btree::BTreeConstIterator<EnumStoreBase::Index, datastore::EntryRef, btree::NoAggregated, const EnumStoreComparatorWrapper, EnumTreeTraits>; + template class btree::BTreeIterator<EnumStoreBase::Index, btree::BTreeNoLeafData, btree::NoAggregated, const EnumStoreComparatorWrapper, EnumTreeTraits>; diff --git a/searchlib/src/vespa/searchlib/attribute/enumstorebase.h b/searchlib/src/vespa/searchlib/attribute/enumstorebase.h index 48bf4a56874..d8604a5a85e 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstorebase.h +++ b/searchlib/src/vespa/searchlib/attribute/enumstorebase.h @@ -467,6 +467,10 @@ extern template class btree::BTreeIteratorBase<EnumStoreBase::Index, datastore::EntryRef, btree::NoAggregated, EnumTreeTraits::INTERNAL_SLOTS, EnumTreeTraits::LEAF_SLOTS, EnumTreeTraits::PATH_SIZE>; +extern template class btree::BTreeConstIterator<EnumStoreBase::Index, btree::BTreeNoLeafData, btree::NoAggregated, const EnumStoreComparatorWrapper, EnumTreeTraits>; + +extern template class btree::BTreeConstIterator<EnumStoreBase::Index, datastore::EntryRef, btree::NoAggregated, const EnumStoreComparatorWrapper, EnumTreeTraits>; + extern template class btree::BTreeIterator<EnumStoreBase::Index, btree::BTreeNoLeafData, btree::NoAggregated, const EnumStoreComparatorWrapper, EnumTreeTraits>; diff --git a/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_blueprint_adapter.cpp b/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_blueprint_adapter.cpp index 018da0e7bcd..4ff9d2f4e30 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_blueprint_adapter.cpp +++ b/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_blueprint_adapter.cpp @@ -29,8 +29,11 @@ struct IntrinsicBlueprint : IntrinsicExpression { : blueprint(std::move(blueprint_in)), type(type_in) {} vespalib::string describe_self() const override { return blueprint->getName(); } const FeatureType &result_type() const override { return type; } - FeatureExecutor &create_executor(const QueryEnv &queryEnv, vespalib::Stash &stash) const override { - return blueprint->createExecutor(queryEnv, stash); + void prepare_shared_state(const QueryEnv & env, fef::IObjectStore & store) const override { + blueprint->prepareSharedState(env, store); + } + FeatureExecutor &create_executor(const QueryEnv &env, vespalib::Stash &stash) const override { + return blueprint->createExecutor(env, stash); } }; diff --git a/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_expression.h b/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_expression.h index 34c4f34b03f..79cb3f9035b 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_expression.h +++ b/searchlib/src/vespa/searchlib/features/rankingexpression/intrinsic_expression.h @@ -11,6 +11,7 @@ namespace search::fef { class FeatureType; class FeatureExecutor; class IQueryEnvironment; +class IObjectStore; } namespace search::features::rankingexpression { @@ -26,8 +27,8 @@ struct IntrinsicExpression { using UP = std::unique_ptr<IntrinsicExpression>; virtual vespalib::string describe_self() const = 0; virtual const FeatureType &result_type() const = 0; - virtual FeatureExecutor &create_executor(const QueryEnv &queryEnv, - vespalib::Stash &stash) const = 0; + virtual void prepare_shared_state(const QueryEnv & env, fef::IObjectStore & store) const = 0; + virtual FeatureExecutor &create_executor(const QueryEnv &queryEnv, vespalib::Stash &stash) const = 0; virtual ~IntrinsicExpression(); }; diff --git a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp index b2c8c64d55a..2733ec62105 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp +++ b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp @@ -134,10 +134,14 @@ CompiledRankingExpressionExecutor::execute(uint32_t) //----------------------------------------------------------------------------- +namespace { + using Context = fef::FeatureExecutor::Inputs; double resolve_input(void *ctx, size_t idx) { return ((const Context *)(ctx))->get_number(idx); } Context *make_ctx(const Context &inputs) { return const_cast<Context *>(&inputs); } +} + LazyCompiledRankingExpressionExecutor::LazyCompiledRankingExpressionExecutor(const CompiledFunction &compiled_function) : _ranking_function(compiled_function.get_lazy_function()) { @@ -278,6 +282,14 @@ RankingExpressionBlueprint::createInstance() const return std::make_unique<RankingExpressionBlueprint>(_expression_replacer); } +void +RankingExpressionBlueprint::prepareSharedState(const fef::IQueryEnvironment & env, fef::IObjectStore & store) const +{ + if (_intrinsic_expression) { + return _intrinsic_expression->prepare_shared_state(env, store); + } +} + fef::FeatureExecutor & RankingExpressionBlueprint::createExecutor(const fef::IQueryEnvironment &env, vespalib::Stash &stash) const { diff --git a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.h b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.h index 8d5144206ea..104e8d63a70 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.h +++ b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.h @@ -26,7 +26,7 @@ private: public: RankingExpressionBlueprint(); RankingExpressionBlueprint(rankingexpression::ExpressionReplacer::SP replacer); - ~RankingExpressionBlueprint(); + ~RankingExpressionBlueprint() override; void visitDumpFeatures(const fef::IIndexEnvironment &env, fef::IDumpFeatureVisitor &visitor) const override; fef::Blueprint::UP createInstance() const override; @@ -37,6 +37,7 @@ public: } bool setup(const fef::IIndexEnvironment & env, const fef::ParameterList & params) override; + void prepareSharedState(const fef::IQueryEnvironment & queryEnv, fef::IObjectStore & objectStore) const override; fef::FeatureExecutor &createExecutor(const fef::IQueryEnvironment &env, vespalib::Stash &stash) const override; }; diff --git a/searchlib/src/vespa/searchlib/features/utils.cpp b/searchlib/src/vespa/searchlib/features/utils.cpp index 3f68e69ff25..7bfea67b7f6 100644 --- a/searchlib/src/vespa/searchlib/features/utils.cpp +++ b/searchlib/src/vespa/searchlib/features/utils.cpp @@ -1,28 +1,43 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "utils.hpp" +#include <charconv> namespace search::features::util { +template <typename T> +T strToInt(vespalib::stringref str) +{ + T retval = 0; + if ((str.size() > 2) && (str[0] == '0') && ((str[1] | 0x20) == 'x')) { + std::from_chars(str.data()+2, str.data()+str.size(), retval, 16); + } else { + std::from_chars(str.data(), str.data()+str.size(), retval, 10); + } + + return retval; +} + template <> uint8_t strToNum<uint8_t>(vespalib::stringref str) { - return strToNum<uint16_t>(str); + return strToInt<uint16_t>(str); } template <> int8_t strToNum<int8_t>(vespalib::stringref str) { - return strToNum<int16_t>(str); + return strToInt<int16_t>(str); } template double strToNum<double>(vespalib::stringref str); template float strToNum<float>(vespalib::stringref str); -template uint16_t strToNum<uint16_t>(vespalib::stringref str); -template uint32_t strToNum<uint32_t>(vespalib::stringref str); -template uint64_t strToNum<uint64_t>(vespalib::stringref str); -template int16_t strToNum<int16_t>(vespalib::stringref str); -template int32_t strToNum<int32_t>(vespalib::stringref str); -template int64_t strToNum<int64_t>(vespalib::stringref str); + +template <> uint16_t strToNum<uint16_t>(vespalib::stringref str) { return strToInt<uint16_t>(str); } +template <> uint32_t strToNum<uint32_t>(vespalib::stringref str) { return strToInt<uint32_t>(str); } +template <> uint64_t strToNum<uint64_t>(vespalib::stringref str) { return strToInt<uint64_t>(str); } +template <> int16_t strToNum<int16_t>(vespalib::stringref str) { return strToInt<int16_t>(str); } +template <> int32_t strToNum<int32_t>(vespalib::stringref str) { return strToInt<int32_t>(str); } +template <> int64_t strToNum<int64_t>(vespalib::stringref str) { return strToInt<int64_t>(str); } } diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp b/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp index e2e1c99a9b9..8daf87e1899 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp +++ b/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp @@ -127,36 +127,35 @@ FieldIndex::compactFeatures() const PostingList *tree = _postingListStore.getTreeEntry(pidx); auto pitr = tree->begin(_postingListStore.getAllocator()); for (; pitr.valid(); ++pitr) { - EntryRef oldFeatures(pitr.getData()); + const PostingListEntry &posting_entry(pitr.getData()); // Filter on which buffers to move features from when // performing incremental compaction. - EntryRef newFeatures = _featureStore.moveFeatures(packedIndex, oldFeatures); + EntryRef newFeatures = _featureStore.moveFeatures(packedIndex, posting_entry.get_features()); // Features must be written before reference is updated. std::atomic_thread_fence(std::memory_order_release); - // Ugly, ugly due to const_cast in iterator - pitr.writeData(newFeatures.ref()); + // Reference the moved data + posting_entry.update_features(newFeatures); } } else { const PostingListKeyDataType *shortArray = _postingListStore.getKeyDataEntry(pidx, clusterSize); const PostingListKeyDataType *ite = shortArray + clusterSize; for (const PostingListKeyDataType *it = shortArray; it < ite; ++it) { - EntryRef oldFeatures(it->getData()); + const PostingListEntry &posting_entry(it->getData()); // Filter on which buffers to move features from when // performing incremental compaction. - EntryRef newFeatures = _featureStore.moveFeatures(packedIndex, oldFeatures); + EntryRef newFeatures = _featureStore.moveFeatures(packedIndex, posting_entry.get_features()); // Features must be written before reference is updated. std::atomic_thread_fence(std::memory_order_release); - // Ugly, ugly due to const_cast, but new data is - // semantically equal to old data - const_cast<PostingListKeyDataType *>(it)->setData(newFeatures.ref()); + // Reference the moved data + posting_entry.update_features(newFeatures); } } } @@ -189,7 +188,7 @@ FieldIndex::dump(search::index::IndexBuilder & indexBuilder) assert(pitr.valid()); for (; pitr.valid(); ++pitr) { uint32_t docId = pitr.getKey(); - EntryRef featureRef(pitr.getData()); + EntryRef featureRef(pitr.getData().get_features()); _featureStore.setupForReadFeatures(featureRef, decoder); decoder.readFeatures(features); features.set_doc_id(docId); @@ -202,7 +201,7 @@ FieldIndex::dump(search::index::IndexBuilder & indexBuilder) const PostingListKeyDataType *kde = kd + clusterSize; for (; kd != kde; ++kd) { uint32_t docId = kd->_key; - EntryRef featureRef(kd->getData()); + EntryRef featureRef(kd->getData().get_features()); _featureStore.setupForReadFeatures(featureRef, decoder); decoder.readFeatures(features); features.set_doc_id(docId); diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_index.h b/searchlib/src/vespa/searchlib/memoryindex/field_index.h index dba57f553b5..d5df2fa49c8 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/field_index.h +++ b/searchlib/src/vespa/searchlib/memoryindex/field_index.h @@ -5,6 +5,7 @@ #include "feature_store.h" #include "field_index_remover.h" #include "word_store.h" +#include "posting_list_entry.h" #include <vespa/searchlib/index/docidandfeatures.h> #include <vespa/searchlib/index/field_length_calculator.h> #include <vespa/searchlib/index/indexbuilder.h> @@ -35,8 +36,8 @@ class OrderedFieldIndexInserter; class FieldIndex { public: // Mapping from docid -> feature ref - using PostingList = btree::BTreeRoot<uint32_t, uint32_t, search::btree::NoAggregated>; - using PostingListStore = btree::BTreeStore<uint32_t, uint32_t, + using PostingList = btree::BTreeRoot<uint32_t, PostingListEntry, search::btree::NoAggregated>; + using PostingListStore = btree::BTreeStore<uint32_t, PostingListEntry, search::btree::NoAggregated, std::less<uint32_t>, btree::BTreeDefaultTraits>; diff --git a/searchlib/src/vespa/searchlib/memoryindex/ordered_field_index_inserter.cpp b/searchlib/src/vespa/searchlib/memoryindex/ordered_field_index_inserter.cpp index 637a13d67be..1d38e88b747 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/ordered_field_index_inserter.cpp +++ b/searchlib/src/vespa/searchlib/memoryindex/ordered_field_index_inserter.cpp @@ -111,7 +111,7 @@ OrderedFieldIndexInserter::add(uint32_t docId, assert(_prevDocId == noDocId || _prevDocId < docId || (_prevDocId == docId && !_prevAdd)); datastore::EntryRef featureRef = _fieldIndex.addFeatures(features); - _adds.push_back(PostingListKeyDataType(docId, featureRef.ref())); + _adds.push_back(PostingListKeyDataType(docId, featureRef)); _listener.insert(_dItr.getKey()._wordRef, docId); _prevDocId = docId; _prevAdd = true; diff --git a/searchlib/src/vespa/searchlib/memoryindex/posting_iterator.cpp b/searchlib/src/vespa/searchlib/memoryindex/posting_iterator.cpp index 63040aab66f..290aa16dfe4 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/posting_iterator.cpp +++ b/searchlib/src/vespa/searchlib/memoryindex/posting_iterator.cpp @@ -6,6 +6,7 @@ #include <vespa/vespalib/btree/btreenodeallocator.hpp> #include <vespa/vespalib/btree/btreenodestore.hpp> #include <vespa/vespalib/btree/btreeroot.hpp> +#include <vespa/vespalib/btree/btreestore.hpp> #include <vespa/log/log.h> LOG_SETUP(".searchlib.memoryindex.posting_iterator"); @@ -62,7 +63,7 @@ PostingIterator::doUnpack(uint32_t docId) assert(docId == getDocId()); assert(_itr.valid()); assert(docId == _itr.getKey()); - datastore::EntryRef featureRef(_itr.getData()); + datastore::EntryRef featureRef(_itr.getData().get_features()); _featureStore.setupForUnpackFeatures(featureRef, _featureDecoder); _featureDecoder.unpackFeatures(_matchData, docId); setUnpacked(); @@ -70,3 +71,59 @@ PostingIterator::doUnpack(uint32_t docId) } +namespace search::btree { + +template class BTreeNodeTT<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + BTreeDefaultTraits::INTERNAL_SLOTS>; + +template class BTreeLeafNode<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + BTreeDefaultTraits::LEAF_SLOTS>; + +template class BTreeNodeStore<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + BTreeDefaultTraits::INTERNAL_SLOTS, + BTreeDefaultTraits::LEAF_SLOTS>; + +template class BTreeIteratorBase<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + BTreeDefaultTraits::INTERNAL_SLOTS, + BTreeDefaultTraits::LEAF_SLOTS, + BTreeDefaultTraits::PATH_SIZE>; + +template class BTreeIterator<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + std::less<uint32_t>, + BTreeDefaultTraits>; + +template class BTree<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + std::less<uint32_t>, + BTreeDefaultTraits>; + +template class BTreeRoot<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + std::less<uint32_t>, + BTreeDefaultTraits>; + +template class BTreeRootBase<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + BTreeDefaultTraits::INTERNAL_SLOTS, + BTreeDefaultTraits::LEAF_SLOTS>; + +template class BTreeNodeAllocator<uint32_t, + search::memoryindex::PostingListEntry, + search::btree::NoAggregated, + BTreeDefaultTraits::INTERNAL_SLOTS, + BTreeDefaultTraits::LEAF_SLOTS>; + +} diff --git a/searchlib/src/vespa/searchlib/memoryindex/posting_list_entry.h b/searchlib/src/vespa/searchlib/memoryindex/posting_list_entry.h new file mode 100644 index 00000000000..b28cd87736c --- /dev/null +++ b/searchlib/src/vespa/searchlib/memoryindex/posting_list_entry.h @@ -0,0 +1,36 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +# pragma once + +#include <vespa/vespalib/datastore/entryref.h> + +namespace search::memoryindex { + +/** + * Entry per document in memory index posting list. + */ +class PostingListEntry { + mutable datastore::EntryRef _features; // reference to compressed features + +public: + PostingListEntry(datastore::EntryRef features) + : _features(features) + { + } + + PostingListEntry() + : _features() + { + } + + datastore::EntryRef get_features() const { return _features; } + + /* + * Reference moved features (used when compacting FeatureStore). + * The moved features must have the same content as the original + * features. + */ + void update_features(datastore::EntryRef features) const { _features = features; } +}; + +} diff --git a/searchlib/src/vespa/searchlib/test/fakedata/fakememtreeocc.cpp b/searchlib/src/vespa/searchlib/test/fakedata/fakememtreeocc.cpp index 54c0aa866b4..f5300430bea 100644 --- a/searchlib/src/vespa/searchlib/test/fakedata/fakememtreeocc.cpp +++ b/searchlib/src/vespa/searchlib/test/fakedata/fakememtreeocc.cpp @@ -261,13 +261,13 @@ FakeMemTreeOccMgr::flush() lastWord = wordIdx; if (i->getRemove()) { if (itr.valid() && itr.getKey() == docId) { - uint64_t bits = _featureStore.bitSize(fw->getPackedIndex(), EntryRef(itr.getData())); + uint64_t bits = _featureStore.bitSize(fw->getPackedIndex(), EntryRef(itr.getData().get_features())); _featureSizes[wordIdx] -= RefType::align((bits + 7) / 8) * 8; tree.remove(itr); } } else { if (!itr.valid() || docId < itr.getKey()) { - tree.insert(itr, docId, i->getFeatureRef().ref()); + tree.insert(itr, docId, i->getFeatureRef()); } } } diff --git a/tenant-cd/src/main/java/ai/vespa/hosted/cd/Endpoint.java b/tenant-cd/src/main/java/ai/vespa/hosted/cd/Endpoint.java index efeb4214ebd..dbbb969efe2 100644 --- a/tenant-cd/src/main/java/ai/vespa/hosted/cd/Endpoint.java +++ b/tenant-cd/src/main/java/ai/vespa/hosted/cd/Endpoint.java @@ -3,6 +3,8 @@ package ai.vespa.hosted.cd; import ai.vespa.hosted.cd.metric.Metrics; import java.net.URI; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; /** * An endpoint in a Vespa application {@link Deployment}, which allows document and metrics retrieval. @@ -16,6 +18,8 @@ public interface Endpoint { URI uri(); + <T> HttpResponse<T> send(HttpRequest.Builder request, HttpResponse.BodyHandler<T> handler); + Search search(Query query); Visit visit(Selection selection); diff --git a/tenant-cd/src/main/java/ai/vespa/hosted/cd/http/HttpEndpoint.java b/tenant-cd/src/main/java/ai/vespa/hosted/cd/http/HttpEndpoint.java index 02c34501dd2..17703d8fbab 100644 --- a/tenant-cd/src/main/java/ai/vespa/hosted/cd/http/HttpEndpoint.java +++ b/tenant-cd/src/main/java/ai/vespa/hosted/cd/http/HttpEndpoint.java @@ -13,6 +13,8 @@ import ai.vespa.hosted.cd.TestEndpoint; import ai.vespa.hosted.cd.Visit; import ai.vespa.hosted.cd.metric.Metrics; +import java.io.IOException; +import java.io.UncheckedIOException; import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; @@ -58,15 +60,23 @@ public class HttpEndpoint implements TestEndpoint { } @Override + public <T> HttpResponse<T> send(HttpRequest.Builder request, HttpResponse.BodyHandler<T> handler) { + try { + return client.send(authenticator.authenticated(request).build(), handler); + } + catch (IOException | InterruptedException e) { + throw new RuntimeException(e); + } + } + + @Override public Search search(Query query) { try { URI target = endpoint.resolve(searchApiPath).resolve("?" + query.rawQuery()); - HttpRequest request = authenticator.authenticated(HttpRequest.newBuilder() - .timeout(query.timeout().orElse(Duration.ofMillis(500)) - .plus(Duration.ofSeconds(1))) - .uri(target)) - .build(); - HttpResponse<byte[]> response = client.send(request, HttpResponse.BodyHandlers.ofByteArray()); + HttpResponse<byte[]> response = send(HttpRequest.newBuilder(target) + .timeout(query.timeout().orElse(Duration.ofMillis(500)) + .plus(Duration.ofSeconds(1))), + HttpResponse.BodyHandlers.ofByteArray()); if (response.statusCode() / 100 != 2) // TODO consider allowing 504 if specified. throw new RuntimeException("Non-OK status code " + response.statusCode() + " at " + target + ", with response \n" + new String(response.body())); diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index b2b895040bc..3b733105d2e 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -104,14 +104,22 @@ "com.yahoo.data.access.ObjectTraverser" ], "attributes": [ - "public", - "final" + "public" ], "methods": [ "public void <init>(java.lang.StringBuilder, boolean)", "public void encode(com.yahoo.data.access.Inspector)", + "protected void encodeEMPTY()", + "protected void encodeBOOL(boolean)", + "protected void encodeLONG(long)", + "protected void encodeDOUBLE(double)", + "protected void encodeSTRING(java.lang.String)", + "protected void encodeDATA(byte[])", + "protected void encodeARRAY(com.yahoo.data.access.Inspector)", + "protected void encodeOBJECT(com.yahoo.data.access.Inspector)", "public void entry(int, com.yahoo.data.access.Inspector)", - "public void field(java.lang.String, com.yahoo.data.access.Inspector)" + "public void field(java.lang.String, com.yahoo.data.access.Inspector)", + "public java.lang.StringBuilder target()" ], "fields": [] }, @@ -124,7 +132,8 @@ ], "methods": [ "public void <init>()", - "public static java.lang.StringBuilder render(com.yahoo.data.access.Inspectable, java.lang.StringBuilder, boolean)" + "public static java.lang.StringBuilder render(com.yahoo.data.access.Inspectable, java.lang.StringBuilder, boolean)", + "public static java.lang.StringBuilder render(com.yahoo.data.access.Inspectable, com.yahoo.data.access.simple.JsonRender$StringEncoder)" ], "fields": [] }, diff --git a/vespajlib/src/main/java/com/yahoo/data/access/simple/JsonRender.java b/vespajlib/src/main/java/com/yahoo/data/access/simple/JsonRender.java index 253b0c60927..9f662c77c59 100644 --- a/vespajlib/src/main/java/com/yahoo/data/access/simple/JsonRender.java +++ b/vespajlib/src/main/java/com/yahoo/data/access/simple/JsonRender.java @@ -11,19 +11,25 @@ import com.yahoo.data.access.ObjectTraverser; * * @author arnej27959 */ -public final class JsonRender -{ +public final class JsonRender { + public static StringBuilder render(Inspectable value, StringBuilder target, - boolean compact) - { - StringEncoder enc = new StringEncoder(target, compact); - enc.encode(value.inspect()); - return target; + boolean compact) { + return render(value, new StringEncoder(target, compact)); + } + + /** + * Renders the given value to the target stringbuilder with a given encoder. + * This is useful to use an encoder where rendering of some value types is customized. + */ + public static StringBuilder render(Inspectable value, StringEncoder encoder) { + encoder.encode(value.inspect()); + return encoder.target(); } - public static final class StringEncoder implements ArrayTraverser, ObjectTraverser - { + public static class StringEncoder implements ArrayTraverser, ObjectTraverser { + private final StringBuilder out; private boolean head = true; private boolean compact; @@ -41,21 +47,21 @@ public final class JsonRender } } - private void encodeEMPTY() { + protected void encodeEMPTY() { out.append("null"); } - private void encodeBOOL(boolean value) { + protected void encodeBOOL(boolean value) { out.append(value ? "true" : "false"); } - private void encodeLONG(long value) { - out.append(String.valueOf(value)); + protected void encodeLONG(long value) { + out.append(value); } - private void encodeDOUBLE(double value) { + protected void encodeDOUBLE(double value) { if (Double.isFinite(value)) { - out.append(String.valueOf(value)); + out.append(value); } else { out.append("null"); } @@ -63,7 +69,7 @@ public final class JsonRender static final char[] hex = "0123456789ABCDEF".toCharArray(); - private void encodeSTRING(String value) { + protected void encodeSTRING(String value) { out.append('"'); for (char c : value.toCharArray()) { switch (c) { @@ -89,7 +95,7 @@ public final class JsonRender out.append('"'); } - private void encodeDATA(byte[] value) { + protected void encodeDATA(byte[] value) { out.append('"'); out.append("0x"); for (int pos = 0; pos < value.length; pos++) { @@ -99,14 +105,14 @@ public final class JsonRender out.append('"'); } - private void encodeARRAY(Inspector inspector) { + protected void encodeARRAY(Inspector inspector) { openScope("["); ArrayTraverser at = this; inspector.traverse(at); closeScope("]"); } - private void encodeOBJECT(Inspector inspector) { + protected void encodeOBJECT(Inspector inspector) { openScope("{"); ObjectTraverser ot = this; inspector.traverse(ot); @@ -164,5 +170,10 @@ public final class JsonRender out.append(' '); encodeValue(inspector); } + + /** Returns the target this is encoding values to */ + public StringBuilder target() { return out; } + } + } diff --git a/vespajlib/src/main/java/com/yahoo/tensor/serialization/JsonFormat.java b/vespajlib/src/main/java/com/yahoo/tensor/serialization/JsonFormat.java index 52635905d72..1a210a614cc 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/serialization/JsonFormat.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/serialization/JsonFormat.java @@ -19,21 +19,33 @@ import java.util.Iterator; * A JSON map containing a 'cells' array. * See http://docs.vespa.ai/documentation/reference/document-json-put-format.html#tensor */ -// TODO: We should probably move reading of this format from the document module to here public class JsonFormat { - /** Serializes the given tensor into JSON format */ + /** Serializes the given tensor value into JSON format */ public static byte[] encode(Tensor tensor) { Slime slime = new Slime(); Cursor root = slime.setObject(); - Cursor cellsArray = root.setArray("cells"); + encodeCells(tensor, root); + return com.yahoo.slime.JsonFormat.toJsonBytes(slime); + } + + /** Serializes the given tensor type and value into JSON format */ + public static byte[] encodeWithType(Tensor tensor) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + root.setString("type", tensor.type().toString()); + encodeCells(tensor, root); + return com.yahoo.slime.JsonFormat.toJsonBytes(slime); + } + + private static void encodeCells(Tensor tensor, Cursor rootObject) { + Cursor cellsArray = rootObject.setArray("cells"); for (Iterator<Tensor.Cell> i = tensor.cellIterator(); i.hasNext(); ) { Tensor.Cell cell = i.next(); Cursor cellObject = cellsArray.addObject(); encodeAddress(tensor.type(), cell.getKey(), cellObject.setObject("address")); cellObject.setDouble("value", cell.getValue()); } - return com.yahoo.slime.JsonFormat.toJsonBytes(slime); } private static void encodeAddress(TensorType type, TensorAddress address, Cursor addressObject) { diff --git a/vespalib/src/vespa/vespalib/btree/btreeiterator.h b/vespalib/src/vespa/vespalib/btree/btreeiterator.h index de9637c00f1..7c247cd01da 100644 --- a/vespalib/src/vespa/vespalib/btree/btreeiterator.h +++ b/vespalib/src/vespa/vespalib/btree/btreeiterator.h @@ -302,8 +302,7 @@ protected: * * @param pathSize New tree height (number of levels of internal nodes) */ - void - clearPath(uint32_t pathSize); + VESPA_DLL_LOCAL void clearPath(uint32_t pathSize); public: bool @@ -396,8 +395,7 @@ public: /** * Setup iterator to be empty and not be associated with any tree. */ - void - setupEmpty(); + VESPA_DLL_LOCAL void setupEmpty(); /** * Move iterator to beyond last element in the current tree. |