diff options
44 files changed, 413 insertions, 357 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java index 599fa962527..f2e6cd0e15f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java @@ -100,20 +100,22 @@ public abstract class LockedTenant { /** A locked CloudTenant. */ public static class Cloud extends LockedTenant { + private final Optional<Principal> creator; private final BiMap<PublicKey, Principal> developerKeys; - private Cloud(TenantName name, BiMap<PublicKey, Principal> developerKeys) { + private Cloud(TenantName name, Optional<Principal> creator, BiMap<PublicKey, Principal> developerKeys) { super(name); this.developerKeys = ImmutableBiMap.copyOf(developerKeys); + this.creator = creator; } private Cloud(CloudTenant tenant) { - this(tenant.name(), tenant.developerKeys()); + this(tenant.name(), Optional.empty(), tenant.developerKeys()); } @Override public CloudTenant get() { - return new CloudTenant(name, developerKeys); + return new CloudTenant(name, creator, developerKeys); } public Cloud withDeveloperKey(PublicKey key, Principal principal) { @@ -121,13 +123,13 @@ public abstract class LockedTenant { if (keys.containsKey(key)) throw new IllegalArgumentException("Key " + KeyUtils.toPem(key) + " is already owned by " + keys.get(key)); keys.put(key, principal); - return new Cloud(name, keys); + return new Cloud(name, creator, keys); } public Cloud withoutDeveloperKey(PublicKey key) { BiMap<PublicKey, Principal> keys = HashBiMap.create(developerKeys); keys.remove(key); - return new Cloud(name, keys); + return new Cloud(name, creator, keys); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java index 26270c092d5..2c0189c0730 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java @@ -10,6 +10,9 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.flags.BooleanFlag; +import com.yahoo.vespa.flags.FetchVector; +import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ContainerEndpoint; @@ -62,12 +65,14 @@ public class RoutingController { private final Controller controller; private final RoutingPolicies routingPolicies; private final RotationRepository rotationRepository; + private final BooleanFlag hideSharedRoutingEndpoint; public RoutingController(Controller controller, RotationsConfig rotationsConfig) { this.controller = Objects.requireNonNull(controller, "controller must be non-null"); this.routingPolicies = new RoutingPolicies(controller); this.rotationRepository = new RotationRepository(rotationsConfig, controller.applications(), controller.curator()); + this.hideSharedRoutingEndpoint = Flags.HIDE_SHARED_ROUTING_ENDPOINT.bindTo(controller.flagSource()); } public RoutingPolicies policies() { @@ -84,9 +89,12 @@ public class RoutingController { boolean isSystemApplication = SystemApplication.matching(deployment.applicationId()).isPresent(); // Avoid reading application more than once per call to this var application = Suppliers.memoize(() -> controller.applications().requireApplication(TenantAndApplicationId.from(deployment.applicationId()))); + var hideSharedEndpoints = hideSharedRoutingEndpoint.with(FetchVector.Dimension.APPLICATION_ID, deployment.applicationId().serializedForm()).value(); for (var policy : routingPolicies.get(deployment).values()) { if (!policy.status().isActive()) continue; for (var routingMethod : controller.zoneRegistry().routingMethods(policy.id().zone())) { + // Hide shared endpoints if configured for application, and the application can be routed to directly + if (hideSharedEndpoints && !routingMethod.isDirect() && !isSystemApplication && canRouteDirectlyTo(deployment, application.get())) continue; if (routingMethod.isDirect() && !isSystemApplication && !canRouteDirectlyTo(deployment, application.get())) continue; endpoints.add(policy.endpointIn(controller.system(), routingMethod, controller.zoneRegistry())); endpoints.add(policy.regionEndpointIn(controller.system(), routingMethod)); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java index 5e9d5c44d4c..2ed71b91850 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java @@ -46,6 +46,8 @@ public class TenantSerializer { private static final String athenzDomainField = "athenzDomain"; private static final String propertyField = "property"; private static final String propertyIdField = "propertyId"; + private static final String creatorField = "creator"; + private static final String createdAtField = "createdAt"; private static final String contactField = "contact"; private static final String contactUrlField = "contactUrl"; private static final String propertyUrlField = "propertyUrl"; @@ -88,6 +90,7 @@ public class TenantSerializer { // field we continue to write the default value and stop reading it. // TODO(ogronnesby, 2020-08-05): Remove when a version where we do not read the field has propagated. var legacyBillingInfo = new BillingInfo("customer", "Vespa"); + tenant.creator().ifPresent(creator -> root.setString(creatorField, creator.getName())); developerKeysToSlime(tenant.developerKeys(), root.setArray(pemDeveloperKeysField)); toSlime(legacyBillingInfo, root.setObject(billingInfoField)); } @@ -128,8 +131,9 @@ public class TenantSerializer { private CloudTenant cloudTenantFrom(Inspector tenantObject) { TenantName name = TenantName.from(tenantObject.field(nameField).asString()); + Optional<Principal> creator = SlimeUtils.optionalString(tenantObject.field(creatorField)).map(SimplePrincipal::new); BiMap<PublicKey, Principal> developerKeys = developerKeysFromSlime(tenantObject.field(pemDeveloperKeysField)); - return new CloudTenant(name, developerKeys); + return new CloudTenant(name, creator, developerKeys); } private BiMap<PublicKey, Principal> developerKeysFromSlime(Inspector array) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index c5fb507749c..427d1d66ee8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -1635,6 +1635,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { case cloud: { CloudTenant cloudTenant = (CloudTenant) tenant; + cloudTenant.creator().ifPresent(creator -> object.setString("creator", creator.getName())); Cursor pemDeveloperKeysArray = object.setArray("pemDeveloperKeys"); cloudTenant.developerKeys().forEach((key, user) -> { Cursor keyObject = pemDeveloperKeysArray.addObject(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java index 4f410f60204..6d2b2d58e78 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java @@ -48,7 +48,7 @@ public class CloudAccessControl implements AccessControl { requireTenantCreationAllowed((Auth0Credentials) credentials); CloudTenantSpec spec = (CloudTenantSpec) tenantSpec; - CloudTenant tenant = CloudTenant.create(spec.tenant()); + CloudTenant tenant = CloudTenant.create(spec.tenant(), credentials.user()); for (Role role : Roles.tenantRoles(spec.tenant())) { userManagement.createRole(role); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java index d208a1e4e63..a74c39dc362 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java @@ -16,20 +16,28 @@ import java.util.Optional; */ public class CloudTenant extends Tenant { + private final Optional<Principal> creator; private final BiMap<PublicKey, Principal> developerKeys; /** Public for the serialization layer — do not use! */ - public CloudTenant(TenantName name, BiMap<PublicKey, Principal> developerKeys) { + public CloudTenant(TenantName name, Optional<Principal> creator, BiMap<PublicKey, Principal> developerKeys) { super(name, Optional.empty()); + this.creator = creator; this.developerKeys = developerKeys; } /** Creates a tenant with the given name, provided it passes validation. */ - public static CloudTenant create(TenantName tenantName) { + public static CloudTenant create(TenantName tenantName, Principal creator) { return new CloudTenant(requireName(tenantName), + Optional.ofNullable(creator), ImmutableBiMap.of()); } + /** The user that created the tenant */ + public Optional<Principal> creator() { + return creator; + } + /** Returns the set of developer keys and their corresponding developers for this tenant. */ public BiMap<PublicKey, Principal> developerKeys() { return developerKeys; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 26f718ae5ff..4116c6c7754 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -18,6 +18,8 @@ import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.path.Path; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; @@ -894,6 +896,45 @@ public class ControllerTest { } @Test + public void testDirectRoutingSupportHidingSharedEndpoint() { + // TODO (mortent): remove this test when shared routing is gone + var context = tester.newDeploymentContext(); + var zone1 = ZoneId.from("prod", "us-west-1"); + var zone2 = ZoneId.from("prod", "us-east-3"); + var zone3 = ZoneId.from("staging", "us-east-3"); + var zone4 = ZoneId.from("test", "us-east-1"); + var applicationPackageBuilder = new ApplicationPackageBuilder() + .region(zone1.region()) + .region(zone2.region()); + tester.controllerTester().zoneRegistry() + .setRoutingMethod(ZoneApiMock.from(zone1), RoutingMethod.shared, RoutingMethod.sharedLayer4) + .setRoutingMethod(ZoneApiMock.from(zone2), RoutingMethod.shared, RoutingMethod.sharedLayer4) + .setRoutingMethod(ZoneApiMock.from(zone3), RoutingMethod.shared, RoutingMethod.sharedLayer4) + .setRoutingMethod(ZoneApiMock.from(zone4), RoutingMethod.shared, RoutingMethod.sharedLayer4); + Supplier<Set<RoutingMethod>> routingMethods = () -> tester.controller().routing().endpointsOf(context.deploymentIdIn(zone1)) + .asList() + .stream() + .map(Endpoint::routingMethod) + .collect(Collectors.toSet()); + + ((InMemoryFlagSource)tester.controller().flagSource()).withBooleanFlag(Flags.HIDE_SHARED_ROUTING_ENDPOINT.id(), true); + // Without satisfying any requirement + context.submit(applicationPackageBuilder.build()).deploy(); + assertEquals(Set.of(RoutingMethod.shared), routingMethods.get()); + + // Without satisfying Athenz service requirement + context.submit(applicationPackageBuilder.compileVersion(RoutingController.DIRECT_ROUTING_MIN_VERSION).build()) + .deploy(); + assertEquals(Set.of(RoutingMethod.shared), routingMethods.get()); + + // Satisfying all requirements + context.submit(applicationPackageBuilder.compileVersion(RoutingController.DIRECT_ROUTING_MIN_VERSION) + .athenzIdentity(AthenzDomain.from("domain"), AthenzService.from("service")) + .build()).deploy(); + assertEquals(Set.of(RoutingMethod.sharedLayer4), routingMethods.get()); + } + + @Test public void testChangeEndpointCluster() { var context = tester.newDeploymentContext(); var west = ZoneId.from("prod", "us-west-1"); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java index 0f4ea3dd2ce..9f36275e196 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java @@ -78,10 +78,12 @@ public class TenantSerializerTest { @Test public void cloud_tenant() { CloudTenant tenant = new CloudTenant(TenantName.from("elderly-lady"), + Optional.of(new SimplePrincipal("foobar-user")), ImmutableBiMap.of(publicKey, new SimplePrincipal("joe"), otherPublicKey, new SimplePrincipal("jane"))); CloudTenant serialized = (CloudTenant) serializer.tenantFrom(serializer.toSlime(tenant)); assertEquals(tenant.name(), serialized.name()); + assertEquals(tenant.creator(), serialized.creator()); assertEquals(tenant.developerKeys(), serialized.developerKeys()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java index 3c13962916a..28db2cf2224 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java @@ -27,6 +27,7 @@ import java.net.URI; import java.net.http.HttpRequest; import java.security.PrivateKey; import java.security.PublicKey; +import java.util.Optional; import java.util.Set; import static org.junit.Assert.assertEquals; @@ -66,6 +67,7 @@ public class SignatureFilterTest { signer = new RequestSigner(privateKey, id.serializedForm(), tester.clock()); tester.curator().writeTenant(new CloudTenant(appId.tenant(), + Optional.empty(), ImmutableBiMap.of())); tester.curator().writeApplication(new Application(appId, tester.clock().instant())); } @@ -104,6 +106,7 @@ public class SignatureFilterTest { // Signed request gets a developer role when a matching developer key is stored for the tenant. tester.curator().writeTenant(new CloudTenant(appId.tenant(), + Optional.empty(), ImmutableBiMap.of(publicKey, () -> "user"))); verifySecurityContext(requestOf(signer.signed(request.copy(), Method.POST, () -> new ByteArrayInputStream(hiBytes)), hiBytes), new SecurityContext(new SimplePrincipal("user"), diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/tenant-without-applications.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/tenant-without-applications.json index 39b6cccbab0..bce3d185977 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/tenant-without-applications.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/tenant-without-applications.json @@ -1,6 +1,7 @@ { "tenant": "my-tenant", "type": "CLOUD", + "creator": "administrator@tenant", "pemDeveloperKeys": [], "applications": [] } diff --git a/document/src/tests/fieldsettest.cpp b/document/src/tests/fieldsettest.cpp index b82cca86ce7..37c55385c0f 100644 --- a/document/src/tests/fieldsettest.cpp +++ b/document/src/tests/fieldsettest.cpp @@ -2,9 +2,6 @@ #include <vespa/document/base/testdocman.h> #include <vespa/document/fieldset/fieldsetrepo.h> -#include <vespa/vespalib/io/fileutil.h> -#include <vespa/document/datatype/annotationreferencedatatype.h> -#include <vespa/document/repo/configbuilder.h> #include <vespa/document/repo/documenttyperepo.h> #include <vespa/vespalib/objects/nbostream.h> #include <algorithm> @@ -12,8 +9,6 @@ using vespalib::nbostream; -using namespace document::config_builder; - namespace document { class FieldSetTest : public ::testing::Test { @@ -290,4 +285,18 @@ TEST_F(FieldSetTest, testStripFields) doStripFields(*src, repo, "testdoctype1:hstringval,content")); } +TEST(FieldCollectionTest, testHash ) { + TestDocMan testDocMan; + const DocumentTypeRepo& repo = testDocMan.getTypeRepo(); + const DocumentType & type = *repo.getDocumentType("testdoctype1"); + Field::Set set; + EXPECT_EQ(0ul, FieldCollection(type, set).hash()); + set.insert(&type.getField("headerval")); + EXPECT_EQ(0x548599858c77ef83ul, FieldCollection(type, set).hash()); + set.insert(&type.getField("hstringval")); + EXPECT_EQ(0x4a7ff2406d36a9b0ul, FieldCollection(type, set).hash()); + set.erase(&type.getField("headerval")); + EXPECT_EQ(0x1e0918531b19734ul, FieldCollection(type, set).hash()); +} + } // document diff --git a/document/src/vespa/document/base/field.cpp b/document/src/vespa/document/base/field.cpp index e85703d203c..316394c289f 100644 --- a/document/src/vespa/document/base/field.cpp +++ b/document/src/vespa/document/base/field.cpp @@ -3,6 +3,7 @@ #include "field.h" #include <vespa/document/fieldvalue/fieldvalue.h> #include <vespa/document/datatype/datatype.h> +#include <vespa/document/fieldset/fieldsets.h> #include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/stllike/asciistream.h> #include <vespa/vespalib/util/bobhash.h> @@ -52,17 +53,16 @@ bool Field::contains(const FieldSet& fields) const { switch (fields.getType()) { - case FIELD: - return static_cast<const Field&>(fields).getId() == getId(); - case SET: - { - // Go through each. - return false; - } - case NONE: - case DOCID: + case Type::FIELD: + return static_cast<const Field&>(fields).getId() == getId(); + case Type::SET: { + const auto & set = static_cast<const FieldCollection &>(fields); + return (set.getFields().size() == 1) && ((*set.getFields().begin())->getId() == getId()); + } + case Type::NONE: + case Type::DOCID: return true; - case ALL: + case Type::ALL: return false; } @@ -92,7 +92,7 @@ Field::validateId(int newId) { getName().data(), newId)); } - if ((newId & 0x80000000) != 0) // Highest bit must not be set + if ((uint32_t(newId) & 0x80000000u) != 0) // Highest bit must not be set { throw vespalib::IllegalArgumentException(vespalib::make_string( "Attempt to set the id of %s to %d" diff --git a/document/src/vespa/document/base/field.h b/document/src/vespa/document/base/field.h index f17a0a41703..a77034932cf 100644 --- a/document/src/vespa/document/base/field.h +++ b/document/src/vespa/document/base/field.h @@ -78,7 +78,7 @@ public: vespalib::string toString(bool verbose=false) const; bool contains(const FieldSet& fields) const override; - Type getType() const override { return FIELD; } + Type getType() const override { return Type::FIELD; } bool valid() const { return _fieldId != 0; } uint32_t hash() const { return getId(); } private: diff --git a/document/src/vespa/document/datatype/datatype.cpp b/document/src/vespa/document/datatype/datatype.cpp index fae606839a7..b7b511d459e 100644 --- a/document/src/vespa/document/datatype/datatype.cpp +++ b/document/src/vespa/document/datatype/datatype.cpp @@ -28,6 +28,7 @@ DocumentType DOCUMENT_OBJ("document"); WeightedSetDataType TAG_OBJ(*DataType::STRING, true, true); PrimitiveDataType URI_OBJ(DataType::T_URI); PrimitiveDataType PREDICATE_OBJ(DataType::T_PREDICATE); +PrimitiveDataType TENSOR_OBJ(DataType::T_TENSOR); } // namespace @@ -44,6 +45,8 @@ const DocumentType *const DataType::DOCUMENT(&DOCUMENT_OBJ); const DataType *const DataType::TAG(&TAG_OBJ); const DataType *const DataType::URI(&URI_OBJ); const DataType *const DataType::PREDICATE(&PREDICATE_OBJ); +const DataType *const DataType::TENSOR(&TENSOR_OBJ); + namespace { diff --git a/document/src/vespa/document/datatype/datatype.h b/document/src/vespa/document/datatype/datatype.h index 95f0e8b9a64..723e7c69ed6 100644 --- a/document/src/vespa/document/datatype/datatype.h +++ b/document/src/vespa/document/datatype/datatype.h @@ -96,6 +96,7 @@ public: static const DataType *const TAG; static const DataType *const URI; static const DataType *const PREDICATE; + static const DataType *const TENSOR; /** Used by type manager to fetch default types to register. */ static std::vector<const DataType *> getDefaultDataTypes(); diff --git a/document/src/vespa/document/fieldset/fieldset.h b/document/src/vespa/document/fieldset/fieldset.h index 6c9acadcf5d..35c912c5e45 100644 --- a/document/src/vespa/document/fieldset/fieldset.h +++ b/document/src/vespa/document/fieldset/fieldset.h @@ -15,7 +15,7 @@ class Document; class FieldSet { public: - enum Type { + enum class Type { FIELD, SET, ALL, diff --git a/document/src/vespa/document/fieldset/fieldsetrepo.cpp b/document/src/vespa/document/fieldset/fieldsetrepo.cpp index e3212f3bd25..c576624b026 100644 --- a/document/src/vespa/document/fieldset/fieldsetrepo.cpp +++ b/document/src/vespa/document/fieldset/fieldsetrepo.cpp @@ -17,13 +17,13 @@ parseSpecialValues(vespalib::stringref name) { FieldSet::UP fs; if ((name.size() == 4) && (name[1] == 'i') && (name[2] == 'd') && (name[3] == ']')) { - fs.reset(new DocIdOnly()); + fs = std::make_unique<DocIdOnly>(); } else if ((name.size() == 5) && (name[1] == 'a') && (name[2] == 'l') && (name[3] == 'l') && (name[4] == ']')) { - fs.reset(new AllFields()); + fs = std::make_unique<AllFields>(); } else if ((name.size() == 6) && (name[1] == 'n') && (name[2] == 'o') && (name[3] == 'n') && (name[4] == 'e') && (name[5] == ']')) { - fs.reset(new NoFields()); + fs = std::make_unique<NoFields>(); } else if ((name.size() == 7) && (name[1] == 'd') && (name[2] == 'o') && (name[3] == 'c') && (name[4] == 'i') && (name[5] == 'd') && (name[6] == ']')) { - fs.reset(new DocIdOnly()); + fs = std::make_unique<DocIdOnly>(); } else { throw vespalib::IllegalArgumentException( "The only special names (enclosed in '[]') allowed are " @@ -44,20 +44,18 @@ parseFieldCollection(const DocumentTypeRepo& repo, const DocumentType& type(*typePtr); StringTokenizer tokenizer(fieldNames, ","); - FieldCollection::UP collection(new FieldCollection(type)); - - for (uint32_t i = 0; i < tokenizer.size(); ++i) { - const DocumentType::FieldSet * fs = type.getFieldSet(tokenizer[i]); + Field::Set fields; + for (const auto & token : tokenizer) { + const DocumentType::FieldSet * fs = type.getFieldSet(token); if (fs) { - for (DocumentType::FieldSet::Fields::const_iterator it(fs->getFields().begin()), mt(fs->getFields().end()); it != mt; it++) { - collection->insert(type.getField(*it)); + for (const auto & fieldName : fs->getFields()) { + fields.insert(&type.getField(fieldName)); } } else { - collection->insert(type.getField(tokenizer[i])); + fields.insert(&type.getField(token)); } } - - return FieldSet::UP(collection.release()); + return std::make_unique<FieldCollection>(type, std::move(fields)); } } @@ -83,34 +81,33 @@ vespalib::string FieldSetRepo::serialize(const FieldSet& fieldSet) { switch (fieldSet.getType()) { - case FieldSet::FIELD: - return static_cast<const Field&>(fieldSet).getName(); - case FieldSet::SET: - { - const FieldCollection& collection = static_cast<const FieldCollection&>(fieldSet); - - vespalib::asciistream stream; - stream << collection.getDocumentType().getName() << ":"; - for (Field::Set::const_iterator iter = collection.getFields().begin(); - iter != collection.getFields().end(); - ++iter) { - if (iter != collection.getFields().begin()) { - stream << ","; + case FieldSet::Type::FIELD: + return static_cast<const Field&>(fieldSet).getName(); + case FieldSet::Type::SET: { + const auto & collection = static_cast<const FieldCollection&>(fieldSet); + + vespalib::asciistream stream; + stream << collection.getDocumentType().getName() << ":"; + bool first = true; + for (const Field * field : collection.getFields()) { + if (first) { + first = false; + } else { + stream << ","; + } + stream << field->getName(); } - stream << (*iter)->getName(); + return stream.str(); } - - return stream.str(); - } - case FieldSet::ALL: - return AllFields::NAME; - case FieldSet::NONE: - return NoFields::NAME; - case FieldSet::DOCID: - return DocIdOnly::NAME; - default: - return ""; + case FieldSet::Type::ALL: + return AllFields::NAME; + case FieldSet::Type::NONE: + return NoFields::NAME; + case FieldSet::Type::DOCID: + return DocIdOnly::NAME; + default: + return ""; } } diff --git a/document/src/vespa/document/fieldset/fieldsets.cpp b/document/src/vespa/document/fieldset/fieldsets.cpp index 1a6cd5e6b84..586cef0e5f4 100644 --- a/document/src/vespa/document/fieldset/fieldsets.cpp +++ b/document/src/vespa/document/fieldset/fieldsets.cpp @@ -3,67 +3,75 @@ #include "fieldsets.h" #include <vespa/document/fieldvalue/document.h> #include <vespa/document/datatype/documenttype.h> +#include <vespa/vespalib/stllike/asciistream.h> +#include <xxhash.h> namespace document { -FieldCollection::FieldCollection(const DocumentType& type, const Field::Set& s) - : _set(s), - _docType(type) -{ +namespace { + +uint64_t +computeHash(const Field::Set & set) { + if (set.empty()) return 0ul; + + vespalib::asciistream os; + for (const Field * field : set) { + os << field->getName() << ':'; + } + return XXH64(os.c_str(), os.size(), 0); +} + } +FieldCollection::FieldCollection(const DocumentType& type, Field::Set set) + : _set(std::move(set)), + _hash(computeHash(_set)), + _docType(type) +{ } + +FieldCollection::FieldCollection(const FieldCollection&) = default; + +FieldCollection::~FieldCollection() = default; + bool FieldCollection::contains(const FieldSet& fields) const { switch (fields.getType()) { - case FIELD: - return _set.find(static_cast<const Field*>(&fields)) != _set.end(); - case SET: - { - const FieldCollection& coll = static_cast<const FieldCollection&>(fields); + case Type::FIELD: + return _set.find(static_cast<const Field*>(&fields)) != _set.end(); + case Type::SET: { + const auto & coll = static_cast<const FieldCollection&>(fields); - if (_set.size() < coll._set.size()) { - return false; - } + if (_set.size() < coll._set.size()) { + return false; + } - Field::Set::const_iterator iter = coll.getFields().begin(); + auto iter = coll.getFields().begin(); - while (iter != coll.getFields().end()) { - if (_set.find(*iter) == _set.end()) { - return false; + while (iter != coll.getFields().end()) { + if (_set.find(*iter) == _set.end()) { + return false; + } + + ++iter; } - ++iter; + return true; } - - return true; - } - case NONE: - case DOCID: - return true; - case ALL: - return false; + case Type::NONE: + case Type::DOCID: + return true; + case Type::ALL: + return false; } return false; } void -FieldCollection::insert(const Field& f) -{ - _set.insert(&f); -} - -void -FieldCollection::insert(const Field::Set& c) -{ - _set.insert(c.begin(), c.end()); -} - -void FieldSet::copyFields(Document& dest, const Document& src, const FieldSet& fields) { - if (fields.getType() == ALL) { + if (fields.getType() == Type::ALL) { dest.getFields() = src.getFields(); return; } @@ -89,10 +97,10 @@ FieldSet::createDocumentSubsetCopy(const Document& src, const FieldSet& fields) void FieldSet::stripFields(Document& doc, const FieldSet& fieldsToKeep) { - if (fieldsToKeep.getType() == ALL) { + if (fieldsToKeep.getType() == Type::ALL) { return; - } else if (fieldsToKeep.getType() == DOCID - || fieldsToKeep.getType() == NONE) + } else if (fieldsToKeep.getType() == Type::DOCID + || fieldsToKeep.getType() == Type::NONE) { doc.clear(); return; @@ -106,8 +114,8 @@ FieldSet::stripFields(Document& doc, const FieldSet& fieldsToKeep) fieldsToRemove.push_back(&f); } } - for (size_t i = 0; i < fieldsToRemove.size(); ++i) { - doc.remove(*fieldsToRemove[i]); + for (const Field * field : fieldsToRemove) { + doc.remove(*field); } } diff --git a/document/src/vespa/document/fieldset/fieldsets.h b/document/src/vespa/document/fieldset/fieldsets.h index 7b8ec6bbff0..d37d72d6109 100644 --- a/document/src/vespa/document/fieldset/fieldsets.h +++ b/document/src/vespa/document/fieldset/fieldsets.h @@ -12,7 +12,7 @@ class AllFields final : public FieldSet public: static constexpr const char * NAME = "[all]"; bool contains(const FieldSet&) const override { return true; } - Type getType() const override { return ALL; } + Type getType() const override { return Type::ALL; } FieldSet* clone() const override { return new AllFields(); } }; @@ -20,8 +20,8 @@ class NoFields final : public FieldSet { public: static constexpr const char * NAME = "[none]"; - bool contains(const FieldSet& f) const override { return f.getType() == NONE; } - Type getType() const override { return NONE; } + bool contains(const FieldSet& f) const override { return f.getType() == Type::NONE; } + Type getType() const override { return Type::NONE; } FieldSet* clone() const override { return new NoFields(); } }; @@ -30,9 +30,9 @@ class DocIdOnly final : public FieldSet public: static constexpr const char * NAME = "[id]"; bool contains(const FieldSet& fields) const override { - return fields.getType() == DOCID || fields.getType() == NONE; + return fields.getType() == Type::DOCID || fields.getType() == Type::NONE; } - Type getType() const override { return DOCID; } + Type getType() const override { return Type::DOCID; } FieldSet* clone() const override { return new DocIdOnly(); } }; @@ -41,11 +41,13 @@ class FieldCollection : public FieldSet public: typedef std::unique_ptr<FieldCollection> UP; - FieldCollection(const DocumentType& docType) : _docType(docType) {}; - FieldCollection(const DocumentType& docType, const Field::Set& set); + FieldCollection(const DocumentType& docType, Field::Set set); + FieldCollection(const FieldCollection &); + FieldCollection(FieldCollection&&) = default; + ~FieldCollection() override; bool contains(const FieldSet& fields) const override; - Type getType() const override { return SET; } + Type getType() const override { return Type::SET; } /** * @return Returns the document type the collection is associated with. @@ -55,24 +57,16 @@ public: } /** - * Inserts the given field into the collection. - */ - void insert(const Field& f); - - /** - * Inserts all the field in the given collection into this collection. - */ - void insert(const Field::Set& f); - - /** * Returns all the fields contained in this collection. */ const Field::Set& getFields() const { return _set; } FieldSet* clone() const override { return new FieldCollection(*this); } + uint64_t hash() const { return _hash; } private: Field::Set _set; + uint64_t _hash; const DocumentType& _docType; }; diff --git a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp index 01b8515400b..7d7153e8461 100644 --- a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp @@ -7,7 +7,6 @@ #include "replymerger.h" #include <vespa/document/util/stringutil.h> #include <vespa/documentapi/documentapi.h> -#include <vespa/messagebus/emptyreply.h> #include <vespa/vespalib/util/exceptions.h> #include <sstream> @@ -97,7 +96,7 @@ DocumentProtocol::createPolicy(const mbus::string &name, const mbus::string &par DocumentProtocol & DocumentProtocol::putRoutingPolicyFactory(const string &name, IRoutingPolicyFactory::SP factory) { - _routingPolicyRepository->putFactory(name, factory); + _routingPolicyRepository->putFactory(name, std::move(factory)); return *this; } @@ -140,7 +139,7 @@ DocumentProtocol & DocumentProtocol::putRoutableFactory(uint32_t type, IRoutableFactory::SP factory, const vespalib::VersionSpecification &version) { - _routableRepository->putFactory(version, type, factory); + _routableRepository->putFactory(version, type, std::move(factory)); return *this; } @@ -215,7 +214,7 @@ DocumentProtocol::merge(mbus::RoutingContext& ctx, ctx.setReply(ctx.getChildIterator().skip(okIdx).removeReply()); } else { assert(res.hasGeneratedReply()); - ctx.setReply(mbus::Reply::UP(res.releaseGeneratedReply().release())); + ctx.setReply(res.releaseGeneratedReply()); } } diff --git a/documentapi/src/vespa/documentapi/messagebus/replymerger.cpp b/documentapi/src/vespa/documentapi/messagebus/replymerger.cpp index 69c1b00295d..4bcf2619031 100644 --- a/documentapi/src/vespa/documentapi/messagebus/replymerger.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/replymerger.cpp @@ -13,30 +13,23 @@ namespace documentapi { ReplyMerger::ReplyMerger() : _error(), _ignored(), - _successReply(0), + _successReply(nullptr), _successIndex(0) { } -ReplyMerger::~ReplyMerger() {} +ReplyMerger::~ReplyMerger() = default; -ReplyMerger::Result::Result(uint32_t successIdx, - std::unique_ptr<mbus::Reply> generatedReply) +ReplyMerger::Result::Result(uint32_t successIdx, std::unique_ptr<mbus::Reply> generatedReply) : _generatedReply(std::move(generatedReply)), _successIdx(successIdx) { } -ReplyMerger::Result::Result(Result&& o) - : _generatedReply(std::move(o._generatedReply)), - _successIdx(o._successIdx) -{ -} - bool ReplyMerger::Result::hasGeneratedReply() const { - return (_generatedReply.get() != 0); + return (bool)_generatedReply; } bool @@ -53,7 +46,7 @@ ReplyMerger::Result::releaseGeneratedReply() } uint32_t -ReplyMerger::Result::getSuccessfulReplyIndex() +ReplyMerger::Result::getSuccessfulReplyIndex() const { assert(!hasGeneratedReply()); return _successIdx; @@ -70,7 +63,7 @@ ReplyMerger::merge(uint32_t idx, const mbus::Reply& r) } bool -ReplyMerger::resourceWasFound(const mbus::Reply& r) const +ReplyMerger::resourceWasFound(const mbus::Reply& r) { switch (r.getType()) { case DocumentProtocol::REPLY_REMOVEDOCUMENT: @@ -112,8 +105,8 @@ ReplyMerger::mergeAllReplyErrors(const mbus::Reply& r) if (handleReplyWithOnlyIgnoredErrors(r)) { return; } - if (!_error.get()) { - _error.reset(new mbus::EmptyReply()); + if ( ! _error ) { + _error = std::make_unique<mbus::EmptyReply>(); } for (uint32_t i = 0; i < r.getNumErrors(); ++i) { _error->addError(r.getError(i)); @@ -124,8 +117,8 @@ bool ReplyMerger::handleReplyWithOnlyIgnoredErrors(const mbus::Reply& r) { if (DocumentProtocol::hasOnlyErrorsOfType(r, DocumentProtocol::ERROR_MESSAGE_IGNORED)) { - if (!_ignored.get()) { - _ignored.reset(new mbus::EmptyReply()); + if ( ! _ignored) { + _ignored = std::make_unique<mbus::EmptyReply>(); } _ignored->addError(r.getError(0)); return true; @@ -136,7 +129,7 @@ ReplyMerger::handleReplyWithOnlyIgnoredErrors(const mbus::Reply& r) bool ReplyMerger::shouldReturnErrorReply() const { - if (_error.get()) { + if (_error) { return true; } return (_ignored.get() && !_successReply); @@ -145,7 +138,7 @@ ReplyMerger::shouldReturnErrorReply() const std::unique_ptr<mbus::Reply> ReplyMerger::releaseGeneratedErrorReply() { - if (_error.get()) { + if (_error) { return std::move(_error); } else { assert(_ignored.get()); @@ -156,13 +149,13 @@ ReplyMerger::releaseGeneratedErrorReply() bool ReplyMerger::successfullyMergedAtLeastOneReply() const { - return (_successReply != 0); + return (_successReply != nullptr); } ReplyMerger::Result -ReplyMerger::createEmptyReplyResult() const +ReplyMerger::createEmptyReplyResult() { - return Result(0u, std::unique_ptr<mbus::Reply>(new mbus::EmptyReply())); + return Result(0u, std::make_unique<mbus::EmptyReply>()); } ReplyMerger::Result diff --git a/documentapi/src/vespa/documentapi/messagebus/replymerger.h b/documentapi/src/vespa/documentapi/messagebus/replymerger.h index f9574d56c2c..b14bf6a5f73 100644 --- a/documentapi/src/vespa/documentapi/messagebus/replymerger.h +++ b/documentapi/src/vespa/documentapi/messagebus/replymerger.h @@ -14,15 +14,17 @@ public: std::unique_ptr<mbus::Reply> _generatedReply; uint32_t _successIdx; - Result(uint32_t successIdx, - std::unique_ptr<mbus::Reply> generatedReply); + Result(uint32_t successIdx, std::unique_ptr<mbus::Reply> generatedReply); public: - Result(Result&&); + Result(Result&& o) noexcept + : _generatedReply(std::move(o._generatedReply)), + _successIdx(o._successIdx) + { } bool hasGeneratedReply() const; bool isSuccessful() const; std::unique_ptr<mbus::Reply> releaseGeneratedReply(); - uint32_t getSuccessfulReplyIndex(); + uint32_t getSuccessfulReplyIndex() const; }; private: std::unique_ptr<mbus::Reply> _error; @@ -38,8 +40,8 @@ private: void setCurrentBestReply(uint32_t idx, const mbus::Reply& r); void updateStateWithSuccessfulReply(uint32_t idx, const mbus::Reply& r); bool successfullyMergedAtLeastOneReply() const; - Result createEmptyReplyResult() const; - bool resourceWasFound(const mbus::Reply& r) const; + static Result createEmptyReplyResult(); + static bool resourceWasFound(const mbus::Reply& r); public: ReplyMerger(); ~ReplyMerger(); diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 2ac5969c726..48ec49f0b96 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -358,6 +358,13 @@ public class Flags { "Takes effect on config server restart" ); + public static final UnboundBooleanFlag HIDE_SHARED_ROUTING_ENDPOINT = defineFeatureFlag( + "hide-shared-routing-endpoint", + false, + "Whether the contorller should hide shared routing layer endpoint", + "Takses effect immediately" + ); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, String description, String modificationEffect, FetchVector.Dimension... dimensions) { diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp index 92fea5ff6e4..73783413061 100644 --- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp +++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp @@ -294,9 +294,7 @@ BucketContent::eraseEntry(Timestamp t) } } -DummyPersistence::DummyPersistence( - const std::shared_ptr<const document::DocumentTypeRepo>& repo, - uint16_t partitionCount) +DummyPersistence::DummyPersistence(const std::shared_ptr<const document::DocumentTypeRepo>& repo, uint16_t partitionCount) : _initialized(false), _repo(repo), _partitions(partitionCount), @@ -314,8 +312,7 @@ DummyPersistence::parseDocumentSelection(const string& documentSelection, bool a { document::select::Node::UP ret; try { - document::select::Parser parser( - *_repo, document::BucketIdFactory()); + document::select::Parser parser(*_repo, document::BucketIdFactory()); ret = parser.parse(documentSelection); } catch (document::select::ParsingFailedException& e) { return document::select::Node::UP(); @@ -539,7 +536,7 @@ DummyPersistence::get(const Bucket& b, const document::FieldSet& fieldSet, const return GetResult::make_for_tombstone(entry->getTimestamp()); } else { Document::UP doc(entry->getDocument()->clone()); - if (fieldSet.getType() != document::FieldSet::ALL) { + if (fieldSet.getType() != document::FieldSet::Type::ALL) { document::FieldSet::stripFields(*doc, fieldSet); } return GetResult(std::move(doc), entry->getTimestamp()); @@ -562,10 +559,7 @@ DummyPersistence::createIterator( assert(b.getBucketSpace() == FixedBucketSpaces::default_space()); std::unique_ptr<document::select::Node> docSelection; if (!s.getDocumentSelection().getDocumentSelection().empty()) { - docSelection.reset( - parseDocumentSelection( - s.getDocumentSelection().getDocumentSelection(), - true).release()); + docSelection = parseDocumentSelection(s.getDocumentSelection().getDocumentSelection(), true); if (!docSelection.get()) { return CreateIteratorResult( Result::ErrorType::PERMANENT_ERROR, @@ -678,7 +672,7 @@ DummyPersistence::iterate(IteratorId id, uint64_t maxByteSize, Context& ctx) con if (currentSize != 0 && currentSize + size > maxByteSize) break; currentSize += size; if (!entry->isRemove() - && it->_fieldSet->getType() != document::FieldSet::ALL) + && it->_fieldSet->getType() != document::FieldSet::Type::ALL) { assert(entry->getDocument()); // Create new document with only wanted fields. diff --git a/searchcore/src/apps/tests/persistenceconformance_test.cpp b/searchcore/src/apps/tests/persistenceconformance_test.cpp index 4056e49e37c..217d1edcd57 100644 --- a/searchcore/src/apps/tests/persistenceconformance_test.cpp +++ b/searchcore/src/apps/tests/persistenceconformance_test.cpp @@ -75,19 +75,19 @@ storeDocType(DocTypeVector *types, const DocumentType &type) struct SchemaConfigFactory { typedef DocumentDBConfig CS; typedef std::shared_ptr<SchemaConfigFactory> SP; - virtual ~SchemaConfigFactory() {} - static SchemaConfigFactory::SP get() { return SchemaConfigFactory::SP(new SchemaConfigFactory()); } + virtual ~SchemaConfigFactory() = default; + static SchemaConfigFactory::SP get() { return std::make_shared<SchemaConfigFactory>(); } virtual CS::IndexschemaConfigSP createIndexSchema(const DocumentType &docType) { (void) docType; - return CS::IndexschemaConfigSP(new IndexschemaConfig()); + return std::make_shared<IndexschemaConfig>(); } virtual CS::AttributesConfigSP createAttributes(const DocumentType &docType) { (void) docType; - return CS::AttributesConfigSP(new AttributesConfig()); + return std::make_shared<AttributesConfig>(); } virtual CS::SummaryConfigSP createSummary(const DocumentType &docType) { (void) docType; - return CS::SummaryConfigSP(new SummaryConfig()); + return std::make_shared<SummaryConfig>(); } }; @@ -98,12 +98,12 @@ private: SchemaConfigFactory::SP _schemaFactory; public: - ConfigFactory(const std::shared_ptr<const DocumentTypeRepo> &repo, - const DocumenttypesConfigSP &typeCfg, - const SchemaConfigFactory::SP &schemaFactory); + ConfigFactory(std::shared_ptr<const DocumentTypeRepo> repo, + DocumenttypesConfigSP typeCfg, + SchemaConfigFactory::SP schemaFactory); ~ConfigFactory(); - const std::shared_ptr<const DocumentTypeRepo> getTypeRepo() const { return _repo; } - const DocumenttypesConfigSP getTypeCfg() const { return _typeCfg; } + std::shared_ptr<const DocumentTypeRepo> getTypeRepo() const { return _repo; } + DocumenttypesConfigSP getTypeCfg() const { return _typeCfg; } DocTypeVector getDocTypes() const { DocTypeVector types; _repo->forEachDocumentType(*makeClosure(storeDocType, &types)); @@ -112,7 +112,7 @@ public: DocumentDBConfig::SP create(const DocTypeName &docTypeName) const { const DocumentType *docType = _repo->getDocumentType(docTypeName.getName()); - if (docType == NULL) { + if (docType == nullptr) { return DocumentDBConfig::SP(); } typedef DocumentDBConfig CS; @@ -123,7 +123,7 @@ public: SchemaBuilder::build(*indexschema, *schema); SchemaBuilder::build(*attributes, *schema); SchemaBuilder::build(*summary, *schema); - return DocumentDBConfig::SP(new DocumentDBConfig( + return std::make_shared<DocumentDBConfig>( 1, std::make_shared<RankProfilesConfig>(), std::make_shared<matching::RankingConstants>(), @@ -140,18 +140,18 @@ public: std::make_shared<DocumentDBMaintenanceConfig>(), search::LogDocumentStore::Config(), "client", - docTypeName.getName())); + docTypeName.getName()); } }; -ConfigFactory::ConfigFactory(const std::shared_ptr<const DocumentTypeRepo> &repo, const DocumenttypesConfigSP &typeCfg, - const SchemaConfigFactory::SP &schemaFactory) - : _repo(repo), - _typeCfg(typeCfg), - _schemaFactory(schemaFactory) +ConfigFactory::ConfigFactory(std::shared_ptr<const DocumentTypeRepo> repo, DocumenttypesConfigSP typeCfg, + SchemaConfigFactory::SP schemaFactory) + : _repo(std::move(repo)), + _typeCfg(std::move(typeCfg)), + _schemaFactory(std::move(schemaFactory)) {} -ConfigFactory::~ConfigFactory() {} +ConfigFactory::~ConfigFactory() = default; class DocumentDBFactory : public DummyDBOwner { private: @@ -167,7 +167,7 @@ private: public: DocumentDBFactory(const vespalib::string &baseDir, int tlsListenPort); - ~DocumentDBFactory(); + ~DocumentDBFactory() override; DocumentDB::SP create(BucketSpace bucketSpace, const DocTypeName &docType, const ConfigFactory &factory) { @@ -191,8 +191,7 @@ public: tuneFileDocDB, HwInfo())); mgr.forwardConfig(b); mgr.nextGeneration(0ms); - return DocumentDB::SP( - new DocumentDB(_baseDir, + return std::make_shared<DocumentDB>(_baseDir, mgr.getConfig(), _tlsSpec, _queryLimiter, @@ -209,7 +208,7 @@ public: _config_stores.getConfigStore(docType.toString()), std::make_shared<vespalib::ThreadStackExecutor> (16, 128 * 1024), - HwInfo())); + HwInfo()); } }; @@ -224,40 +223,33 @@ DocumentDBFactory::DocumentDBFactory(const vespalib::string &baseDir, int tlsLis _metricsWireService(), _summaryExecutor(8, 128 * 1024) {} -DocumentDBFactory::~DocumentDBFactory() {} +DocumentDBFactory::~DocumentDBFactory() = default; class DocumentDBRepo { private: DocumentDBMap _docDbs; public: typedef std::unique_ptr<DocumentDBRepo> UP; - DocumentDBRepo(const ConfigFactory &cfgFactory, - DocumentDBFactory &docDbFactory) : - _docDbs() + DocumentDBRepo(const ConfigFactory &cfgFactory, DocumentDBFactory &docDbFactory) + : _docDbs() { DocTypeVector types = cfgFactory.getDocTypes(); - for (size_t i = 0; i < types.size(); ++i) { - BucketSpace bucketSpace(makeBucketSpace(types[i].getName())); - DocumentDB::SP docDb = docDbFactory.create(bucketSpace, - types[i], - cfgFactory); + for (const auto & type : types) { + BucketSpace bucketSpace(makeBucketSpace(type.getName())); + DocumentDB::SP docDb = docDbFactory.create(bucketSpace, type, cfgFactory); docDb->start(); docDb->waitForOnlineState(); - _docDbs[types[i]] = docDb; + _docDbs[type] = docDb; } } - void - close() - { - for (DocumentDBMap::iterator itr = _docDbs.begin(); - itr != _docDbs.end(); ++itr) { - itr->second->close(); + void close() { + for (auto & dbEntry : _docDbs) { + dbEntry.second->close(); } } - ~DocumentDBRepo() - { + ~DocumentDBRepo() { close(); } const DocumentDBMap &getDocDbs() const { return _docDbs; } @@ -269,20 +261,15 @@ class DocDBRepoHolder protected: DocumentDBRepo::UP _docDbRepo; - DocDBRepoHolder(DocumentDBRepo::UP docDbRepo) + explicit DocDBRepoHolder(DocumentDBRepo::UP docDbRepo) : _docDbRepo(std::move(docDbRepo)) { } - virtual - ~DocDBRepoHolder() - { - } + virtual ~DocDBRepoHolder() = default; - void - close() - { - if (_docDbRepo.get() != NULL) + void close() { + if (_docDbRepo) _docDbRepo->close(); } }; @@ -290,17 +277,13 @@ protected: class MyPersistenceEngineOwner : public IPersistenceEngineOwner { - virtual void - setClusterState(BucketSpace, const storage::spi::ClusterState &calc) override - { - (void) calc; - } + void setClusterState(BucketSpace, const storage::spi::ClusterState &) override { } }; struct MyResourceWriteFilter : public IResourceWriteFilter { - virtual bool acceptWriteOperation() const override { return true; } - virtual State getAcceptState() const override { return IResourceWriteFilter::State(); } + bool acceptWriteOperation() const override { return true; } + State getAcceptState() const override { return IResourceWriteFilter::State(); } }; class MyPersistenceEngine : public DocDBRepoHolder, @@ -320,35 +303,32 @@ public: void addHandlers(const vespalib::string &docType) { - if (_docDbRepo.get() == NULL) + if (!_docDbRepo) return; const DocumentDBMap &docDbs = _docDbRepo->getDocDbs(); - for (DocumentDBMap::const_iterator itr = docDbs.begin(); - itr != docDbs.end(); ++itr) { - if (!docType.empty() && docType != itr->first.getName()) { + for (const auto & dbEntry : docDbs) { + if (!docType.empty() && docType != dbEntry.first.getName()) { continue; } - LOG(info, "putHandler(%s)", itr->first.toString().c_str()); - IPersistenceHandler::SP proxy( - new PersistenceHandlerProxy(itr->second)); - putHandler(getWLock(), itr->second->getBucketSpace(), itr->first, proxy); + LOG(info, "putHandler(%s)", dbEntry.first.toString().c_str()); + auto proxy = std::make_shared<PersistenceHandlerProxy>(dbEntry.second); + putHandler(getWLock(), dbEntry.second->getBucketSpace(), dbEntry.first, proxy); } } void removeHandlers() { - if (_docDbRepo.get() == NULL) + if ( ! _docDbRepo) return; const DocumentDBMap &docDbs = _docDbRepo->getDocDbs(); - for (DocumentDBMap::const_iterator itr = docDbs.begin(); - itr != docDbs.end(); ++itr) { - IPersistenceHandler::SP proxy(removeHandler(getWLock(), itr->second->getBucketSpace(), itr->first)); + for (const auto & dbEntry : docDbs) { + IPersistenceHandler::SP proxy(removeHandler(getWLock(), dbEntry.second->getBucketSpace(), dbEntry.first)); + (void) proxy; } } - virtual - ~MyPersistenceEngine() + ~MyPersistenceEngine() override { destroyIterators(); removeHandlers(); // Block calls to document db from engine @@ -367,11 +347,11 @@ private: MyResourceWriteFilter _writeFilter; public: MyPersistenceFactory(const vespalib::string &baseDir, int tlsListenPort, - const SchemaConfigFactory::SP &schemaFactory, - const vespalib::string &docType = "") + SchemaConfigFactory::SP schemaFactory, + const vespalib::string & docType = "") : _baseDir(baseDir), _docDbFactory(baseDir, tlsListenPort), - _schemaFactory(schemaFactory), + _schemaFactory(std::move(schemaFactory)), _docDbRepo(), _docType(docType), _engineOwner(), @@ -382,25 +362,25 @@ public: ~MyPersistenceFactory() override { clear(); } - virtual PersistenceProvider::UP getPersistenceImplementation(const std::shared_ptr<const DocumentTypeRepo> &repo, - const DocumenttypesConfig &typesCfg) override { - ConfigFactory cfgFactory(repo, DocumenttypesConfigSP(new DocumenttypesConfig(typesCfg)), _schemaFactory); - _docDbRepo.reset(new DocumentDBRepo(cfgFactory, _docDbFactory)); + PersistenceProvider::UP getPersistenceImplementation(const std::shared_ptr<const DocumentTypeRepo> &repo, + const DocumenttypesConfig &typesCfg) override { + ConfigFactory cfgFactory(repo, std::make_shared<DocumenttypesConfig>(typesCfg), _schemaFactory); + _docDbRepo = std::make_unique<DocumentDBRepo>(cfgFactory, _docDbFactory); PersistenceEngine::UP engine(new MyPersistenceEngine(_engineOwner, _writeFilter, std::move(_docDbRepo), _docType)); - assert(_docDbRepo.get() == NULL); // Repo should be handed over - return PersistenceProvider::UP(engine.release()); + assert( ! _docDbRepo); // Repo should be handed over + return engine; } - virtual void clear() override { + void clear() override { FastOS_FileInterface::EmptyAndRemoveDirectory(_baseDir.c_str()); } - virtual bool hasPersistence() const override { return true; } - virtual bool supportsActiveState() const override { return true; } - virtual bool supportsBucketSpaces() const override { return true; } + bool hasPersistence() const override { return true; } + bool supportsActiveState() const override { return true; } + bool supportsBucketSpaces() const override { return true; } }; diff --git a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp index f52ca606900..64e67672f0f 100644 --- a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp +++ b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp @@ -748,8 +748,9 @@ TEST("require that document selection and timestamp range works together") { } TEST("require that fieldset limits fields returned") { - document::FieldCollection limited(getDocType()); - limited.insert(getDocType().getField("header")); + document::Field::Set fields; + fields.insert(&getDocType().getField("header")); + document::FieldCollection limited(getDocType(), std::move(fields)); DocumentIterator itr(bucket(5), limited, selectAll(), newestV(), -1, false); itr.add(doc_with_fields("id:ns:foo::xxx1", Timestamp(1), bucket(5))); IterateResult res = itr.iterate(largeNum); diff --git a/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp b/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp index 0c72d11899a..8eace5ac657 100644 --- a/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp +++ b/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp @@ -16,7 +16,7 @@ struct DummyPersistenceHandler : public IPersistenceHandler { using SP = std::shared_ptr<DummyPersistenceHandler>; void initialize() override {} void handlePut(FeedToken, const storage::spi::Bucket &, storage::spi::Timestamp, DocumentSP) override {} - void handleUpdate(FeedToken, const storage::spi::Bucket &, storage::spi::Timestamp, const document::DocumentUpdate::SP &) override {} + void handleUpdate(FeedToken, const storage::spi::Bucket &, storage::spi::Timestamp, DocumentUpdateSP) override {} void handleRemove(FeedToken, const storage::spi::Bucket &, storage::spi::Timestamp, const document::DocumentId &) override {} void handleListBuckets(IBucketIdListResultHandler &) override {} void handleSetClusterState(const storage::spi::ClusterState &, IGenericResultHandler &) override {} diff --git a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp index f5346213a7a..214ca186656 100644 --- a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp +++ b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp @@ -204,7 +204,7 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer { } void handleUpdate(FeedToken token, const Bucket& bucket, - Timestamp timestamp, const document::DocumentUpdate::SP& upd) override { + Timestamp timestamp, DocumentUpdateSP upd) override { token->setResult(std::make_unique<UpdateResult>(existingTimestamp), existingTimestamp > 0); handle(token, bucket, timestamp, upd->getId()); } diff --git a/searchcore/src/tests/proton/server/documentretriever_test.cpp b/searchcore/src/tests/proton/server/documentretriever_test.cpp index 30a87bd216e..40224902b8f 100644 --- a/searchcore/src/tests/proton/server/documentretriever_test.cpp +++ b/searchcore/src/tests/proton/server/documentretriever_test.cpp @@ -314,6 +314,17 @@ struct Fixture { attr->commit(); } + Fixture & + addIndexField(const Schema::IndexField &field) { + schema.addIndexField(field); + return *this; + } + + void + build() { + _retriever = std::make_unique<DocumentRetriever>(_dtName, repo, schema, meta_store, attr_manager, doc_store); + } + Fixture() : repo(getRepoConfig()), meta_store(std::make_shared<BucketDBOwner>()), @@ -335,46 +346,29 @@ struct Fixture { lid = putRes.getLid(); ASSERT_TRUE(putRes.ok()); schema::CollectionType ct = schema::CollectionType::SINGLE; - addAttribute<IntegerAttribute>( - dyn_field_i, dyn_value_i, DataType::INT32, ct); - addAttribute<FloatingPointAttribute>( - dyn_field_d, dyn_value_d, DataType::DOUBLE, ct); - addAttribute<StringAttribute>( - dyn_field_s, dyn_value_s, DataType::STRING, ct); - addAttribute<FloatingPointAttribute>( - dyn_field_n, DataType::FLOAT, ct); - addAttribute<IntegerAttribute>( - dyn_field_nai, DataType::INT32, ct); - addAttribute<StringAttribute>( - dyn_field_nas, DataType::STRING, ct); - addAttribute<IntegerAttribute>( - zcurve_field, dynamic_zcurve_value, DataType::INT64, ct); + addAttribute<IntegerAttribute>(dyn_field_i, dyn_value_i, DataType::INT32, ct); + addAttribute<FloatingPointAttribute>(dyn_field_d, dyn_value_d, DataType::DOUBLE, ct); + addAttribute<StringAttribute>(dyn_field_s, dyn_value_s, DataType::STRING, ct); + addAttribute<FloatingPointAttribute>(dyn_field_n, DataType::FLOAT, ct); + addAttribute<IntegerAttribute>(dyn_field_nai, DataType::INT32, ct); + addAttribute<StringAttribute>(dyn_field_nas, DataType::STRING, ct); + addAttribute<IntegerAttribute>(zcurve_field, dynamic_zcurve_value, DataType::INT64, ct); addTensorAttribute(dyn_field_tensor.c_str(), *dynamic_tensor); - PredicateAttribute *attr = addAttribute<PredicateAttribute>( - dyn_field_p, DataType::BOOLEANTREE, ct); + PredicateAttribute *attr = addAttribute<PredicateAttribute>(dyn_field_p, DataType::BOOLEANTREE, ct); attr->getIndex().indexEmptyDocument(lid); attr->commit(); ct = schema::CollectionType::ARRAY; - addAttribute<IntegerAttribute>( - dyn_arr_field_i, dyn_value_i, DataType::INT32, ct); - addAttribute<FloatingPointAttribute>( - dyn_arr_field_d, dyn_value_d, DataType::DOUBLE, ct); - addAttribute<StringAttribute>( - dyn_arr_field_s, dyn_value_s, DataType::STRING, ct); - addAttribute<FloatingPointAttribute>( - dyn_arr_field_n, DataType::FLOAT, ct); - addAttribute<IntegerAttribute>( - zcurve_array_field, dynamic_zcurve_value, DataType::INT64, ct); + addAttribute<IntegerAttribute>(dyn_arr_field_i, dyn_value_i, DataType::INT32, ct); + addAttribute<FloatingPointAttribute>(dyn_arr_field_d, dyn_value_d, DataType::DOUBLE, ct); + addAttribute<StringAttribute>(dyn_arr_field_s, dyn_value_s, DataType::STRING, ct); + addAttribute<FloatingPointAttribute>(dyn_arr_field_n, DataType::FLOAT, ct); + addAttribute<IntegerAttribute>(zcurve_array_field, dynamic_zcurve_value, DataType::INT64, ct); ct = schema::CollectionType::WEIGHTEDSET; - addAttribute<IntegerAttribute>( - dyn_wset_field_i, dyn_value_i, DataType::INT32, ct); - addAttribute<FloatingPointAttribute>( - dyn_wset_field_d, dyn_value_d, DataType::DOUBLE, ct); - addAttribute<StringAttribute>( - dyn_wset_field_s, dyn_value_s, DataType::STRING, ct); - addAttribute<FloatingPointAttribute>( - dyn_wset_field_n, DataType::FLOAT, ct); - _retriever = std::make_unique<DocumentRetriever>(_dtName, repo, schema, meta_store, attr_manager, doc_store); + addAttribute<IntegerAttribute>(dyn_wset_field_i, dyn_value_i, DataType::INT32, ct); + addAttribute<FloatingPointAttribute>(dyn_wset_field_d, dyn_value_d, DataType::DOUBLE, ct); + addAttribute<StringAttribute>(dyn_wset_field_s, dyn_value_s, DataType::STRING, ct); + addAttribute<FloatingPointAttribute>(dyn_wset_field_n, DataType::FLOAT, ct); + build(); } void clearAttributes(std::vector<vespalib::string> names) { @@ -386,15 +380,13 @@ struct Fixture { } }; -TEST_F("require that document retriever can retrieve document meta data", - Fixture) { +TEST_F("require that document retriever can retrieve document meta data", Fixture) { DocumentMetaData meta_data = f._retriever->getDocumentMetaData(doc_id); EXPECT_EQUAL(f.lid, meta_data.lid); EXPECT_EQUAL(f.timestamp, meta_data.timestamp); } -TEST_F("require that document retriever can retrieve bucket meta data", - Fixture) { +TEST_F("require that document retriever can retrieve bucket meta data", Fixture) { DocumentMetaData::Vector result; f._retriever->getBucketMetaData(makeSpiBucket(f.bucket_id, PartitionId(0)), result); ASSERT_EQUAL(1u, result.size()); @@ -476,7 +468,7 @@ TEST_F("require that attributes are patched into stored document", Fixture) { } TEST_F("require that attributes are patched into stored document unless also index field", Fixture) { - f.schema.addIndexField(Schema::IndexField(dyn_field_s, DataType::STRING)); + f.addIndexField(Schema::IndexField(dyn_field_s, DataType::STRING)).build(); DocumentMetaData meta_data = f._retriever->getDocumentMetaData(doc_id); Document::UP doc = f._retriever->getDocument(meta_data.lid); ASSERT_TRUE(doc.get()); @@ -574,9 +566,8 @@ TEST_F("require that tensor attribute can be retrieved", Fixture) { ASSERT_TRUE(doc.get()); FieldValue::UP value = doc->getValue(dyn_field_tensor); - ASSERT_TRUE(value.get()); - TensorFieldValue *tensor_value = - dynamic_cast<TensorFieldValue *>(value.get()); + ASSERT_TRUE(value); + TensorFieldValue *tensor_value = dynamic_cast<TensorFieldValue *>(value.get()); ASSERT_TRUE(tensor_value->getAsTensorPtr()->equals(*dynamic_tensor)); } diff --git a/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.cpp b/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.cpp index 75086f7a2dd..051e9726452 100644 --- a/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.cpp +++ b/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.cpp @@ -31,18 +31,20 @@ UpdateOperation::UpdateOperation(Type type) UpdateOperation::UpdateOperation(Type type, const BucketId &bucketId, - const Timestamp ×tamp, const DocumentUpdate::SP &upd) + const Timestamp ×tamp, DocumentUpdate::SP upd) : DocumentOperation(type, bucketId, timestamp), - _upd(upd) + _upd(std::move(upd)) { } -UpdateOperation::UpdateOperation(const BucketId &bucketId, const Timestamp ×tamp, const DocumentUpdate::SP &upd) - : UpdateOperation(FeedOperation::UPDATE, bucketId, timestamp, upd) +UpdateOperation::UpdateOperation(const BucketId &bucketId, const Timestamp ×tamp, DocumentUpdate::SP upd) + : UpdateOperation(FeedOperation::UPDATE, bucketId, timestamp, std::move(upd)) { } +UpdateOperation::~UpdateOperation() = default; + void UpdateOperation::serializeUpdate(vespalib::nbostream &os) const { diff --git a/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.h b/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.h index 83e87fea096..7975452cfc6 100644 --- a/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.h +++ b/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.h @@ -17,7 +17,7 @@ private: DocumentUpdateSP _upd; UpdateOperation(Type type, const document::BucketId &bucketId, const storage::spi::Timestamp ×tamp, - const DocumentUpdateSP &upd); + DocumentUpdateSP upd); void serializeUpdate(vespalib::nbostream &os) const; void deserializeUpdate(vespalib::nbostream && is, const document::DocumentTypeRepo &repo); public: @@ -25,8 +25,8 @@ public: UpdateOperation(Type type); UpdateOperation(const document::BucketId &bucketId, const storage::spi::Timestamp ×tamp, - const DocumentUpdateSP &upd); - ~UpdateOperation() override {} + DocumentUpdateSP upd); + ~UpdateOperation() override; const DocumentUpdateSP &getUpdate() const { return _upd; } void serialize(vespalib::nbostream &os) const override; void deserialize(vespalib::nbostream &is, const document::DocumentTypeRepo &repo) override; diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp index 8175e612bd4..fcb9a672039 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp @@ -78,7 +78,7 @@ DocumentIterator::DocumentIterator(const storage::spi::Bucket &bucket, _fields(fields.clone()), _defaultSerializedSize((readConsistency == ReadConsistency::WEAK) ? defaultSerializedSize : -1), _readConsistency(readConsistency), - _metaOnly(fields.getType() == document::FieldSet::NONE), + _metaOnly(fields.getType() == document::FieldSet::Type::NONE), _ignoreMaxBytes((readConsistency == ReadConsistency::WEAK) && ignoreMaxBytes), _fetchedData(false), _sources(), @@ -140,7 +140,7 @@ public: _selectSession = _cachedSelect->createSession(); using document::select::GidFilter; _gidFilter = GidFilter::for_selection_root_node(_selectSession->selectNode()); - _selectCxt.reset(new SelectContext(*_cachedSelect)); + _selectCxt = std::make_unique<SelectContext>(*_cachedSelect); _selectCxt->getAttributeGuards(); } } else { @@ -216,7 +216,7 @@ public: } } - virtual bool allowVisitCaching() const override { + bool allowVisitCaching() const override { return _allowVisitCaching; } diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h b/searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h index c95f51b29dc..a4bede28d7b 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h @@ -30,10 +30,7 @@ public: IPersistenceHandler(const IPersistenceHandler &) = delete; IPersistenceHandler & operator = (const IPersistenceHandler &) = delete; - /** - * Virtual destructor to allow inheritance. - */ - virtual ~IPersistenceHandler() { } + virtual ~IPersistenceHandler() = default; /** * Called before all other functions so that the persistence handler @@ -45,7 +42,7 @@ public: storage::spi::Timestamp timestamp, DocumentSP doc) = 0; virtual void handleUpdate(FeedToken token, const storage::spi::Bucket &bucket, - storage::spi::Timestamp timestamp, const DocumentUpdateSP &upd) = 0; + storage::spi::Timestamp timestamp, DocumentUpdateSP upd) = 0; virtual void handleRemove(FeedToken token, const storage::spi::Bucket &bucket, storage::spi::Timestamp timestamp, const document::DocumentId &id) = 0; diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp index 2659952ae03..da1b90aa167 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp @@ -436,7 +436,7 @@ PersistenceEngine::get(const Bucket& b, const document::FieldSet& fields, const if (meta.removed) { return GetResult::make_for_tombstone(meta.timestamp); } - if (fields.NONE == fields.getType()) { + if (document::FieldSet::Type::NONE == fields.getType()) { return GetResult::make_for_metadata_only(meta.timestamp); } document::Document::UP doc = retriever.getDocument(meta.lid); diff --git a/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp b/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp index 09550bd74dd..7466bcd99d5 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp @@ -143,7 +143,7 @@ public: } } - virtual bool allowVisitCaching() const override { + bool allowVisitCaching() const override { return _visitor.allowVisitCaching(); } @@ -154,7 +154,8 @@ private: } // namespace -Document::UP DocumentRetriever::getDocument(DocumentIdT lid) const +Document::UP +DocumentRetriever::getDocument(DocumentIdT lid) const { Document::UP doc = _doc_store.read(lid, getDocumentTypeRepo()); if (doc) { diff --git a/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.cpp b/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.cpp index 65a4f7e7c4a..183e9eb0c0a 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.cpp @@ -26,7 +26,7 @@ DocumentRetrieverBase::DocumentRetrieverBase( _hasFields(hasFields) { const document::DocumentType * docType(_repo.getDocumentType(_docTypeName.getName())); - _emptyDoc.reset(new document::Document(*docType, DocumentId("id:empty:" + _docTypeName.getName() + "::empty"))); + _emptyDoc = std::make_unique<document::Document>(*docType, DocumentId("id:empty:" + _docTypeName.getName() + "::empty")); _emptyDoc->setRepo(_repo); } @@ -58,7 +58,7 @@ DocumentRetrieverBase::getDocumentMetaData(const DocumentId &id) const { const search::IAttributeManager * DocumentRetrieverBase::getAttrMgr() const { - return NULL; + return nullptr; } @@ -71,7 +71,7 @@ DocumentRetrieverBase::parseSelect(const vespalib::string &selection) const return _selectCache[selection]; } - CachedSelect::SP nselect(new CachedSelect()); + auto nselect = std::make_shared<CachedSelect>(); nselect->set(selection, _docTypeName.getName(), diff --git a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp index af4af7f64fe..7d151a9ef14 100644 --- a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "persistencehandlerproxy.h" -#include "documentretriever.h" #include "documentdb.h" #include <vespa/searchcore/proton/feedoperation/createbucketoperation.h> #include <vespa/searchcore/proton/feedoperation/deletebucketoperation.h> @@ -10,7 +9,6 @@ #include <vespa/searchcore/proton/feedoperation/removeoperation.h> #include <vespa/searchcore/proton/feedoperation/splitbucketoperation.h> #include <vespa/searchcore/proton/feedoperation/updateoperation.h> -#include <vespa/document/fieldvalue/document.h> #include <vespa/document/update/documentupdate.h> using storage::spi::Bucket; @@ -18,8 +16,8 @@ using storage::spi::Timestamp; namespace proton { -PersistenceHandlerProxy::PersistenceHandlerProxy(const DocumentDB::SP &documentDB) - : _documentDB(documentDB), +PersistenceHandlerProxy::PersistenceHandlerProxy(DocumentDB::SP documentDB) + : _documentDB(std::move(documentDB)), _feedHandler(_documentDB->getFeedHandler()), _bucketHandler(_documentDB->getBucketHandler()), _clusterStateHandler(_documentDB->getClusterStateHandler()) @@ -46,9 +44,9 @@ PersistenceHandlerProxy::handlePut(FeedToken token, const Bucket &bucket, Timest } void -PersistenceHandlerProxy::handleUpdate(FeedToken token, const Bucket &bucket, Timestamp timestamp, const DocumentUpdateSP &upd) +PersistenceHandlerProxy::handleUpdate(FeedToken token, const Bucket &bucket, Timestamp timestamp, DocumentUpdateSP upd) { - auto op = std::make_unique<UpdateOperation>(bucket.getBucketId().stripUnused(), timestamp, upd); + auto op = std::make_unique<UpdateOperation>(bucket.getBucketId().stripUnused(), timestamp, std::move(upd)); _feedHandler.handleOperation(std::move(token), std::move(op)); } diff --git a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h index 5a82b46f91c..ee6e8a7ee42 100644 --- a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h +++ b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h @@ -18,9 +18,9 @@ private: BucketHandler &_bucketHandler; ClusterStateHandler &_clusterStateHandler; public: - PersistenceHandlerProxy(const std::shared_ptr<DocumentDB> &documentDB); + explicit PersistenceHandlerProxy(std::shared_ptr<DocumentDB> documentDB); - virtual ~PersistenceHandlerProxy(); + ~PersistenceHandlerProxy() override; void initialize() override; @@ -28,7 +28,7 @@ public: storage::spi::Timestamp timestamp, DocumentSP doc) override; void handleUpdate(FeedToken token, const storage::spi::Bucket &bucket, - storage::spi::Timestamp timestamp, const DocumentUpdateSP &upd) override; + storage::spi::Timestamp timestamp, DocumentUpdateSP upd) override; void handleRemove(FeedToken token, const storage::spi::Bucket &bucket, storage::spi::Timestamp timestamp, diff --git a/storage/src/vespa/storage/bucketdb/storagebucketinfo.h b/storage/src/vespa/storage/bucketdb/storagebucketinfo.h index 2a3d9aa8f91..d9081432feb 100644 --- a/storage/src/vespa/storage/bucketdb/storagebucketinfo.h +++ b/storage/src/vespa/storage/bucketdb/storagebucketinfo.h @@ -23,7 +23,7 @@ struct StorageBucketInfo { info.setTotalDocumentSize(0); } bool verifyLegal() const { return (disk != 0xff); } - uint32_t getMetaCount() { return info.getMetaCount(); } + uint32_t getMetaCount() const { return info.getMetaCount(); } void setChecksum(uint32_t crc) { info.setChecksum(crc); } bool operator == (const StorageBucketInfo & b) const; bool operator != (const StorageBucketInfo & b) const; diff --git a/storage/src/vespa/storage/persistence/fieldvisitor.cpp b/storage/src/vespa/storage/persistence/fieldvisitor.cpp index df9d3df6379..0071d7a010f 100644 --- a/storage/src/vespa/storage/persistence/fieldvisitor.cpp +++ b/storage/src/vespa/storage/persistence/fieldvisitor.cpp @@ -6,10 +6,10 @@ namespace storage { -FieldVisitor::~FieldVisitor() {} +FieldVisitor::~FieldVisitor() = default; void FieldVisitor::visitFieldValueNode(const document::select::FieldValueNode & node) { - _fields.insert(_docType.getField(node.getRealFieldName())); + _fields.insert(&_docType.getField(node.getRealFieldName())); } void FieldVisitor::visitComparison(const document::select::Compare & node) { diff --git a/storage/src/vespa/storage/persistence/fieldvisitor.h b/storage/src/vespa/storage/persistence/fieldvisitor.h index 4b47c68e33b..f2b07e193d6 100644 --- a/storage/src/vespa/storage/persistence/fieldvisitor.h +++ b/storage/src/vespa/storage/persistence/fieldvisitor.h @@ -16,17 +16,17 @@ namespace storage { class FieldVisitor : public document::select::Visitor { private: document::DocumentType _docType; - document::FieldCollection _fields; + document::Field::Set _fields; public: - FieldVisitor(const document::DocumentType & docType) + explicit FieldVisitor(const document::DocumentType & docType) : _docType(docType), - _fields(_docType) + _fields() {} - ~FieldVisitor(); + ~FieldVisitor() override; - const document::FieldSet & getFieldSet() { - return _fields; + document::FieldCollection getFieldSet() { + return document::FieldCollection(_docType, std::move(_fields)); } void visitFieldValueNode(const document::select::FieldValueNode &) override; diff --git a/storage/src/vespa/storage/persistence/persistencethread.cpp b/storage/src/vespa/storage/persistence/persistencethread.cpp index 31858d2a875..d31157cb11d 100644 --- a/storage/src/vespa/storage/persistence/persistencethread.cpp +++ b/storage/src/vespa/storage/persistence/persistencethread.cpp @@ -45,10 +45,10 @@ private: template<class FunctionType> class LambdaResultTask : public ResultTask { public: - LambdaResultTask(FunctionType && func) + explicit LambdaResultTask(FunctionType && func) : _func(std::move(func)) {} - ~LambdaResultTask() override {} + ~LambdaResultTask() override = default; void run() override { handle(*_result); _func(std::move(_result)); @@ -215,7 +215,7 @@ PersistenceThread::handleRemove(api::RemoveCommand& cmd, MessageTracker::UP trac } else { // Note that the &cmd capture is OK since its lifetime is guaranteed by the tracker auto task = makeResultTask([&metrics, &cmd, tracker = std::move(trackerUP)](spi::Result::UP responseUP) { - const spi::RemoveResult & response = dynamic_cast<const spi::RemoveResult &>(*responseUP); + auto & response = dynamic_cast<const spi::RemoveResult &>(*responseUP); if (tracker->checkForError(response)) { tracker->setReply(std::make_shared<api::RemoveReply>(cmd, response.wasFound() ? cmd.getTimestamp() : 0)); } @@ -253,7 +253,7 @@ PersistenceThread::handleUpdate(api::UpdateCommand& cmd, MessageTracker::UP trac } else { // Note that the &cmd capture is OK since its lifetime is guaranteed by the tracker auto task = makeResultTask([&cmd, tracker = std::move(trackerUP)](spi::Result::UP responseUP) { - const spi::UpdateResult & response = dynamic_cast<const spi::UpdateResult &>(*responseUP); + auto & response = dynamic_cast<const spi::UpdateResult &>(*responseUP); if (tracker->checkForError(response)) { auto reply = std::make_shared<api::UpdateReply>(cmd); reply->setOldTimestamp(response.getExistingTimestamp()); @@ -292,7 +292,7 @@ PersistenceThread::handleGet(api::GetCommand& cmd, MessageTracker::UP tracker) _spi.get(getBucket(cmd.getDocumentId(), cmd.getBucket()), *fieldSet, cmd.getDocumentId(), tracker->context()); if (tracker->checkForError(result)) { - if (!result.hasDocument() && (document::FieldSet::NONE != fieldSet->getType())) { + if (!result.hasDocument() && (document::FieldSet::Type::NONE != fieldSet->getType())) { _env._metrics.get[cmd.getLoadType()].notFound.inc(); } tracker->setReply(std::make_shared<api::GetReply>(cmd, result.getDocumentPtr(), result.getTimestamp(), @@ -569,30 +569,30 @@ PersistenceThread::handleSplitBucket(api::SplitBucketCommand& cmd, MessageTracke _env._fileStorHandler.remapQueueAfterSplit(source, targets[0].second, targets[1].second); bool ownershipChanged(!_bucketOwnershipNotifier->distributorOwns(cmd.getSourceIndex(), cmd.getBucket())); // Now release all the bucketdb locks. - for (uint32_t i = 0; i < targets.size(); i++) { + for (auto & target : targets) { if (ownershipChanged) { - notifyGuard.notifyAlways(targets[i].second.bucket, targets[i].first->getBucketInfo()); + notifyGuard.notifyAlways(target.second.bucket, target.first->getBucketInfo()); } // The entries vector has the source bucket in element zero, so indexing // that with i+1 - if (targets[i].second.foundInQueue || targets[i].first->getMetaCount() > 0) { - if (targets[i].first->getMetaCount() == 0) { + if (target.second.foundInQueue || target.first->getMetaCount() > 0) { + if (target.first->getMetaCount() == 0) { // Fake that the bucket has content so it is not deleted. - targets[i].first->info.setMetaCount(1); + target.first->info.setMetaCount(1); // Must make sure target bucket exists when we have pending ops // to an empty target bucket, since the provider will have // implicitly erased it by this point. - spi::Bucket createTarget(spi::Bucket(targets[i].second.bucket, - spi::PartitionId(targets[i].second.diskIndex))); + spi::Bucket createTarget(spi::Bucket(target.second.bucket, + spi::PartitionId(target.second.diskIndex))); LOG(debug, "Split target %s was empty, but re-creating it since there are remapped operations queued to it", createTarget.toString().c_str()); _spi.createBucket(createTarget, tracker->context()); } - splitReply.getSplitInfo().emplace_back(targets[i].second.bucket.getBucketId(), - targets[i].first->getBucketInfo()); - targets[i].first.write(); + splitReply.getSplitInfo().emplace_back(target.second.bucket.getBucketId(), + target.first->getBucketInfo()); + target.first.write(); } else { - targets[i].first.remove(); + target.first.remove(); } } if (sourceEntry.exist()) { diff --git a/storage/src/vespa/storage/persistence/testandsethelper.cpp b/storage/src/vespa/storage/persistence/testandsethelper.cpp index e1909252c8f..ed396cd522e 100644 --- a/storage/src/vespa/storage/persistence/testandsethelper.cpp +++ b/storage/src/vespa/storage/persistence/testandsethelper.cpp @@ -5,6 +5,7 @@ #include <vespa/storage/persistence/testandsethelper.h> #include <vespa/document/select/parser.h> #include <vespa/document/repo/documenttyperepo.h> +#include <vespa/vespalib/util/stringfmt.h> using namespace std::string_literals; @@ -45,6 +46,7 @@ TestAndSetHelper::TestAndSetHelper(PersistenceThread & thread, const api::TestAn _component(thread._env._component), _cmd(cmd), _docId(cmd.getDocumentId()), + _docTypePtr(nullptr), _missingDocumentImpliesMatch(missingDocumentImpliesMatch) { getDocumentType(); @@ -66,7 +68,10 @@ TestAndSetHelper::retrieveAndMatch(spi::Context & context) { if (result.hasDocument()) { auto docPtr = result.getDocumentPtr(); if (_docSelectionUp->contains(*docPtr) != document::select::Result::True) { - return api::ReturnCode(api::ReturnCode::TEST_AND_SET_CONDITION_FAILED, "Condition did not match document"); + return api::ReturnCode(api::ReturnCode::TEST_AND_SET_CONDITION_FAILED, + vespalib::make_string("Condition did not match document partition=%d, nodeIndex=%d bucket=%lx %s", + _thread._env._partition, _thread._env._nodeIndex, _cmd.getBucketId().getRawId(), + _cmd.hasBeenRemapped() ? "remapped" : "")); } // Document matches @@ -75,7 +80,10 @@ TestAndSetHelper::retrieveAndMatch(spi::Context & context) { return api::ReturnCode(); } - return api::ReturnCode(api::ReturnCode::TEST_AND_SET_CONDITION_FAILED, "Document does not exist"); + return api::ReturnCode(api::ReturnCode::TEST_AND_SET_CONDITION_FAILED, + vespalib::make_string("Document does not exist partition=%d, nodeIndex=%d bucket=%lx %s", + _thread._env._partition, _thread._env._nodeIndex, _cmd.getBucketId().getRawId(), + _cmd.hasBeenRemapped() ? "remapped" : "")); } } // storage diff --git a/vespalib/src/vespa/vespalib/stllike/hash_map.cpp b/vespalib/src/vespa/vespalib/stllike/hash_map.cpp index 9b00d7cb548..54564010d35 100644 --- a/vespalib/src/vespa/vespalib/stllike/hash_map.cpp +++ b/vespalib/src/vespa/vespalib/stllike/hash_map.cpp @@ -20,5 +20,6 @@ VESPALIB_HASH_MAP_INSTANTIATE(uint32_t, int32_t); VESPALIB_HASH_MAP_INSTANTIATE(uint32_t, uint32_t); VESPALIB_HASH_MAP_INSTANTIATE(uint64_t, uint32_t); VESPALIB_HASH_MAP_INSTANTIATE(uint64_t, uint64_t); +VESPALIB_HASH_MAP_INSTANTIATE(uint64_t, bool); VESPALIB_HASH_MAP_INSTANTIATE(double, uint32_t); VESPALIB_HASH_MAP_INSTANTIATE(float, uint32_t); |