opendev/gerrit
Code Issues Proposed changes

Fix running git-upload-archive command for Gerrit slaves

Browse Source 
Running the git-upload-archive command against Gerrit slaves fails with
a NullPointerException because it tries to access the change index which
is not available on Gerrit slaves.

Querying for visible changes to check if a commit is readable is an
optimization that was added by commit 50c8b63f67 [2] (the corresponding
change seems to be missing in the change index and hence I can't find
it).

Since querying for visible changes is an optimization only we can simply
skip it on slaves where a change index is not available and fall back
to checking the visibility of refs from which the commit is reachable.

[1]
[2018-01-22 13:16:45,059] [SSH git-upload-archive '/platform/packages/apps/Browser2' (xxx)] ERROR com.google.gerrit.sshd.BaseCommand : Internal server error (userxxx account 1000002) during git-upload-archive '/platform/packages/apps/Browser2'
java.lang.NullPointerException
        at com.google.gerrit.server.index.change.ChangeIndexRewriter.isIndexPredicate(ChangeIndexRewriter.java:245)
        at com.google.gerrit.server.index.change.ChangeIndexRewriter.rewriteImpl(ChangeIndexRewriter.java:183)
        at com.google.gerrit.server.index.change.ChangeIndexRewriter.rewriteImpl(ChangeIndexRewriter.java:208)
        at com.google.gerrit.server.index.change.ChangeIndexRewriter.rewriteImpl(ChangeIndexRewriter.java:156)
        at com.google.gerrit.server.index.change.ChangeIndexRewriter.rewrite(ChangeIndexRewriter.java:140)
        at com.google.gerrit.index.query.QueryProcessor.query(QueryProcessor.java:224)
        at com.google.gerrit.index.query.QueryProcessor.query(QueryProcessor.java:174)
        at com.google.gerrit.index.query.QueryProcessor.query(QueryProcessor.java:157)
        at com.google.gerrit.index.query.InternalQuery.query(InternalQuery.java:74)
        at com.google.gerrit.server.query.change.InternalChangeQuery.byProjectCommit(InternalChangeQuery.java:250)
        at com.google.gerrit.server.query.change.InternalChangeQuery.byProjectCommit(InternalChangeQuery.java:245)
        at com.google.gerrit.server.project.CommitsCollection.canRead(CommitsCollection.java:109)
        at com.google.gerrit.sshd.commands.UploadArchive.canRead(UploadArchive.java:253)
        at com.google.gerrit.sshd.commands.UploadArchive.runImpl(UploadArchive.java:188)
        at com.google.gerrit.sshd.AbstractGitCommand.service(AbstractGitCommand.java:102)
        at com.google.gerrit.sshd.AbstractGitCommand.access$000(AbstractGitCommand.java:32)
        at com.google.gerrit.sshd.AbstractGitCommand$1.run(AbstractGitCommand.java:67)
        at com.google.gerrit.sshd.BaseCommand$TaskThunk.run(BaseCommand.java:427)
        ...

[2] https://gerrit.googlesource.com/gerrit/+/50c8b63f6721d8aba6074b238664f3eee7a2dc64%5E%21/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java

Change-Id: Ic27ee3ccc5c9887a6a460c89fa29cb128b4c7af4
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-01-22 08:38:45 +01:00
parent fc6ec5a316
commit 418c240713
1 changed files with 13 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java
View File

@@ -23,6 +23,7 @@ import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.change.IncludedInResolver;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.OrmException;
@@ -49,6 +50,7 @@ public class CommitsCollection implements ChildCollection<ProjectResource, Commi
private final DynamicMap<RestView<CommitResource>> views;
private final GitRepositoryManager repoManager;
private final VisibleRefFilter.Factory refFilter;
private final ChangeIndexCollection indexes;
private final Provider<InternalChangeQuery> queryProvider;
@Inject
@@ -56,10 +58,12 @@ public class CommitsCollection implements ChildCollection<ProjectResource, Commi
DynamicMap<RestView<CommitResource>> views,
GitRepositoryManager repoManager,
VisibleRefFilter.Factory refFilter,
ChangeIndexCollection indexes,
Provider<InternalChangeQuery> queryProvider) {
this.views = views;
this.repoManager = repoManager;
this.refFilter = refFilter;
this.indexes = indexes;
this.queryProvider = queryProvider;
}
@@ -104,14 +108,16 @@ public class CommitsCollection implements ChildCollection<ProjectResource, Commi
Project.NameKey project = state.getNameKey();
// Look for changes associated with the commit.
try {
List<ChangeData> changes =
queryProvider.get().enforceVisibility(true).byProjectCommit(project, commit);
if (!changes.isEmpty()) {
return true;
if (indexes.getSearchIndex() != null) {
try {
List<ChangeData> changes =
queryProvider.get().enforceVisibility(true).byProjectCommit(project, commit);
if (!changes.isEmpty()) {
return true;
}
} catch (OrmException e) {
log.error("Cannot look up change for commit " + commit.name() + " in " + project, e);
}
} catch (OrmException e) {
log.error("Cannot look up change for commit " + commit.name() + " in " + project, e);
}
return isReachableFrom(state, repo, commit, repo.getAllRefs());