Merge changes I64000a57,Icc7c7586

* changes:
  ChangeIndexer: Disable auto-rebuilding changes
  ChangeNotes: Log when recheckUpToDate finds it wasn't
This commit is contained in:
Dave Borowitz
2016-06-06 19:19:38 +00:00
committed by Gerrit Code Review
7 changed files with 40 additions and 7 deletions

View File

@@ -420,7 +420,7 @@ class ChangeApiImpl implements ChangeApi {
public void index() throws RestApiException { public void index() throws RestApiException {
try { try {
index.apply(change, new Index.Input()); index.apply(change, new Index.Input());
} catch (IOException e) { } catch (IOException | OrmException e) {
throw new RestApiException("Cannot index change", e); throw new RestApiException("Cannot index change", e);
} }
} }

View File

@@ -21,6 +21,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.Index.Input; import com.google.gerrit.server.change.Index.Input;
import com.google.gerrit.server.index.change.ChangeIndexer; import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
@@ -44,7 +45,7 @@ public class Index implements RestModifyView<ChangeResource, Input> {
@Override @Override
public Response<?> apply(ChangeResource rsrc, Input input) public Response<?> apply(ChangeResource rsrc, Input input)
throws IOException, AuthException { throws IOException, AuthException, OrmException {
ChangeControl ctl = rsrc.getControl(); ChangeControl ctl = rsrc.getControl();
if (!ctl.isOwner() if (!ctl.isOwner()
&& !ctl.getUser().getCapabilities().canMaintainServer()) { && !ctl.getUser().getCapabilities().canMaintainServer()) {

View File

@@ -36,6 +36,7 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeIndexer; import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
@@ -119,9 +120,10 @@ public class ChangeEditModifier {
* @throws IOException * @throws IOException
* @throws ResourceConflictException When change edit already * @throws ResourceConflictException When change edit already
* exists for the change * exists for the change
* @throws OrmException
*/ */
public RefUpdate.Result createEdit(Change change, PatchSet ps) public RefUpdate.Result createEdit(Change change, PatchSet ps)
throws AuthException, IOException, ResourceConflictException { throws AuthException, IOException, ResourceConflictException, OrmException {
if (!currentUser.get().isIdentifiedUser()) { if (!currentUser.get().isIdentifiedUser()) {
throw new AuthException("Authentication required"); throw new AuthException("Authentication required");
} }

View File

@@ -200,9 +200,10 @@ public class ChangeEditUtil {
* *
* @param edit change edit to delete * @param edit change edit to delete
* @throws IOException * @throws IOException
* @throws OrmException
*/ */
public void delete(ChangeEdit edit) public void delete(ChangeEdit edit)
throws IOException { throws IOException, OrmException {
Change change = edit.getChange(); Change change = edit.getChange();
try (Repository repo = gitManager.openRepository(change.getProject())) { try (Repository repo = gitManager.openRepository(change.getProject())) {
deleteRef(repo, edit); deleteRef(repo, edit);

View File

@@ -27,6 +27,8 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.extensions.events.ChangeIndexedListener; import com.google.gerrit.server.extensions.events.ChangeIndexedListener;
import com.google.gerrit.server.index.Index; import com.google.gerrit.server.index.Index;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.util.RequestContext; import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext; import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -94,6 +96,8 @@ public class ChangeIndexer {
private final ChangeIndexCollection indexes; private final ChangeIndexCollection indexes;
private final ChangeIndex index; private final ChangeIndex index;
private final SchemaFactory<ReviewDb> schemaFactory; private final SchemaFactory<ReviewDb> schemaFactory;
private final NotesMigration notesMigration;
private final ChangeNotes.Factory changeNotesFactory;
private final ChangeData.Factory changeDataFactory; private final ChangeData.Factory changeDataFactory;
private final ThreadLocalRequestContext context; private final ThreadLocalRequestContext context;
private final ListeningExecutorService executor; private final ListeningExecutorService executor;
@@ -101,6 +105,8 @@ public class ChangeIndexer {
@AssistedInject @AssistedInject
ChangeIndexer(SchemaFactory<ReviewDb> schemaFactory, ChangeIndexer(SchemaFactory<ReviewDb> schemaFactory,
NotesMigration notesMigration,
ChangeNotes.Factory changeNotesFactory,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
ThreadLocalRequestContext context, ThreadLocalRequestContext context,
DynamicSet<ChangeIndexedListener> indexedListener, DynamicSet<ChangeIndexedListener> indexedListener,
@@ -108,6 +114,8 @@ public class ChangeIndexer {
@Assisted ChangeIndex index) { @Assisted ChangeIndex index) {
this.executor = executor; this.executor = executor;
this.schemaFactory = schemaFactory; this.schemaFactory = schemaFactory;
this.notesMigration = notesMigration;
this.changeNotesFactory = changeNotesFactory;
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.context = context; this.context = context;
this.index = index; this.index = index;
@@ -117,6 +125,8 @@ public class ChangeIndexer {
@AssistedInject @AssistedInject
ChangeIndexer(SchemaFactory<ReviewDb> schemaFactory, ChangeIndexer(SchemaFactory<ReviewDb> schemaFactory,
NotesMigration notesMigration,
ChangeNotes.Factory changeNotesFactory,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
ThreadLocalRequestContext context, ThreadLocalRequestContext context,
DynamicSet<ChangeIndexedListener> indexedListener, DynamicSet<ChangeIndexedListener> indexedListener,
@@ -124,6 +134,8 @@ public class ChangeIndexer {
@Assisted ChangeIndexCollection indexes) { @Assisted ChangeIndexCollection indexes) {
this.executor = executor; this.executor = executor;
this.schemaFactory = schemaFactory; this.schemaFactory = schemaFactory;
this.notesMigration = notesMigration;
this.changeNotesFactory = changeNotesFactory;
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.context = context; this.context = context;
this.index = null; this.index = null;
@@ -189,8 +201,23 @@ public class ChangeIndexer {
* @param db review database. * @param db review database.
* @param change change to index. * @param change change to index.
*/ */
public void index(ReviewDb db, Change change) throws IOException { public void index(ReviewDb db, Change change)
index(changeDataFactory.create(db, change)); throws IOException, OrmException {
ChangeData cd;
if (notesMigration.readChanges()) {
cd = changeDataFactory.create(db, change);
} else if (notesMigration.writeChanges()) {
// Auto-rebuilding when NoteDb reads are disabled just increases
// contention on the meta ref from a background indexing thread with
// little benefit. The next actual write to the entity may still incur a
// less-contentious rebuild.
ChangeNotes notes =
changeNotesFactory.createWithAutoRebuildingDisabled(change, null);
cd = changeDataFactory.create(db, notes);
} else {
cd = changeDataFactory.create(db, change);
}
index(cd);
} }
/** /**

View File

@@ -621,6 +621,8 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
throw e; throw e;
} }
if (!upToDate) { if (!upToDate) {
log.warn("Rechecked change {} after a rebuild error, but it was not up to"
+ " date; rethrowing exception", getChangeId());
throw e; throw e;
} }
change = new Change(newChange); change = new Change(newChange);

View File

@@ -58,7 +58,7 @@ final class IndexChangesCommand extends SshCommand {
for (ChangeResource rsrc : changes.values()) { for (ChangeResource rsrc : changes.values()) {
try { try {
index.apply(rsrc, new Index.Input()); index.apply(rsrc, new Index.Input());
} catch (IOException | RestApiException e) { } catch (IOException | RestApiException | OrmException e) {
ok = false; ok = false;
writeError("error", String.format( writeError("error", String.format(
"failed to index change %s: %s", rsrc.getId(), e.getMessage())); "failed to index change %s: %s", rsrc.getId(), e.getMessage()));