From a4e58931ac98e8b6e9e470033ba04ee60180b135 Mon Sep 17 00:00:00 2001 From: minux Date: Fri, 16 Aug 2024 16:49:38 +0900 Subject: [PATCH] Check if the internal project and repos exist before creating them (#1010) Motivation: Central Dogma server currently attempts to create internal projects and repositories during starting up, even if they already exist. This is unnecessary for most scenarios, except when setting up a new cluster. Modifications: - Added checks to verify if the internal project and repositories exist before attempting to create them. - Propagated the read-only exception to the caller if it's raised while creating the internal project. - It doesn't make sense to continue to run the cluster because the internal project must exist when the cluster gets back to non read-only mode. Result: - Central Dogma servers now checks for the existence of internal projects and repos before attempting to create them. --- dist/src/conf/dogma.json | 4 -- .../centraldogma/server/CentralDogma.java | 2 +- .../centraldogma/server/command/Command.java | 9 +++- .../project/InternalProjectInitializer.java | 51 +++++++++++++------ .../internal/api/auth/PermissionTest.java | 2 +- .../internal/ProjectManagerExtension.java | 2 +- 6 files changed, 45 insertions(+), 25 deletions(-) diff --git a/dist/src/conf/dogma.json b/dist/src/conf/dogma.json index b7563b988..437cd152a 100644 --- a/dist/src/conf/dogma.json +++ b/dist/src/conf/dogma.json @@ -28,10 +28,6 @@ }, "webAppEnabled": true, "webAppTitle": null, - "mirroringEnabled": true, - "numMirroringThreads": null, - "maxNumFilesPerMirror": null, - "maxNumBytesPerMirror": null, "replication": { "method": "NONE" }, diff --git a/server/src/main/java/com/linecorp/centraldogma/server/CentralDogma.java b/server/src/main/java/com/linecorp/centraldogma/server/CentralDogma.java index 72d46f7de..a7d8af4af 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/CentralDogma.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/CentralDogma.java @@ -496,7 +496,7 @@ private CommandExecutor startCommandExecutor( default: throw new Error("unknown replication method: " + replicationMethod); } - projectInitializer = new InternalProjectInitializer(executor); + projectInitializer = new InternalProjectInitializer(executor, pm); final ServerStatus initialServerStatus = statusManager.serverStatus(); executor.setWritable(initialServerStatus.writable()); diff --git a/server/src/main/java/com/linecorp/centraldogma/server/command/Command.java b/server/src/main/java/com/linecorp/centraldogma/server/command/Command.java index dc5f810ee..11b5fa583 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/command/Command.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/command/Command.java @@ -375,8 +375,13 @@ static Command updateServerStatus(ServerStatus serverStatus) { */ static Command forcePush(Command delegate) { requireNonNull(delegate, "delegate"); - checkArgument(delegate.type() == CommandType.NORMALIZING_PUSH || delegate.type() == CommandType.PUSH, - "delegate: %s (expected: NORMALIZING_PUSH or PUSH)", delegate); + checkArgument(delegate.type() == CommandType.CREATE_PROJECT || + delegate.type() == CommandType.CREATE_REPOSITORY || + delegate.type() == CommandType.NORMALIZING_PUSH || delegate.type() == CommandType.PUSH, + "delegate: %s (expected: CREATE_PROJECT, CREATE_REPOSITORY, NORMALIZING_PUSH or PUSH)", + delegate); + checkArgument(delegate.author().equals(Author.SYSTEM), "delegate.author: %s (expected: SYSTEM)", + delegate.author()); return new ForcePushCommand<>(delegate); } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/storage/project/InternalProjectInitializer.java b/server/src/main/java/com/linecorp/centraldogma/server/storage/project/InternalProjectInitializer.java index 9fe7b295c..a0fa0e7d1 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/storage/project/InternalProjectInitializer.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/storage/project/InternalProjectInitializer.java @@ -25,18 +25,22 @@ import java.util.List; import java.util.concurrent.CompletableFuture; +import com.fasterxml.jackson.databind.JsonNode; import com.google.common.collect.ImmutableList; import com.linecorp.armeria.common.util.Exceptions; import com.linecorp.centraldogma.common.Author; import com.linecorp.centraldogma.common.Change; import com.linecorp.centraldogma.common.ChangeConflictException; +import com.linecorp.centraldogma.common.Entry; import com.linecorp.centraldogma.common.Markup; import com.linecorp.centraldogma.common.ProjectExistsException; +import com.linecorp.centraldogma.common.Query; import com.linecorp.centraldogma.common.ReadOnlyException; import com.linecorp.centraldogma.common.RepositoryExistsException; import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.internal.Jackson; +import com.linecorp.centraldogma.server.command.Command; import com.linecorp.centraldogma.server.command.CommandExecutor; import com.linecorp.centraldogma.server.metadata.MetadataService; import com.linecorp.centraldogma.server.metadata.Tokens; @@ -49,13 +53,15 @@ public final class InternalProjectInitializer { public static final String INTERNAL_PROJECT_DOGMA = "dogma"; private final CommandExecutor executor; + private final ProjectManager projectManager; private final CompletableFuture initialFuture = new CompletableFuture<>(); /** * Creates a new instance. */ - public InternalProjectInitializer(CommandExecutor executor) { + public InternalProjectInitializer(CommandExecutor executor, ProjectManager projectManager) { this.executor = executor; + this.projectManager = projectManager; } /** @@ -86,17 +92,16 @@ public void initialize(String projectName) { */ public void initialize0(String projectName) { final long creationTimeMillis = System.currentTimeMillis(); - try { - executor.execute(createProject(creationTimeMillis, Author.SYSTEM, projectName)) - .get(); - } catch (Throwable cause) { - final Throwable peeled = Exceptions.peel(cause); - if (peeled instanceof ReadOnlyException) { - // The executor has stopped right after starting up. - return; - } - if (!(peeled instanceof ProjectExistsException)) { - throw new Error("failed to initialize an internal project: " + projectName, peeled); + if (!projectManager.exists(projectName)) { + try { + executor.execute(Command.forcePush( + createProject(creationTimeMillis, Author.SYSTEM, projectName))) + .get(); + } catch (Throwable cause) { + final Throwable peeled = Exceptions.peel(cause); + if (!(peeled instanceof ProjectExistsException)) { + throw new Error("failed to initialize an internal project: " + projectName, peeled); + } } } @@ -106,17 +111,25 @@ public void initialize0(String projectName) { } private void initializeTokens() { + final Entry entry = + projectManager.get(INTERNAL_PROJECT_DOGMA).repos().get(Project.REPO_DOGMA) + .getOrNull(Revision.HEAD, + Query.ofJson(MetadataService.TOKEN_JSON)).join(); + if (entry != null && entry.hasContent()) { + return; + } try { final Change change = Change.ofJsonPatch(MetadataService.TOKEN_JSON, null, Jackson.valueToTree(new Tokens())); final String commitSummary = "Initialize the token list file: /" + INTERNAL_PROJECT_DOGMA + '/' + Project.REPO_DOGMA + MetadataService.TOKEN_JSON; - executor.execute(push(Author.SYSTEM, INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, Revision.HEAD, - commitSummary, "", Markup.PLAINTEXT, ImmutableList.of(change))) + executor.execute(Command.forcePush(push(Author.SYSTEM, INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, + Revision.HEAD, commitSummary, "", Markup.PLAINTEXT, + ImmutableList.of(change)))) .get(); } catch (Throwable cause) { final Throwable peeled = Exceptions.peel(cause); - if (peeled instanceof ReadOnlyException || peeled instanceof ChangeConflictException) { + if (peeled instanceof ChangeConflictException) { return; } throw new Error("failed to initialize the token list file", peeled); @@ -137,9 +150,15 @@ public CompletableFuture whenInitialized() { private void initializeInternalRepos(String projectName, List internalRepos, long creationTimeMillis) { requireNonNull(internalRepos, "internalRepos"); + final Project project = projectManager.get(projectName); + assert project != null; for (final String repo : internalRepos) { + if (project.repos().exists(repo)) { + continue; + } try { - executor.execute(createRepository(creationTimeMillis, Author.SYSTEM, projectName, repo)) + executor.execute(Command.forcePush( + createRepository(creationTimeMillis, Author.SYSTEM, projectName, repo))) .get(); } catch (Throwable cause) { final Throwable peeled = Exceptions.peel(cause); diff --git a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/auth/PermissionTest.java b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/auth/PermissionTest.java index 5cafa90fa..db02703f7 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/auth/PermissionTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/auth/PermissionTest.java @@ -98,7 +98,7 @@ protected void configure(ServerBuilder sb) throws Exception { final CommandExecutor executor = new StandaloneCommandExecutor( pm, ForkJoinPool.commonPool(), statusManager, null, null, null); executor.start().join(); - new InternalProjectInitializer(executor).initialize(); + new InternalProjectInitializer(executor, pm).initialize(); executor.execute(Command.createProject(AUTHOR, "project1")).join(); diff --git a/testing-internal/src/main/java/com/linecorp/centraldogma/testing/internal/ProjectManagerExtension.java b/testing-internal/src/main/java/com/linecorp/centraldogma/testing/internal/ProjectManagerExtension.java index c07000778..48cebbe3e 100644 --- a/testing-internal/src/main/java/com/linecorp/centraldogma/testing/internal/ProjectManagerExtension.java +++ b/testing-internal/src/main/java/com/linecorp/centraldogma/testing/internal/ProjectManagerExtension.java @@ -78,7 +78,7 @@ public void before(ExtensionContext context) throws Exception { executor = newCommandExecutor(projectManager, repositoryWorker, dataDir); executor.start().get(); - internalProjectInitializer = new InternalProjectInitializer(executor); + internalProjectInitializer = new InternalProjectInitializer(executor, projectManager); internalProjectInitializer.initialize(); afterExecutorStarted();