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 =
