diff options
author | Tor Brede Vekterli <vekterli@oath.com> | 2018-01-25 16:13:28 +0100 |
---|---|---|
committer | Geir Storli <geirst@oath.com> | 2018-01-29 15:32:15 +0000 |
commit | e6201d5891787abd3b0aca3c534910cad1d7960e (patch) | |
tree | 9964ab91548745d8bfda7c344f9310a23129581d /vespaclient-container-plugin | |
parent | 26208ac25f0de3848b9e65fe7017d8ceb6d93b70 (diff) |
WIP for auto-deducing bucket space based on doctype for document V1 API
Diffstat (limited to 'vespaclient-container-plugin')
4 files changed, 141 insertions, 23 deletions
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/BucketSpaceEnumerator.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/BucketSpaceEnumerator.java new file mode 100644 index 00000000000..4f3768e3740 --- /dev/null +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/BucketSpaceEnumerator.java @@ -0,0 +1,34 @@ +package com.yahoo.document.restapi; + +import com.yahoo.config.subscription.ConfigGetter; +import com.yahoo.vespa.config.content.core.BucketspacesConfig; + +import java.util.Collections; +import java.util.Map; +import java.util.stream.Collectors; + +/** + * TODO description + */ +class BucketSpaceEnumerator { + + private final Map<String, String> doctypeToSpace; + + private BucketSpaceEnumerator(String configId) { + doctypeToSpace = Collections.unmodifiableMap(buildMappingFromConfig(configId)); + } + + public static BucketSpaceEnumerator fromConfig(String configId) { + return new BucketSpaceEnumerator(configId); + } + + public Map<String, String> getDoctypeToSpaceMapping() { + return doctypeToSpace; + } + + private static Map<String, String> buildMappingFromConfig(String configId) { + BucketspacesConfig config = new ConfigGetter<>(BucketspacesConfig.class).getConfig(configId); + return config.documenttype().stream().collect(Collectors.toMap(dt -> dt.name(), dt -> dt.bucketspace())); + } + +} diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java index 46678ea67e3..8fe2007e88a 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java @@ -7,6 +7,7 @@ import com.yahoo.document.DocumentRemove; import com.yahoo.document.TestAndSetCondition; import com.yahoo.document.json.JsonWriter; import com.yahoo.document.DocumentPut; +import com.yahoo.document.restapi.resource.RestApi; import com.yahoo.documentapi.DocumentAccess; import com.yahoo.documentapi.DocumentAccessException; import com.yahoo.documentapi.SyncParameters; @@ -48,11 +49,34 @@ public class OperationHandlerImpl implements OperationHandler { List<ClusterDef> enumerateClusters(); } + public interface BucketSpaceResolver { + Optional<String> clusterBucketSpaceFromDocumentType(String clusterConfigId, String docType); + } + + public static class BucketSpaceRoute { + private final String clusterRoute; + private final String bucketSpace; + + public BucketSpaceRoute(String clusterRoute, String bucketSpace) { + this.clusterRoute = clusterRoute; + this.bucketSpace = bucketSpace; + } + + public String getClusterRoute() { + return clusterRoute; + } + + public String getBucketSpace() { + return bucketSpace; + } + } + public static final int VISIT_TIMEOUT_MS = 120000; public static final int WANTED_DOCUMENT_COUNT_UPPER_BOUND = 1000; // Approximates the max default size of a bucket private final DocumentAccess documentAccess; private final DocumentApiMetrics metricsHelper; private final ClusterEnumerator clusterEnumerator; + private final BucketSpaceResolver bucketSpaceResolver; private static final class SyncSessionFactory extends ResourceFactory<SyncSession> { private final DocumentAccess documentAccess; @@ -67,13 +91,25 @@ public class OperationHandlerImpl implements OperationHandler { private final ConcurrentResourcePool<SyncSession> syncSessions; + private static ClusterEnumerator defaultClusterEnumerator() { + return () -> new ClusterList("client").getStorageClusters(); + } + + private static BucketSpaceResolver defaultBucketResolver() { + return (clusterConfigId, docType) -> Optional.ofNullable(BucketSpaceEnumerator + .fromConfig(clusterConfigId).getDoctypeToSpaceMapping() + .get(docType)); + } + public OperationHandlerImpl(DocumentAccess documentAccess, MetricReceiver metricReceiver) { - this(documentAccess, () -> new ClusterList("client").getStorageClusters(), metricReceiver); + this(documentAccess, defaultClusterEnumerator(), defaultBucketResolver(), metricReceiver); } - public OperationHandlerImpl(DocumentAccess documentAccess, ClusterEnumerator clusterEnumerator, MetricReceiver metricReceiver) { + public OperationHandlerImpl(DocumentAccess documentAccess, ClusterEnumerator clusterEnumerator, + BucketSpaceResolver bucketSpaceResolver, MetricReceiver metricReceiver) { this.documentAccess = documentAccess; this.clusterEnumerator = clusterEnumerator; + this.bucketSpaceResolver = bucketSpaceResolver; syncSessions = new ConcurrentResourcePool<>(new SyncSessionFactory(documentAccess)); metricsHelper = new DocumentApiMetrics(metricReceiver, "documentV1"); } @@ -280,13 +316,19 @@ public class OperationHandlerImpl implements OperationHandler { } } - private String resolveClusterRoute(Optional<String> wantedCluster) throws RestApiException { + protected BucketSpaceRoute resolveBucketSpaceRoute(Optional<String> wantedCluster, String docType) throws RestApiException { final List<ClusterDef> clusters = clusterEnumerator.enumerateClusters(); - return resolveClusterRoute(wantedCluster, clusters); + ClusterDef clusterDef = resolveClusterDef(wantedCluster, clusters); + Optional<String> targetBucketSpace = bucketSpaceResolver.clusterBucketSpaceFromDocumentType(clusterDef.getConfigId(), docType); + if (!targetBucketSpace.isPresent()) { + throw new RestApiException(Response.createErrorResponse(400, String.format( + "Document type '%s' in cluster '%s' is not mapped to a known bucket space", docType, clusterDef.getName()), + RestUri.apiErrorCodes.UNKNOWN_BUCKET_SPACE)); // TODO own code + } + return new BucketSpaceRoute(clusterDefToRoute(clusterDef), targetBucketSpace.get()); } - // Based on resolveClusterRoute in VdsVisit, protected for testability - protected static String resolveClusterRoute(Optional<String> wantedCluster, List<ClusterDef> clusters) throws RestApiException { + protected static ClusterDef resolveClusterDef(Optional<String> wantedCluster, List<ClusterDef> clusters) throws RestApiException { if (clusters.size() == 0) { throw new IllegalArgumentException("Your Vespa cluster does not have any content clusters " + "declared. Visiting feature is not available."); @@ -296,20 +338,25 @@ public class OperationHandlerImpl implements OperationHandler { throw new RestApiException(Response.createErrorResponse(400, "Several clusters exist: " + clusterListToString(clusters) + " you must specify one. ", RestUri.apiErrorCodes.SEVERAL_CLUSTERS)); } - return clusterDefToRoute(clusters.get(0)); + return clusters.get(0); } for (ClusterDef clusterDef : clusters) { if (clusterDef.getName().equals(wantedCluster.get())) { - return clusterDefToRoute(clusterDef); + return clusterDef; } } throw new RestApiException(Response.createErrorResponse(400, "Your vespa cluster contains the content clusters " + clusterListToString(clusters) + " not " + wantedCluster.get() + ". Please select a valid vespa cluster.", RestUri.apiErrorCodes.MISSING_CLUSTER)); + } + // Based on resolveClusterRoute in VdsVisit, protected for testability + // TODO remove in favor of resolveClusterDef + protected static String resolveClusterRoute(Optional<String> wantedCluster, List<ClusterDef> clusters) throws RestApiException { + return clusterDefToRoute(resolveClusterDef(wantedCluster, clusters)); } - private static String clusterDefToRoute(ClusterDef clusterDef) { + protected static String clusterDefToRoute(ClusterDef clusterDef) { return "[Storage:cluster=" + clusterDef.getName() + ";clusterconfigid=" + clusterDef.getConfigId() + "]"; } @@ -353,7 +400,9 @@ public class OperationHandlerImpl implements OperationHandler { params.visitInconsistentBuckets(true); // TODO document this as part of consistency doc params.setVisitorOrdering(VisitorOrdering.ASCENDING); - params.setRoute(resolveClusterRoute(options.cluster)); + BucketSpaceRoute bucketSpaceRoute = resolveBucketSpaceRoute(options.cluster, restUri.getDocumentType()); + params.setRoute(bucketSpaceRoute.getClusterRoute()); + params.setBucketSpace(bucketSpaceRoute.getBucketSpace()); params.setTraceLevel(0); params.setPriority(DocumentProtocol.Priority.NORMAL_4); diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/RestUri.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/RestUri.java index 4eaac48a511..15d1b54adbe 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/RestUri.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/RestUri.java @@ -34,7 +34,7 @@ public class RestUri { URL_PARSING(-6), INVALID_CREATE_VALUE(-7), TOO_MANY_PARALLEL_REQUESTS(-8), - MISSING_CLUSTER(-9), INTERNAL_EXCEPTION(-9), + MISSING_CLUSTER(-9), UNKNOWN_BUCKET_SPACE(-9), INTERNAL_EXCEPTION(-9), DOCUMENT_CONDITION_NOT_MET(-10), DOCUMENT_EXCPETION(-11), PARSER_ERROR(-11), diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java index 5735e84f3fe..730930d80cb 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java @@ -14,10 +14,7 @@ import org.junit.Test; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.net.URI; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Optional; +import java.util.*; import java.util.concurrent.atomic.AtomicReference; import static org.hamcrest.CoreMatchers.containsString; @@ -33,27 +30,31 @@ public class OperationHandlerImplTest { @Test(expected = IllegalArgumentException.class) public void missingClusterDef() throws RestApiException { List<ClusterDef> clusterDef = new ArrayList<>(); - OperationHandlerImpl.resolveClusterRoute(Optional.empty(), clusterDef); + OperationHandlerImpl.resolveClusterDef(Optional.empty(), clusterDef); } @Test(expected = IllegalArgumentException.class) public void missingClusterDefSpecifiedCluster() throws RestApiException { List<ClusterDef> clusterDef = new ArrayList<>(); - OperationHandlerImpl.resolveClusterRoute(Optional.of("cluster"), clusterDef); + OperationHandlerImpl.resolveClusterDef(Optional.of("cluster"), clusterDef); } @Test(expected = RestApiException.class) public void oneClusterPresentNotMatching() throws RestApiException { List<ClusterDef> clusterDef = new ArrayList<>(); clusterDef.add(new ClusterDef("foo", "configId")); - OperationHandlerImpl.resolveClusterRoute(Optional.of("cluster"), clusterDef); + OperationHandlerImpl.resolveClusterDef(Optional.of("cluster"), clusterDef); + } + + private static String toRoute(ClusterDef clusterDef) { + return OperationHandlerImpl.clusterDefToRoute(clusterDef); } @Test() public void oneClusterMatching() throws RestApiException { List<ClusterDef> clusterDef = new ArrayList<>(); clusterDef.add(new ClusterDef("foo", "configId")); - assertThat(OperationHandlerImpl.resolveClusterRoute(Optional.of("foo"), clusterDef), + assertThat(toRoute(OperationHandlerImpl.resolveClusterDef(Optional.of("foo"), clusterDef)), is("[Storage:cluster=foo;clusterconfigid=configId]")); } @@ -63,18 +64,18 @@ public class OperationHandlerImplTest { clusterDef.add(new ClusterDef("foo2", "configId2")); clusterDef.add(new ClusterDef("foo", "configId")); clusterDef.add(new ClusterDef("foo3", "configId2")); - assertThat(OperationHandlerImpl.resolveClusterRoute(Optional.of("foo"), clusterDef), + assertThat(toRoute(OperationHandlerImpl.resolveClusterDef(Optional.of("foo"), clusterDef)), is("[Storage:cluster=foo;clusterconfigid=configId]")); } @Test() - public void checkErrorMessage() throws RestApiException, IOException { + public void unknown_target_cluster_throws_exception() throws RestApiException, IOException { List<ClusterDef> clusterDef = new ArrayList<>(); clusterDef.add(new ClusterDef("foo2", "configId2")); clusterDef.add(new ClusterDef("foo", "configId")); clusterDef.add(new ClusterDef("foo3", "configId2")); try { - OperationHandlerImpl.resolveClusterRoute(Optional.of("wrong"), clusterDef); + OperationHandlerImpl.resolveClusterDef(Optional.of("wrong"), clusterDef); } catch(RestApiException e) { String errorMsg = renderRestApiExceptionAsString(e); assertThat(errorMsg, is("{\"errors\":[{\"description\":" + @@ -96,6 +97,12 @@ public class OperationHandlerImplTest { AtomicReference<VisitorParameters> assignedParameters = new AtomicReference<>(); VisitorControlHandler.CompletionCode completionCode = VisitorControlHandler.CompletionCode.SUCCESS; int bucketsVisited = 0; + Map<String, String> bucketSpaces = new HashMap<>(); + + OperationHandlerImplFixture() { + bucketSpaces.put("foo", "global"); + bucketSpaces.put("document-type", "default"); + } OperationHandlerImpl createHandler() throws Exception { VisitorSession visitorSession = mock(VisitorSession.class); @@ -115,7 +122,8 @@ public class OperationHandlerImplTest { return visitorSession; }); OperationHandlerImpl.ClusterEnumerator clusterEnumerator = () -> Arrays.asList(new ClusterDef("foo", "configId")); - return new OperationHandlerImpl(documentAccess, clusterEnumerator, MetricReceiver.nullImplementation); + OperationHandlerImpl.BucketSpaceResolver bucketSpaceResolver = (configId, docType) -> Optional.ofNullable(bucketSpaces.get(docType)); + return new OperationHandlerImpl(documentAccess, clusterEnumerator, bucketSpaceResolver, MetricReceiver.nullImplementation); } } @@ -175,6 +183,33 @@ public class OperationHandlerImplTest { } @Test + public void document_type_is_mapped_to_correct_bucket_space() throws Exception { + OperationHandlerImplFixture fixture = new OperationHandlerImplFixture(); + fixture.bucketSpaces.put("document-type", "langbein"); + OperationHandlerImpl handler = fixture.createHandler(); + handler.visit(dummyVisitUri(), "", emptyVisitOptions()); + + VisitorParameters parameters = fixture.assignedParameters.get(); + assertEquals("langbein", parameters.getBucketSpace()); + } + + @Test + public void unknown_bucket_space_mapping_throws_exception() throws Exception { + OperationHandlerImplFixture fixture = new OperationHandlerImplFixture(); + fixture.bucketSpaces.remove("document-type"); + try { + OperationHandlerImpl handler = fixture.createHandler(); + handler.visit(dummyVisitUri(), "", emptyVisitOptions()); + } catch (RestApiException e) { + assertThat(e.getResponse().getStatus(), is(400)); + String errorMsg = renderRestApiExceptionAsString(e); + // FIXME isn't this really more of a case of unknown document type..? + assertThat(errorMsg, is("{\"errors\":[{\"description\":" + + "\"UNKNOWN_BUCKET_SPACE Document type 'document-type' in cluster 'foo' is not mapped to a known bucket space\",\"id\":-15}]}")); + } + } + + @Test public void provided_wanted_document_count_is_propagated_to_visitor_parameters() throws Exception { VisitorParameters params = generatedParametersFromVisitOptions(visitOptionsWithWantedDocumentCount(123)); assertThat(params.getMaxTotalHits(), is((long)123)); |