diff options
50 files changed, 249 insertions, 669 deletions
diff --git a/cloud-tenant-base-dependencies-enforcer/pom.xml b/cloud-tenant-base-dependencies-enforcer/pom.xml index 166236f91a0..5d5c63934d3 100644 --- a/cloud-tenant-base-dependencies-enforcer/pom.xml +++ b/cloud-tenant-base-dependencies-enforcer/pom.xml @@ -40,8 +40,8 @@ <aopalliance.version>1.0</aopalliance.version> <guava.version>27.1-jre</guava.version> <guice.version>4.2.3</guice.version> - <jackson2.version>2.15.0</jackson2.version> - <jackson-databind.version>2.15.0</jackson-databind.version> + <jackson2.version>2.15.2</jackson2.version> + <jackson-databind.version>2.15.2</jackson-databind.version> <javax.inject.version>1</javax.inject.version> <javax.servlet-api.version>3.1.0</javax.servlet-api.version> <javax.ws.rs-api.version>2.0.1</javax.ws.rs-api.version> diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/CoredumpGatherer.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/CoredumpGatherer.java deleted file mode 100644 index c82f0cb436f..00000000000 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/CoredumpGatherer.java +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.container.jdisc.state; - -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ObjectNode; -import com.yahoo.vespa.defaults.Defaults; - -import java.io.IOException; -import java.io.UncheckedIOException; -import java.nio.file.NoSuchFileException; -import java.nio.file.Path; -import java.time.Instant; -import java.util.stream.Stream; - -/** - * @author olaa - */ -public class CoredumpGatherer { - - private static final ObjectMapper jsonMapper = new ObjectMapper(); - - private static final Path COREDUMP_PATH = Path.of(Defaults.getDefaults().underVespaHome("var/crash/processing")); - - public static JsonNode gatherCoredumpMetrics(FileWrapper fileWrapper) { - int coredumps = getNumberOfCoredumps(fileWrapper); - ObjectNode packet = jsonMapper.createObjectNode(); - packet.put("status_code", coredumps == 0 ? 0 : 1); - packet.put("status_msg", coredumps == 0 ? "OK" : String.format("Found %d coredump(s)", coredumps)); - packet.put("timestamp", Instant.now().getEpochSecond()); - packet.put("application", "system-coredumps-processing"); - return packet; - } - - private static int getNumberOfCoredumps(FileWrapper fileWrapper) { - try (Stream<Path> stream = fileWrapper.walkTree(COREDUMP_PATH)){ - return (int) stream - .filter(fileWrapper::isRegularFile) - .count(); - } catch (NoSuchFileException e) { - return 0; - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } -} diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricGatherer.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricGatherer.java index fbe6c5c0aec..103a4363ac2 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricGatherer.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricGatherer.java @@ -16,7 +16,6 @@ public class MetricGatherer { static List<JsonNode> getAdditionalMetrics() { FileWrapper fileWrapper = new FileWrapper(); List<JsonNode> packetList = new ArrayList<>(); - packetList.add(CoredumpGatherer.gatherCoredumpMetrics(fileWrapper)); if (System.getProperty("os.name").contains("nux")) packetList.add(HostLifeGatherer.getHostLifePacket(fileWrapper)); return packetList; diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java index df4f4bcb9ea..e06c7c8aa32 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java @@ -51,25 +51,20 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { static final String APPLICATION_KEY = "application"; static final String TIMESTAMP_KEY = "timestamp"; - static final String STATUS_CODE_KEY = "status_code"; - static final String STATUS_MSG_KEY = "status_msg"; static final String METRICS_KEY = "metrics"; static final String DIMENSIONS_KEY = "dimensions"; static final String PACKET_SEPARATOR = "\n\n"; - private final StateMonitor monitor; private final Timer timer; private final SnapshotProvider snapshotProvider; private final String applicationName; private final String hostDimension; @Inject - public MetricsPacketsHandler(StateMonitor monitor, - Timer timer, + public MetricsPacketsHandler(Timer timer, ComponentRegistry<SnapshotProvider> snapshotProviders, MetricsPacketsHandlerConfig config) { - this.monitor = monitor; this.timer = timer; snapshotProvider = getSnapshotProviderOrThrow(snapshotProviders); applicationName = config.application(); @@ -105,7 +100,7 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { return buildPrometheusOutput(); } - String output = jsonToString(getStatusPacket()) + getAllMetricsPackets() + "\n"; + String output = getAllMetricsPackets() + "\n"; return output.getBytes(StandardCharsets.UTF_8); } catch (JsonProcessingException e) { throw new RuntimeException("Bad JSON construction.", e); @@ -117,7 +112,6 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { private byte[] getMetricsArray() throws JsonProcessingException { ObjectNode root = jsonMapper.createObjectNode(); ArrayNode jsonArray = jsonMapper.createArrayNode(); - jsonArray.add(getStatusPacket()); getPacketsForSnapshot(getSnapshot(), applicationName, timer.currentTimeMillis()) .forEach(jsonArray::add); MetricGatherer.getAdditionalMetrics().forEach(jsonArray::add); @@ -133,19 +127,6 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { return PrometheusHelper.buildPrometheusOutput(getSnapshot(), applicationName, timer.currentTimeMillis()); } - /** - * Exactly one status packet is added to the response. - */ - private JsonNode getStatusPacket() { - ObjectNode packet = jsonMapper.createObjectNode(); - packet.put(APPLICATION_KEY, applicationName); - - StateMonitor.Status status = monitor.status(); - packet.put(STATUS_CODE_KEY, status.ordinal()); - packet.put(STATUS_MSG_KEY, status.name()); - return packet; - } - private static String jsonToString(JsonNode jsonObject) throws JsonProcessingException { return jsonMapper.writerWithDefaultPrettyPrinter() .writeValueAsString(jsonObject); @@ -154,9 +135,11 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { private String getAllMetricsPackets() throws JsonProcessingException { StringBuilder ret = new StringBuilder(); List<JsonNode> metricsPackets = getPacketsForSnapshot(getSnapshot(), applicationName, timer.currentTimeMillis()); + String delimiter = ""; for (JsonNode packet : metricsPackets) { - ret.append(PACKET_SEPARATOR); // For legibility and parsing in unit tests + ret.append(delimiter); // For legibility and parsing in unit tests ret.append(jsonToString(packet)); + delimiter = PACKET_SEPARATOR; } return ret.toString(); } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/ssl/impl/ConfiguredSslContextFactoryProvider.java b/container-core/src/main/java/com/yahoo/jdisc/http/ssl/impl/ConfiguredSslContextFactoryProvider.java index b99bc007b32..8e2f080d4ce 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/ssl/impl/ConfiguredSslContextFactoryProvider.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/ssl/impl/ConfiguredSslContextFactoryProvider.java @@ -111,13 +111,16 @@ public class ConfiguredSslContextFactoryProvider implements SslProvider { private static boolean hasNeither(String a, String b) { return a.isBlank() && b.isBlank(); } Optional<String> getCaCertificates(ConnectorConfig.Ssl sslConfig) { + var sb = new StringBuilder(); + if (sslConfig.caCertificateFile().isBlank() && sslConfig.caCertificate().isBlank()) return Optional.empty(); if (!sslConfig.caCertificate().isBlank()) { - return Optional.of(sslConfig.caCertificate()); - } else if (!sslConfig.caCertificateFile().isBlank()) { - return Optional.of(readToString(sslConfig.caCertificateFile())); - } else { - return Optional.empty(); + sb.append(sslConfig.caCertificate()); + } + if (!sslConfig.caCertificateFile().isBlank()) { + if (sb.length() > 0) sb.append('\n'); + sb.append(readToString(sslConfig.caCertificateFile())); } + return Optional.of(sb.toString()); } private static String getPrivateKey(ConnectorConfig.Ssl config) { diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/CoredumpGathererTest.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/CoredumpGathererTest.java deleted file mode 100644 index 5cec6c471fe..00000000000 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/CoredumpGathererTest.java +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.container.jdisc.state; - -import com.fasterxml.jackson.databind.JsonNode; -import org.junit.jupiter.api.Test; - -import java.nio.file.Path; -import java.util.stream.Stream; - -import static org.junit.jupiter.api.Assertions.assertEquals; - - -/** - * @author olaa - */ -public class CoredumpGathererTest { - - @Test - void finds_one_coredump() { - JsonNode packet = CoredumpGatherer.gatherCoredumpMetrics(new MockFileWrapper()); - - assertEquals("system-coredumps-processing", packet.get("application").textValue()); - assertEquals(1, packet.get("status_code").intValue()); - assertEquals("Found 1 coredump(s)", packet.get("status_msg").textValue()); - - } - - static class MockFileWrapper extends FileWrapper { - - - @Override - Stream<Path> walkTree(Path path) { - return Stream.of(Path.of("dummy-path")); - } - - @Override - boolean isRegularFile(Path path) { - return true; - } - } - -} diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java index 434622852e9..3f5c31e5e7f 100644 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java +++ b/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java @@ -15,8 +15,6 @@ import static com.yahoo.container.jdisc.state.MetricsPacketsHandler.APPLICATION_ import static com.yahoo.container.jdisc.state.MetricsPacketsHandler.DIMENSIONS_KEY; import static com.yahoo.container.jdisc.state.MetricsPacketsHandler.METRICS_KEY; import static com.yahoo.container.jdisc.state.MetricsPacketsHandler.PACKET_SEPARATOR; -import static com.yahoo.container.jdisc.state.MetricsPacketsHandler.STATUS_CODE_KEY; -import static com.yahoo.container.jdisc.state.MetricsPacketsHandler.STATUS_MSG_KEY; import static com.yahoo.container.jdisc.state.MetricsPacketsHandler.TIMESTAMP_KEY; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -35,30 +33,17 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { public void setupHandler() { metricsPacketsHandlerConfig = new MetricsPacketsHandlerConfig(new MetricsPacketsHandlerConfig.Builder() .application(APPLICATION_NAME).hostname(HOST_DIMENSION)); - metricsPacketsHandler = new MetricsPacketsHandler(monitor, timer, snapshotProviderRegistry, metricsPacketsHandlerConfig); + metricsPacketsHandler = new MetricsPacketsHandler(timer, snapshotProviderRegistry, metricsPacketsHandlerConfig); testDriver = new RequestHandlerTestDriver(metricsPacketsHandler); } @Test - void status_packet_is_returned_prior_to_first_snapshot() throws Exception { - String response = requestAsString("http://localhost/metrics-packets"); - - List<JsonNode> packets = toJsonPackets(response); - assertEquals(1, packets.size()); - - JsonNode statusPacket = packets.get(0); - assertEquals(APPLICATION_NAME, statusPacket.get(APPLICATION_KEY).asText(), statusPacket.toString()); - assertEquals(0, statusPacket.get(STATUS_CODE_KEY).asInt(), statusPacket.toString()); - assertEquals("up", statusPacket.get(STATUS_MSG_KEY).asText(), statusPacket.toString()); - } - - @Test void metrics_are_included_after_snapshot() throws Exception { createSnapshotWithCountMetric("counter", 1, null); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); - assertEquals(2, packets.size()); + assertEquals(1, packets.size()); - JsonNode counterPacket = packets.get(1); + JsonNode counterPacket = packets.get(0); assertCountMetric(counterPacket, "counter.count", 1); } @@ -66,7 +51,7 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { void metadata_is_included_in_each_metrics_packet() throws Exception { createSnapshotWithCountMetric("counter", 1, null); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); - JsonNode counterPacket = packets.get(1); + JsonNode counterPacket = packets.get(0); assertTrue(counterPacket.has(TIMESTAMP_KEY)); assertTrue(counterPacket.has(APPLICATION_KEY)); @@ -77,7 +62,7 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { void timestamp_resolution_is_in_seconds() throws Exception { createSnapshotWithCountMetric("counter", 1, null); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); - JsonNode counterPacket = packets.get(1); + JsonNode counterPacket = packets.get(0); assertEquals(SNAPSHOT_INTERVAL / 1000L, counterPacket.get(TIMESTAMP_KEY).asLong()); } @@ -90,7 +75,7 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { snapshotProvider.setSnapshot(snapshot); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); - JsonNode gaugeMetric = packets.get(1).get(METRICS_KEY); + JsonNode gaugeMetric = packets.get(0).get(METRICS_KEY); assertEquals(0.2, gaugeMetric.get("gauge.last").asDouble(), 0.1); assertEquals(0.2, gaugeMetric.get("gauge.average").asDouble(), 0.1); @@ -103,7 +88,7 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { createSnapshotWithCountMetric("counter", 1, context); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); - JsonNode counterPacket = packets.get(1); + JsonNode counterPacket = packets.get(0); assertTrue(counterPacket.has(DIMENSIONS_KEY)); JsonNode dimensions = counterPacket.get(DIMENSIONS_KEY); @@ -119,8 +104,8 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { snapshotProvider.setSnapshot(snapshot); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); - assertEquals(2, packets.size()); - JsonNode countersPacket = packets.get(1); + assertEquals(1, packets.size()); + JsonNode countersPacket = packets.get(0); assertEquals("value1", countersPacket.get(DIMENSIONS_KEY).get("dim1").asText()); assertCountMetric(countersPacket, "counter1.count", 1); @@ -137,7 +122,7 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { snapshotProvider.setSnapshot(snapshot); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); - assertEquals(3, packets.size()); + assertEquals(2, packets.size()); } @Test @@ -150,7 +135,7 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { snapshotProvider.setSnapshot(snapshot); var packets = incrementTimeAndGetJsonPackets(); - assertEquals(3, packets.size()); + assertEquals(2, packets.size()); packets.forEach(packet -> { if (!packet.has(DIMENSIONS_KEY)) return; diff --git a/container-dependency-versions/pom.xml b/container-dependency-versions/pom.xml index a511e14de0a..92d06a5c1bd 100644 --- a/container-dependency-versions/pom.xml +++ b/container-dependency-versions/pom.xml @@ -243,8 +243,8 @@ <error-prone-annotations.version>2.18.0</error-prone-annotations.version> <guava.version>27.1-jre</guava.version> <guice.version>4.2.3</guice.version> - <jackson2.version>2.15.0</jackson2.version> - <jackson-databind.version>2.15.0</jackson-databind.version> + <jackson2.version>2.15.2</jackson2.version> + <jackson-databind.version>2.15.2</jackson-databind.version> <javax.inject.version>1</javax.inject.version> <javax.servlet-api.version>3.1.0</javax.servlet-api.version> <javax.ws.rs-api.version>2.0.1</javax.ws.rs-api.version> diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/DataplaneProxyService.java b/container-disc/src/main/java/com/yahoo/container/jdisc/DataplaneProxyService.java index 83e793dace2..6d871b7283f 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/DataplaneProxyService.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/DataplaneProxyService.java @@ -99,7 +99,7 @@ public class DataplaneProxyService extends AbstractComponent { try { Process stopCommand = new ProcessBuilder().command( "nginx", - "-s", "reload" + "-s", "stop" ).start(); int exitCode = stopCommand.waitFor(); if (exitCode != 0) { diff --git a/container-search/src/main/java/com/yahoo/search/logging/LoggerEntry.java b/container-search/src/main/java/com/yahoo/search/logging/LoggerEntry.java index a7ef0b9300f..80ff967f779 100644 --- a/container-search/src/main/java/com/yahoo/search/logging/LoggerEntry.java +++ b/container-search/src/main/java/com/yahoo/search/logging/LoggerEntry.java @@ -123,10 +123,7 @@ public class LoggerEntry { } public Builder blob(String blob) { - byte[] bytes = Utf8.toBytes(blob); - this.blob = ByteBuffer.allocate(bytes.length); - this.blob.put(bytes); - return this; + return this.blob(Utf8.toBytes(blob)); } public Builder track(String track) { diff --git a/container-search/src/main/java/com/yahoo/search/searchers/QueryValidator.java b/container-search/src/main/java/com/yahoo/search/searchers/QueryValidator.java index 0b435c2e32d..a4abab3cbef 100644 --- a/container-search/src/main/java/com/yahoo/search/searchers/QueryValidator.java +++ b/container-search/src/main/java/com/yahoo/search/searchers/QueryValidator.java @@ -31,7 +31,7 @@ public class QueryValidator extends Searcher { public Result search(Query query, Execution execution) { var session = execution.context().schemaInfo().newSession(query); ToolBox.visit(new TermSearchValidator(session), query.getModel().getQueryTree().getRoot()); - // ToolBox.visit(new PrefixSearchValidator(session), query.getModel().getQueryTree().getRoot()); TODO: Enable check and QueryValidatorPrefixTest + ToolBox.visit(new PrefixSearchValidator(session), query.getModel().getQueryTree().getRoot()); return execution.search(query); } diff --git a/container-search/src/test/java/com/yahoo/search/searchers/test/QueryValidatorPrefixTest.java b/container-search/src/test/java/com/yahoo/search/searchers/test/QueryValidatorPrefixTest.java index b653e4d97aa..fdc8c773cf2 100644 --- a/container-search/src/test/java/com/yahoo/search/searchers/test/QueryValidatorPrefixTest.java +++ b/container-search/src/test/java/com/yahoo/search/searchers/test/QueryValidatorPrefixTest.java @@ -9,7 +9,6 @@ import com.yahoo.search.schema.SchemaInfo; import com.yahoo.search.searchchain.Execution; import com.yahoo.search.searchers.QueryValidator; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.Disabled; import java.util.List; @@ -21,7 +20,6 @@ import static org.junit.jupiter.api.Assertions.fail; */ public class QueryValidatorPrefixTest { - @Disabled @Test void testPrefixRequiresAttribute() { var indexing = new Cluster.Builder("indexing").addSchema("test1").build(); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dataplanetoken/package-info.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dataplanetoken/package-info.java new file mode 100644 index 00000000000..09cc3a84cd3 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dataplanetoken/package-info.java @@ -0,0 +1,5 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package com.yahoo.vespa.hosted.controller.api.integration.dataplanetoken; + +import com.yahoo.osgi.annotation.ExportPackage;
\ No newline at end of file diff --git a/hosted-tenant-base/pom.xml b/hosted-tenant-base/pom.xml index adde2a32720..6fc8b6b18b8 100644 --- a/hosted-tenant-base/pom.xml +++ b/hosted-tenant-base/pom.xml @@ -38,7 +38,7 @@ <maven-compiler-plugin.version>3.10.1</maven-compiler-plugin.version> <maven-surefire-plugin.version>2.22.2</maven-surefire-plugin.version> <maven-dependency-plugin.version>3.5.0</maven-dependency-plugin.version> - <jackson2.version>2.15.0</jackson2.version> + <jackson2.version>2.15.2</jackson2.version> <!-- NOTE: this must not be overriden by users, and must be in sync with junit version specified in 'tenant-cd-api' --> <vespa.junit.version>5.8.1</vespa.junit.version> <test.categories>!integration</test.categories> diff --git a/maven-plugins/allowed-maven-dependencies.txt b/maven-plugins/allowed-maven-dependencies.txt index f2334a6ef00..d504357b07c 100644 --- a/maven-plugins/allowed-maven-dependencies.txt +++ b/maven-plugins/allowed-maven-dependencies.txt @@ -3,9 +3,9 @@ #[non-test] # Contains dependencies that are not used exclusively in 'test' scope aopalliance:aopalliance:1.0 -com.fasterxml.jackson.core:jackson-annotations:2.15.0 -com.fasterxml.jackson.core:jackson-core:2.15.0 -com.fasterxml.jackson.core:jackson-databind:2.15.0 +com.fasterxml.jackson.core:jackson-annotations:2.15.2 +com.fasterxml.jackson.core:jackson-core:2.15.2 +com.fasterxml.jackson.core:jackson-databind:2.15.2 com.google.errorprone:error_prone_annotations:2.18.0 com.google.guava:failureaccess:1.0.1 com.google.guava:guava:27.1-jre diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/yamas/YamasHandler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/yamas/YamasHandler.java index eb635d8c641..ef23a5ad070 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/yamas/YamasHandler.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/yamas/YamasHandler.java @@ -52,7 +52,7 @@ public class YamasHandler extends HttpHandlerBase { NodeDimensions nodeDimensions) { super(executor); valuesFetcher = new ValuesFetcher(metricsManager, vespaServices, metricsConsumers); - this.nodeMetricGatherer = new NodeMetricGatherer(metricsManager, vespaServices, applicationDimensions, nodeDimensions); + this.nodeMetricGatherer = new NodeMetricGatherer(metricsManager, applicationDimensions, nodeDimensions); this.metricsConsumers = metricsConsumers; } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/yamas/YamasResponse.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/yamas/YamasResponse.java index 49f5036b3fd..6c94de49140 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/yamas/YamasResponse.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/yamas/YamasResponse.java @@ -29,7 +29,7 @@ public class YamasResponse extends HttpResponse { @Override public void render(OutputStream outputStream) throws IOException { - YamasJsonUtil.toJson(metrics, outputStream, true); + YamasJsonUtil.toJson(metrics, outputStream, false); } } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtil.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtil.java index 13b2a8d859c..5086846293b 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtil.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtil.java @@ -107,7 +107,7 @@ public class YamasJsonUtil { for (int i = 0; i < metrics.size() - 1; i++) { toJson(metrics.get(i), generator, addStatus); } - toJson(metrics.get(metrics.size() - 1), generator, true); + toJson(metrics.get(metrics.size() - 1), generator, addStatus); generator.writeEndArray(); generator.writeEndObject(); generator.close(); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/prometheus/PrometheusUtil.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/prometheus/PrometheusUtil.java index 5fe9af24c3d..c2d05a8636e 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/prometheus/PrometheusUtil.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/prometheus/PrometheusUtil.java @@ -57,14 +57,6 @@ public class PrometheusUtil { sampleList.add(new Sample(metricName, labels, labelValues, metric.getValue().doubleValue(), packet.timestamp * 1000)); } } - // convert status message to 0,1 metric - var firstPacket = packets.get(0); - String statusMetricName = serviceName + "_status"; - // MetricsPacket status 0 means OK, but it's the opposite in Prometheus. - double statusMetricValue = (firstPacket.statusCode == 0) ? 1.0 : 0.0; - List<Sample> sampleList = singletonList(new Sample(statusMetricName, emptyList(), emptyList(), - statusMetricValue, firstPacket.timestamp * 1000)); - metricFamilySamples.add(new MetricFamilySamples(statusMetricName, Collector.Type.UNTYPED, "status of service", sampleList)); })); return new PrometheusModel(metricFamilySamples); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java index 02785674103..3da7aef0a12 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java @@ -10,10 +10,8 @@ import ai.vespa.metricsproxy.metric.model.MetricId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; import ai.vespa.metricsproxy.metric.model.ServiceId; import ai.vespa.metricsproxy.service.SystemPollerProvider; -import ai.vespa.metricsproxy.service.VespaServices; import com.fasterxml.jackson.databind.JsonNode; import com.yahoo.component.annotation.Inject; -import com.yahoo.container.jdisc.state.CoredumpGatherer; import com.yahoo.container.jdisc.state.FileWrapper; import com.yahoo.container.jdisc.state.HostLifeGatherer; @@ -21,9 +19,6 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; - -import static ai.vespa.metricsproxy.node.ServiceHealthGatherer.gatherServiceHealthMetrics; /** * Fetches miscellaneous system metrics for node, including @@ -35,15 +30,13 @@ import static ai.vespa.metricsproxy.node.ServiceHealthGatherer.gatherServiceHeal */ public class NodeMetricGatherer { - private final VespaServices vespaServices; private final ApplicationDimensions applicationDimensions; private final NodeDimensions nodeDimensions; private final MetricsManager metricsManager; @Inject - public NodeMetricGatherer(MetricsManager metricsManager, VespaServices vespaServices, ApplicationDimensions applicationDimensions, NodeDimensions nodeDimensions) { + public NodeMetricGatherer(MetricsManager metricsManager, ApplicationDimensions applicationDimensions, NodeDimensions nodeDimensions) { this.metricsManager = metricsManager; - this.vespaServices = vespaServices; this.applicationDimensions = applicationDimensions; this.nodeDimensions = nodeDimensions; } @@ -51,10 +44,7 @@ public class NodeMetricGatherer { public List<MetricsPacket> gatherMetrics() { FileWrapper fileWrapper = new FileWrapper(); List<MetricsPacket.Builder> metricPacketBuilders = new ArrayList<>(); - metricPacketBuilders.addAll(gatherServiceHealthMetrics(vespaServices)); - JsonNode coredumpPacket = CoredumpGatherer.gatherCoredumpMetrics(fileWrapper); - addObjectToBuilders(metricPacketBuilders, coredumpPacket); if (SystemPollerProvider.runningOnLinux()) { JsonNode packet = HostLifeGatherer.getHostLifePacket(fileWrapper); addObjectToBuilders(metricPacketBuilders, packet); @@ -71,8 +61,6 @@ public class NodeMetricGatherer { protected static void addObjectToBuilders(List<MetricsPacket.Builder> builders, JsonNode object) { MetricsPacket.Builder builder = new MetricsPacket.Builder(ServiceId.toServiceId(object.get("application").textValue())); builder.timestamp(object.get("timestamp").longValue()); - if (object.has("status_code")) builder.statusCode(object.get("status_code").intValue()); - if (object.has("status_msg")) builder.statusMessage(object.get("status_msg").textValue()); if (object.has("metrics")) { JsonNode metrics = object.get("metrics"); Iterator<?> keys = metrics.fieldNames(); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/ServiceHealthGatherer.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/ServiceHealthGatherer.java deleted file mode 100644 index d09f2aff3e5..00000000000 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/ServiceHealthGatherer.java +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package ai.vespa.metricsproxy.node; - -import ai.vespa.metricsproxy.metric.HealthMetric; -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.VespaServices; - -import java.time.Instant; -import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; - -/** - * @author olaa - */ -public class ServiceHealthGatherer { - - protected static List<MetricsPacket.Builder> gatherServiceHealthMetrics(VespaServices vespaServices) { - return vespaServices.getVespaServices() - .stream() - .map(service -> { - HealthMetric healt = service.getHealth(); - return new MetricsPacket.Builder(service.getMonitoringName()) - .timestamp(Instant.now().getEpochSecond()) - .statusMessage(healt.getStatus().status) - .statusCode(healt.getStatus().code) - .putDimension(DimensionId.toDimensionId("instance"), service.getInstanceName()) - .putDimension(DimensionId.toDimensionId("metrictype"), "health") - .addConsumers(Set.of(ConsumerId.toConsumerId("Vespa"))); - }) - .toList(); - } - -} diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/prometheus/PrometheusHandlerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/prometheus/PrometheusHandlerTest.java index 2c216272022..90536ddcfdb 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/prometheus/PrometheusHandlerTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/prometheus/PrometheusHandlerTest.java @@ -60,23 +60,12 @@ public class PrometheusHandlerTest extends HttpHandlerTestBase { } @Test - public void response_contains_node_status() { - assertTrue(valuesResponse.contains("vespa_node_status 1.0")); - } - - @Test public void response_contains_node_metrics() { String cpu = getLine(valuesResponse, CPU_METRIC + "{"); assertTrue(cpu.contains("} 12.345")); // metric value } @Test - public void response_contains_service_status() { - assertTrue(valuesResponse.contains("vespa_dummy_status 1.0")); - assertTrue(valuesResponse.contains("vespa_down_service_status 0.0")); - } - - @Test public void response_contains_service_metrics() { String dummy0 = getLine(valuesResponse, DummyService.NAME + "0"); assertTrue(dummy0.contains("c_test")); // metric name diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/yamas/YamasHandlerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/yamas/YamasHandlerTest.java index a4e61d5965e..346fc6a462b 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/yamas/YamasHandlerTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/yamas/YamasHandlerTest.java @@ -13,6 +13,7 @@ import java.util.List; import java.util.concurrent.Executors; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class YamasHandlerTest extends HttpHandlerTestBase { @@ -48,8 +49,9 @@ public class YamasHandlerTest extends HttpHandlerTestBase { } @Test - public void value_response_contains_coredump_metric() { - assertTrue(valuesResponse.contains("\"application\":\"system-coredumps-processing\",\"routing\":{\"yamas\":{\"namespaces\":[\"Vespa\"]}}")); + public void value_response_does_not_contain_status() { + assertFalse(valuesResponse.contains("status_code")); + assertFalse(valuesResponse.contains("status_msg")); } } diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModelTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModelTest.java index a5bc7a0877a..3e85166430d 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModelTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModelTest.java @@ -28,7 +28,7 @@ import static org.junit.Assert.assertTrue; */ public class YamasJsonModelTest { - private static final String EXPECTED_JSON = "{\"metrics\":[{\"status_code\":0,\"timestamp\":1400047900,\"application\":\"vespa.searchnode\",\"metrics\":{\"cpu\":55.5555555555555,\"memory_virt\":22222222222,\"memory_rss\":5555555555},\"dimensions\":{\"applicationName\":\"app\",\"tenantName\":\"tenant\",\"metrictype\":\"system\",\"instance\":\"searchnode\",\"applicationInstance\":\"default\",\"clustername\":\"cluster\"},\"routing\":{\"yamas\":{\"namespaces\":[\"Vespa\"]}},\"status_msg\":\"Data collected successfully\"}]}"; + private static final String EXPECTED_JSON = "{\"metrics\":[{\"timestamp\":1400047900,\"application\":\"vespa.searchnode\",\"metrics\":{\"cpu\":55.5555555555555,\"memory_virt\":22222222222,\"memory_rss\":5555555555},\"dimensions\":{\"applicationName\":\"app\",\"tenantName\":\"tenant\",\"metrictype\":\"system\",\"instance\":\"searchnode\",\"applicationInstance\":\"default\",\"clustername\":\"cluster\"},\"routing\":{\"yamas\":{\"namespaces\":[\"Vespa\"]}}}]}"; @Test public void array_definition_creates_correct_json() throws IOException { @@ -69,7 +69,7 @@ public class YamasJsonModelTest { assertEquals(5.555555555E9, metricsPacket.metrics().get(toMetricId("memory_rss")).doubleValue(), 0.1d); //Not using custom double rendrer // Serialize and verify - String string = YamasJsonUtil.toJson(List.of(metricsPacket), true); + String string = YamasJsonUtil.toJson(List.of(metricsPacket), false); assertEquals(EXPECTED_JSON, string); } diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtilTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtilTest.java index ebd80b38a42..133461e3658 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtilTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtilTest.java @@ -31,15 +31,6 @@ public class YamasJsonUtilTest { private static ArrayNode metrics(List<MetricsPacket> packets, boolean addStatus) throws IOException { return (ArrayNode) jsonMapper.readTree(YamasJsonUtil.toJson(packets, addStatus)).get("metrics"); } - @Test - public void json_model_gets_null_status_by_default() throws IOException { - ArrayNode json = metrics(List.of(new MetricsPacket.Builder(toServiceId("foo")).build(), - new MetricsPacket.Builder(toServiceId("bar")).build()), false); - assertFalse(json.get(0).has("status_code")); - assertFalse(json.get(0).has("status_msg")); - assertTrue(json.get(1).has("status_code")); - assertTrue(json.get(1).has("status_msg")); - } @Test public void status_is_included_in_json_model_when_explicitly_asked_for() throws IOException { diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java index 0de3526b40d..635365f9462 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/node/NodeMetricGathererTest.java @@ -30,8 +30,6 @@ public class NodeMetricGathererTest { MetricsPacket packet = builders.remove(0).build(); assertEquals("host_life", packet.service.id); - assertEquals(0, packet.statusCode); - assertEquals("OK", packet.statusMessage); assertEquals(123, packet.timestamp); assertEquals(12l, packet.metrics().get(MetricId.toMetricId("uptime"))); assertEquals(1l, packet.metrics().get(MetricId.toMetricId("alive"))); @@ -41,8 +39,6 @@ public class NodeMetricGathererTest { private JsonNode generateHostLifePacket() { ObjectNode jsonObject = jsonMapper.createObjectNode(); - jsonObject.put("status_code", 0); - jsonObject.put("status_msg", "OK"); jsonObject.put("timestamp", 123); jsonObject.put("application", "host_life"); ObjectNode metrics = jsonMapper.createObjectNode(); diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcMetricsTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcMetricsTest.java index 1e7a398b3d0..142740356d7 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcMetricsTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcMetricsTest.java @@ -167,7 +167,6 @@ public class RpcMetricsTest { } } - verifyStatusMessage(metrics.get(metrics.size() - 1)); } private void verfiyMetricsFromServiceObject(VespaService service) { @@ -266,12 +265,4 @@ public class RpcMetricsTest { return returnValue; } - private static void verifyStatusMessage(JsonNode jsonObject) { - assertEquals(0, jsonObject.get("status_code").intValue()); - assertNotNull(jsonObject.get("status_msg")); - assertNotNull(jsonObject.get("application")); - assertNotNull(jsonObject.get("routing")); - assertEquals(4, jsonObject.size()); - } - } diff --git a/parent/pom.xml b/parent/pom.xml index f68b2d0c068..d63e2f4d8f9 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -1078,7 +1078,7 @@ <dependency> <groupId>org.xerial.snappy</groupId> <artifactId>snappy-java</artifactId> - <version>1.1.7</version> + <version>1.1.10.1</version> </dependency> <dependency> <groupId>io.dropwizard.metrics</groupId> diff --git a/searchlib/src/tests/docstore/document_store/visitcache_test.cpp b/searchlib/src/tests/docstore/document_store/visitcache_test.cpp index 14e3c19fe33..3f80bb6004f 100644 --- a/searchlib/src/tests/docstore/document_store/visitcache_test.cpp +++ b/searchlib/src/tests/docstore/document_store/visitcache_test.cpp @@ -62,7 +62,7 @@ TEST("require that BlobSet can be built") { a.append(9, B("bbbbb",5)); verifyAB(a); CompressionConfig cfg(CompressionConfig::LZ4); - CompressedBlobSet ca(cfg, a); + CompressedBlobSet ca(cfg, std::move(a)); BlobSet b = ca.getBlobSet(); verifyAB(b); } diff --git a/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp b/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp index 1080d44f2fb..2bf2a38b7e6 100644 --- a/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp +++ b/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp @@ -368,13 +368,13 @@ TEST("test visit cache does not cache empty ones and is able to access some back IDataStore & datastore = store.getStore(); VisitCache visitCache(datastore, 100000, CompressionConfig::Type::LZ4); - EXPECT_EQUAL(0u, visitCache.read({1}).size()); + EXPECT_EQUAL(12u, visitCache.read({1}).bytesAllocated()); EXPECT_TRUE(visitCache.read({1}).empty()); datastore.write(1,1, A7, 7); - EXPECT_EQUAL(0u, visitCache.read({2}).size()); + EXPECT_EQUAL(12u, visitCache.read({2}).bytesAllocated()); CompressedBlobSet cbs = visitCache.read({1}); EXPECT_FALSE(cbs.empty()); - EXPECT_EQUAL(19u, cbs.size()); + EXPECT_EQUAL(19u, cbs.bytesAllocated()); BlobSet bs(cbs.getBlobSet()); EXPECT_EQUAL(7u, bs.get(1).size()); EXPECT_EQUAL(0, strncmp(A7, bs.get(1).c_str(), 7)); @@ -664,14 +664,14 @@ TEST("test that the integrated visit cache works.") { vcs.remove(17); TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 104, 97, BASE_SZ-671)); vcs.verifyVisit({7,9,17,19,67,88,89}, {7,9,19,67,88,89}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 105, 98, BASE_SZ-89)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 105, 98, BASE_SZ-70)); vcs.verifyVisit({41, 42}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 106, 99, BASE_SZ+215)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 106, 99, BASE_SZ+230)); vcs.verifyVisit({43, 44}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 107, 100, BASE_SZ+520)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 107, 100, BASE_SZ+540)); vcs.verifyVisit({41, 42, 43, 44}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 108, 99, BASE_SZ+340)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 108, 99, BASE_SZ+360)); } TEST("testWriteRead") { diff --git a/searchlib/src/vespa/searchlib/docstore/documentstore.cpp b/searchlib/src/vespa/searchlib/docstore/documentstore.cpp index 49beed34f08..7d585007d76 100644 --- a/searchlib/src/vespa/searchlib/docstore/documentstore.cpp +++ b/searchlib/src/vespa/searchlib/docstore/documentstore.cpp @@ -437,7 +437,8 @@ DocumentStore::getFileChunkStats() const return _backingStore.getFileChunkStats(); } -CacheStats DocumentStore::getCacheStats() const { +CacheStats +DocumentStore::getCacheStats() const { CacheStats visitStats = _visitCache->getCacheStats(); CacheStats singleStats = _cache->get_stats(); singleStats.add_extra_misses(_uncached_lookups.load(std::memory_order_relaxed)); diff --git a/searchlib/src/vespa/searchlib/docstore/visitcache.cpp b/searchlib/src/vespa/searchlib/docstore/visitcache.cpp index c99bb50d4f8..b61cf49c438 100644 --- a/searchlib/src/vespa/searchlib/docstore/visitcache.cpp +++ b/searchlib/src/vespa/searchlib/docstore/visitcache.cpp @@ -2,12 +2,14 @@ #include "visitcache.h" #include "ibucketizer.h" -#include <vespa/vespalib/stllike/cache.hpp> -#include <vespa/vespalib/stllike/hash_map.hpp> +#include <vespa/vespalib/stllike/hash_set.h> +#include <vespa/vespalib/stllike/hash_map.h> +#include <vespa/vespalib/stllike/cache.h> #include <vespa/vespalib/data/databuffer.h> #include <vespa/vespalib/util/compressor.h> #include <vespa/vespalib/util/memory_allocator.h> -#include <algorithm> +#include <vespa/vespalib/stllike/cache.hpp> +#include <vespa/vespalib/stllike/hash_map.hpp> namespace search::docstore { @@ -17,14 +19,14 @@ using vespalib::DataBuffer; using vespalib::alloc::Alloc; using vespalib::alloc::MemoryAllocator; -KeySet::KeySet(uint32_t key) : - _keys() +KeySet::KeySet(uint32_t key) + : _keys() { _keys.push_back(key); } -KeySet::KeySet(const IDocumentStore::LidVector &keys) : - _keys(keys) +KeySet::KeySet(const IDocumentStore::LidVector &keys) + : _keys(keys) { std::sort(_keys.begin(), _keys.end()); } @@ -34,26 +36,26 @@ KeySet::contains(const KeySet &rhs) const { return std::includes(_keys.begin(), _keys.end(), rhs._keys.begin(), rhs._keys.end()); } -BlobSet::BlobSet() : - _positions(), - _buffer(Alloc::alloc(0, 16 * MemoryAllocator::HUGEPAGE_SIZE), 0) +BlobSet::BlobSet() + : _positions(), + _buffer(Alloc::alloc(0, 16 * MemoryAllocator::HUGEPAGE_SIZE), 0) { } BlobSet::~BlobSet() = default; namespace { -size_t getBufferSize(const BlobSet::Positions & p) { +size_t +getBufferSize(const BlobSet::Positions & p) { return p.empty() ? 0 : p.back().offset() + p.back().size(); } } -BlobSet::BlobSet(const Positions & positions, Alloc && buffer) : - _positions(positions), - _buffer(std::move(buffer), getBufferSize(_positions)) -{ -} +BlobSet::BlobSet(Positions positions, Alloc && buffer) noexcept + : _positions(std::move(positions)), + _buffer(std::move(buffer), getBufferSize(_positions)) +{ } void BlobSet::append(uint32_t lid, ConstBufferRef blob) { @@ -74,29 +76,30 @@ BlobSet::get(uint32_t lid) const return buf; } -CompressedBlobSet::CompressedBlobSet() : - _compression(CompressionConfig::Type::LZ4), - _positions(), - _buffer() -{ -} +CompressedBlobSet::CompressedBlobSet() noexcept + : _positions(), + _buffer(), + _used(0), + _compression(CompressionConfig::Type::LZ4) +{ } CompressedBlobSet::~CompressedBlobSet() = default; - -CompressedBlobSet::CompressedBlobSet(CompressionConfig compression, const BlobSet & uncompressed) : - _compression(compression.type), - _positions(uncompressed.getPositions()), - _buffer() +CompressedBlobSet::CompressedBlobSet(CompressionConfig compression, BlobSet uncompressed) + : _positions(uncompressed.stealPositions()), + _buffer(), + _used(0), + _compression(compression.type) { if ( ! _positions.empty() ) { DataBuffer compressed; ConstBufferRef org = uncompressed.getBuffer(); _compression = vespalib::compression::compress(compression, org, compressed, false); - _buffer = std::make_shared<vespalib::MallocPtr>(compressed.getDataLen()); - memcpy(*_buffer, compressed.getData(), compressed.getDataLen()); + _used = compressed.getDataLen(); + _buffer = std::make_shared<Alloc>(Alloc::alloc(_used)); + memcpy(_buffer->get(), compressed.getData(), _used); } else { - _buffer = std::make_shared<vespalib::MallocPtr>(); + _buffer = std::make_shared<Alloc>(); } } @@ -108,12 +111,13 @@ CompressedBlobSet::getBlobSet() const DataBuffer uncompressed(0, 1, Alloc::alloc(0, 16 * MemoryAllocator::HUGEPAGE_SIZE)); if ( ! _positions.empty() ) { decompress(_compression, getBufferSize(_positions), - ConstBufferRef(_buffer->c_str(), _buffer->size()), uncompressed, false); + ConstBufferRef(_buffer->get(), _used), uncompressed, false); } return BlobSet(_positions, std::move(uncompressed).stealBuffer()); } -size_t CompressedBlobSet::size() const { +size_t +CompressedBlobSet::bytesAllocated() const { return _positions.capacity() * sizeof(BlobSet::Positions::value_type) + _buffer->size(); } @@ -122,13 +126,10 @@ namespace { class VisitCollector : public IBufferVisitor { public: - VisitCollector() : - _blobSet() - { } + VisitCollector(BlobSet & blobSet) : _blobSet(blobSet) { } void visit(uint32_t lid, ConstBufferRef buf) override; - const BlobSet & getBlobSet() const { return _blobSet; } private: - BlobSet _blobSet; + BlobSet & _blobSet; }; void @@ -138,13 +139,52 @@ VisitCollector::visit(uint32_t lid, ConstBufferRef buf) { } } +struct ByteSize { + size_t operator() (const CompressedBlobSet & arg) const noexcept { return arg.bytesAllocated(); } +}; + } +using CacheParams = vespalib::CacheParam< + vespalib::LruParam<KeySet, CompressedBlobSet>, + VisitCache::BackingStore, + vespalib::zero<KeySet>, + ByteSize +>; + +/** + * This extends the default thread safe cache implementation so that + * it will correctly invalidate the cached sets when objects are removed/updated. + * It will also detect the addition of new objects to any of the sets upon first + * usage of the set and then invalidate and perform fresh visit of the backing store. + */ +class VisitCache::Cache : public vespalib::cache<CacheParams> { +public: + Cache(BackingStore & b, size_t maxBytes); + ~Cache() override; + CompressedBlobSet readSet(const KeySet & keys); + void removeKey(uint32_t key); + vespalib::MemoryUsage getStaticMemoryUsage() const override; +private: + void locateAndInvalidateOtherSubsets(const UniqueLock & cacheGuard, const KeySet & keys); + using IdSet = vespalib::hash_set<uint64_t>; + using Parent = vespalib::cache<CacheParams>; + using LidUniqueKeySetId = vespalib::hash_map<uint32_t, uint64_t>; + using IdKeySetMap = vespalib::hash_map<uint64_t, KeySet>; + IdSet findSetsContaining(const UniqueLock &, const KeySet & keys) const; + void onInsert(const K & key) override; + void onRemove(const K & key) override; + LidUniqueKeySetId _lid2Id; + IdKeySetMap _id2KeySet; +}; + bool VisitCache::BackingStore::read(const KeySet &key, CompressedBlobSet &blobs) const { - VisitCollector collector; + BlobSet blobSet; + blobSet.reserve(key.getKeys().size()); + VisitCollector collector(blobSet); _backingStore.read(key.getKeys(), collector); - blobs = CompressedBlobSet(_compression.load(std::memory_order_relaxed), collector.getBlobSet()); + blobs = CompressedBlobSet(_compression.load(std::memory_order_relaxed), std::move(blobSet)); return ! blobs.empty(); } @@ -157,8 +197,9 @@ VisitCache::BackingStore::reconfigure(CompressionConfig compression) { VisitCache::VisitCache(IDataStore &store, size_t cacheSize, CompressionConfig compression) : _store(store, compression), _cache(std::make_unique<Cache>(_store, cacheSize)) -{ -} +{ } + +VisitCache::~VisitCache() = default; void VisitCache::reconfigure(size_t cacheSize, CompressionConfig compression) { @@ -166,6 +207,10 @@ VisitCache::reconfigure(size_t cacheSize, CompressionConfig compression) { _cache->setCapacityBytes(cacheSize); } +vespalib::MemoryUsage +VisitCache::getStaticMemoryUsage() const { + return _cache->getStaticMemoryUsage(); +} VisitCache::Cache::IdSet VisitCache::Cache::findSetsContaining(const UniqueLock &, const KeySet & keys) const { @@ -237,8 +282,7 @@ VisitCache::Cache::removeKey(uint32_t subKey) { auto cacheGuard = getGuard(); const auto foundLid = _lid2Id.find(subKey); if (foundLid != _lid2Id.end()) { - K keySet = _id2KeySet[foundLid->second]; - invalidate(cacheGuard, keySet); + invalidate(cacheGuard, _id2KeySet[foundLid->second]); } } diff --git a/searchlib/src/vespa/searchlib/docstore/visitcache.h b/searchlib/src/vespa/searchlib/docstore/visitcache.h index baa594b8d28..2f312ec3011 100644 --- a/searchlib/src/vespa/searchlib/docstore/visitcache.h +++ b/searchlib/src/vespa/searchlib/docstore/visitcache.h @@ -3,14 +3,9 @@ #pragma once #include "idocumentstore.h" -#include <vespa/vespalib/stllike/cache.h> -#include <vespa/vespalib/stllike/hash_set.h> -#include <vespa/vespalib/stllike/hash_map.h> #include <vespa/vespalib/util/alloc.h> -#include <vespa/vespalib/util/memory.h> #include <vespa/vespalib/util/compressionconfig.h> #include <vespa/vespalib/objects/nbostream.h> -#include <vespa/document/util/bytebuffer.h> namespace search::docstore { @@ -19,7 +14,7 @@ namespace search::docstore { **/ class KeySet { public: - KeySet() : _keys() { } + KeySet() noexcept : _keys() { } KeySet(uint32_t key); explicit KeySet(const IDocumentStore::LidVector &keys); uint32_t hash() const noexcept { return _keys.empty() ? 0 : _keys[0]; } @@ -51,12 +46,14 @@ public: using Positions = std::vector<LidPosition>; BlobSet(); - BlobSet(const Positions & positions, vespalib::alloc::Alloc && buffer); - BlobSet(BlobSet &&) = default; - BlobSet &operator = (BlobSet &&) = default; + BlobSet(Positions positions, vespalib::alloc::Alloc && buffer) noexcept; + BlobSet(BlobSet &&) noexcept = default; + BlobSet &operator = (BlobSet &&) noexcept = default; ~BlobSet(); + void reserve(size_t elems) { _positions.reserve(elems);} void append(uint32_t lid, vespalib::ConstBufferRef blob); const Positions & getPositions() const { return _positions; } + Positions && stealPositions() { return std::move(_positions); } vespalib::ConstBufferRef get(uint32_t lid) const; vespalib::ConstBufferRef getBuffer() const { return vespalib::ConstBufferRef(_buffer.data(), _buffer.size()); } private: @@ -73,20 +70,22 @@ private: class CompressedBlobSet { public: using CompressionConfig = vespalib::compression::CompressionConfig; - CompressedBlobSet(); - CompressedBlobSet(CompressionConfig compression, const BlobSet & uncompressed); - CompressedBlobSet(CompressedBlobSet && rhs) = default; - CompressedBlobSet & operator=(CompressedBlobSet && rhs) = default; + CompressedBlobSet() noexcept; + CompressedBlobSet(CompressionConfig compression, BlobSet uncompressed); + CompressedBlobSet(CompressedBlobSet && rhs) noexcept = default; + CompressedBlobSet & operator=(CompressedBlobSet && rhs) noexcept = default; CompressedBlobSet(const CompressedBlobSet & rhs) = default; CompressedBlobSet & operator=(const CompressedBlobSet & rhs) = default; ~CompressedBlobSet(); - size_t size() const; + size_t bytesAllocated() const; bool empty() const { return _positions.empty(); } BlobSet getBlobSet() const; private: - CompressionConfig::Type _compression; + using Alloc = vespalib::alloc::Alloc; BlobSet::Positions _positions; - std::shared_ptr<vespalib::MallocPtr> _buffer; + std::shared_ptr<Alloc> _buffer; + uint32_t _used; + CompressionConfig::Type _compression; }; /** @@ -98,26 +97,27 @@ class VisitCache { public: using CompressionConfig = vespalib::compression::CompressionConfig; VisitCache(IDataStore &store, size_t cacheSize, CompressionConfig compression); + ~VisitCache(); CompressedBlobSet read(const IDocumentStore::LidVector & keys) const; void remove(uint32_t key); void invalidate(uint32_t key) { remove(key); } vespalib::CacheStats getCacheStats() const; - vespalib::MemoryUsage getStaticMemoryUsage() const { return _cache->getStaticMemoryUsage(); } + vespalib::MemoryUsage getStaticMemoryUsage() const; void reconfigure(size_t cacheSize, CompressionConfig compression); -private: + /** - * This implments the interface the cache uses when it has a cache miss. - * It wraps an IDataStore. Given a set of lids it will visit all objects - * and compress them as a complete set to maximize compression rate. - * As this is a readonly cache the write/erase methods are noops. - */ + * This implments the interface the cache uses when it has a cache miss. + * It wraps an IDataStore. Given a set of lids it will visit all objects + * and compress them as a complete set to maximize compression rate. + * As this is a readonly cache the write/erase methods are noops. + */ class BackingStore { public: - BackingStore(IDataStore &store, CompressionConfig compression) : - _backingStore(store), - _compression(compression) + BackingStore(IDataStore &store, CompressionConfig compression) + : _backingStore(store), + _compression(compression) { } bool read(const KeySet &key, CompressedBlobSet &blobs) const; void write(const KeySet &, const CompressedBlobSet &) { } @@ -128,40 +128,9 @@ private: IDataStore &_backingStore; std::atomic<CompressionConfig> _compression; }; +private: - using CacheParams = vespalib::CacheParam< - vespalib::LruParam<KeySet, CompressedBlobSet>, - BackingStore, - vespalib::zero<KeySet>, - vespalib::size<CompressedBlobSet> - >; - - /** - * This extends the default thread safe cache implementation so that - * it will correctly invalidate the cached sets when objects are removed/updated. - * It will also detect the addition of new objects to any of the sets upon first - * usage of the set and then invalidate and perform fresh visit of the backing store. - */ - class Cache : public vespalib::cache<CacheParams> { - public: - Cache(BackingStore & b, size_t maxBytes); - ~Cache() override; - CompressedBlobSet readSet(const KeySet & keys); - void removeKey(uint32_t key); - vespalib::MemoryUsage getStaticMemoryUsage() const override; - private: - void locateAndInvalidateOtherSubsets(const UniqueLock & cacheGuard, const KeySet & keys); - using IdSet = vespalib::hash_set<uint64_t>; - using Parent = vespalib::cache<CacheParams>; - using LidUniqueKeySetId = vespalib::hash_map<uint32_t, uint64_t>; - using IdKeySetMap = vespalib::hash_map<uint64_t, KeySet>; - IdSet findSetsContaining(const UniqueLock &, const KeySet & keys) const; - void onInsert(const K & key) override; - void onRemove(const K & key) override; - LidUniqueKeySetId _lid2Id; - IdKeySetMap _id2KeySet; - }; - + class Cache; BackingStore _store; std::unique_ptr<Cache> _cache; }; diff --git a/storage/src/tests/common/dummystoragelink.h b/storage/src/tests/common/dummystoragelink.h index 8da92917c08..8e68f2e5a70 100644 --- a/storage/src/tests/common/dummystoragelink.h +++ b/storage/src/tests/common/dummystoragelink.h @@ -8,7 +8,6 @@ #include <string> #include <vector> #include <vespa/storage/common/storagelink.h> -#include <vespa/storage/common/bucketmessages.h> #include <vespa/storageapi/message/internal.h> namespace storage { diff --git a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp index bbe2af732f5..f39222722fc 100644 --- a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp +++ b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp @@ -1713,21 +1713,6 @@ TEST_F(FileStorManagerTest, set_bucket_active_state) { StorBucketDatabase::WrappedEntry entry(_node->getStorageBucketDatabase().get(bid, "foo")); EXPECT_TRUE(entry->info.isActive()); } - // Trigger bucket info to be read back into the database - { - auto cmd = std::make_shared<ReadBucketInfo>(makeDocumentBucket(bid)); - top.sendDown(cmd); - top.waitForMessages(1, _waitTime); - ASSERT_EQ(1, top.getNumReplies()); - auto reply = std::dynamic_pointer_cast<ReadBucketInfoReply>(top.getReply(0)); - top.reset(); - ASSERT_TRUE(reply.get()); - } - // Should not have lost active flag - { - StorBucketDatabase::WrappedEntry entry(_node->getStorageBucketDatabase().get(bid, "foo")); - EXPECT_TRUE(entry->info.isActive()); - } { auto cmd = std::make_shared<api::SetBucketStateCommand>( diff --git a/storage/src/vespa/storage/bucketdb/bucketmanager.h b/storage/src/vespa/storage/bucketdb/bucketmanager.h index ce60ec88bff..cb0bc6a9f95 100644 --- a/storage/src/vespa/storage/bucketdb/bucketmanager.h +++ b/storage/src/vespa/storage/bucketdb/bucketmanager.h @@ -11,7 +11,6 @@ #include "storbucketdb.h" #include <vespa/config/subscription/configuri.h> #include <vespa/storage/bucketdb/config-stor-bucketdb.h> -#include <vespa/storage/common/bucketmessages.h> #include <vespa/storage/common/servicelayercomponent.h> #include <vespa/storage/common/storagelinkqueued.h> #include <vespa/storageapi/message/bucket.h> diff --git a/storage/src/vespa/storage/common/CMakeLists.txt b/storage/src/vespa/storage/common/CMakeLists.txt index 4a712719d53..6165106f871 100644 --- a/storage/src/vespa/storage/common/CMakeLists.txt +++ b/storage/src/vespa/storage/common/CMakeLists.txt @@ -2,7 +2,6 @@ vespa_add_library(storage_common OBJECT SOURCES bucket_stripe_utils.cpp - bucketmessages.cpp content_bucket_space.cpp content_bucket_space_repo.cpp distributorcomponent.cpp diff --git a/storage/src/vespa/storage/common/bucketmessages.cpp b/storage/src/vespa/storage/common/bucketmessages.cpp deleted file mode 100644 index 1523ad1b0ef..00000000000 --- a/storage/src/vespa/storage/common/bucketmessages.cpp +++ /dev/null @@ -1,108 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "bucketmessages.h" -#include <vespa/vespalib/stllike/asciistream.h> -#include <ostream> - -using document::BucketSpace; - -namespace storage { - -ReadBucketList::ReadBucketList(BucketSpace bucketSpace) - : api::InternalCommand(ID), - _bucketSpace(bucketSpace) -{ } - -ReadBucketList::~ReadBucketList() = default; - -document::Bucket -ReadBucketList::getBucket() const -{ - return document::Bucket(_bucketSpace, document::BucketId()); -} - -void -ReadBucketList::print(std::ostream& out, bool verbose, const std::string& indent) const { - out << "ReadBucketList()"; - - if (verbose) { - out << " : "; - InternalCommand::print(out, true, indent); - } -} - -ReadBucketListReply::ReadBucketListReply(const ReadBucketList& cmd) - : api::InternalReply(ID, cmd), - _bucketSpace(cmd.getBucketSpace()) -{ } - -ReadBucketListReply::~ReadBucketListReply() = default; - -document::Bucket -ReadBucketListReply::getBucket() const -{ - return document::Bucket(_bucketSpace, document::BucketId()); -} - -void -ReadBucketListReply::print(std::ostream& out, bool verbose, const std::string& indent) const -{ - out << "ReadBucketListReply(" << _buckets.size() << " buckets)"; - if (verbose) { - out << " : "; - InternalReply::print(out, true, indent); - } -} - -std::unique_ptr<api::StorageReply> -ReadBucketList::makeReply() { - return std::make_unique<ReadBucketListReply>(*this); -} - -ReadBucketInfo::ReadBucketInfo(const document::Bucket &bucket) - : api::InternalCommand(ID), - _bucket(bucket) -{ } - -ReadBucketInfo::~ReadBucketInfo() = default; - -void -ReadBucketInfo::print(std::ostream& out, bool verbose, const std::string& indent) const -{ - out << "ReadBucketInfo(" << _bucket.getBucketId() << ")"; - - if (verbose) { - out << " : "; - InternalCommand::print(out, true, indent); - } -} - -vespalib::string -ReadBucketInfo::getSummary() const { - vespalib::string s("ReadBucketInfo("); - s.append(_bucket.toString()); - s.append(')'); - return s; -} - -ReadBucketInfoReply::ReadBucketInfoReply(const ReadBucketInfo& cmd) - : api::InternalReply(ID, cmd), - _bucket(cmd.getBucket()) -{ } - -ReadBucketInfoReply::~ReadBucketInfoReply() = default; -void -ReadBucketInfoReply::print(std::ostream& out, bool verbose, const std::string& indent) const { - out << "ReadBucketInfoReply()"; - if (verbose) { - out << " : "; - InternalReply::print(out, true, indent); - } -} - -std::unique_ptr<api::StorageReply> ReadBucketInfo::makeReply() { - return std::make_unique<ReadBucketInfoReply>(*this); -} - -} // storage - diff --git a/storage/src/vespa/storage/common/bucketmessages.h b/storage/src/vespa/storage/common/bucketmessages.h deleted file mode 100644 index ccee12938b2..00000000000 --- a/storage/src/vespa/storage/common/bucketmessages.h +++ /dev/null @@ -1,108 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include <vespa/persistence/spi/result.h> -#include <vespa/storageapi/message/internal.h> -#include <vespa/storageapi/buckets/bucketinfo.h> - -namespace storage { - -/** - * @class ReadBucketList - * @ingroup common - * - * @brief List buckets existing in a bucket space. - */ -class ReadBucketList : public api::InternalCommand { - document::BucketSpace _bucketSpace; - -public: - using UP = std::unique_ptr<ReadBucketList>; - static constexpr uint32_t ID = 2003; - - ReadBucketList(document::BucketSpace bucketSpace); - ~ReadBucketList(); - document::BucketSpace getBucketSpace() const { return _bucketSpace; } - document::Bucket getBucket() const override; - - std::unique_ptr<api::StorageReply> makeReply() override; - - void print(std::ostream& out, bool verbose, const std::string& indent) const override; -}; - - -/** - * @class ReadBucketListReply - * @ingroup common - */ -class ReadBucketListReply : public api::InternalReply { - document::BucketSpace _bucketSpace; - spi::BucketIdListResult::List _buckets; - -public: - using UP = std::unique_ptr<ReadBucketListReply>; - using SP = std::shared_ptr<ReadBucketListReply>; - static constexpr uint32_t ID = 2004; - - ReadBucketListReply(const ReadBucketList& cmd); - ~ReadBucketListReply(); - - document::BucketSpace getBucketSpace() const { return _bucketSpace; } - document::Bucket getBucket() const override; - - spi::BucketIdListResult::List& getBuckets() { return _buckets; } - const spi::BucketIdListResult::List& getBuckets() const { - return _buckets; - } - - void print(std::ostream& out, bool verbose, const std::string& indent) const override; -}; - -/** - * @class ReadBucketInfo - * @ingroup common - * - * @brief Get more detailed information about a set of buckets. - * - * The distributor wants some information for each bucket, that one - * have to open the bucket and read its headers to find. This class is - * used to retrieve such information. - */ -class ReadBucketInfo : public api::InternalCommand { - document::Bucket _bucket; - -public: - static constexpr uint32_t ID = 2005; - - ReadBucketInfo(const document::Bucket &bucket); - ~ReadBucketInfo(); - - document::Bucket getBucket() const override { return _bucket; } - - std::unique_ptr<api::StorageReply> makeReply() override; - - void print(std::ostream& out, bool verbose, const std::string& indent) const override; -private: - vespalib::string getSummary() const override; -}; - - -/** - * @class ReadBucketInfoReply - * @ingroup common - */ -class ReadBucketInfoReply : public api::InternalReply { - document::Bucket _bucket; - -public: - static constexpr uint32_t ID = 2006; - - ReadBucketInfoReply(const ReadBucketInfo& cmd); - ~ReadBucketInfoReply(); - - document::Bucket getBucket() const override { return _bucket; } - - void print(std::ostream& out, bool verbose, const std::string& indent) const override; -}; - -} // storage diff --git a/storage/src/vespa/storage/common/messagebucket.cpp b/storage/src/vespa/storage/common/messagebucket.cpp index 73b83936e98..286eef39e16 100644 --- a/storage/src/vespa/storage/common/messagebucket.cpp +++ b/storage/src/vespa/storage/common/messagebucket.cpp @@ -2,7 +2,6 @@ #include "messagebucket.h" #include "statusmessages.h" -#include "bucketmessages.h" #include <vespa/storageapi/message/bucket.h> #include <vespa/storageapi/message/bucketsplitting.h> #include <vespa/storageapi/message/persistence.h> @@ -60,10 +59,6 @@ getStorageMessageBucket(const api::StorageMessage& msg) return static_cast<const GetIterCommand&>(msg).getBucket(); case CreateIteratorCommand::ID: return static_cast<const CreateIteratorCommand&>(msg).getBucket(); - case ReadBucketList::ID: - return static_cast<const ReadBucketList&>(msg).getBucket(); - case ReadBucketInfo::ID: - return static_cast<const ReadBucketInfo&>(msg).getBucket(); case RecheckBucketInfoCommand::ID: return static_cast<const RecheckBucketInfoCommand&>(msg).getBucket(); case RunTaskCommand::ID: diff --git a/storage/src/vespa/storage/common/storagelink.cpp b/storage/src/vespa/storage/common/storagelink.cpp index 85ec26fda2f..2d566f1fc29 100644 --- a/storage/src/vespa/storage/common/storagelink.cpp +++ b/storage/src/vespa/storage/common/storagelink.cpp @@ -1,7 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "storagelink.h" -#include "bucketmessages.h" +#include <vespa/storageapi/messageapi/storagecommand.h> +#include <vespa/storageapi/messageapi/storagereply.h> #include <vespa/vespalib/util/backtrace.h> #include <sstream> #include <cassert> diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.cpp b/storage/src/vespa/storage/distributor/idealstatemanager.cpp index 2c33bc490fe..cad141e76ed 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.cpp +++ b/storage/src/vespa/storage/distributor/idealstatemanager.cpp @@ -8,7 +8,6 @@ #include "distributor_bucket_space.h" #include <vespa/vespalib/stllike/asciistream.h> #include <vespa/storageapi/message/persistence.h> -#include <vespa/storage/common/bucketmessages.h> #include <vespa/document/bucket/fixed_bucket_spaces.h> #include <vespa/vespalib/util/assert.h> #include <vespa/vespalib/stllike/hash_map.hpp> diff --git a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp index 1d29a8795d5..33022c65e24 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp @@ -6,7 +6,6 @@ #include <vespa/storageapi/message/bucketsplitting.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/storage/bucketdb/storbucketdb.h> -#include <vespa/storage/common/bucketmessages.h> #include <vespa/storage/common/statusmessages.h> #include <vespa/storage/common/messagebucket.h> #include <vespa/storage/persistence/asynchandler.h> @@ -666,7 +665,6 @@ FileStorHandlerImpl::remapMessage(api::StorageMessage& msg, const document::Buck } } break; - case ReadBucketInfo::ID: case RecheckBucketInfoCommand::ID: { LOG(debug, "While remapping load for bucket %s for reason %u, " diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp index da9d16c9356..d2c3cea44b0 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp @@ -4,7 +4,6 @@ #include "filestorhandlerimpl.h" #include <vespa/storageframework/generic/thread/thread.h> #include <vespa/storage/bucketdb/minimumusedbitstracker.h> -#include <vespa/storage/common/bucketmessages.h> #include <vespa/storage/common/content_bucket_space_repo.h> #include <vespa/storage/common/doneinitializehandler.h> #include <vespa/storage/common/hostreporter/hostinfo.h> @@ -755,21 +754,6 @@ FileStorManager::onInternal(const shared_ptr<api::InternalCommand>& msg) msg->getTrace().addChild(context.steal_trace()); return true; } - case ReadBucketList::ID: - { - shared_ptr<ReadBucketList> cmd(std::static_pointer_cast<ReadBucketList>(msg)); - handlePersistenceMessage(cmd); - return true; - } - case ReadBucketInfo::ID: - { - shared_ptr<ReadBucketInfo> cmd(std::static_pointer_cast<ReadBucketInfo>(msg)); - StorBucketDatabase::WrappedEntry entry(mapOperationToDisk(*cmd, cmd->getBucket())); - if (entry.exists()) { - handlePersistenceMessage(cmd); - } - return true; - } case RecheckBucketInfoCommand::ID: { shared_ptr<RecheckBucketInfoCommand> cmd(std::static_pointer_cast<RecheckBucketInfoCommand>(msg)); diff --git a/storage/src/vespa/storage/persistence/persistencehandler.cpp b/storage/src/vespa/storage/persistence/persistencehandler.cpp index 69f910d0910..00ab61f2304 100644 --- a/storage/src/vespa/storage/persistence/persistencehandler.cpp +++ b/storage/src/vespa/storage/persistence/persistencehandler.cpp @@ -100,10 +100,6 @@ PersistenceHandler::handleCommandSplitByType(api::StorageCommand& msg, MessageTr auto usage = vespalib::CpuUsage::use(CpuUsage::Category::READ); return _simpleHandler.handleCreateIterator(static_cast<CreateIteratorCommand&>(msg), std::move(tracker)); } - case ReadBucketList::ID: - return _simpleHandler.handleReadBucketList(static_cast<ReadBucketList&>(msg), std::move(tracker)); - case ReadBucketInfo::ID: - return _simpleHandler.handleReadBucketInfo(static_cast<ReadBucketInfo&>(msg), std::move(tracker)); case RecheckBucketInfoCommand::ID: return _splitJoinHandler.handleRecheckBucketInfo(static_cast<RecheckBucketInfoCommand&>(msg), std::move(tracker)); case RunTaskCommand::ID: diff --git a/storage/src/vespa/storage/persistence/simplemessagehandler.cpp b/storage/src/vespa/storage/persistence/simplemessagehandler.cpp index ea929bf8620..1ac0939c21e 100644 --- a/storage/src/vespa/storage/persistence/simplemessagehandler.cpp +++ b/storage/src/vespa/storage/persistence/simplemessagehandler.cpp @@ -131,29 +131,6 @@ SimpleMessageHandler::handleGetIter(GetIterCommand& cmd, MessageTracker::UP trac } MessageTracker::UP -SimpleMessageHandler::handleReadBucketList(ReadBucketList& cmd, MessageTracker::UP tracker) const -{ - tracker->setMetric(_env._metrics.readBucketList); - - spi::BucketIdListResult result(_spi.listBuckets(cmd.getBucketSpace())); - if (tracker->checkForError(result)) { - auto reply = std::make_shared<ReadBucketListReply>(cmd); - result.getList().swap(reply->getBuckets()); - tracker->setReply(reply); - } - - return tracker; -} - -MessageTracker::UP -SimpleMessageHandler::handleReadBucketInfo(ReadBucketInfo& cmd, MessageTracker::UP tracker) const -{ - tracker->setMetric(_env._metrics.readBucketInfo); - _env.updateBucketDatabase(cmd.getBucket(), _env.getBucketInfo(cmd.getBucket())); - return tracker; -} - -MessageTracker::UP SimpleMessageHandler::handleCreateIterator(CreateIteratorCommand& cmd, MessageTracker::UP tracker) const { tracker->setMetric(_env._metrics.createIterator); diff --git a/storage/src/vespa/storage/persistence/simplemessagehandler.h b/storage/src/vespa/storage/persistence/simplemessagehandler.h index a5a19772556..49432c1ccb7 100644 --- a/storage/src/vespa/storage/persistence/simplemessagehandler.h +++ b/storage/src/vespa/storage/persistence/simplemessagehandler.h @@ -4,7 +4,6 @@ #include "types.h" #include "messages.h" -#include <vespa/storage/common/bucketmessages.h> #include <vespa/storageapi/message/persistence.h> namespace document { class BucketIdFactory; } @@ -28,8 +27,6 @@ public: MessageTrackerUP handleRevert(api::RevertCommand& cmd, MessageTrackerUP tracker) const; MessageTrackerUP handleCreateIterator(CreateIteratorCommand& cmd, MessageTrackerUP tracker) const; MessageTrackerUP handleGetIter(GetIterCommand& cmd, MessageTrackerUP tracker) const; - MessageTrackerUP handleReadBucketList(ReadBucketList& cmd, MessageTrackerUP tracker) const; - MessageTrackerUP handleReadBucketInfo(ReadBucketInfo& cmd, MessageTrackerUP tracker) const; private: MessageTrackerUP handle_conditional_get(api::GetCommand& cmd, MessageTrackerUP tracker) const; diff --git a/vespa-dependencies-enforcer/allowed-maven-dependencies.txt b/vespa-dependencies-enforcer/allowed-maven-dependencies.txt index e04c19a8486..a57580c46b0 100644 --- a/vespa-dependencies-enforcer/allowed-maven-dependencies.txt +++ b/vespa-dependencies-enforcer/allowed-maven-dependencies.txt @@ -14,15 +14,15 @@ com.amazonaws:aws-java-sdk-ssm:1.12.460 com.amazonaws:aws-java-sdk-sts:1.12.460 com.amazonaws:jmespath-java:1.12.460 com.auth0:java-jwt:3.10.0 -com.fasterxml.jackson.core:jackson-annotations:2.15.0 -com.fasterxml.jackson.core:jackson-core:2.15.0 -com.fasterxml.jackson.core:jackson-databind:2.15.0 -com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.15.0 -com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.15.0 -com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.15.0 -com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:2.15.0 -com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:2.15.0 -com.fasterxml.jackson.module:jackson-module-jaxb-annotations:2.15.0 +com.fasterxml.jackson.core:jackson-annotations:2.15.2 +com.fasterxml.jackson.core:jackson-core:2.15.2 +com.fasterxml.jackson.core:jackson-databind:2.15.2 +com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.15.2 +com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.15.2 +com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.15.2 +com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:2.15.2 +com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:2.15.2 +com.fasterxml.jackson.module:jackson-module-jaxb-annotations:2.15.2 com.github.spotbugs:spotbugs-annotations:3.1.9 com.google.code.findbugs:jsr305:3.0.2 com.google.errorprone:error_prone_annotations:2.18.0 @@ -210,7 +210,7 @@ org.sonatype.sisu:sisu-guice:2.1.7:noaop org.sonatype.sisu:sisu-inject-bean:1.4.2 org.sonatype.sisu:sisu-inject-plexus:1.4.2 org.tukaani:xz:1.8 -org.xerial.snappy:snappy-java:1.1.7 +org.xerial.snappy:snappy-java:1.1.10.1 software.amazon.ion:ion-java:1.0.2 xerces:xercesImpl:2.12.2 xml-apis:xml-apis:1.4.01 diff --git a/vespalib/src/tests/stllike/cache_test.cpp b/vespalib/src/tests/stllike/cache_test.cpp index 358a3591dbf..715fda1cbfd 100644 --- a/vespalib/src/tests/stllike/cache_test.cpp +++ b/vespalib/src/tests/stllike/cache_test.cpp @@ -136,4 +136,64 @@ TEST("testThatMultipleRemoveOnOverflowIsFine") { EXPECT_EQUAL(2924u, cache.sizeBytes()); } +class ExtendedCache : public cache< CacheParam<P, B> > { +public: + ExtendedCache(BackingStore & b, size_t maxBytes) + : cache<CacheParam<P, B>>(b, maxBytes), + _insert_count(0), + _remove_count(0) + {} + size_t _insert_count; + size_t _remove_count; +private: + void onRemove(const K &) override { + _remove_count++; + } + + void onInsert(const K &) override { + _insert_count++; + } +}; + +TEST("testOnInsertonRemoveCalledWhenFull") { + B m; + ExtendedCache cache(m, 200); + EXPECT_EQUAL(0u, cache._insert_count); + EXPECT_EQUAL(0u, cache._remove_count); + cache.write(1, "15 bytes string"); + EXPECT_EQUAL(1u, cache.size()); + EXPECT_EQUAL(80u, cache.sizeBytes()); + EXPECT_EQUAL(1u, cache._insert_count); + EXPECT_EQUAL(0u, cache._remove_count); + cache.write(2, "16 bytes stringg"); + EXPECT_EQUAL(2u, cache.size()); + EXPECT_EQUAL(160u, cache.sizeBytes()); + EXPECT_EQUAL(2u, cache._insert_count); + EXPECT_EQUAL(0u, cache._remove_count); + cache.write(3, "17 bytes stringgg"); + EXPECT_EQUAL(3u, cache.size()); + EXPECT_EQUAL(240u, cache.sizeBytes()); + EXPECT_EQUAL(3u, cache._insert_count); + EXPECT_EQUAL(0u, cache._remove_count); + EXPECT_TRUE(cache.hasKey(1)); + cache.write(4, "18 bytes stringggg"); + EXPECT_EQUAL(3u, cache.size()); + EXPECT_EQUAL(240u, cache.sizeBytes()); + EXPECT_EQUAL(4u, cache._insert_count); + EXPECT_EQUAL(1u, cache._remove_count); + EXPECT_FALSE(cache.hasKey(1)); + cache.invalidate(2); + EXPECT_EQUAL(2u, cache.size()); + EXPECT_EQUAL(160u, cache.sizeBytes()); + EXPECT_EQUAL(4u, cache._insert_count); + EXPECT_EQUAL(2u, cache._remove_count); + EXPECT_FALSE(cache.hasKey(2)); + cache.invalidate(3); + EXPECT_EQUAL(1u, cache.size()); + EXPECT_EQUAL(80u, cache.sizeBytes()); + EXPECT_EQUAL(4u, cache._insert_count); + EXPECT_EQUAL(3u, cache._remove_count); + EXPECT_FALSE(cache.hasKey(3)); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/vespalib/src/vespa/vespalib/stllike/lrucache_map.hpp b/vespalib/src/vespa/vespalib/stllike/lrucache_map.hpp index ca1b075d68b..f063f277e3e 100644 --- a/vespalib/src/vespa/vespalib/stllike/lrucache_map.hpp +++ b/vespalib/src/vespa/vespalib/stllike/lrucache_map.hpp @@ -204,9 +204,7 @@ lrucache_map<P>::removeOld() { (_tail != _head) && removeOldest(*last); last = & HashTable::getByInternalIndex(_tail)) { - _tail = last->second._prev; - HashTable::getByInternalIndex(_tail).second._next = LinkedValueBase::npos; - HashTable::erase(*this, HashTable::hash(last->first), HashTable::find(last->first)); + erase(last->first); } } } |