diff options
40 files changed, 259 insertions, 97 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java index 8f3bc83984a..09a687657c6 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java @@ -418,8 +418,9 @@ public class TenantApplications implements RequestHandler, HostValidator { /** * Waiter for removing application. Will wait for some time for all servers to remove application, - * but will accept majority of servers to have removed app if it takes a long time. + * but will accept the majority of servers to have removed app if it takes a long time. */ + // TODO: Merge with CuratorCompletionWaiter static class RemoveApplicationWaiter implements CompletionWaiter { private static final java.util.logging.Logger log = Logger.getLogger(RemoveApplicationWaiter.class.getName()); @@ -485,7 +486,7 @@ public class TenantApplications implements RequestHandler, HostValidator { gotQuorumTime = clock.instant(); // Give up if more than some time has passed since we got quorum, otherwise continue - if (Duration.between(Instant.now(), gotQuorumTime.plus(waitForAll)).isNegative()) { + if (Duration.between(clock.instant(), gotQuorumTime.plus(waitForAll)).isNegative()) { logBarrierCompleted(respondents, startTime); break; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpConfigRequest.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpConfigRequest.java index 3baa57b2b01..e043afdbf43 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpConfigRequest.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpConfigRequest.java @@ -30,15 +30,18 @@ import java.util.Set; */ public class HttpConfigRequest implements GetConfigRequest, TenantRequest { - private static final String HTTP_PROPERTY_NOCACHE = "noCache"; + private static final String NOCACHE = "noCache"; + private static final String REQUIRED_GENERATION = "requiredGeneration"; private final ConfigKey<?> key; private final ApplicationId appId; private final boolean noCache; + private final Optional<Long> requiredGeneration; - private HttpConfigRequest(ConfigKey<?> key, ApplicationId appId, boolean noCache) { + private HttpConfigRequest(ConfigKey<?> key, ApplicationId appId, boolean noCache, Optional<Long> requiredGeneration) { this.key = key; this.appId = appId; this.noCache = noCache; + this.requiredGeneration = requiredGeneration; } private static ConfigKey<?> fromRequestV1(HttpRequest req) { @@ -59,7 +62,10 @@ public class HttpConfigRequest implements GetConfigRequest, TenantRequest { } public static HttpConfigRequest createFromRequestV1(HttpRequest req) { - return new HttpConfigRequest(fromRequestV1(req), ApplicationId.defaultId(), req.getBooleanProperty(HTTP_PROPERTY_NOCACHE)); + return new HttpConfigRequest(fromRequestV1(req), + ApplicationId.defaultId(), + req.getBooleanProperty(NOCACHE), + requiredGeneration(req)); } public static HttpConfigRequest createFromRequestV2(HttpRequest req) { @@ -89,7 +95,8 @@ public class HttpConfigRequest implements GetConfigRequest, TenantRequest { cNamespace = nns.second; return new HttpConfigRequest(new ConfigKey<>(cName, cId, cNamespace), new ApplicationId.Builder().applicationName(application).tenant(tenant).build(), - req.getBooleanProperty(HTTP_PROPERTY_NOCACHE)); + req.getBooleanProperty(NOCACHE), + requiredGeneration(req)); } // The URL pattern with full app id given @@ -117,7 +124,10 @@ public class HttpConfigRequest implements GetConfigRequest, TenantRequest { .applicationName(application) .instanceName(instance) .build(); - return new HttpConfigRequest(new ConfigKey<>(cName, cId, cNamespace), appId, req.getBooleanProperty(HTTP_PROPERTY_NOCACHE)); + return new HttpConfigRequest(new ConfigKey<>(cName, cId, cNamespace), + appId, + req.getBooleanProperty(NOCACHE), + requiredGeneration(req)); } /** @@ -144,7 +154,11 @@ public class HttpConfigRequest implements GetConfigRequest, TenantRequest { public static void throwModelNotReady() { throw new NotFoundException("Config not available, verify that an application package has been deployed and activated."); } - + + public static void throwPreconditionFailed(long requiredGeneration) { + throw new PreconditionFailedException("Config for required generation " + requiredGeneration + " could not be found."); + } + /** * If the given config is produced by the model at all * @@ -199,4 +213,11 @@ public class HttpConfigRequest implements GetConfigRequest, TenantRequest { @Override public PayloadChecksums configPayloadChecksums() { return PayloadChecksums.empty(); } + public Optional<Long> requiredGeneration() { return requiredGeneration; } + + static Optional<Long> requiredGeneration(HttpRequest req) { + Optional<String> requiredGeneration = Optional.ofNullable(req.getProperty(REQUIRED_GENERATION)); + return requiredGeneration.map(Long::parseLong); + } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpErrorResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpErrorResponse.java index 40ce16145e7..3b5269cdf11 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpErrorResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpErrorResponse.java @@ -16,6 +16,7 @@ import static com.yahoo.jdisc.Response.Status.CONFLICT; import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR; import static com.yahoo.jdisc.Response.Status.METHOD_NOT_ALLOWED; import static com.yahoo.jdisc.Response.Status.NOT_FOUND; +import static com.yahoo.jdisc.Response.Status.PRECONDITION_FAILED; import static com.yahoo.jdisc.Response.Status.REQUEST_TIMEOUT; /** @@ -51,7 +52,8 @@ public class HttpErrorResponse extends HttpResponse { CERTIFICATE_NOT_READY, LOAD_BALANCER_NOT_READY, CONFIG_NOT_CONVERGED, - REINDEXING_STATUS_UNAVAILABLE + REINDEXING_STATUS_UNAVAILABLE, + PRECONDITION_FAILED } public static HttpErrorResponse notFoundError(String msg) { @@ -114,6 +116,10 @@ public class HttpErrorResponse extends HttpResponse { return new HttpErrorResponse(CONFLICT, ErrorCode.REINDEXING_STATUS_UNAVAILABLE.name(), msg); } + public static HttpResponse preconditionFailed(String msg) { + return new HttpErrorResponse(PRECONDITION_FAILED, ErrorCode.PRECONDITION_FAILED.name(), msg); + } + @Override public void render(OutputStream stream) throws IOException { new JsonFormat(true).encode(stream, slime); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java index dc3a05e65f9..a0e814f32d8 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java @@ -71,6 +71,8 @@ public class HttpHandler extends ThreadedHttpRequestHandler { return HttpErrorResponse.loadBalancerNotReady(getMessage(e, request)); } catch (ReindexingStatusException e) { return HttpErrorResponse.reindexingStatusUnavailable(getMessage(e, request)); + } catch (PreconditionFailedException e) { + return HttpErrorResponse.preconditionFailed(getMessage(e, request)); } catch (Exception e) { log.log(Level.WARNING, "Unexpected exception handling a config server request", e); return HttpErrorResponse.internalServerError(getMessage(e, request)); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/NotFoundException.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/NotFoundException.java index eb008de6ee5..688890c75b2 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/NotFoundException.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/NotFoundException.java @@ -5,7 +5,6 @@ package com.yahoo.vespa.config.server.http; * Exception that will create a http response with NOT_FOUND response code (404) * * @author hmusum - * @since 5.1.17 */ public class NotFoundException extends RuntimeException { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/PreconditionFailedException.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/PreconditionFailedException.java new file mode 100644 index 00000000000..ef3924425c2 --- /dev/null +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/PreconditionFailedException.java @@ -0,0 +1,16 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.config.server.http; + +/** + * Exception that will create a http response with NOT_FOUND response code (404) + * + * @author hmusum + */ +public class PreconditionFailedException extends RuntimeException { + + public PreconditionFailedException(String message) { + super(message); + } + +} + diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandler.java index 3ab3df99a10..0389b2a6c98 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandler.java @@ -4,31 +4,27 @@ package com.yahoo.vespa.config.server.http.v2; import com.yahoo.component.annotation.Inject; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; -import java.util.logging.Level; import com.yahoo.vespa.config.protocol.ConfigResponse; import com.yahoo.vespa.config.server.RequestHandler; -import com.yahoo.vespa.config.server.http.v2.request.HttpConfigRequests; -import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.config.server.http.HttpConfigRequest; import com.yahoo.vespa.config.server.http.HttpConfigResponse; import com.yahoo.vespa.config.server.http.HttpHandler; - +import com.yahoo.vespa.config.server.http.v2.request.HttpConfigRequests; +import com.yahoo.vespa.config.server.tenant.TenantRepository; import java.util.Optional; +import java.util.logging.Level; /** * HTTP handler for a getConfig operation * * @author Ulf Lilleengen - * @since 5.1 */ public class HttpGetConfigHandler extends HttpHandler { private final TenantRepository tenantRepository; @Inject - public HttpGetConfigHandler(HttpHandler.Context ctx, - TenantRepository tenantRepository) - { + public HttpGetConfigHandler(HttpHandler.Context ctx, TenantRepository tenantRepository) { super(ctx); this.tenantRepository = tenantRepository; } @@ -45,6 +41,8 @@ public class HttpGetConfigHandler extends HttpHandler { log.log(Level.FINE, () -> "nocache=" + request.noCache()); ConfigResponse config = requestHandler.resolveConfig(request.getApplicationId(), request, Optional.empty()); if (config == null) HttpConfigRequest.throwModelNotReady(); + if (request.requiredGeneration().isPresent() && request.requiredGeneration().get() != config.getGeneration()) + HttpConfigRequest.throwPreconditionFailed(request.requiredGeneration().get()); return config; } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java index a0b5b879e45..401bd1ae55b 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java @@ -25,6 +25,7 @@ import org.junit.rules.TemporaryFolder; import java.io.File; import java.io.IOException; import java.util.Collections; +import java.util.Map; import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; import static com.yahoo.jdisc.Response.Status.NOT_FOUND; @@ -131,6 +132,20 @@ public class HttpGetConfigHandlerTest { assertTrue(renderedString, renderedString.startsWith(expected)); } + @Test + public void require_that_required_generation_property_works() throws IOException { + HttpRequest request = HttpRequest.createTestRequest(configUri, GET, null, Map.of("requiredGeneration", "2")); + HttpResponse response = handler.handle(request); + String renderedString = SessionHandlerTest.getRenderedString(response); + assertTrue(renderedString, renderedString.startsWith(expected)); + + request = HttpRequest.createTestRequest(configUri, GET, null, Map.of("requiredGeneration", "3")); + response = handler.handle(request); + assertEquals(412, response.getStatus()); + renderedString = SessionHandlerTest.getRenderedString(response); + assertEquals("{\"error-code\":\"PRECONDITION_FAILED\",\"message\":\"Config for required generation 3 could not be found.\"}", renderedString); + } + private PrepareParams prepareParams() { return new PrepareParams.Builder().applicationId(applicationId).build(); } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java index f128d13b7c4..9aa6ef414fc 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java @@ -85,7 +85,7 @@ public class PermanentFlags { "host-flavor", "", "Specifies the Vespa flavor name that the hosts of the matching nodes should have.", "Takes effect on next deployment (including internal redeployment).", - APPLICATION_ID, CLUSTER_TYPE); + APPLICATION_ID, CLUSTER_TYPE, CLUSTER_ID); public static final UnboundBooleanFlag SKIP_MAINTENANCE_DEPLOYMENT = defineFeatureFlag( "node-repository-skip-maintenance-deployment", false, diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java index 1d5581b511d..83dadddf76c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java @@ -196,8 +196,8 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer { Version osVersion = nodeRepository().osVersions().targetFor(NodeType.host).orElse(Version.emptyVersion); List<Integer> provisionIndices = nodeRepository().database().readProvisionIndices(count); List<Node> hosts = new ArrayList<>(); - hostProvisioner.provisionHosts(provisionIndices, NodeType.host, nodeResources, ApplicationId.defaultId(), - osVersion, HostSharing.shared, Optional.empty(), nodeRepository().zone().cloud().account(), + hostProvisioner.provisionHosts(provisionIndices, NodeType.host, nodeResources, ApplicationId.defaultId(), osVersion, + HostSharing.shared, Optional.empty(), Optional.empty(), nodeRepository().zone().cloud().account(), provisionedHosts -> { hosts.addAll(provisionedHosts.stream().map(ProvisionedHost::generateHost).toList()); nodeRepository().nodes().addNodes(hosts, Agent.HostCapacityMaintainer); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index 5c2ca58a6b7..691f88a9be3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -115,7 +115,7 @@ public class GroupPreparer { try { hostProvisioner.get().provisionHosts( allocation.provisionIndices(deficit.count()), hostType, deficit.resources(), application, - osVersion, sharing, Optional.of(cluster.type()), requestedNodes.cloudAccount(), + osVersion, sharing, Optional.of(cluster.type()), Optional.of(cluster.id()), requestedNodes.cloudAccount(), provisionedHostsConsumer); } catch (NodeAllocationException e) { // Mark the nodes that were written to ZK in the consumer for deprovisioning. While these hosts do diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java index 38fa1abf8e2..f07185cbe60 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java @@ -50,6 +50,8 @@ public interface HostProvisioner { * @param sharing puts requirements on sharing or exclusivity of the host to be provisioned. * @param clusterType the cluster we are provisioning for, or empty if we are provisioning hosts * to be shared by multiple cluster nodes + * @param clusterId the id of the cluster we are provisioning for, or empty if we are provisioning hosts + * to be shared by multiple cluster nodes * @param cloudAccount the cloud account to use * @param provisionedHostConsumer consumer of {@link ProvisionedHost}s describing the provisioned nodes, * the {@link Node} returned from {@link ProvisionedHost#generateHost()} must be @@ -64,6 +66,7 @@ public interface HostProvisioner { Version osVersion, HostSharing sharing, Optional<ClusterSpec.Type> clusterType, + Optional<ClusterSpec.Id> clusterId, CloudAccount cloudAccount, Consumer<List<ProvisionedHost>> provisionedHostConsumer) throws NodeAllocationException; 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 ab5cd577ea1..c6971f0fe02 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 @@ -95,6 +95,7 @@ class NodeAllocation { this.requiredHostFlavor = Optional.of(PermanentFlags.HOST_FLAVOR.bindTo(nodeRepository.flagSource()) .with(FetchVector.Dimension.APPLICATION_ID, application.serializedForm()) .with(FetchVector.Dimension.CLUSTER_TYPE, cluster.type().name()) + .with(FetchVector.Dimension.CLUSTER_ID, cluster.id().value()) .value()) .filter(s -> !s.isBlank()); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java index 5f3cb873e10..7e83e265496 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java @@ -65,8 +65,8 @@ public class MockHostProvisioner implements HostProvisioner { @Override public void provisionHosts(List<Integer> provisionIndices, NodeType hostType, NodeResources resources, ApplicationId applicationId, Version osVersion, HostSharing sharing, - Optional<ClusterSpec.Type> clusterType, CloudAccount cloudAccount, - Consumer<List<ProvisionedHost>> provisionedHostsConsumer) { + Optional<ClusterSpec.Type> clusterType, Optional<ClusterSpec.Id> clusterId, + CloudAccount cloudAccount, Consumer<List<ProvisionedHost>> provisionedHostsConsumer) { Flavor hostFlavor = hostFlavors.get(clusterType.orElse(ClusterSpec.Type.content)); if (hostFlavor == null) hostFlavor = flavors.stream() diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java index 6362a07ae00..0e19c48591d 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java @@ -271,7 +271,7 @@ public class DynamicProvisioningTest { assertEquals(Set.of("x86"), nodes.parentsOf(tester.getNodes(app, Node.State.active).retired()).stream().map(n -> n.flavor().name()).collect(Collectors.toSet())); assertEquals(Set.of("arm"), nodes.parentsOf(tester.getNodes(app, Node.State.active).not().retired()).stream().map(n -> n.flavor().name()).collect(Collectors.toSet())); - flagSource.removeFlag(PermanentFlags.HOST_FLAVOR.id()); // Resetting flag does not moves the nodes back + flagSource.removeFlag(PermanentFlags.HOST_FLAVOR.id()); // Resetting flag does not move the nodes back tester.activate(app, cluster, capacity); nodes = tester.nodeRepository().nodes().list(); assertEquals(4, nodes.owner(app).state(Node.State.active).retired().size()); diff --git a/searchlib/src/tests/attribute/raw_attribute/raw_attribute_test.cpp b/searchlib/src/tests/attribute/raw_attribute/raw_attribute_test.cpp index bc9d361e29a..82e4fd065cf 100644 --- a/searchlib/src/tests/attribute/raw_attribute/raw_attribute_test.cpp +++ b/searchlib/src/tests/attribute/raw_attribute/raw_attribute_test.cpp @@ -69,4 +69,24 @@ TEST_F(RawAttributeTest, can_set_and_clear_value) EXPECT_EQ(empty, get_raw(1)); } +TEST_F(RawAttributeTest, implements_serialize_for_sort) { + vespalib::string long_hello("hello, is there anybody out there"); + vespalib::ConstArrayRef<char> raw_long_hello(long_hello.c_str(), long_hello.size()); + uint8_t buf[8]; + memset(buf, 0, sizeof(buf)); + _attr->addDocs(10); + _attr->commit(); + EXPECT_EQ(0, _attr->serializeForAscendingSort(1, buf, sizeof(buf))); + EXPECT_EQ(0, _attr->serializeForDescendingSort(1, buf, sizeof(buf))); + _raw->set_raw(1, raw_hello); + EXPECT_EQ(5, _attr->serializeForAscendingSort(1, buf, sizeof(buf))); + EXPECT_EQ(0, memcmp("hello", buf, 5)); + EXPECT_EQ(5, _attr->serializeForDescendingSort(1, buf, sizeof(buf))); + uint8_t expected [] = {0xff-'h', 0xff-'e', 0xff-'l', 0xff-'l', 0xff-'o'}; + EXPECT_EQ(0, memcmp(expected, buf, 5)); + _raw->set_raw(1, raw_long_hello); + EXPECT_EQ(-1, _attr->serializeForAscendingSort(1, buf, sizeof(buf))); + EXPECT_EQ(-1, _attr->serializeForDescendingSort(1, buf, sizeof(buf))); +} + GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp b/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp index 20373fbb3a9..e217e8c8533 100644 --- a/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp +++ b/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp @@ -339,6 +339,9 @@ testSingleValue(Attribute & svsa, Config &cfg) for (uint32_t j = i - 9; j <= i; ++j) { snprintf(tmp, sizeof(tmp), "enum%u", j % 10); EXPECT_TRUE( strcmp(t = v.get(j), tmp) == 0 ); + auto raw = v.get_raw(j); + EXPECT_EQUAL(strlen(tmp), raw.size()); + EXPECT_EQUAL(0, memcmp(raw.data(), tmp, raw.size())); e1 = v.getEnum(j); EXPECT_TRUE( v.findEnum(t, e2) ); EXPECT_TRUE( e1 == e2 ); diff --git a/searchlib/src/vespa/searchcommon/attribute/iattributevector.h b/searchlib/src/vespa/searchcommon/attribute/iattributevector.h index 34bf6f49cba..884d34c78c6 100644 --- a/searchlib/src/vespa/searchcommon/attribute/iattributevector.h +++ b/searchlib/src/vespa/searchcommon/attribute/iattributevector.h @@ -377,7 +377,11 @@ public: virtual bool isPredicateType() const { return getBasicType() == BasicType::PREDICATE; } virtual bool isTensorType() const { return getBasicType() == BasicType::TENSOR; } virtual bool isReferenceType() const { return getBasicType() == BasicType::REFERENCE; } - virtual bool is_raw_type() const noexcept { return getBasicType() == BasicType::RAW; } + virtual bool is_raw_type() const noexcept { + BasicType::Type t = getBasicType(); + return t == BasicType::RAW || + t == BasicType::STRING; + } /** * Returns whether this is a multi value attribute. diff --git a/searchlib/src/vespa/searchlib/attribute/single_raw_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/single_raw_attribute.cpp index 9bd3a81482a..04d99d3a59a 100644 --- a/searchlib/src/vespa/searchlib/attribute/single_raw_attribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/single_raw_attribute.cpp @@ -167,4 +167,40 @@ SingleRawAttribute::clearDoc(DocId docId) return 0u; } +long +SingleRawAttribute::onSerializeForAscendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const +{ + auto raw = get_raw(doc); + vespalib::ConstBufferRef buf(raw.data(), raw.size()); + if (bc != nullptr) { + buf = bc->convert(buf); + } + if (available >= (long)buf.size()) { + memcpy(serTo, buf.data(), buf.size()); + } else { + return -1; + } + return buf.size(); +} + +long +SingleRawAttribute::onSerializeForDescendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const +{ + auto raw = get_raw(doc); + vespalib::ConstBufferRef buf(raw.data(), raw.size()); + if (bc != nullptr) { + buf = bc->convert(buf); + } + if (available >= (long)buf.size()) { + auto *dst = static_cast<unsigned char *>(serTo); + const auto * src(static_cast<const uint8_t *>(buf.data())); + for (size_t i(0); i < buf.size(); ++i) { + dst[i] = 0xff - src[i]; + } + } else { + return -1; + } + return buf.size(); +} + } diff --git a/searchlib/src/vespa/searchlib/attribute/single_raw_attribute.h b/searchlib/src/vespa/searchlib/attribute/single_raw_attribute.h index 7477b13bc5a..52b81a782b9 100644 --- a/searchlib/src/vespa/searchlib/attribute/single_raw_attribute.h +++ b/searchlib/src/vespa/searchlib/attribute/single_raw_attribute.h @@ -38,6 +38,8 @@ public: vespalib::ConstArrayRef<char> get_raw(DocId docid) const override; void set_raw(DocId docid, vespalib::ConstArrayRef<char> raw); uint32_t clearDoc(DocId docId) override; + long onSerializeForAscendingSort(DocId, void *, long, const common::BlobConverter *) const override; + long onSerializeForDescendingSort(DocId, void *, long, const common::BlobConverter *) const override; }; } diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp index 22a2eab1111..2800d7c3f6d 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp @@ -86,9 +86,10 @@ StringAttribute::getFloat(DocId doc) const { } vespalib::ConstArrayRef<char> -StringAttribute::get_raw(DocId) const +StringAttribute::get_raw(DocId doc) const { - return {}; + const char * s = get(doc); + return {s, s ? ::strlen(s) : 0u}; } uint32_t @@ -118,7 +119,6 @@ StringAttribute::get(DocId doc, largeint_t * v, uint32_t sz) const long StringAttribute::onSerializeForAscendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const { - auto *dst = static_cast<unsigned char *>(serTo); const char *value(get(doc)); int size = strlen(value) + 1; vespalib::ConstBufferRef buf(value, size); @@ -126,7 +126,7 @@ StringAttribute::onSerializeForAscendingSort(DocId doc, void * serTo, long avail buf = bc->convert(buf); } if (available >= (long)buf.size()) { - memcpy(dst, buf.data(), buf.size()); + memcpy(serTo, buf.data(), buf.size()); } else { return -1; } @@ -136,8 +136,6 @@ StringAttribute::onSerializeForAscendingSort(DocId doc, void * serTo, long avail long StringAttribute::onSerializeForDescendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const { - (void) bc; - auto *dst = static_cast<unsigned char *>(serTo); const char *value(get(doc)); int size = strlen(value) + 1; vespalib::ConstBufferRef buf(value, size); @@ -145,6 +143,7 @@ StringAttribute::onSerializeForDescendingSort(DocId doc, void * serTo, long avai buf = bc->convert(buf); } if (available >= (long)buf.size()) { + auto *dst = static_cast<unsigned char *>(serTo); const auto * src(static_cast<const uint8_t *>(buf.data())); for (size_t i(0); i < buf.size(); ++i) { dst[i] = 0xff - src[i]; diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.h b/searchlib/src/vespa/searchlib/attribute/stringbase.h index f40a89f76b4..3de7df5aa28 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.h +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.h @@ -51,6 +51,10 @@ public: static void generateOffsets(const char * bt, size_t sz, OffsetVector & offsets); virtual const char * getFromEnum(EnumHandle e) const = 0; virtual const char *get(DocId doc) const = 0; + largeint_t getInt(DocId doc) const override { return strtoll(get(doc), nullptr, 0); } + double getFloat(DocId doc) const override; + vespalib::ConstArrayRef<char> get_raw(DocId) const override; + const char * getString(DocId doc, char * v, size_t sz) const override { (void) v; (void) sz; return get(doc); } protected: StringAttribute(const vespalib::string & name); StringAttribute(const vespalib::string & name, const Config & c); @@ -79,11 +83,6 @@ private: virtual void load_enumerated_data(ReaderBase &attrReader, enumstore::EnumeratedLoader& loader); virtual void load_posting_lists_and_update_enum_store(enumstore::EnumeratedPostingsLoader& loader); - largeint_t getInt(DocId doc) const override { return strtoll(get(doc), nullptr, 0); } - double getFloat(DocId doc) const override; - vespalib::ConstArrayRef<char> get_raw(DocId) const override; - const char * getString(DocId doc, char * v, size_t sz) const override { (void) v; (void) sz; return get(doc); } - long onSerializeForAscendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const override; long onSerializeForDescendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const override; }; diff --git a/searchlib/src/vespa/searchlib/common/identifiable.h b/searchlib/src/vespa/searchlib/common/identifiable.h index 1f5aff6d2d0..4b401633a2d 100644 --- a/searchlib/src/vespa/searchlib/common/identifiable.h +++ b/searchlib/src/vespa/searchlib/common/identifiable.h @@ -151,6 +151,7 @@ #define CID_search_expression_AttributeMapLookupNode SEARCHLIB_CID(145) #define CID_search_expression_BoolResultNode SEARCHLIB_CID(146) #define CID_search_expression_BoolResultNodeVector SEARCHLIB_CID(147) +#define CID_search_expression_RawAttributeResult SEARCHLIB_CID(148) #define CID_search_QueryNode SEARCHLIB_CID(150) diff --git a/searchlib/src/vespa/searchlib/expression/attributenode.cpp b/searchlib/src/vespa/searchlib/expression/attributenode.cpp index f8ae4bd698d..73f306fc708 100644 --- a/searchlib/src/vespa/searchlib/expression/attributenode.cpp +++ b/searchlib/src/vespa/searchlib/expression/attributenode.cpp @@ -77,7 +77,9 @@ createResult(const IAttributeVector * attribute) { IAttributeVector::EnumRefs enumRefs = attribute->make_enum_read_view(); return (enumRefs.empty()) - ? std::make_unique<AttributeResult>(attribute, 0) + ? attribute->is_raw_type() + ? std::make_unique<RawAttributeResult>(attribute, 0) + : std::make_unique<AttributeResult>(attribute, 0) : std::make_unique<EnumAttributeResult>(enumRefs, attribute, 0); } @@ -221,6 +223,13 @@ AttributeNode::onPrepare(bool preserveAccurateTypes) setResultType(std::make_unique<StringResultNode>()); } } + } else if (attribute->is_raw_type()) { + if (_hasMultiValue) { + throw std::runtime_error(make_string("Does not support multivalue raw attribute vector '%s'", + attribute->getName().c_str())); + } else { + setResultType(std::make_unique<RawResultNode>()); + } } else { throw std::runtime_error(make_string("Can not deduce correct resultclass for attribute vector '%s'", attribute->getName().c_str())); diff --git a/searchlib/src/vespa/searchlib/expression/attributenode.h b/searchlib/src/vespa/searchlib/expression/attributenode.h index 03b7909e581..d668bd3f662 100644 --- a/searchlib/src/vespa/searchlib/expression/attributenode.h +++ b/searchlib/src/vespa/searchlib/expression/attributenode.h @@ -46,7 +46,7 @@ public: AttributeNode(const search::attribute::IAttributeVector & attribute); AttributeNode(const AttributeNode & attribute); AttributeNode & operator = (const AttributeNode & attribute); - ~AttributeNode(); + ~AttributeNode() override; void setDocId(DocId docId) const { _scratchResult->setDocId(docId); } const search::attribute::IAttributeVector *getAttribute() const { return _scratchResult ? _scratchResult->getAttribute() : nullptr; @@ -59,7 +59,7 @@ public: class Handler { public: - virtual ~Handler() { } + virtual ~Handler() = default; virtual void handle(const AttributeResult & r) = 0; }; private: diff --git a/searchlib/src/vespa/searchlib/expression/attributeresult.cpp b/searchlib/src/vespa/searchlib/expression/attributeresult.cpp index 9eb8b35d83c..8a4574265c4 100644 --- a/searchlib/src/vespa/searchlib/expression/attributeresult.cpp +++ b/searchlib/src/vespa/searchlib/expression/attributeresult.cpp @@ -5,5 +5,6 @@ namespace search::expression { IMPLEMENT_RESULTNODE(AttributeResult, ResultNode); +IMPLEMENT_RESULTNODE(RawAttributeResult, ResultNode); } diff --git a/searchlib/src/vespa/searchlib/expression/attributeresult.h b/searchlib/src/vespa/searchlib/expression/attributeresult.h index 0501b6477cf..5fd271b6ae0 100644 --- a/searchlib/src/vespa/searchlib/expression/attributeresult.h +++ b/searchlib/src/vespa/searchlib/expression/attributeresult.h @@ -13,13 +13,18 @@ public: using UP = std::unique_ptr<AttributeResult>; DECLARE_RESULTNODE(AttributeResult); AttributeResult() : _attribute(nullptr), _docId(0) { } - AttributeResult(const attribute::IAttributeVector * attribute, DocId docId) : - _attribute(attribute), - _docId(docId) + AttributeResult(const attribute::IAttributeVector * attribute, DocId docId) + : _attribute(attribute), + _docId(docId) { } void setDocId(DocId docId) { _docId = docId; } const search::attribute::IAttributeVector *getAttribute() const { return _attribute; } DocId getDocId() const { return _docId; } +protected: + ConstBufferRef get_raw() const { + auto raw = getAttribute()->get_raw(_docId); + return {raw.data(), raw.size()}; + } private: int64_t onGetInteger(size_t index) const override { (void) index; return _attribute->getInt(_docId); } double onGetFloat(size_t index) const override { (void) index; return _attribute->getFloat(_docId); } @@ -36,4 +41,17 @@ private: DocId _docId; }; +class RawAttributeResult : public AttributeResult { +public: + DECLARE_RESULTNODE(RawAttributeResult); + RawAttributeResult() : AttributeResult() {} + RawAttributeResult(const attribute::IAttributeVector * attribute, DocId docId) + : AttributeResult(attribute, docId) + { } + ConstBufferRef onGetString(size_t index, BufferRef buf) const override { + (void) index; (void) buf; + return get_raw(); + } +}; + } diff --git a/searchlib/src/vespa/searchlib/expression/catfunctionnode.h b/searchlib/src/vespa/searchlib/expression/catfunctionnode.h index 0667b408500..33df55c891a 100644 --- a/searchlib/src/vespa/searchlib/expression/catfunctionnode.h +++ b/searchlib/src/vespa/searchlib/expression/catfunctionnode.h @@ -3,8 +3,7 @@ #include "multiargfunctionnode.h" -namespace search { -namespace expression { +namespace search::expression { class CatFunctionNode : public MultiArgFunctionNode { @@ -19,5 +18,3 @@ private: }; } -} - diff --git a/searchlib/src/vespa/searchlib/expression/documentfieldnode.cpp b/searchlib/src/vespa/searchlib/expression/documentfieldnode.cpp index f48be061d15..bd13c032a03 100644 --- a/searchlib/src/vespa/searchlib/expression/documentfieldnode.cpp +++ b/searchlib/src/vespa/searchlib/expression/documentfieldnode.cpp @@ -140,7 +140,8 @@ DocumentFieldNode::onPrepare(bool preserveAccurateTypes) } } -void DocumentFieldNode::onDocType(const DocumentType & docType) +void +DocumentFieldNode::onDocType(const DocumentType & docType) { LOG(debug, "DocumentFieldNode::onDocType(this=%p)", this); _fieldPath.clear(); @@ -173,12 +174,14 @@ private: char DefaultValue::null = 0; -void DefaultValue::set(const ResultNode&) +void +DefaultValue::set(const ResultNode&) { throw std::runtime_error("DefaultValue::set(const ResultNode&) is not possible."); } -void FieldValue2ResultNode::set(const ResultNode&) +void +FieldValue2ResultNode::set(const ResultNode&) { throw std::runtime_error("FieldValue2ResultNode::set(const ResultNode&) is not possible."); } @@ -192,7 +195,8 @@ void DocumentFieldNode::onDoc(const Document & doc) _handler->reset(); } -bool DocumentFieldNode::onExecute() const +bool +DocumentFieldNode::onExecute() const { _doc->iterateNested(_fieldPath.getFullRange(), *_handler); return true; @@ -237,12 +241,14 @@ DocumentFieldNode::Handler::onStructStart(const Content & c) } -Serializer & DocumentFieldNode::onSerialize(Serializer & os) const +Serializer & +DocumentFieldNode::onSerialize(Serializer & os) const { return os << _fieldName << _value; } -Deserializer & DocumentFieldNode::onDeserialize(Deserializer & is) +Deserializer & +DocumentFieldNode::onDeserialize(Deserializer & is) { return is >> _fieldName >> _value; } diff --git a/searchlib/src/vespa/searchlib/expression/enumresultnode.h b/searchlib/src/vespa/searchlib/expression/enumresultnode.h index 14dacd75651..6d201cb2b5d 100644 --- a/searchlib/src/vespa/searchlib/expression/enumresultnode.h +++ b/searchlib/src/vespa/searchlib/expression/enumresultnode.h @@ -3,8 +3,7 @@ #include "integerresultnode.h" -namespace search { -namespace expression { +namespace search::expression { class EnumResultNode : public IntegerResultNodeT<int64_t> { @@ -20,5 +19,3 @@ private: }; } -} - diff --git a/searchlib/src/vespa/searchlib/expression/floatresultnode.h b/searchlib/src/vespa/searchlib/expression/floatresultnode.h index c31f9a2de40..e79911fe985 100644 --- a/searchlib/src/vespa/searchlib/expression/floatresultnode.h +++ b/searchlib/src/vespa/searchlib/expression/floatresultnode.h @@ -4,8 +4,7 @@ #include "numericresultnode.h" #include <vespa/vespalib/util/sort.h> -namespace search { -namespace expression { +namespace search ::expression { class FloatResultNode final : public NumericResultNode { @@ -54,5 +53,3 @@ private: }; } -} - diff --git a/searchlib/src/vespa/searchlib/expression/integerbucketresultnode.h b/searchlib/src/vespa/searchlib/expression/integerbucketresultnode.h index 95a4555e6e4..ffd0fb11701 100644 --- a/searchlib/src/vespa/searchlib/expression/integerbucketresultnode.h +++ b/searchlib/src/vespa/searchlib/expression/integerbucketresultnode.h @@ -3,8 +3,7 @@ #include "bucketresultnode.h" -namespace search { -namespace expression { +namespace search::expression { class IntegerBucketResultNode : public BucketResultNode { @@ -48,5 +47,3 @@ public: }; } -} - diff --git a/searchlib/src/vespa/searchlib/expression/nullresultnode.h b/searchlib/src/vespa/searchlib/expression/nullresultnode.h index e873d85d0f1..b16fa2245de 100644 --- a/searchlib/src/vespa/searchlib/expression/nullresultnode.h +++ b/searchlib/src/vespa/searchlib/expression/nullresultnode.h @@ -3,8 +3,7 @@ #include "singleresultnode.h" -namespace search { -namespace expression { +namespace search::expression { class NullResultNode : public SingleResultNode { @@ -32,5 +31,3 @@ private: }; } -} - diff --git a/searchlib/src/vespa/searchlib/expression/positiveinfinityresultnode.h b/searchlib/src/vespa/searchlib/expression/positiveinfinityresultnode.h index 261b60b3613..a12bcaa0a32 100644 --- a/searchlib/src/vespa/searchlib/expression/positiveinfinityresultnode.h +++ b/searchlib/src/vespa/searchlib/expression/positiveinfinityresultnode.h @@ -3,8 +3,7 @@ #include "singleresultnode.h" -namespace search { -namespace expression { +namespace search::expression { class PositiveInfinityResultNode : public SingleResultNode { @@ -26,5 +25,3 @@ private: }; } -} - diff --git a/searchlib/src/vespa/searchlib/expression/resultnode.h b/searchlib/src/vespa/searchlib/expression/resultnode.h index 4c81259325b..6a62600b993 100644 --- a/searchlib/src/vespa/searchlib/expression/resultnode.h +++ b/searchlib/src/vespa/searchlib/expression/resultnode.h @@ -53,7 +53,6 @@ private: public: DECLARE_ABSTRACT_RESULTNODE(ResultNode); - ~ResultNode() { } using UP = std::unique_ptr<ResultNode>; using CP = vespalib::IdentifiablePtr<ResultNode>; virtual void set(const ResultNode & rhs) = 0; diff --git a/searchlib/src/vespa/searchlib/expression/stringresultnode.h b/searchlib/src/vespa/searchlib/expression/stringresultnode.h index 79d849bdd15..303d8778e99 100644 --- a/searchlib/src/vespa/searchlib/expression/stringresultnode.h +++ b/searchlib/src/vespa/searchlib/expression/stringresultnode.h @@ -3,8 +3,7 @@ #include "singleresultnode.h" -namespace search { -namespace expression { +namespace search::expression { class StringResultNode : public SingleResultNode { @@ -60,5 +59,3 @@ private: }; } -} - diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp index 74d67aabe88..e606c6f08bb 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp @@ -97,8 +97,8 @@ SingleAttrDFW::insertField(uint32_t docid, GetDocsumsState& state, Inserter &tar break; } case BasicType::STRING: { - const char *s = v.getString(docid, nullptr, 0); // no need to pass in a buffer, this attribute has a string storage. - target.insertString(vespalib::Memory(s)); + auto s = v.get_raw(docid); + target.insertString(vespalib::Memory(s.data(), s.size())); break; } case BasicType::REFERENCE: diff --git a/vespalib/src/vespa/vespalib/util/ref_counted.cpp b/vespalib/src/vespa/vespalib/util/ref_counted.cpp index 43a4647e9ec..47bb59bb287 100644 --- a/vespalib/src/vespa/vespalib/util/ref_counted.cpp +++ b/vespalib/src/vespa/vespalib/util/ref_counted.cpp @@ -12,6 +12,7 @@ enable_ref_counted::internal_addref(uint32_t cnt) const noexcept // the thread obtaining the new reference already has a reference auto prev = _refs.fetch_add(cnt, std::memory_order_relaxed); assert(prev > 0); + assert(_guard == MAGIC); } void @@ -21,6 +22,7 @@ enable_ref_counted::internal_subref(uint32_t cnt, [[maybe_unused]] uint32_t rese // our changes to the object must be visible to the deleter auto prev = _refs.fetch_sub(cnt, std::memory_order_release); assert(prev >= (reserve + cnt)); + assert(_guard == MAGIC); if (prev == cnt) { // acquire because: // we need to see all object changes before deleting it @@ -33,6 +35,7 @@ uint32_t enable_ref_counted::count_refs() const noexcept { auto result = _refs.load(std::memory_order_relaxed); assert(result > 0); + assert(_guard == MAGIC); return result; } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java index d5207c6fab8..8df37d1f6ce 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java @@ -11,9 +11,9 @@ import java.util.List; import java.util.logging.Level; /** - * Implementation of a Barrier that handles the case where more than number of members can call synchronize. If - * the number of members that synchronize exceed the expected number, the other members are immediately allowed - * to pass through the barrier. + * Implementation of a Barrier that handles the case where more than number of members can call synchronize. + * Will wait for some time for all servers to do the operation, but will accept the majority of servers to have + * done the operation if it takes longer than a specified amount of time. * * @author Vegard Havdal * @author Ulf Lilleengen @@ -21,16 +21,20 @@ import java.util.logging.Level; class CuratorCompletionWaiter implements Curator.CompletionWaiter { private static final java.util.logging.Logger log = java.util.logging.Logger.getLogger(CuratorCompletionWaiter.class.getName()); + private static final Duration waitForAllDefault = Duration.ofSeconds(1); // Make this configurable? + private final Curator curator; private final String barrierPath; private final String myId; private final Clock clock; + private final Duration waitForAll; - CuratorCompletionWaiter(Curator curator, String barrierPath, String myId, Clock clock) { + CuratorCompletionWaiter(Curator curator, String barrierPath, String myId, Clock clock, Duration waitForAll) { this.myId = barrierPath + "/" + myId; this.curator = curator; this.barrierPath = barrierPath; this.clock = clock; + this.waitForAll = waitForAll; } @Override @@ -55,6 +59,7 @@ class CuratorCompletionWaiter implements Curator.CompletionWaiter { private List<String> awaitInternal(Duration timeout) throws Exception { Instant startTime = clock.instant(); Instant endTime = startTime.plus(timeout); + Instant gotQuorumTime = Instant.EPOCH; List<String> respondents = new ArrayList<>(); do { @@ -65,15 +70,20 @@ class CuratorCompletionWaiter implements Curator.CompletionWaiter { respondents + ", all participants: " + curator.zooKeeperEnsembleConnectionSpec()); } - // First, check if all config servers responded + // If all config servers responded, return if (respondents.size() == curator.zooKeeperEnsembleCount()) { - log.log(Level.FINE, () -> barrierCompletedMessage(respondents, startTime)); + logBarrierCompleted(respondents, startTime); break; } - // If some are missing, quorum is enough + // If some are missing, quorum is enough, but wait for all up to ´waitForAll´ seconds before returning if (respondents.size() >= barrierMemberCount()) { - log.log(Level.FINE, () -> barrierCompletedMessage(respondents, startTime)); - break; + if (gotQuorumTime.isBefore(startTime)) + gotQuorumTime = clock.instant(); + + if (Duration.between(clock.instant(), gotQuorumTime.plus(waitForAll)).isNegative()) { + logBarrierCompleted(respondents, startTime); + break; + } } Thread.sleep(100); @@ -82,9 +92,15 @@ class CuratorCompletionWaiter implements Curator.CompletionWaiter { return respondents; } - private String barrierCompletedMessage(List<String> respondents, Instant startTime) { - return barrierPath + " completed in " + Duration.between(startTime, Instant.now()).toString() + - ", " + respondents.size() + "/" + curator.zooKeeperEnsembleCount() + " responded: " + respondents; + private void logBarrierCompleted(List<String> respondents, Instant startTime) { + Duration duration = Duration.between(startTime, Instant.now()); + Level level = duration.minus(Duration.ofSeconds(5)).isNegative() ? Level.FINE : Level.INFO; + log.log(level, () -> barrierCompletedMessage(respondents, duration)); + } + + private String barrierCompletedMessage(List<String> respondents, Duration duration) { + return barrierPath + " completed in " + duration.toString() + + ", " + respondents.size() + "/" + curator.zooKeeperEnsembleCount() + " responded: " + respondents; } @Override @@ -106,10 +122,18 @@ class CuratorCompletionWaiter implements Curator.CompletionWaiter { } public static Curator.CompletionWaiter create(Curator curator, Path barrierPath, String id) { - return new CuratorCompletionWaiter(curator, barrierPath.getAbsolute(), id, Clock.systemUTC()); + return create(curator, barrierPath, id, waitForAllDefault); + } + + public static Curator.CompletionWaiter create(Curator curator, Path barrierPath, String id, Duration waitForAll) { + return new CuratorCompletionWaiter(curator, barrierPath.getAbsolute(), id, Clock.systemUTC(), waitForAll); } public static Curator.CompletionWaiter createAndInitialize(Curator curator, Path parentPath, String waiterNode, String id) { + return createAndInitialize(curator, parentPath, waiterNode, id, waitForAllDefault); + } + + public static Curator.CompletionWaiter createAndInitialize(Curator curator, Path parentPath, String waiterNode, String id, Duration waitForAll) { Path waiterPath = parentPath.append(waiterNode); String debugMessage = log.isLoggable(Level.FINE) ? "Recreating ZK path " + waiterPath : null; @@ -120,7 +144,7 @@ class CuratorCompletionWaiter implements Curator.CompletionWaiter { if (debugMessage != null) log.fine(debugMessage + ": Done"); - return new CuratorCompletionWaiter(curator, waiterPath.getAbsolute(), id, Clock.systemUTC()); + return new CuratorCompletionWaiter(curator, waiterPath.getAbsolute(), id, Clock.systemUTC(), waitForAll); } private int barrierMemberCount() { diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCuratorFramework.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCuratorFramework.java index c8566015ea1..e5810763bf2 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCuratorFramework.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCuratorFramework.java @@ -84,13 +84,10 @@ import org.apache.curator.retry.RetryForever; import org.apache.curator.utils.EnsurePath; import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.KeeperException; -import org.apache.zookeeper.KeeperException.BadVersionException; -import org.apache.zookeeper.KeeperException.NoNodeException; import org.apache.zookeeper.Watcher; import org.apache.zookeeper.data.ACL; import org.apache.zookeeper.data.Stat; import org.apache.zookeeper.server.quorum.flexible.QuorumVerifier; - import java.nio.file.Paths; import java.time.Duration; import java.util.ArrayList; |