diff options
9 files changed, 78 insertions, 90 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 98772c73b49..ef45b1aa0dd 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -30,6 +30,7 @@ import com.yahoo.path.Path; import com.yahoo.transaction.NestedTransaction; import com.yahoo.transaction.Transaction; import com.yahoo.vespa.config.server.application.Application; +import com.yahoo.vespa.config.server.application.ApplicationReindexing; import com.yahoo.vespa.config.server.application.ApplicationSet; import com.yahoo.vespa.config.server.application.CompressedApplicationInputStream; import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker; @@ -93,6 +94,7 @@ import java.util.Optional; import java.util.OptionalLong; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.UnaryOperator; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -895,6 +897,15 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye return getLocalSession(tenant, sessionId).getMetaData(); } + public ApplicationReindexing getReindexing(ApplicationId id) { + return getTenant(id).getApplicationRepo().database().readReindexingStatus(id) + .orElse(ApplicationReindexing.ready(clock.instant())); + } + + public void modifyReindexing(ApplicationId id, UnaryOperator<ApplicationReindexing> modifications) { + getTenant(id).getApplicationRepo().database().modifyReindexing(id, ApplicationReindexing.ready(clock.instant()), modifications); + } + public ConfigserverConfig configserverConfig() { return configserverConfig; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationCuratorDatabase.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationCuratorDatabase.java index 4cd112606c0..ed9f12484b8 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationCuratorDatabase.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationCuratorDatabase.java @@ -24,6 +24,7 @@ import java.time.Instant; import java.util.List; import java.util.Optional; import java.util.concurrent.ExecutorService; +import java.util.function.UnaryOperator; import java.util.stream.Collectors; import static java.util.stream.Collectors.toUnmodifiableMap; @@ -58,6 +59,13 @@ public class ApplicationCuratorDatabase { return curator.lock(lockPath(id), Duration.ofMinutes(1)); // These locks shouldn't be held for very long. } + /** Reads, modifies and writes the application reindexing for this application, while holding its lock. */ + public void modifyReindexing(ApplicationId id, ApplicationReindexing emptyValue, UnaryOperator<ApplicationReindexing> modifications) { + try (Lock lock = curator.lock(reindexingLockPath(id), Duration.ofMinutes(1))) { + writeReindexingStatus(id, modifications.apply(readReindexingStatus(id).orElse(emptyValue))); + } + } + public boolean exists(ApplicationId id) { return curator.exists(applicationPath(id)); } @@ -119,7 +127,7 @@ public class ApplicationCuratorDatabase { .map(ReindexingStatusSerializer::fromBytes); } - public void writeReindexingStatus(ApplicationId id, ApplicationReindexing status) { + void writeReindexingStatus(ApplicationId id, ApplicationReindexing status) { curator.set(reindexingDataPath(id), ReindexingStatusSerializer.toBytes(status)); } @@ -130,6 +138,10 @@ public class ApplicationCuratorDatabase { } + private Path reindexingLockPath(ApplicationId id) { + return locksPath.append(id.serializedForm()).append("reindexing"); + } + private Path lockPath(ApplicationId id) { return locksPath.append(id.serializedForm()); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java index 3ff1351fe39..b3599911b5b 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java @@ -160,15 +160,12 @@ public class Deployment implements com.yahoo.config.provision.Deployment { } private void storeReindexing(ApplicationId applicationId, long requiredSession) { - try (Lock sessionLock = tenant.getApplicationRepo().lock(applicationId)) { - ApplicationReindexing reindexing = tenant.getApplicationRepo().database().readReindexingStatus(applicationId) - .orElse(ApplicationReindexing.ready(clock.instant())); - + applicationRepository.modifyReindexing(applicationId, reindexing -> { for (ReindexActions.Entry entry : configChangeActions.getReindexActions().getEntries()) reindexing = reindexing.withPending(entry.getClusterName(), entry.getDocumentType(), requiredSession); - tenant.getApplicationRepo().database().writeReindexingStatus(applicationId, reindexing); - } + return reindexing; + }); } /** diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java index f336f183d56..a49d98176d2 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java @@ -17,7 +17,6 @@ import com.yahoo.jdisc.application.BindingMatch; import com.yahoo.jdisc.application.UriPattern; import com.yahoo.slime.Cursor; import com.yahoo.vespa.config.server.ApplicationRepository; -import com.yahoo.vespa.config.server.application.ApplicationCuratorDatabase; import com.yahoo.vespa.config.server.application.ApplicationReindexing; import com.yahoo.vespa.config.server.http.ContentHandler; import com.yahoo.vespa.config.server.http.ContentRequest; @@ -25,13 +24,11 @@ import com.yahoo.vespa.config.server.http.HttpErrorResponse; import com.yahoo.vespa.config.server.http.HttpHandler; import com.yahoo.vespa.config.server.http.JSONResponse; import com.yahoo.vespa.config.server.http.NotFoundException; -import com.yahoo.vespa.curator.Lock; import java.io.IOException; import java.time.Duration; import java.time.Instant; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.stream.Stream; @@ -84,7 +81,7 @@ public class ApplicationHandler extends HttpHandler { ApplicationId applicationId = getApplicationIdFromRequest(request); if (isReindexingRequest(request)) { - setReindexingEnabled(applicationId, false); + applicationRepository.modifyReindexing(applicationId, reindexing -> reindexing.enabled(false)); return new JSONResponse(Response.Status.OK); } @@ -215,7 +212,7 @@ public class ApplicationHandler extends HttpHandler { } if (isReindexingRequest(request)) { - setReindexingEnabled(applicationId, true); + applicationRepository.modifyReindexing(applicationId, reindexing -> reindexing.enabled(true)); return new JSONResponse(Response.Status.OK); } @@ -232,10 +229,7 @@ public class ApplicationHandler extends HttpHandler { .filter(type -> ! type.isBlank()) .collect(toList()); Instant now = applicationRepository.clock().instant(); - ApplicationCuratorDatabase database = applicationRepository.getTenant(applicationId).getApplicationRepo().database(); - try (Lock lock = database.lock(applicationId)) { - ApplicationReindexing reindexing = database.readReindexingStatus(applicationId) - .orElse(ApplicationReindexing.ready(now)); + applicationRepository.modifyReindexing(applicationId, reindexing -> { if (clusters.isEmpty()) reindexing = reindexing.withReady(now); else @@ -245,24 +239,14 @@ public class ApplicationHandler extends HttpHandler { else for (String type : types) reindexing = reindexing.withReady(cluster, type, now); - database.writeReindexingStatus(applicationId, reindexing); - } - } - - void setReindexingEnabled(ApplicationId applicationId, boolean enabled) { - Instant now = applicationRepository.clock().instant(); - ApplicationCuratorDatabase database = applicationRepository.getTenant(applicationId).getApplicationRepo().database(); - try (Lock lock = database.lock(applicationId)) { - database.writeReindexingStatus(applicationId, - database.readReindexingStatus(applicationId) - .orElse(ApplicationReindexing.ready(now)) - .enabled(enabled)); - } + return reindexing; + }); } private HttpResponse getReindexingStatus(ApplicationId applicationId) { return new ReindexResponse(applicationRepository.getTenant(applicationId).getApplicationRepo().database() - .readReindexingStatus(applicationId)); + .readReindexingStatus(applicationId) + .orElseThrow(() -> new NotFoundException("Reindexing status not found for " + applicationId))); } private HttpResponse restart(HttpRequest request, ApplicationId applicationId) { @@ -451,36 +435,25 @@ public class ApplicationHandler extends HttpHandler { } private static class ReindexResponse extends JSONResponse { - ReindexResponse(Optional<ApplicationReindexing> applicationReindexing) { + ReindexResponse(ApplicationReindexing reindexing) { super(Response.Status.OK); - applicationReindexing.ifPresent(reindexing -> { object.setBool("enabled", reindexing.enabled()); setStatus(object.setObject("status"), reindexing.common()); - Cursor clustersArray = object.setArray("clusters"); + Cursor clustersObject = object.setObject("clusters"); reindexing.clusters().entrySet().stream().sorted(comparingByKey()) .forEach(cluster -> { - Cursor clusterObject = clustersArray.addObject(); - clusterObject.setString("name", cluster.getKey()); + Cursor clusterObject = clustersObject.setObject(cluster.getKey()); setStatus(clusterObject.setObject("status"), cluster.getValue().common()); - Cursor pendingArray = clusterObject.setArray("pending"); + Cursor pendingObject = clusterObject.setObject("pending"); cluster.getValue().pending().entrySet().stream().sorted(comparingByKey()) - .forEach(pending -> { - Cursor pendingObject = pendingArray.addObject(); - pendingObject.setString("type", pending.getKey()); - pendingObject.setLong("requiredGeneration", pending.getValue()); - }); + .forEach(pending -> pendingObject.setLong(pending.getKey(), pending.getValue())); - Cursor readyArray = clusterObject.setArray("ready"); + Cursor readyObject = clusterObject.setObject("ready"); cluster.getValue().ready().entrySet().stream().sorted(comparingByKey()) - .forEach(ready -> { - Cursor readyObject = readyArray.addObject(); - readyObject.setString("type", ready.getKey()); - setStatus(readyObject, ready.getValue()); - }); + .forEach(ready -> setStatus(readyObject.setObject(ready.getKey()), ready.getValue())); }); - }); } private static void setStatus(Cursor object, ApplicationReindexing.Status status) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ReindexingMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ReindexingMaintainer.java index 4aa89bb8c93..40165450696 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ReindexingMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ReindexingMaintainer.java @@ -58,11 +58,8 @@ public class ReindexingMaintainer extends ConfigServerMaintainer { .map(application -> application.getForVersionOrLatest(Optional.empty(), clock.instant())) .ifPresent(application -> { try { - try (Lock lock = database.lock(id)) { - ApplicationReindexing reindexing = database.readReindexingStatus(id) - .orElse(ApplicationReindexing.ready(clock.instant())); - database.writeReindexingStatus(id, withConvergenceOn(reindexing, lazyGeneration(application), clock.instant())); - } + applicationRepository.modifyReindexing(id, reindexing -> + withConvergenceOn(reindexing, lazyGeneration(application), clock.instant())); } catch (RuntimeException e) { log.log(Level.INFO, "Failed to update reindexing status for " + id + ": " + Exceptions.toMessageString(e)); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java index 49a40e7e33c..7612aa5e01f 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java @@ -210,6 +210,8 @@ public class ApplicationHandlerTest { @Test public void testReindex() throws Exception { ApplicationCuratorDatabase database = applicationRepository.getTenant(applicationId).getApplicationRepo().database(); + reindexing(applicationId, GET, "{\"error-code\": \"NOT_FOUND\", \"message\": \"Reindexing status not found for default.default\"}", 404); + applicationRepository.deploy(testApp, prepareParams(applicationId)); ApplicationReindexing expected = ApplicationReindexing.ready(clock.instant()); assertEquals(expected, @@ -259,7 +261,7 @@ public class ApplicationHandlerTest { assertEquals(expected, database.readReindexingStatus(applicationId).orElseThrow()); - database.writeReindexingStatus(applicationId, expected.withPending("boo", "bar", 123L)); + applicationRepository.modifyReindexing(applicationId, reindexing -> reindexing.withPending("boo", "bar", 123L)); long now = clock.instant().toEpochMilli(); reindexing(applicationId, GET, "{" + @@ -267,47 +269,38 @@ public class ApplicationHandlerTest { " \"status\": {" + " \"readyMillis\": " + (now - 2000) + " }," + - " \"clusters\": [" + - " {" + - " \"name\": \"boo\"," + + " \"clusters\": {" + + " \"boo\": {" + " \"status\": {" + " \"readyMillis\": " + (now - 1000) + " }," + - " \"pending\": [" + - " {" + - " \"type\": \"bar\"," + - " \"requiredGeneration\": 123" + - " }" + - " ]," + - " \"ready\": [" + - " {" + - " \"type\": \"bar\"," + + " \"pending\": {" + + " \"bar\": 123" + + " }," + + " \"ready\": {" + + " \"bar\": {" + " \"readyMillis\": " + now + " }," + - " {" + - " \"type\": \"baz\"," + + " \"baz\": {" + " \"readyMillis\": " + now + " }" + - " ]" + + " }" + " }," + - " {" + - " \"name\": \"foo\", " + + " \"foo\": {" + " \"status\": {" + " \"readyMillis\": " + (now - 1000) + " }," + - " \"pending\": []," + - " \"ready\": [" + - " {" + - " \"type\": \"bar\"," + + " \"pending\": {}," + + " \"ready\": {" + + " \"bar\": {" + " \"readyMillis\": " + now + " }," + - " {" + - " \"type\": \"baz\"," + + " \"baz\": {" + " \"readyMillis\": " + now + " }" + - " ]" + + " }" + " }" + - " ]" + + " }" + "}"); } @@ -524,15 +517,19 @@ public class ApplicationHandlerTest { GET); } - private void reindexing(ApplicationId application, Method method, String expectedBody) throws IOException { + private void reindexing(ApplicationId application, Method method, String expectedBody, int statusCode) throws IOException { String reindexingUrl = toUrlPath(application, Zone.defaultZone(), true) + "/reindexing"; HttpResponse response = createApplicationHandler().handle(createTestRequest(reindexingUrl, method)); - assertEquals(200, response.getStatus()); if (expectedBody != null) { ByteArrayOutputStream out = new ByteArrayOutputStream(); response.render(out); assertJsonEquals(out.toString(), expectedBody); } + assertEquals(statusCode, response.getStatus()); + } + + private void reindexing(ApplicationId application, Method method, String expectedBody) throws IOException { + reindexing(application, method, expectedBody, 200); } private void reindexing(ApplicationId application, Method method) throws IOException { diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java index e9e39e1f449..b384f349f92 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java @@ -37,7 +37,7 @@ public interface ConfigServer { void reindex(DeploymentId deployment, List<String> clusterNames, List<String> documentTypes); - ApplicationReindexing getReindexing(DeploymentId deployment); + Optional<ApplicationReindexing> getReindexing(DeploymentId deployment); void disableReindexing(DeploymentId deployment); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 9854d884bd3..a52ee768406 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -186,7 +186,8 @@ public class ApplicationController { /** Returns the reindexing status for the given application in the given zone. */ public ApplicationReindexing applicationReindexing(ApplicationId id, ZoneId zoneId) { - return configServer.getReindexing(new DeploymentId(id, zoneId)); + return configServer.getReindexing(new DeploymentId(id, zoneId)) + .orElseThrow(() -> new NotExistsException("Reindexing status not found for " + id + " in " + zoneId)); } /** Enables reindexing for the given application in the given zone. */ diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index f48b6f1f687..8acce352d5a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -419,13 +419,13 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer public void reindex(DeploymentId deployment, List<String> clusterNames, List<String> documentTypes) { } @Override - public ApplicationReindexing getReindexing(DeploymentId deployment) { - return new ApplicationReindexing(true, - new Status(Instant.ofEpochMilli(123)), - Map.of("cluster", - new ApplicationReindexing.Cluster(new Status(Instant.ofEpochMilli(234)), - Map.of("type", 100L), - Map.of("type", new Status(Instant.ofEpochMilli(345)))))); + public Optional<ApplicationReindexing> getReindexing(DeploymentId deployment) { + return Optional.of(new ApplicationReindexing(true, + new Status(Instant.ofEpochMilli(123)), + Map.of("cluster", + new ApplicationReindexing.Cluster(new Status(Instant.ofEpochMilli(234)), + Map.of("type", 100L), + Map.of("type", new Status(Instant.ofEpochMilli(345))))))); } @Override |