From 31cee3af709cdd8c76b09f9ba766d8941d589180 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 5 May 2016 17:18:23 -0400 Subject: [PATCH] ChangeNotes: Don't auto-rebuild if both state and ref are missing This is the normal state when only writing NoteDb changes but not reading. The whole point of the optimization in Ic8c6c28e was to keep old changes in this state until an explicit rebuild. Change-Id: I9353bcd36ead4f4f7350884c795b0bad9c0da572 --- .../gerrit/server/notedb/ChangeNotes.java | 2 +- .../server/notedb/DraftCommentNotes.java | 4 ++-- .../server/notedb/NoteDbChangeState.java | 19 +++++++++++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java index 4dd211b4cb..ddf09ccbbc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -627,7 +627,7 @@ public class ChangeNotes extends AbstractChangeNotes { if (autoRebuild) { NoteDbChangeState state = NoteDbChangeState.parse(change); RefCache refs = this.refs != null ? this.refs : new RepoRefCache(repo); - if (state == null || !state.isChangeUpToDate(refs)) { + if (!NoteDbChangeState.isChangeUpToDate(state, refs, getChangeId())) { return rebuildAndOpen(repo); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java index e887ff121d..068c008e5c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java @@ -153,8 +153,8 @@ public class DraftCommentNotes extends AbstractChangeNotes { NoteDbChangeState state = NoteDbChangeState.parse(change); // Only check if this particular user's drafts are up to date, to avoid // reading unnecessary refs. - if (state == null - || !state.areDraftsUpToDate(new RepoRefCache(repo), author)) { + if (!NoteDbChangeState.areDraftsUpToDate( + state, new RepoRefCache(repo), getChangeId(), author)) { return rebuildAndOpen(repo); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java index f5d1178810..c08bdd88cf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java @@ -26,6 +26,7 @@ import com.google.common.base.Splitter; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.gerrit.common.Nullable; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.server.ReviewDbUtil; @@ -140,6 +141,24 @@ public class NoteDbChangeState { return state; } + public static boolean isChangeUpToDate(@Nullable NoteDbChangeState state, + RefCache changeRepoRefs, Change.Id changeId) throws IOException { + if (state == null) { + return !changeRepoRefs.get(changeMetaRef(changeId)).isPresent(); + } + return state.isChangeUpToDate(changeRepoRefs); + } + + public static boolean areDraftsUpToDate(@Nullable NoteDbChangeState state, + RefCache draftsRepoRefs, Change.Id changeId, Account.Id accountId) + throws IOException { + if (state == null) { + return !draftsRepoRefs.get(refsDraftComments(changeId, accountId)) + .isPresent(); + } + return state.areDraftsUpToDate(draftsRepoRefs, accountId); + } + public static String toString(ObjectId changeMetaId, Map draftIds) { List accountIds = Lists.newArrayList(draftIds.keySet());