opendev/gerrit
Code Issues Proposed changes

Change kind cache: Don't fail if prior ps has no parent, but next ps has 1 parent

Browse Source 
This case failed with:

[2016-11-14 18:44:53,805] [ReceiveCommits-1] ERROR com.google.gerrit.server.git.ReceiveCommits : [com.google.gerrit.acceptance.api.change.ChangeIT_changeNoParentToOneParent_project-1479177893785-ed68f500]Can't insert change/patch set for com.google.gerrit.acceptance.api.change.ChangeIT_changeNoParentToOneParent_project
com.google.gerrit.extensions.restapi.RestApiException: Error inserting change/patchset
  at com.google.gerrit.server.git.ReceiveCommits$1.apply(ReceiveCommits.java:274)
  at com.google.gerrit.server.git.ReceiveCommits$1.apply(ReceiveCommits.java:1)
  at com.google.gerrit.server.git.ReceiveCommits.insertChangesAndPatchSets(ReceiveCommits.java:851)
  at com.google.gerrit.server.git.ReceiveCommits.processCommands(ReceiveCommits.java:605)
  at com.google.gerrit.server.git.AsyncReceiveCommits$Worker.run(AsyncReceiveCommits.java:89)
  at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
  at com.google.gerrit.server.util.RequestScopePropagator$5.call(RequestScopePropagator.java:221)
  at com.google.gerrit.server.util.RequestScopePropagator$4.call(RequestScopePropagator.java:200)
  at com.google.gerrit.server.util.ThreadLocalRequestScopePropagator$1.call(ThreadLocalRequestScopePropagator.java:55)
  at com.google.gerrit.server.util.RequestScopePropagator$1.call(RequestScopePropagator.java:99)
  at com.google.gerrit.server.util.RequestScopePropagator$2.run(RequestScopePropagator.java:131)
  at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
  at java.util.concurrent.FutureTask.run(FutureTask.java:266)
  at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
  at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
  at com.google.gerrit.server.git.WorkQueue$Task.run(WorkQueue.java:417)
  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
  at java.lang.Thread.run(Thread.java:745)
Caused by: com.google.gerrit.server.git.UpdateException: com.google.common.util.concurrent.UncheckedExecutionException: java.lang.ArrayIndexOutOfBoundsException: 0
  at com.google.gerrit.server.git.BatchUpdate.executeUpdateRepo(BatchUpdate.java:697)
  at com.google.gerrit.server.git.BatchUpdate.execute(BatchUpdate.java:421)
  at com.google.gerrit.server.git.BatchUpdate.execute(BatchUpdate.java:673)
  at com.google.gerrit.server.git.BatchUpdate.execute(BatchUpdate.java:668)
  at com.google.gerrit.server.git.ReceiveCommits.insertChangesAndPatchSets(ReceiveCommits.java:849)
  ... 16 more
Caused by: com.google.common.util.concurrent.UncheckedExecutionException: java.lang.ArrayIndexOutOfBoundsException: 0
  at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2214)
  at com.google.common.cache.LocalCache.get(LocalCache.java:4053)
  at com.google.common.cache.LocalCache$LocalManualCache.get(LocalCache.java:4899)
  at com.google.gerrit.server.change.ChangeKindCacheImpl.getChangeKind(ChangeKindCacheImpl.java:338)
  at com.google.gerrit.server.git.ReplaceOp.updateRepo(ReplaceOp.java:194)
  at com.google.gerrit.server.git.BatchUpdate.executeUpdateRepo(BatchUpdate.java:681)
  ... 20 more
Caused by: java.lang.ArrayIndexOutOfBoundsException: 0
  at org.eclipse.jgit.revwalk.RevCommit.getParent(RevCommit.java:353)
  at com.google.gerrit.server.change.ChangeKindCacheImpl$Loader.call(ChangeKindCacheImpl.java:245)
  at com.google.gerrit.server.change.ChangeKindCacheImpl$Loader.call(ChangeKindCacheImpl.java:1)
  at com.google.common.cache.LocalCache$LocalManualCache$1.load(LocalCache.java:4904)
  at com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache.java:3628)
  at com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2336)
  at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2295)
  at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2208)
  ... 25 more

Change-Id: I689b703d4912d54bfc659def577b689fc1cda887
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-11-14 18:43:53 -08:00
committed by Paladox
parent f652f1f043
commit d89b703131
2 changed files with 48 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@@ -16,6 +16,8 @@ package com.google.gerrit.acceptance.api.change;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.acceptance.GitUtil.assertPushOk;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
import static com.google.gerrit.extensions.client.ReviewerState.CC;
@@ -50,6 +52,7 @@ import com.google.gerrit.extensions.api.changes.RebaseInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.SubmitType;
@@ -88,6 +91,7 @@ import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.PushResult;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -537,6 +541,48 @@ public class ChangeIT extends AbstractDaemonTest {
.rebase(ri);
}
@Test
@TestProjectInput(createEmptyCommit = false)
public void changeNoParentToOneParent() throws Exception {
// create initial commit with no parent and push it as change, so that patch
// set 1 has no parent
RevCommit c =
testRepo.commit().message("Initial commit").insertChangeId().create();
String id = GitUtil.getChangeId(testRepo, c).get();
testRepo.reset(c);
PushResult pr = pushHead(testRepo, "refs/for/master", false);
assertPushOk(pr, "refs/for/master");
ChangeInfo change = gApi.changes().id(id).get();
assertThat(change.revisions.get(change.currentRevision).commit.parents)
.isEmpty();
// create another initial commit with no parent and push it directly into
// the remote repository
c = testRepo.amend(c.getId()).message("Initial Empty Commit").create();
testRepo.reset(c);
pr = pushHead(testRepo, "refs/heads/master", false);
assertPushOk(pr, "refs/heads/master");
// create a successor commit and push it as second patch set to the change,
// so that patch set 2 has 1 parent
RevCommit c2 = testRepo.commit().message("Initial commit").parent(c)
.insertChangeId(id.substring(1)).create();
testRepo.reset(c2);
pr = pushHead(testRepo, "refs/for/master", false);
assertPushOk(pr, "refs/for/master");
change = gApi.changes().id(id).get();
RevisionInfo rev = change.revisions.get(change.currentRevision);
assertThat(rev.commit.parents).hasSize(1);
assertThat(rev.commit.parents.get(0).commit).isEqualTo(c.name());
// check that change kind is correctly detected as REWORK
assertThat(rev.kind).isEqualTo(ChangeKind.REWORK);
}
@Test
public void addReviewerThatCannotSeeChange() throws Exception {
// create hidden project that is only visible to administrators

3
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
View File

@@ -222,7 +222,8 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
}
if ((prior.getParentCount() != 1 || next.getParentCount() != 1)
&& !onlyFirstParentChanged(prior, next)) {
&& (!onlyFirstParentChanged(prior, next)
|| prior.getParentCount() == 0)) {
// Trivial rebases done by machine only work well on 1 parent.
return ChangeKind.REWORK;
}