Send small refs as a fetch call payload To improve performance of fetch operation if total size of the ref is smaller than 'payloadMaxRefSize' parameter send all objects for that ref as a REST Api call payload. Otherwise use git fetch operation. Bug: Issue 13915 Change-Id: I3b39bd57120016617128f841a62c66341b968fb0
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueue.java index 8c7dca6..6c8ada0 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueue.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueue.java
@@ -18,7 +18,6 @@ import com.google.common.collect.Queues; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.Project.NameKey; -import com.google.gerrit.entities.RefNames; import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.extensions.registration.DynamicItem; @@ -36,6 +35,7 @@ import java.io.IOException; import java.net.URISyntaxException; import java.util.HashSet; +import java.util.Optional; import java.util.Queue; import java.util.Set; import java.util.concurrent.ExecutionException; @@ -183,18 +183,18 @@ } private CallFunction getCallFunction(NameKey project, String refName, ReplicationState state) { - if (RefNames.isNoteDbMetaRef(refName)) { - try { - RevisionData revision = revisionReader.read(project, refName); - return (source) -> callSendObject(source, project, refName, revision, state); - } catch (IOException | RefUpdateException e) { - stateLog.error( - String.format( - "Exception during reading ref: %s, project:%s, message: %s", - refName, project.get(), e.getMessage()), - e, - state); + try { + Optional<RevisionData> revisionData = revisionReader.read(project, refName); + if (revisionData.isPresent()) { + return ((source) -> callSendObject(source, project, refName, revisionData.get(), state)); } + } catch (IOException | RefUpdateException e) { + stateLog.error( + String.format( + "Exception during reading ref: %s, project:%s, message: %s", + refName, project.get(), e.getMessage()), + e, + state); } return (source) -> callFetch(source, project, refName, state);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/RevisionReader.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/RevisionReader.java index a10f1da..d88c5f5 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/RevisionReader.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/RevisionReader.java
@@ -14,6 +14,8 @@ package com.googlesource.gerrit.plugins.replication.pull; +import static com.googlesource.gerrit.plugins.replication.pull.PullReplicationLogger.repLog; + import com.google.common.collect.Lists; import com.google.gerrit.entities.Project; import com.google.gerrit.server.git.GitRepositoryManager; @@ -23,11 +25,15 @@ import com.googlesource.gerrit.plugins.replication.pull.api.exception.RefUpdateException; import java.io.IOException; import java.util.List; +import java.util.Optional; +import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.diff.DiffEntry.ChangeType; import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.errors.IncorrectObjectTypeException; +import org.eclipse.jgit.errors.LargeObjectException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.RepositoryNotFoundException; -import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.Ref; @@ -35,16 +41,22 @@ import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.treewalk.TreeWalk; +import org.eclipse.jgit.treewalk.filter.TreeFilter; public class RevisionReader { + private static final String CONFIG_MAX_API_PAYLOAD_SIZE = "maxApiPayloadSize"; + private static final Long DEFAULT_MAX_PAYLOAD_SIZE_IN_BYTES = 10000L; private GitRepositoryManager gitRepositoryManager; + private Long maxRefSize; @Inject - public RevisionReader(GitRepositoryManager gitRepositoryManager) { + public RevisionReader(GitRepositoryManager gitRepositoryManager, Config cfg) { this.gitRepositoryManager = gitRepositoryManager; + this.maxRefSize = + cfg.getLong("replication", CONFIG_MAX_API_PAYLOAD_SIZE, DEFAULT_MAX_PAYLOAD_SIZE_IN_BYTES); } - public RevisionData read(Project.NameKey project, String refName) + public Optional<RevisionData> read(Project.NameKey project, String refName) throws MissingObjectException, IncorrectObjectTypeException, CorruptObjectException, RepositoryNotFoundException, RefUpdateException, IOException { try (Repository git = gitRepositoryManager.openRepository(project)) { @@ -54,30 +66,97 @@ String.format("Cannot find ref %s in project %s", refName, project.get())); } + Long totalRefSize = 0l; + ObjectLoader commitLoader = git.open(head.getObjectId()); + totalRefSize += commitLoader.getSize(); + verifySize(totalRefSize, commitLoader); + RevCommit commit = RevCommit.parse(commitLoader.getCachedBytes()); RevisionObjectData commitRev = new RevisionObjectData(commit.getType(), commitLoader.getCachedBytes()); RevTree tree = commit.getTree(); ObjectLoader treeLoader = git.open(commit.getTree().toObjectId()); + totalRefSize += treeLoader.getSize(); + verifySize(totalRefSize, treeLoader); + RevisionObjectData treeRev = new RevisionObjectData(tree.getType(), treeLoader.getCachedBytes()); List<RevisionObjectData> blobs = Lists.newLinkedList(); try (TreeWalk walk = new TreeWalk(git)) { - walk.addTree(tree); - while (walk.next()) { - ObjectId blobObjectId = walk.getObjectId(0); - ObjectLoader blobLoader = git.open(blobObjectId); - if (blobLoader.getType() == Constants.OBJ_BLOB) { - RevisionObjectData rev = - new RevisionObjectData(blobLoader.getType(), blobLoader.getCachedBytes()); - blobs.add(rev); - } + if (commit.getParentCount() > 0) { + List<DiffEntry> diffEntries = readDiffs(git, commit, tree, walk); + blobs = readBlobs(git, totalRefSize, diffEntries); + } else { + walk.setRecursive(true); + walk.addTree(tree); + blobs = readBlobs(git, totalRefSize, walk); } } - return new RevisionData(commitRev, treeRev, blobs); + return Optional.of(new RevisionData(commitRev, treeRev, blobs)); + } catch (LargeObjectException e) { + repLog.trace( + "Ref %s size for project %s is greater than configured '%s'", + refName, project, CONFIG_MAX_API_PAYLOAD_SIZE); + return Optional.empty(); + } + } + + private List<DiffEntry> readDiffs(Repository git, RevCommit commit, RevTree tree, TreeWalk walk) + throws MissingObjectException, IncorrectObjectTypeException, CorruptObjectException, + IOException { + walk.setFilter(TreeFilter.ANY_DIFF); + walk.reset(getParentTree(git, commit), tree); + return DiffEntry.scan(walk, true); + } + + private List<RevisionObjectData> readBlobs(Repository git, Long totalRefSize, TreeWalk walk) + throws MissingObjectException, IncorrectObjectTypeException, CorruptObjectException, + IOException { + List<RevisionObjectData> blobs = Lists.newLinkedList(); + while (walk.next()) { + ObjectId objectId = walk.getObjectId(0); + ObjectLoader objectLoader = git.open(objectId); + totalRefSize += objectLoader.getSize(); + verifySize(totalRefSize, objectLoader); + + RevisionObjectData rev = + new RevisionObjectData(objectLoader.getType(), objectLoader.getCachedBytes()); + blobs.add(rev); + } + return blobs; + } + + private List<RevisionObjectData> readBlobs( + Repository git, Long totalRefSize, List<DiffEntry> diffEntries) + throws MissingObjectException, IOException { + List<RevisionObjectData> blobs = Lists.newLinkedList(); + for (DiffEntry diffEntry : diffEntries) { + if (!ChangeType.DELETE.equals(diffEntry.getChangeType())) { + ObjectLoader objectLoader = git.open(diffEntry.getNewId().toObjectId()); + totalRefSize += objectLoader.getSize(); + verifySize(totalRefSize, objectLoader); + RevisionObjectData rev = + new RevisionObjectData(objectLoader.getType(), objectLoader.getCachedBytes()); + blobs.add(rev); + } + } + return blobs; + } + + private RevTree getParentTree(Repository git, RevCommit commit) + throws MissingObjectException, IOException { + RevCommit parent = commit.getParent(0); + ObjectLoader parentLoader = git.open(parent.getId()); + RevCommit parentCommit = RevCommit.parse(parentLoader.getCachedBytes()); + return parentCommit.getTree(); + } + + private void verifySize(Long totalRefSize, ObjectLoader loader) throws LargeObjectException { + if (loader.isLarge() || totalRefSize > maxRefSize) { + throw new LargeObjectException(); } } }
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index ae2c2fd..c3e5ad4 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -181,6 +181,13 @@ By default, all other refs are included. +replication.maxApiPayloadSize +: Maximum size in bytes of the ref to be sent as a REST Api call + payload. For refs larger than threshold git fetch operation + will be used. + + Default: 10000 + remote.NAME.url : Address of the remote server to fetch from. Single URL can be specified within a single remote block. A remote node can request
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueueTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueueTest.java index e582b4f..44b69db 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueueTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueueTest.java
@@ -40,7 +40,9 @@ import com.googlesource.gerrit.plugins.replication.pull.client.HttpResult; import java.io.IOException; import java.nio.file.Path; +import java.util.Optional; import org.apache.http.client.ClientProtocolException; +import org.eclipse.jgit.errors.LargeObjectException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.util.FS; @@ -73,7 +75,7 @@ private Path pluginDataPath; @Before - public void setup() throws IOException, RefUpdateException { + public void setup() throws IOException, LargeObjectException, RefUpdateException { Path sitePath = createTempPath("site"); sitePaths = new SitePaths(sitePath); Path pluginDataPath = createTempPath("data"); @@ -86,7 +88,7 @@ when(source.getApis()).thenReturn(apis); when(sourceCollection.getAll()).thenReturn(Lists.newArrayList(source)); when(rd.get()).thenReturn(sourceCollection); - when(revReader.read(any(), anyString())).thenReturn(revisionData); + when(revReader.read(any(), anyString())).thenReturn(Optional.of(revisionData)); when(fetchClientFactory.create(any())).thenReturn(fetchRestApiClient); when(fetchRestApiClient.callSendObject(any(), anyString(), any(), any())) .thenReturn(httpResult); @@ -107,21 +109,34 @@ } @Test - public void shouldCallFetchWhenPatchSetRef() throws ClientProtocolException, IOException { + public void shouldCallSendObjectWhenPatchSetRef() throws ClientProtocolException, IOException { Event event = new TestEvent("refs/changes/01/1/1"); objectUnderTest.start(); objectUnderTest.onGitReferenceUpdated(event); + verify(fetchRestApiClient).callSendObject(any(), anyString(), any(), any()); + } + + @Test + public void shouldFallbackToCallFetchWhenIOException() + throws ClientProtocolException, IOException, LargeObjectException, RefUpdateException { + Event event = new TestEvent("refs/changes/01/1/meta"); + objectUnderTest.start(); + + when(revReader.read(any(), anyString())).thenThrow(IOException.class); + + objectUnderTest.onGitReferenceUpdated(event); + verify(fetchRestApiClient).callFetch(any(), anyString(), any()); } @Test - public void shouldFallbackToCallFetchWhenIOException() - throws ClientProtocolException, IOException, RefUpdateException { - Event event = new TestEvent("refs/changes/01/1/meta"); + public void shouldFallbackToCallFetchWhenLargeRef() + throws ClientProtocolException, IOException, LargeObjectException, RefUpdateException { + Event event = new TestEvent("refs/changes/01/1/1"); objectUnderTest.start(); - when(revReader.read(any(), anyString())).thenThrow(IOException.class); + when(revReader.read(any(), anyString())).thenReturn(Optional.empty()); objectUnderTest.onGitReferenceUpdated(event);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/RevisionReaderIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/RevisionReaderIT.java index dc36021..ca410d5 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/RevisionReaderIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/RevisionReaderIT.java
@@ -34,6 +34,7 @@ import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionData; import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionObjectData; import com.googlesource.gerrit.plugins.replication.pull.fetch.ApplyObject; +import java.util.Optional; import org.eclipse.jgit.lib.Constants; import org.junit.Test; @@ -49,8 +50,10 @@ Result pushResult = createChange(); String refName = RefNames.changeMetaRef(pushResult.getChange().getId()); - RevisionData revisionData = objectUnderTest.read(project, refName); + Optional<RevisionData> revisionDataOption = objectUnderTest.read(project, refName); + assertThat(revisionDataOption.isPresent()).isTrue(); + RevisionData revisionData = revisionDataOption.get(); assertThat(revisionData).isNotNull(); assertThat(revisionData.getCommitObject()).isNotNull(); assertThat(revisionData.getCommitObject().getType()).isEqualTo(Constants.OBJ_COMMIT); @@ -75,8 +78,10 @@ reviewInput.comments = ImmutableMap.of(Patch.COMMIT_MSG, ImmutableList.of(comment)); gApi.changes().id(changeId.get()).current().review(reviewInput); - RevisionData revisionData = objectUnderTest.read(project, refName); + Optional<RevisionData> revisionDataOption = objectUnderTest.read(project, refName); + assertThat(revisionDataOption.isPresent()).isTrue(); + RevisionData revisionData = revisionDataOption.get(); assertThat(revisionData).isNotNull(); assertThat(revisionData.getCommitObject()).isNotNull(); assertThat(revisionData.getCommitObject().getType()).isEqualTo(Constants.OBJ_COMMIT);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObjectIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObjectIT.java index 05aa596..5d80d94 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObjectIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObjectIT.java
@@ -41,6 +41,7 @@ import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionObjectData; import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Repository; @@ -67,13 +68,14 @@ Result pushResult = createChange(); String refName = RefNames.changeMetaRef(pushResult.getChange().getId()); - RevisionData revisionData = reader.read(Project.nameKey(testRepoProjectName), refName); + Optional<RevisionData> revisionData = + reader.read(Project.nameKey(testRepoProjectName), refName); RefSpec refSpec = new RefSpec(refName); - objectUnderTest.apply(project, refSpec, revisionData); - RevisionData newRevisionData = reader.read(project, refName); + objectUnderTest.apply(project, refSpec, revisionData.get()); + Optional<RevisionData> newRevisionData = reader.read(project, refName); - compareObjects(revisionData, newRevisionData); + compareObjects(revisionData.get(), newRevisionData); try (Repository repo = repoManager.openRepository(project); TestRepository<Repository> testRepo = new TestRepository<>(repo); ) { @@ -91,22 +93,23 @@ String refName = RefNames.changeMetaRef(changeId); RefSpec refSpec = new RefSpec(refName); - RevisionData revisionData = reader.read(Project.nameKey(testRepoProjectName), refName); - objectUnderTest.apply(project, refSpec, revisionData); + Optional<RevisionData> revisionData = + reader.read(Project.nameKey(testRepoProjectName), refName); + objectUnderTest.apply(project, refSpec, revisionData.get()); ReviewInput reviewInput = new ReviewInput(); CommentInput comment = createCommentInput(1, 0, 1, 1, "Test comment"); reviewInput.comments = ImmutableMap.of(Patch.COMMIT_MSG, ImmutableList.of(comment)); gApi.changes().id(changeId.get()).current().review(reviewInput); - RevisionData revisionDataWithComment = + Optional<RevisionData> revisionDataWithComment = reader.read(Project.nameKey(testRepoProjectName), refName); - objectUnderTest.apply(project, refSpec, revisionDataWithComment); + objectUnderTest.apply(project, refSpec, revisionDataWithComment.get()); - RevisionData newRevisionData = reader.read(project, refName); + Optional<RevisionData> newRevisionData = reader.read(project, refName); - compareObjects(revisionDataWithComment, newRevisionData); + compareObjects(revisionDataWithComment.get(), newRevisionData); try (Repository repo = repoManager.openRepository(project); TestRepository<Repository> testRepo = new TestRepository<>(repo)) { @@ -124,20 +127,22 @@ String refName = RefNames.changeMetaRef(changeId); CommentInput comment = createCommentInput(1, 0, 1, 1, "Test comment"); - ReviewInput reviewInput = new ReviewInput(); reviewInput.comments = ImmutableMap.of(Patch.COMMIT_MSG, ImmutableList.of(comment)); gApi.changes().id(changeId.get()).current().review(reviewInput); - RevisionData revisionData = reader.read(Project.nameKey(testRepoProjectName), refName); + Optional<RevisionData> revisionData = + reader.read(Project.nameKey(testRepoProjectName), refName); RefSpec refSpec = new RefSpec(refName); assertThrows( MissingParentObjectException.class, - () -> objectUnderTest.apply(project, refSpec, revisionData)); + () -> objectUnderTest.apply(project, refSpec, revisionData.get())); } - private void compareObjects(RevisionData expected, RevisionData actual) { + private void compareObjects(RevisionData expected, Optional<RevisionData> actualOption) { + assertThat(actualOption.isPresent()).isTrue(); + RevisionData actual = actualOption.get(); compareContent(expected.getCommitObject(), actual.getCommitObject()); compareContent(expected.getTreeObject(), expected.getTreeObject()); List<List<Byte>> actualBlobs =