diff options
36 files changed, 294 insertions, 98 deletions
diff --git a/README.md b/README.md index 280d3ae8ee8..6a452c10f77 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ To get started using Vespa pick one of the quick start documents: - [Run on a Mac or Linux machine using Docker](https://docs.vespa.ai/en/vespa-quick-start.html) - [Run on a Windows machine using Docker](https://docs.vespa.ai/en/vespa-quick-start-windows.html) -- [Run on a Mac or Linux machine using VirtualBox+Vagrant](https://docs.vespa.ai/en/vespa-quick-start-centos.html) +- [Run on a Mac or Linux machine using VirtualBox+Vagrant](https://docs.vespa.ai/en/vespa-quick-start-vagrant.html) - [Multinode install on AWS EC2](https://docs.vespa.ai/en/vespa-quick-start-multinode-aws.html) - [Multinode install on AWS ECS](https://docs.vespa.ai/en/vespa-quick-start-multinode-aws-ecs.html) @@ -43,7 +43,7 @@ To get started using Vespa pick one of the quick start documents: - The application created in the quickstart is fully functional and production ready, but you may want to [add more nodes](https://docs.vespa.ai/en/multinode-systems.html) for redundancy. - Try the [Blog search and recommendation tutorial](https://docs.vespa.ai/en/tutorials/blog-search.html) to learn more about using Vespa -- See [developing applications](https://docs.vespa.ai/en/jdisc/developing-applications.html) on adding your own Java components to your Vespa application. +- See [developing applications](https://docs.vespa.ai/en/developer-guide.html) on adding your own Java components to your Vespa application. - [Vespa APIs](https://docs.vespa.ai/en/api.html) is useful to understand how to interface with Vespa - Explore the [sample applications](https://github.com/vespa-engine/sample-apps/tree/master) diff --git a/client/src/main/java/ai/vespa/client/dsl/FixedQuery.java b/client/src/main/java/ai/vespa/client/dsl/FixedQuery.java index 94c571dba5a..9bab35c44bc 100644 --- a/client/src/main/java/ai/vespa/client/dsl/FixedQuery.java +++ b/client/src/main/java/ai/vespa/client/dsl/FixedQuery.java @@ -12,7 +12,7 @@ import java.util.stream.Collectors; /** * FixedQuery contains a 'Query' which is terminated by a 'semicolon' * This object holds vespa or user defined parameters - * https://docs.vespa.ai/en/reference/search-api-reference.html + * https://docs.vespa.ai/en/reference/query-api-reference.html */ public class FixedQuery { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java index 9153450a39e..97c59675b63 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java @@ -432,6 +432,15 @@ public class VespaMetricSet { metrics.add(new Metric("content.proton.documentdb.ready.lid_space.lid_limit.last")); metrics.add(new Metric("content.proton.documentdb.notready.lid_space.lid_limit.last")); metrics.add(new Metric("content.proton.documentdb.removed.lid_space.lid_limit.last")); + metrics.add(new Metric("content.proton.documentdb.ready.lid_space.highest_used_lid.last")); + metrics.add(new Metric("content.proton.documentdb.notready.lid_space.highest_used_lid.last")); + metrics.add(new Metric("content.proton.documentdb.removed.lid_space.highest_used_lid.last")); + metrics.add(new Metric("content.proton.documentdb.ready.lid_space.used_lids.last")); + metrics.add(new Metric("content.proton.documentdb.notready.lid_space.used_lids.last")); + metrics.add(new Metric("content.proton.documentdb.removed.lid_space.used_lids.last")); + + // bucket move + metrics.add(new Metric("content.proton.documentdb.bucket_move.buckets_pending.last")); // resource usage metrics.add(new Metric("content.proton.resource_usage.disk.average")); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/BlockFeedGlobalEndpointsFilter.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/BlockFeedGlobalEndpointsFilter.java index 1d9bf053331..28d94ca7d4a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/BlockFeedGlobalEndpointsFilter.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/BlockFeedGlobalEndpointsFilter.java @@ -51,7 +51,10 @@ public class BlockFeedGlobalEndpointsFilter extends Filter implements RuleBasedF .action(BLOCK) .name("block-feed-global-endpoints") .blockResponseMessage("Feed to global endpoints are not allowed") - .blockResponseCode(404); + .blockResponseCode(405) + .blockResponseHeaders(new RuleBasedFilterConfig.Rule.BlockResponseHeaders.Builder() + .name("Allow") + .value("GET, OPTIONS, HEAD")); builder.rule(rule); } builder.dryrun(dryRun); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java index 3bbfd5b9165..9f98fdb4ea2 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java @@ -73,6 +73,9 @@ public class HostedSslConnectorFactory extends ConnectorFactory { .pathWhitelist(INSECURE_WHITELISTED_PATHS) .enable(enforceClientAuth)); } + // Disables TLSv1.3 as it causes some browsers to prompt user for client certificate (when connector has 'want' auth) + connectorBuilder.ssl.enabledProtocols(List.of("TLSv1.2")); + connectorBuilder .proxyProtocol(new ConnectorConfig.ProxyProtocol.Builder().enabled(true).mixedMode(true)) .idleTimeout(Duration.ofMinutes(3).toSeconds()) diff --git a/hosted-api/src/main/java/ai/vespa/hosted/api/ControllerHttpClient.java b/hosted-api/src/main/java/ai/vespa/hosted/api/ControllerHttpClient.java index 0cc80bcb111..6a733bd8942 100644 --- a/hosted-api/src/main/java/ai/vespa/hosted/api/ControllerHttpClient.java +++ b/hosted-api/src/main/java/ai/vespa/hosted/api/ControllerHttpClient.java @@ -30,12 +30,14 @@ import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.nio.file.Files; import java.nio.file.Path; +import java.security.NoSuchAlgorithmException; import java.security.PrivateKey; import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.OptionalLong; import java.util.concurrent.Callable; import java.util.function.Consumer; @@ -61,11 +63,18 @@ public abstract class ControllerHttpClient { private final URI endpoint; /** Creates an HTTP client against the given endpoint, using the given HTTP client builder to create a client. */ - protected ControllerHttpClient(URI endpoint, HttpClient.Builder client) { + protected ControllerHttpClient(URI endpoint, SSLContext sslContext) { + if (sslContext == null) { + try { sslContext = SSLContext.getDefault(); } + catch (NoSuchAlgorithmException e) { throw new IllegalStateException(e); } + } + this.endpoint = endpoint.resolve("/"); - this.client = client.connectTimeout(Duration.ofSeconds(5)) - .version(HttpClient.Version.HTTP_1_1) - .build(); + this.client = HttpClient.newBuilder().connectTimeout(Duration.ofSeconds(5)) + .version(HttpClient.Version.HTTP_1_1) + .sslContext(sslContext) + .sslParameters(tlsv12Parameters(sslContext)) + .build(); } /** Creates an HTTP client against the given endpoint, which uses the given key to authenticate as the given application. */ @@ -407,6 +416,12 @@ public abstract class ControllerHttpClient { } } + private static SSLParameters tlsv12Parameters(SSLContext sslContext) { + SSLParameters parameters = sslContext.getDefaultSSLParameters(); + parameters.setProtocols(new String[]{ "TLSv1.2" }); + return parameters; + } + /** Client that signs requests with a private key whose public part is assigned to an application in the remote controller. */ private static class SigningControllerHttpClient extends ControllerHttpClient { @@ -414,7 +429,7 @@ public abstract class ControllerHttpClient { private final RequestSigner signer; private SigningControllerHttpClient(URI endpoint, String privateKey, ApplicationId id) { - super(endpoint, HttpClient.newBuilder()); + super(endpoint, null); this.signer = new RequestSigner(privateKey, id.serializedForm()); } @@ -434,19 +449,13 @@ public abstract class ControllerHttpClient { private static class MutualTlsControllerHttpClient extends ControllerHttpClient { private MutualTlsControllerHttpClient(URI endpoint, SSLContext sslContext) { - super(endpoint, HttpClient.newBuilder().sslContext(sslContext).sslParameters(tlsv12Parameters(sslContext))); + super(endpoint, Objects.requireNonNull(sslContext)); } private MutualTlsControllerHttpClient(URI endpoint, PrivateKey privateKey, List<X509Certificate> certs) { this(endpoint, new SslContextBuilder().withKeyStore(privateKey, certs).build()); } - private static SSLParameters tlsv12Parameters(SSLContext sslContext) { - SSLParameters parameters = sslContext.getDefaultSSLParameters(); - parameters.setProtocols(new String[]{ "TLSv1.2" }); - return parameters; - } - } diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/rule/RuleBasedRequestFilter.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/rule/RuleBasedRequestFilter.java index 71f1965c764..7bdc386e4b4 100644 --- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/rule/RuleBasedRequestFilter.java +++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/rule/RuleBasedRequestFilter.java @@ -3,6 +3,7 @@ package com.yahoo.jdisc.http.filter.security.rule; import com.google.inject.Inject; import com.yahoo.jdisc.Metric; +import com.yahoo.jdisc.Response; import com.yahoo.jdisc.http.filter.DiscFilterRequest; import com.yahoo.jdisc.http.filter.security.base.JsonSecurityRequestFilterBase; import com.yahoo.jdisc.http.filter.security.rule.RuleBasedFilterConfig.Rule.Action; @@ -56,7 +57,11 @@ public class RuleBasedRequestFilter extends JsonSecurityRequestFilterBase { private static ErrorResponse createDefaultResponse(RuleBasedFilterConfig.DefaultRule defaultRule) { switch (defaultRule.action()) { case ALLOW: return null; - case BLOCK: return new ErrorResponse(defaultRule.blockResponseCode(), defaultRule.blockResponseMessage()); + case BLOCK: { + Response response = new Response(defaultRule.blockResponseCode()); + defaultRule.blockResponseHeaders().forEach(h -> response.headers().add(h.name(), h.value())); + return new ErrorResponse(response, defaultRule.blockResponseMessage()); + } default: throw new IllegalArgumentException(defaultRule.action().name()); } } @@ -100,9 +105,13 @@ public class RuleBasedRequestFilter extends JsonSecurityRequestFilterBase { .map(m -> m.name().toUpperCase()) .collect(Collectors.toSet()); this.pathGlobExpressions = Set.copyOf(config.pathExpressions()); - this.response = config.action() == Action.Enum.BLOCK - ? new ErrorResponse(config.blockResponseCode(), config.blockResponseMessage()) - : null; + this.response = config.action() == Action.Enum.BLOCK ? createResponse(config) : null; + } + + private static ErrorResponse createResponse(RuleBasedFilterConfig.Rule config) { + Response response = new Response(config.blockResponseCode()); + config.blockResponseHeaders().forEach(h -> response.headers().add(h.name(), h.value())); + return new ErrorResponse(response, config.blockResponseMessage()); } boolean matches(String method, URI uri) { diff --git a/jdisc-security-filters/src/main/resources/configdefinitions/jdisc.http.filter.security.rule.rule-based-filter.def b/jdisc-security-filters/src/main/resources/configdefinitions/jdisc.http.filter.security.rule.rule-based-filter.def index 374c3ca69c6..6abf3d43a7d 100644 --- a/jdisc-security-filters/src/main/resources/configdefinitions/jdisc.http.filter.security.rule.rule-based-filter.def +++ b/jdisc-security-filters/src/main/resources/configdefinitions/jdisc.http.filter.security.rule.rule-based-filter.def @@ -5,6 +5,8 @@ dryrun bool default=false defaultRule.action enum { ALLOW, BLOCK } defaultRule.blockResponseCode int default=403 defaultRule.blockResponseMessage string default="" +defaultRule.blockResponseHeaders[].name string +defaultRule.blockResponseHeaders[].value string rule[].name string rule[].action enum { ALLOW, BLOCK } rule[].hostNames[] string @@ -12,5 +14,5 @@ rule[].methods[] enum { GET, POST, PUT, PATCH, DELETE } rule[].pathExpressions[] string rule[].blockResponseCode int default=403 rule[].blockResponseMessage string default="" - - +rule[].blockResponseHeaders[].name string +rule[].blockResponseHeaders[].value string diff --git a/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/rule/RuleBasedRequestFilterTest.java b/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/rule/RuleBasedRequestFilterTest.java index c67d3b430c8..68879b4dd32 100644 --- a/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/rule/RuleBasedRequestFilterTest.java +++ b/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/rule/RuleBasedRequestFilterTest.java @@ -11,11 +11,11 @@ import com.yahoo.jdisc.http.filter.DiscFilterRequest; import com.yahoo.jdisc.http.filter.security.rule.RuleBasedFilterConfig.DefaultRule; import com.yahoo.jdisc.http.filter.security.rule.RuleBasedFilterConfig.Rule; import com.yahoo.test.json.JsonTestHelper; -import com.yahoo.vespa.jdk8compat.List; import org.junit.jupiter.api.Test; import java.io.IOException; import java.net.URI; +import java.util.List; import java.util.Set; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -148,6 +148,75 @@ class RuleBasedRequestFilterTest { assertAllowed(responseHandler, metric); } + @Test + void includes_default_rule_response_headers_in_response_for_blocked_request() throws IOException { + RuleBasedFilterConfig config = new RuleBasedFilterConfig.Builder() + .dryrun(false) + .defaultRule(new DefaultRule.Builder() + .action(DefaultRule.Action.Enum.BLOCK) + .blockResponseHeaders(new DefaultRule.BlockResponseHeaders.Builder() + .name("Response-Header-1").value("first-header")) + .blockResponseHeaders(new DefaultRule.BlockResponseHeaders.Builder() + .name("Response-Header-2").value("second-header"))) + .build(); + + Metric metric = mock(Metric.class); + RuleBasedRequestFilter filter = new RuleBasedRequestFilter(metric, config); + MockResponseHandler responseHandler = new MockResponseHandler(); + filter.filter(request("GET", "http://myserver:80/"), responseHandler); + + assertBlocked(responseHandler, metric, 403, ""); + Response response = responseHandler.getResponse(); + assertResponseHeader(response, "Response-Header-1", "first-header"); + assertResponseHeader(response, "Response-Header-2", "second-header"); + } + + @Test + void includes_rule_response_headers_in_response_for_blocked_request() throws IOException { + RuleBasedFilterConfig config = new RuleBasedFilterConfig.Builder() + .dryrun(false) + .defaultRule(new DefaultRule.Builder() + .action(DefaultRule.Action.Enum.ALLOW)) + .rule(new Rule.Builder() + .name("rule") + .pathExpressions("/path-to-resource") + .action(Rule.Action.Enum.BLOCK) + .blockResponseHeaders(new Rule.BlockResponseHeaders.Builder() + .name("Response-Header-1").value("first-header"))) + .build(); + + Metric metric = mock(Metric.class); + RuleBasedRequestFilter filter = new RuleBasedRequestFilter(metric, config); + MockResponseHandler responseHandler = new MockResponseHandler(); + filter.filter(request("GET", "http://myserver/path-to-resource"), responseHandler); + + assertBlocked(responseHandler, metric, 403, ""); + Response response = responseHandler.getResponse(); + assertResponseHeader(response, "Response-Header-1", "first-header"); + } + + @Test + void dryrun_does_not_block() { + RuleBasedFilterConfig config = new RuleBasedFilterConfig.Builder() + .dryrun(true) + .defaultRule(new DefaultRule.Builder() + .action(DefaultRule.Action.Enum.BLOCK)) + .build(); + + Metric metric = mock(Metric.class); + RuleBasedRequestFilter filter = new RuleBasedRequestFilter(metric, config); + MockResponseHandler responseHandler = new MockResponseHandler(); + filter.filter(request("GET", "http://myserver/"), responseHandler); + assertNull(responseHandler.getResponse()); + } + + private void assertResponseHeader(Response response, String name, String expectedValue) { + List<String> actualValues = response.headers().get(name); + assertNotNull(actualValues); + assertEquals(1, actualValues.size()); + assertEquals(expectedValue, actualValues.get(0)); + } + private static DiscFilterRequest request(String method, String uri) { DiscFilterRequest request = mock(DiscFilterRequest.class); when(request.getMethod()).thenReturn(method); diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java index f61939de90b..b0dfd228c8b 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java @@ -44,10 +44,11 @@ import org.eclipse.jetty.client.ProxyProtocolClientConnectionFactory.V2; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.server.handler.AbstractHandlerContainer; import org.eclipse.jetty.util.ssl.SslContextFactory; -import org.junit.Ignore; +import org.hamcrest.Matchers; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.mockito.Mockito; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLException; @@ -67,6 +68,7 @@ import java.security.cert.X509Certificate; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.List; @@ -102,11 +104,14 @@ import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.junit.Assume.assumeTrue; +import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -645,7 +650,7 @@ public class HttpServerTest { .build(); assertHttpsRequestTriggersSslHandshakeException( driver, clientCtx, null, null, "Received fatal alert: bad_certificate"); - verify(metricConsumer.mockitoMock()) + verify(metricConsumer.mockitoMock(), atLeast(1)) .add(MetricDefinitions.SSL_HANDSHAKE_FAILURE_MISSING_CLIENT_CERT, 1L, MetricConsumerMock.STATIC_CONTEXT); assertTrue(driver.close()); Assertions.assertThat(connectionLog.logEntries()).hasSize(1); @@ -654,7 +659,6 @@ public class HttpServerTest { } @Test - @Ignore public void requireThatMetricIsIncrementedWhenClientUsesIncompatibleTlsVersion() throws IOException { Path privateKeyFile = tmpFolder.newFile().toPath(); Path certificateFile = tmpFolder.newFile().toPath(); @@ -668,9 +672,11 @@ public class HttpServerTest { .withKeyStore(privateKeyFile, certificateFile) .build(); - assertHttpsRequestTriggersSslHandshakeException( - driver, clientCtx, "TLSv1.1", null, "protocol"); - verify(metricConsumer.mockitoMock()) + boolean tlsv11Enabled = List.of(clientCtx.getDefaultSSLParameters().getProtocols()).contains("TLSv1.1"); + assumeTrue("TLSv1.1 must be enabled in installed JDK", tlsv11Enabled); + + assertHttpsRequestTriggersSslHandshakeException(driver, clientCtx, "TLSv1.1", null, "protocol"); + verify(metricConsumer.mockitoMock(), atLeast(1)) .add(MetricDefinitions.SSL_HANDSHAKE_FAILURE_INCOMPATIBLE_PROTOCOLS, 1L, MetricConsumerMock.STATIC_CONTEXT); assertTrue(driver.close()); Assertions.assertThat(connectionLog.logEntries()).hasSize(1); @@ -694,7 +700,7 @@ public class HttpServerTest { assertHttpsRequestTriggersSslHandshakeException( driver, clientCtx, null, "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", "Received fatal alert: handshake_failure"); - verify(metricConsumer.mockitoMock()) + verify(metricConsumer.mockitoMock(), atLeast(1)) .add(MetricDefinitions.SSL_HANDSHAKE_FAILURE_INCOMPATIBLE_CIPHERS, 1L, MetricConsumerMock.STATIC_CONTEXT); assertTrue(driver.close()); Assertions.assertThat(connectionLog.logEntries()).hasSize(1); @@ -722,11 +728,10 @@ public class HttpServerTest { assertHttpsRequestTriggersSslHandshakeException( driver, clientCtx, null, null, "Received fatal alert: certificate_unknown"); - verify(metricConsumer.mockitoMock()) + verify(metricConsumer.mockitoMock(), atLeast(1)) .add(MetricDefinitions.SSL_HANDSHAKE_FAILURE_INVALID_CLIENT_CERT, 1L, MetricConsumerMock.STATIC_CONTEXT); assertTrue(driver.close()); Assertions.assertThat(connectionLog.logEntries()).hasSize(1); - ConnectionLogEntry logEntry = connectionLog.logEntries().get(0); assertSslHandshakeFailurePresent( connectionLog.logEntries().get(0), SSLHandshakeException.class, SslHandshakeFailure.INVALID_CLIENT_CERT.failureType()); } @@ -750,7 +755,7 @@ public class HttpServerTest { assertHttpsRequestTriggersSslHandshakeException( driver, clientCtx, null, null, "Received fatal alert: certificate_unknown"); - verify(metricConsumer.mockitoMock()) + verify(metricConsumer.mockitoMock(), atLeast(1)) .add(MetricDefinitions.SSL_HANDSHAKE_FAILURE_EXPIRED_CLIENT_CERT, 1L, MetricConsumerMock.STATIC_CONTEXT); assertTrue(driver.close()); Assertions.assertThat(connectionLog.logEntries()).hasSize(1); @@ -969,7 +974,8 @@ public class HttpServerTest { } catch (SSLException e) { // This exception is thrown if Apache httpclient's write thread detects the handshake failure before the read thread. log.log(Level.WARNING, "Client failed to get a proper TLS handshake response: " + e.getMessage(), e); - assertThat(e.getMessage(), containsString("readHandshakeRecord")); // Only ignore this specific ssl exception + // Only ignore a subset of exceptions + assertThat(e.getMessage(), anyOf(containsString("readHandshakeRecord"), containsString("Broken pipe"))); } } diff --git a/metrics/src/vespa/metrics/countmetric.h b/metrics/src/vespa/metrics/countmetric.h index de34e9330e6..02a6827d1ce 100644 --- a/metrics/src/vespa/metrics/countmetric.h +++ b/metrics/src/vespa/metrics/countmetric.h @@ -54,11 +54,9 @@ public: CountMetric(const CountMetric<T, SumOnAdd>& other, CopyType, MetricSet* owner); - ~CountMetric(); + ~CountMetric() override; - MetricValueClass::UP getValues() const override { - return MetricValueClass::UP(new Values(_values.getValues())); - } + MetricValueClass::UP getValues() const override; void set(T value); void inc(T value = 1); diff --git a/metrics/src/vespa/metrics/countmetric.hpp b/metrics/src/vespa/metrics/countmetric.hpp index 8b5c226a5f5..dfdc2586a05 100644 --- a/metrics/src/vespa/metrics/countmetric.hpp +++ b/metrics/src/vespa/metrics/countmetric.hpp @@ -23,7 +23,13 @@ CountMetric<T, SumOnAdd>::CountMetric(const CountMetric<T, SumOnAdd>& other, } template <typename T, bool SumOnAdd> -CountMetric<T, SumOnAdd>::~CountMetric() { } +CountMetric<T, SumOnAdd>::~CountMetric() = default; + +template <typename T, bool SumOnAdd> +MetricValueClass::UP +CountMetric<T, SumOnAdd>::getValues() const { + return std::make_unique<Values>(_values.getValues()); +} template <typename T, bool SumOnAdd> CountMetric<T, SumOnAdd>& diff --git a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/tensorflow/VariableConverter.java b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/tensorflow/VariableConverter.java index c0db65c5c99..85ae5238bae 100644 --- a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/tensorflow/VariableConverter.java +++ b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/tensorflow/VariableConverter.java @@ -37,7 +37,7 @@ class VariableConverter { if ( args.length != 3) { System.out.println("Converts a TensorFlow variable into Vespa tensor document field value JSON:"); System.out.println("A JSON map containing a 'cells' array, see"); - System.out.println("https://docs.vespa.ai/en/reference/document-json-put-format.html#tensor)"); + System.out.println("https://docs.vespa.ai/en/reference/document-json-format.html#tensor"); System.out.println(""); System.out.println("Arguments: modelDirectory tensorFlowVariableName orderedTypeSpec"); System.out.println(" - modelDirectory: The directory of the TensorFlow SavedModel"); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMover.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMover.java index 9fe9b7db478..29dcfddd821 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMover.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMover.java @@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.provision.provisioning.HostCapacity; import java.time.Duration; import java.time.Instant; +import java.util.Random; import java.util.Set; /** @@ -25,11 +26,13 @@ public abstract class NodeMover<MOVE> extends NodeRepositoryMaintainer { private final Deployer deployer; private final MOVE emptyMove; + private final Random random; public NodeMover(Deployer deployer, NodeRepository nodeRepository, Duration interval, Metric metric, MOVE emptyMove) { super(nodeRepository, interval, metric); this.deployer = deployer; this.emptyMove = emptyMove; + this.random = new Random(nodeRepository.clock().millis()); } /** Returns a suggested move for given node */ @@ -39,7 +42,11 @@ public abstract class NodeMover<MOVE> extends NodeRepositoryMaintainer { protected final MOVE findBestMove(NodeList allNodes) { HostCapacity capacity = new HostCapacity(allNodes, nodeRepository().resourcesCalculator()); MOVE bestMove = emptyMove; - NodeList activeNodes = allNodes.nodeType(NodeType.tenant).state(Node.State.active); + // Shuffle nodes so we did not get stuck if the chosen move is consistently discarded. Node moves happen through + // a soft request to retire (preferToRetire), which node allocation can disregard + NodeList activeNodes = allNodes.nodeType(NodeType.tenant) + .state(Node.State.active) + .shuffle(random); Set<Node> spares = capacity.findSpareHosts(allNodes.asList(), nodeRepository().spareCount()); for (Node node : activeNodes) { if (node.parentHostname().isEmpty()) continue; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index 042d227f8f1..d5d221cba7c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -154,7 +154,7 @@ class NodeAllocation { if (violatesParentHostPolicy(candidate)) return true; if ( ! hasCompatibleFlavor(candidate)) return true; if (candidate.wantToRetire()) return true; - if (candidate.preferToRetire() && !candidate.replacementIncreasesSkew(candidates)) return true; + if (candidate.preferToRetire() && candidate.replacableBy(candidates)) return true; if (violatesExclusivity(candidate)) return true; return false; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java index 297a8ed77e6..143ae8f99ad 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java @@ -99,18 +99,18 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat /** Returns whether this node can - as far as we know - be used to run the application workload */ public abstract boolean isValid(); - /** Returns whether replacing this with any of the reserved candidates will increase skew */ - public boolean replacementIncreasesSkew(List<NodeCandidate> candidates) { + /** Returns whether this can be replaced by any of the reserved candidates */ + public boolean replacableBy(List<NodeCandidate> candidates) { return candidates.stream() .filter(candidate -> candidate.state() == Node.State.reserved) - .allMatch(reserved -> { - int switchPriority = switchPriority(reserved); + .anyMatch(candidate -> { + int switchPriority = candidate.switchPriority(this); if (switchPriority < 0) { return true; } else if (switchPriority > 0) { return false; } - return hostPriority(reserved) < 0; + return candidate.hostPriority(this) < 0; }); } diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h index e2cd5e268d7..b74a2410fa0 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/bucketmover_common.h @@ -27,6 +27,7 @@ struct MyMoveOperationLimiter : public IMoveOperationLimiter { ++beginOpCount; return {}; } + size_t numPending() const override { return beginOpCount; } }; struct MyMoveHandler : public IDocumentMoveHandler { diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_v2_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_v2_test.cpp index 9db18091268..3ee0a77eca7 100644 --- a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_v2_test.cpp +++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_v2_test.cpp @@ -6,6 +6,8 @@ #include <vespa/vespalib/gtest/gtest.h> #include <vespa/log/log.h> +#include <vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h> + LOG_SETUP("document_bucket_mover_test"); using namespace proton; @@ -35,6 +37,7 @@ struct ControllerFixtureBase : public ::testing::Test ExecutorThreadService _master; DummyBucketExecutor _bucketExecutor; MyMoveHandler _moveHandler; + DocumentDBTaggedMetrics _metrics; BucketMoveJobV2 _bmj; MyCountJobRunner _runner; ControllerFixtureBase(const BlockableMaintenanceJobConfig &blockableConfig, bool storeMoveDoneContexts); @@ -73,6 +76,10 @@ struct ControllerFixtureBase : public ::testing::Test const BucketId::List &calcAsked() const { return _calc->asked(); } + size_t numPending() { + _bmj.updateMetrics(_metrics); + return _metrics.bucketMove.bucketsPending.getLast(); + } void runLoop() { while (!_bmj.isBlocked() && !_bmj.run()) { } @@ -98,6 +105,7 @@ ControllerFixtureBase::ControllerFixtureBase(const BlockableMaintenanceJobConfig _master(_singleExecutor), _bucketExecutor(4), _moveHandler(*_bucketDB, storeMoveDoneContexts), + _metrics("test", 1), _bmj(_calc, _moveHandler, _modifiedHandler, _master, _bucketExecutor, _ready._subDb, _notReady._subDb, _bucketCreateNotifier, _clusterStateHandler, _bucketHandler, _diskMemUsageNotifier, blockableConfig, @@ -156,11 +164,15 @@ TEST_F(ControllerFixture, require_that_not_ready_bucket_is_moved_to_ready_if_buc addReady(_ready.bucket(1)); addReady(_ready.bucket(2)); addReady(_notReady.bucket(4)); + + EXPECT_EQ(0, numPending()); _bmj.recompute(); + EXPECT_EQ(1, numPending()); EXPECT_FALSE(_bmj.done()); EXPECT_TRUE(_bmj.scanAndMove(4, 3)); EXPECT_TRUE(_bmj.done()); sync(); + EXPECT_EQ(0, numPending()); EXPECT_EQ(3u, docsMoved().size()); assertEqual(_notReady.bucket(4), _notReady.docs(4)[0], 2, 1, docsMoved()[0]); assertEqual(_notReady.bucket(4), _notReady.docs(4)[1], 2, 1, docsMoved()[1]); @@ -194,27 +206,32 @@ TEST_F(ControllerFixture, require_that_we_move_buckets_in_several_steps) addReady(_notReady.bucket(4)); _bmj.recompute(); + EXPECT_EQ(3, numPending()); EXPECT_FALSE(_bmj.done()); EXPECT_FALSE(_bmj.scanAndMove(1, 2)); EXPECT_FALSE(_bmj.done()); sync(); + EXPECT_EQ(2, numPending()); EXPECT_EQ(2u, docsMoved().size()); EXPECT_FALSE(_bmj.scanAndMove(1, 2)); EXPECT_FALSE(_bmj.done()); sync(); + EXPECT_EQ(2, numPending()); EXPECT_EQ(4u, docsMoved().size()); EXPECT_FALSE(_bmj.scanAndMove(1, 2)); EXPECT_FALSE(_bmj.done()); sync(); + EXPECT_EQ(1, numPending()); EXPECT_EQ(6u, docsMoved().size()); // move bucket 4, docs 3 EXPECT_TRUE(_bmj.scanAndMove(1,2)); EXPECT_TRUE(_bmj.done()); sync(); + EXPECT_EQ(0, numPending()); EXPECT_EQ(7u, docsMoved().size()); EXPECT_EQ(3u, bucketsModified().size()); EXPECT_EQ(_ready.bucket(2), bucketsModified()[0]); diff --git a/searchcore/src/vespa/searchcore/proton/common/attribute_updater.cpp b/searchcore/src/vespa/searchcore/proton/common/attribute_updater.cpp index 8fb165cb0b8..4d12fdfcf5b 100644 --- a/searchcore/src/vespa/searchcore/proton/common/attribute_updater.cpp +++ b/searchcore/src/vespa/searchcore/proton/common/attribute_updater.cpp @@ -205,23 +205,6 @@ AttributeUpdater::handleUpdate(PredicateAttribute &vec, uint32_t lid, const Valu } } -namespace { - -void -applyTensorUpdate(TensorAttribute &vec, uint32_t lid, const document::TensorUpdate &update, - bool create_empty_if_non_existing) -{ - auto oldTensor = vec.getTensor(lid); - if (!oldTensor && create_empty_if_non_existing) { - oldTensor = vec.getEmptyTensor(); - } - if (oldTensor) { - vec.update_tensor(lid, update, *oldTensor); - } -} - -} - template <> void AttributeUpdater::handleUpdate(TensorAttribute &vec, uint32_t lid, const ValueUpdate &upd) @@ -236,11 +219,11 @@ AttributeUpdater::handleUpdate(TensorAttribute &vec, uint32_t lid, const ValueUp updateValue(vec, lid, assign.getValue()); } } else if (op == ValueUpdate::TensorModifyUpdate) { - applyTensorUpdate(vec, lid, static_cast<const TensorModifyUpdate &>(upd), false); + vec.update_tensor(lid, static_cast<const TensorModifyUpdate &>(upd), false); } else if (op == ValueUpdate::TensorAddUpdate) { - applyTensorUpdate(vec, lid, static_cast<const TensorAddUpdate &>(upd), true); + vec.update_tensor(lid, static_cast<const TensorAddUpdate &>(upd), true); } else if (op == ValueUpdate::TensorRemoveUpdate) { - applyTensorUpdate(vec, lid, static_cast<const TensorRemoveUpdate &>(upd), false); + vec.update_tensor(lid, static_cast<const TensorRemoveUpdate &>(upd), false); } else if (op == ValueUpdate::Clear) { vec.clearDoc(lid); } else { diff --git a/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.cpp b/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.cpp index 7b7fcc9e45d..e032cc1cef6 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.cpp +++ b/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.cpp @@ -250,6 +250,13 @@ DocumentDBTaggedMetrics::DocumentsMetrics::DocumentsMetrics(metrics::MetricSet * DocumentDBTaggedMetrics::DocumentsMetrics::~DocumentsMetrics() = default; +DocumentDBTaggedMetrics::BucketMoveMetrics::BucketMoveMetrics(metrics::MetricSet *parent) + : metrics::MetricSet("bucket_move", {}, "Metrics for bucket move job in this document db", parent), + bucketsPending("buckets_pending", {}, "The number of buckets left to move", this) +{ } + +DocumentDBTaggedMetrics::BucketMoveMetrics::~BucketMoveMetrics() = default; + DocumentDBTaggedMetrics::DocumentDBTaggedMetrics(const vespalib::string &docTypeName, size_t maxNumThreads_) : MetricSet("documentdb", {{"documenttype", docTypeName}}, "Document DB metrics", nullptr), job(this), @@ -262,6 +269,7 @@ DocumentDBTaggedMetrics::DocumentDBTaggedMetrics(const vespalib::string &docType matching(this), sessionCache(this), documents(this), + bucketMove(this), totalMemoryUsage(this), totalDiskUsage("disk_usage", {}, "The total disk usage (in bytes) for this document db", this), maxNumThreads(maxNumThreads_) diff --git a/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h b/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h index 133df81a9e6..c1bc5633d19 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h +++ b/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h @@ -185,6 +185,13 @@ struct DocumentDBTaggedMetrics : metrics::MetricSet ~DocumentsMetrics() override; }; + struct BucketMoveMetrics : metrics::MetricSet { + metrics::LongValueMetric bucketsPending; + + BucketMoveMetrics(metrics::MetricSet *parent); + ~BucketMoveMetrics() override; + }; + JobMetrics job; AttributeMetrics attribute; IndexMetrics index; @@ -195,6 +202,7 @@ struct DocumentDBTaggedMetrics : metrics::MetricSet MatchingMetrics matching; SessionCacheMetrics sessionCache; DocumentsMetrics documents; + BucketMoveMetrics bucketMove; MemoryUsageMetrics totalMemoryUsage; metrics::LongValueMetric totalDiskUsage; size_t maxNumThreads; diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejobv2.cpp b/searchcore/src/vespa/searchcore/proton/server/bucketmovejobv2.cpp index 62de8a83173..becaa0c9e31 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejobv2.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejobv2.cpp @@ -9,6 +9,7 @@ #include "ibucketmodifiedhandler.h" #include "move_operation_limiter.h" #include "document_db_maintenance_config.h" +#include "document_db_explorer.h" #include <vespa/searchcore/proton/bucketdb/i_bucket_create_notifier.h> #include <vespa/searchcore/proton/feedoperation/moveoperation.h> #include <vespa/searchcore/proton/documentmetastore/i_document_meta_store.h> @@ -87,6 +88,7 @@ BucketMoveJobV2::BucketMoveJobV2(const std::shared_ptr<IBucketStateCalculator> & _stopped(false), _startedCount(0), _executedCount(0), + _bucketsPending(0), _bucketCreateNotifier(bucketCreateNotifier), _clusterStateChangedNotifier(clusterStateChangedNotifier), _bucketStateChangedNotifier(bucketStateChangedNotifier), @@ -289,6 +291,7 @@ BucketMoveJobV2::moveDocs(size_t maxDocsToMove) { } else { _movers.erase(_movers.begin() + index); } + _bucketsPending.store(_movers.size() + _buckets2Move.size(), std::memory_order_relaxed); } } return done(); @@ -338,6 +341,7 @@ BucketMoveJobV2::backFillMovers() { while ( ! _buckets2Move.empty() && (_movers.size() < _movers.capacity())) { _movers.push_back(greedyCreateMover()); } + _bucketsPending.store(_movers.size() + _buckets2Move.size(), std::memory_order_relaxed); } void BucketMoveJobV2::notifyClusterStateChanged(const std::shared_ptr<IBucketStateCalculator> &newCalc) @@ -380,4 +384,11 @@ BucketMoveJobV2::onStop() { } } +void +BucketMoveJobV2::updateMetrics(DocumentDBTaggedMetrics & metrics) { + // This is an over estimate to ensure we do not count down to zero until everything has been and completed and acked. + metrics.bucketMove.bucketsPending.set(_bucketsPending.load(std::memory_order_relaxed) + + getLimiter().numPending()); +} + } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejobv2.h b/searchcore/src/vespa/searchcore/proton/server/bucketmovejobv2.h index e20e6eeaf42..7e0c45ba8fa 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejobv2.h +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejobv2.h @@ -68,6 +68,7 @@ private: std::atomic<bool> _stopped; std::atomic<size_t> _startedCount; std::atomic<size_t> _executedCount; + std::atomic<size_t> _bucketsPending; bucketdb::IBucketCreateNotifier &_bucketCreateNotifier; IClusterStateChangedNotifier &_clusterStateChangedNotifier; @@ -114,6 +115,7 @@ public: void notifyDiskMemUsage(DiskMemUsageState state) override; void notifyCreateBucket(const bucketdb::Guard & guard, const BucketId &bucket) override; void onStop() override; + void updateMetrics(DocumentDBTaggedMetrics & metrics) override; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp index 55ce842e0cf..7f51227103f 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp @@ -191,7 +191,8 @@ DocumentDB::DocumentDB(const vespalib::string &baseDir, _writeFilter.setConfig(loaded_config->getMaintenanceConfigSP()->getAttributeUsageFilterConfig()); } -void DocumentDB::registerReference() +void +DocumentDB::registerReference() { if (_state.getAllowReconfig()) { auto registry = _owner.getDocumentDBReferenceRegistry(); @@ -204,7 +205,8 @@ void DocumentDB::registerReference() } } -void DocumentDB::setActiveConfig(const DocumentDBConfig::SP &config, int64_t generation) { +void +DocumentDB::setActiveConfig(const DocumentDBConfig::SP &config, int64_t generation) { lock_guard guard(_configMutex); registerReference(); _activeConfigSnapshot = config; @@ -215,7 +217,8 @@ void DocumentDB::setActiveConfig(const DocumentDBConfig::SP &config, int64_t gen _configCV.notify_all(); } -DocumentDBConfig::SP DocumentDB::getActiveConfig() const { +DocumentDBConfig::SP +DocumentDB::getActiveConfig() const { lock_guard guard(_configMutex); return _activeConfigSnapshot; } @@ -868,7 +871,8 @@ DocumentDB::replayConfig(search::SerialNum serialNum) _docTypeName.toString().c_str(), serialNum); } -int64_t DocumentDB::getActiveGeneration() const { +int64_t +DocumentDB::getActiveGeneration() const { lock_guard guard(_configMutex); return _activeConfigSnapshotGeneration; } @@ -1004,7 +1008,8 @@ DocumentDB::notifyClusterStateChanged(const std::shared_ptr<IBucketStateCalculat namespace { -void notifyBucketsChanged(const documentmetastore::IBucketHandler &metaStore, +void +notifyBucketsChanged(const documentmetastore::IBucketHandler &metaStore, IBucketModifiedHandler &handler, const vespalib::string &name) { @@ -1037,6 +1042,7 @@ DocumentDB::updateMetrics(const metrics::MetricLockGuard & guard) return; } _metricsUpdater.updateMetrics(guard, _metrics); + _maintenanceController.updateMetrics(_metrics); } void diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb_metrics_updater.h b/searchcore/src/vespa/searchcore/proton/server/documentdb_metrics_updater.h index 475da7f4e4c..4a70e756a86 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb_metrics_updater.h +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb_metrics_updater.h @@ -30,12 +30,12 @@ public: private: const DocumentSubDBCollection &_subDBs; - ExecutorThreadingService &_writeService; - DocumentDBJobTrackers &_jobTrackers; - matching::SessionManager &_sessionManager; - const AttributeUsageFilter &_writeFilter; + ExecutorThreadingService &_writeService; + DocumentDBJobTrackers &_jobTrackers; + matching::SessionManager &_sessionManager; + const AttributeUsageFilter &_writeFilter; // Last updated document store cache statistics. Necessary due to metrics implementation is upside down. - DocumentStoreCacheStats _lastDocStoreCacheStats; + DocumentStoreCacheStats _lastDocStoreCacheStats; void updateMiscMetrics(DocumentDBTaggedMetrics &metrics, const ExecutorThreadingServiceStats &threadingServiceStats); void updateAttributeResourceUsageMetrics(DocumentDBTaggedMetrics::AttributeMetrics &metrics); diff --git a/searchcore/src/vespa/searchcore/proton/server/i_maintenance_job.h b/searchcore/src/vespa/searchcore/proton/server/i_maintenance_job.h index 869dd2dfdb5..c7fcfc5c508 100644 --- a/searchcore/src/vespa/searchcore/proton/server/i_maintenance_job.h +++ b/searchcore/src/vespa/searchcore/proton/server/i_maintenance_job.h @@ -9,6 +9,7 @@ namespace proton { class IBlockableMaintenanceJob; class IMaintenanceJobRunner; +class DocumentDBTaggedMetrics; /** * Interface for a maintenance job that is executed after "delay" seconds and @@ -40,6 +41,7 @@ public: virtual bool isBlocked() const { return false; } virtual IBlockableMaintenanceJob *asBlockable() { return nullptr; } virtual void onStop() {} + virtual void updateMetrics(DocumentDBTaggedMetrics &) {} /** * Register maintenance job runner, in case event passed to the diff --git a/searchcore/src/vespa/searchcore/proton/server/i_move_operation_limiter.h b/searchcore/src/vespa/searchcore/proton/server/i_move_operation_limiter.h index cc91413826c..0eae0c5924f 100644 --- a/searchcore/src/vespa/searchcore/proton/server/i_move_operation_limiter.h +++ b/searchcore/src/vespa/searchcore/proton/server/i_move_operation_limiter.h @@ -13,6 +13,7 @@ namespace proton { struct IMoveOperationLimiter { virtual ~IMoveOperationLimiter() = default; virtual std::shared_ptr<vespalib::IDestructorCallback> beginOperation() = 0; + virtual size_t numPending() const = 0; }; } diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp index 5cedcaf1c3d..c71e9d832f3 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp @@ -107,6 +107,15 @@ MaintenanceController::killJobs() } void +MaintenanceController::updateMetrics(DocumentDBTaggedMetrics & metrics) +{ + Guard guard(_jobsLock); + for (auto &job : _jobs) { + job->getJob().updateMetrics(metrics); // Make sure no more tasks are added to the executor + } +} + +void MaintenanceController::performHoldJobs(JobList jobs) { // Called by master write thread diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.h b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.h index 3c8510e6c66..fdb6f4fa880 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.h +++ b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.h @@ -52,6 +52,7 @@ public: void stop(); void start(const DocumentDBMaintenanceConfigSP &config); void newConfig(const DocumentDBMaintenanceConfigSP &config); + void updateMetrics(DocumentDBTaggedMetrics & metrics); void syncSubDBs(const MaintenanceDocumentSubDB &readySubDB, diff --git a/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.cpp b/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.cpp index e3d565afb17..d8c3cbafc88 100644 --- a/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.cpp @@ -50,18 +50,11 @@ MoveOperationLimiter::clearJob() _job = nullptr; } -bool -MoveOperationLimiter::isAboveLimit() const -{ - LockGuard guard(_mutex); - return (_outstandingOps >= _maxOutstandingOps); -} - -bool -MoveOperationLimiter::hasPending() const +size_t +MoveOperationLimiter::numPending() const { LockGuard guard(_mutex); - return (_outstandingOps > 0); + return _outstandingOps; } std::shared_ptr<vespalib::IDestructorCallback> diff --git a/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.h b/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.h index b5c8b1b9998..096e45d3cc6 100644 --- a/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.h +++ b/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.h @@ -37,9 +37,10 @@ public: MoveOperationLimiter(IBlockableMaintenanceJob *job, uint32_t maxOutstandingOps); ~MoveOperationLimiter() override; void clearJob(); - bool isAboveLimit() const; - bool hasPending() const; + bool isAboveLimit() const { return numPending() >= _maxOutstandingOps; } + bool hasPending() const { return numPending() > 0;} std::shared_ptr<vespalib::IDestructorCallback> beginOperation() override; + size_t numPending() const override; }; } diff --git a/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.cpp index f4010857c76..c89f83defea 100644 --- a/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.cpp +++ b/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.cpp @@ -79,11 +79,27 @@ DirectTensorAttribute::setTensor(DocId lid, const vespalib::eval::Value &tensor) void DirectTensorAttribute::update_tensor(DocId docId, const document::TensorUpdate &update, - const vespalib::eval::Value &old_tensor) + bool create_if_non_existing) { - auto new_value = update.apply_to(old_tensor, FastValueBuilderFactory::get()); - if (new_value) { - set_tensor(docId, std::move(new_value)); + EntryRef ref; + if (docId < getCommittedDocIdLimit()) { + ref = _refVector[docId]; + } + if (ref.valid()) { + auto ptr = _direct_store.get_tensor(ref); + if (ptr) { + auto new_value = update.apply_to(*ptr, FastValueBuilderFactory::get()); + if (new_value) { + set_tensor(docId, std::move(new_value)); + } + return; + } + } + if (create_if_non_existing) { + auto new_value = update.apply_to(*_emptyTensor, FastValueBuilderFactory::get()); + if (new_value) { + set_tensor(docId, std::move(new_value)); + } } } diff --git a/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.h b/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.h index a87526342ef..dfa5ff0b3ce 100644 --- a/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.h +++ b/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.h @@ -19,7 +19,7 @@ public: virtual void setTensor(DocId docId, const vespalib::eval::Value &tensor) override; void update_tensor(DocId docId, const document::TensorUpdate &update, - const vespalib::eval::Value &old_tensor) override; + bool create_empty_if_non_existing) override; virtual std::unique_ptr<vespalib::eval::Value> getTensor(DocId docId) const override; virtual bool onLoad() override; virtual std::unique_ptr<AttributeSaver> onInitSave(vespalib::stringref fileName) override; diff --git a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp index f5de1de640f..042ecba0901 100644 --- a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp +++ b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp @@ -254,10 +254,21 @@ TensorAttribute::getRefCopy() const void TensorAttribute::update_tensor(DocId docId, const document::TensorUpdate &update, - const vespalib::eval::Value &old_tensor) + bool create_empty_if_non_existing) { - auto new_value = update.apply_to(old_tensor, FastValueBuilderFactory::get()); - setTensor(docId, *new_value); + const vespalib::eval::Value * old_v = nullptr; + auto old_tensor = getTensor(docId); + if (old_tensor) { + old_v = old_tensor.get(); + } else if (create_empty_if_non_existing) { + old_v = _emptyTensor.get(); + } else { + return; + } + auto new_value = update.apply_to(*old_v, FastValueBuilderFactory::get()); + if (new_value) { + setTensor(docId, *new_value); + } } std::unique_ptr<PrepareResult> diff --git a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h index adb9e7bca8c..9d92e226139 100644 --- a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h +++ b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h @@ -61,7 +61,7 @@ public: virtual void setTensor(DocId docId, const vespalib::eval::Value &tensor) = 0; virtual void update_tensor(DocId docId, const document::TensorUpdate &update, - const vespalib::eval::Value &oldTensor); + bool create_empty_if_non_existing); /** * Performs the prepare step in a two-phase operation to set a tensor for a document. * diff --git a/vespa_jersey2/pom.xml b/vespa_jersey2/pom.xml index e19b6bde538..61777d745f1 100644 --- a/vespa_jersey2/pom.xml +++ b/vespa_jersey2/pom.xml @@ -56,6 +56,11 @@ <build> <plugins> <plugin> + <!-- Explicit for IntelliJ to detect correct language level from parent --> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-compiler-plugin</artifactId> + </plugin> + <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-dependency-plugin</artifactId> <executions> |