diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java index 764d41dc9e..a4e8513fba 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -418,6 +418,10 @@ public class ChangeNotes extends AbstractChangeNotes { return commentKeys; } + public int getUpdateCount() { + return state.updateCount(); + } + public ImmutableListMultimap getDraftComments(Account.Id author) { return getDraftComments(author, null); } diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java index e2af855f45..1e48907bc0 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java @@ -112,8 +112,11 @@ public class ChangeNotesCache { // Single pointer overhead. private static final int P = 8; + // Single int overhead. + private static final int I = 4; + // Single IntKey overhead. - private static final int K = O + 4; + private static final int K = O + I; // Single Timestamp overhead. private static final int T = O + 8; @@ -173,7 +176,8 @@ public class ChangeNotesCache { + map(state.publishedComments().asMap(), comment()) + 1 // isPrivate + 1 // workInProgress - + 1; // reviewStarted + + 1 // reviewStarted + + I; // updateCount } private static int ptr(Object o, int size) { diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java index 7ba5679fcf..1e7005b13f 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java @@ -166,6 +166,7 @@ class ChangeNotesParser { private ReviewerSet pendingReviewers; private ReviewerByEmailSet pendingReviewersByEmail; private Change.Id revertOf; + private int updateCount; ChangeNotesParser( Change.Id changeId, @@ -264,7 +265,8 @@ class ChangeNotesParser { firstNonNull(isPrivate, false), firstNonNull(workInProgress, false), firstNonNull(hasReviewStarted, true), - revertOf); + revertOf, + updateCount); } private PatchSet.Id buildCurrentPatchSetId() { @@ -311,6 +313,7 @@ class ChangeNotesParser { } private void parse(ChangeNotesCommit commit) throws ConfigInvalidException { + updateCount++; Timestamp ts = new Timestamp(commit.getCommitterIdent().getWhen().getTime()); createdOn = ts; diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java index fd260e7449..3a983be12f 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java @@ -121,7 +121,8 @@ public abstract class ChangeNotesState { boolean isPrivate, boolean workInProgress, boolean reviewStarted, - @Nullable Change.Id revertOf) { + @Nullable Change.Id revertOf, + int updateCount) { requireNonNull( metaId, () -> @@ -165,6 +166,7 @@ public abstract class ChangeNotesState { .submitRecords(submitRecords) .changeMessages(changeMessages) .publishedComments(publishedComments) + .updateCount(updateCount) .build(); } @@ -297,6 +299,8 @@ public abstract class ChangeNotesState { abstract ImmutableListMultimap publishedComments(); + abstract int updateCount(); + Change newChange(Project.NameKey project) { ChangeColumns c = requireNonNull(columns(), "columns are required"); Change change = @@ -363,7 +367,8 @@ public abstract class ChangeNotesState { .reviewerUpdates(ImmutableList.of()) .submitRecords(ImmutableList.of()) .changeMessages(ImmutableList.of()) - .publishedComments(ImmutableListMultimap.of()); + .publishedComments(ImmutableListMultimap.of()) + .updateCount(0); } abstract Builder metaId(ObjectId metaId); @@ -398,6 +403,8 @@ public abstract class ChangeNotesState { abstract Builder publishedComments(ListMultimap publishedComments); + abstract Builder updateCount(int updateCount); + abstract ChangeNotesState build(); } @@ -459,6 +466,7 @@ public abstract class ChangeNotesState { .changeMessages() .forEach(m -> b.addChangeMessage(toByteString(m, ChangeMessageProtoConverter.INSTANCE))); object.publishedComments().values().forEach(c -> b.addPublishedComment(GSON.toJson(c))); + b.setUpdateCount(object.updateCount()); return Protos.toByteArray(b.build()); } @@ -577,7 +585,8 @@ public abstract class ChangeNotesState { .publishedComments( proto.getPublishedCommentList().stream() .map(r -> GSON.fromJson(r, Comment.class)) - .collect(toImmutableListMultimap(c -> new RevId(c.revId), c -> c))); + .collect(toImmutableListMultimap(c -> new RevId(c.revId), c -> c))) + .updateCount(proto.getUpdateCount()); return b.build(); } diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java index 2931b1706c..93c00d7c50 100644 --- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java +++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java @@ -703,6 +703,18 @@ public class ChangeNotesStateTest extends GerritBaseTests { .build()); } + @Test + public void serializeUpdateCount() throws Exception { + assertRoundTrip( + newBuilder().updateCount(234).build(), + ChangeNotesStateProto.newBuilder() + .setMetaId(SHA_BYTES) + .setChangeId(ID.get()) + .setColumns(colsProto) + .setUpdateCount(234) + .build()); + } + @Test public void changeNotesStateMethods() throws Exception { assertThatSerializedClass(ChangeNotesState.class) @@ -733,6 +745,7 @@ public class ChangeNotesStateTest extends GerritBaseTests { .put( "publishedComments", new TypeLiteral>() {}.getType()) + .put("updateCount", int.class) .build()); } diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java index a6c0224465..a57a984d0e 100644 --- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -3021,6 +3021,22 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { update.commit(); } + @Test + public void updateCount() throws Exception { + Change c = newChange(); + assertThat(newNotes(c).getUpdateCount()).isEqualTo(1); + + ChangeUpdate update = newUpdate(c, changeOwner); + update.putApproval("Code-Review", (short) -1); + update.commit(); + assertThat(newNotes(c).getUpdateCount()).isEqualTo(2); + + update = newUpdate(c, changeOwner); + update.putApproval("Code-Review", (short) 1); + update.commit(); + assertThat(newNotes(c).getUpdateCount()).isEqualTo(3); + } + private String readNote(ChangeNotes notes, ObjectId noteId) throws Exception { ObjectId dataId = notes.revisionNoteMap.noteMap.getNote(noteId).getData(); return new String(rw.getObjectReader().open(dataId, OBJ_BLOB).getCachedBytes(), UTF_8); diff --git a/proto/cache.proto b/proto/cache.proto index b34dbf3b86..77b6908b0d 100644 --- a/proto/cache.proto +++ b/proto/cache.proto @@ -75,7 +75,7 @@ message ChangeNotesKeyProto { // Instead, we just take the tedious yet simple approach of having a "has_foo" // field for each nullable field "foo", indicating whether or not foo is null. // -// Next ID: 19 +// Next ID: 20 message ChangeNotesStateProto { // Effectively required, even though the corresponding ChangeNotesState field // is optional, since the field is only absent when NoteDb is disabled, in @@ -183,6 +183,9 @@ message ChangeNotesStateProto { reserved 17; // read_only_until reserved 18; // has_read_only_until + + // Number of updates to the change's meta ref. + int32 update_count = 19; }