From a331568d38ed469566c810ead6785247ae9e6ec9 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Sat, 2 Oct 2021 00:10:21 +0100 Subject: Load an arbitrary version of change notes The SHA1 of the change notes to load may be already known to the caller of ChangeNotes.create() method hence it can be passed as a version parameter to avoid an unneeded ref lookup operation. When the version passed isn't known, it can be given as null parameter causing the lookup of the latest /meta ref SHA1 from the repository's ref-database. Allow skipping potentially hundreds of thousands of refs lookups for large mono-repos when filtering refs from the repository based on the change status. Also document the different variants of ChangeNotes.create() methods, highlighting their specific purpose and use-case. This is a partial cherry-pick of Ia54acb37e1187. Original-Author: Han-Wen Nienhuys Change-Id: I359578a05ecad595fb64909398ea3fd069ff1a5f --- .../gerrit/server/notedb/AbstractChangeNotes.java | 16 ++++++----- .../google/gerrit/server/notedb/ChangeNotes.java | 33 ++++++++++++++++++---- .../gerrit/server/notedb/DraftCommentNotes.java | 2 +- .../gerrit/server/notedb/RobotCommentNotes.java | 2 +- 4 files changed, 39 insertions(+), 14 deletions(-) diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java index a5acbe3ee3..e81160abaa 100644 --- a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java +++ b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java @@ -118,9 +118,10 @@ public abstract class AbstractChangeNotes { private ObjectId revision; private boolean loaded; - protected AbstractChangeNotes(Args args, Change.Id changeId) { + protected AbstractChangeNotes(Args args, Change.Id changeId, @Nullable ObjectId metaSha1) { this.args = requireNonNull(args); this.changeId = requireNonNull(changeId); + this.revision = metaSha1; } public Change.Id getChangeId() { @@ -152,7 +153,7 @@ public abstract class AbstractChangeNotes { try (Timer0.Context timer = args.metrics.readLatency.start(); // Call openHandle even if reading is disabled, to trigger // auto-rebuilding before this object may get passed to a ChangeUpdate. - LoadHandle handle = openHandle(repo)) { + LoadHandle handle = openHandle(repo, revision)) { revision = handle.id(); onLoad(handle); loaded = true; @@ -174,15 +175,16 @@ public abstract class AbstractChangeNotes { *

Implementations may override this method to provide auto-rebuilding behavior. * * @param repo open repository. + * @param id version SHA1 of the change notes to load * @return handle for reading the entity. * @throws NoSuchChangeException change does not exist. * @throws IOException a repo-level error occurred. */ - protected LoadHandle openHandle(Repository repo) throws NoSuchChangeException, IOException { - return openHandle(repo, readRef(repo)); - } - - protected LoadHandle openHandle(Repository repo, ObjectId id) { + protected LoadHandle openHandle(Repository repo, @Nullable ObjectId id) + throws NoSuchChangeException, IOException { + if (id == null) { + id = readRef(repo); + } return new LoadHandle(repo, id); } diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java index 95095686e3..8eb5c5cdaa 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -110,9 +110,23 @@ public class ChangeNotes extends AbstractChangeNotes { return createChecked(c.getProject(), c.getId()); } - public ChangeNotes createChecked(Project.NameKey project, Change.Id changeId) { + public ChangeNotes createChecked( + Repository repo, + Project.NameKey project, + Change.Id changeId, + @Nullable ObjectId metaRevId) { Change change = newChange(project, changeId); - return new ChangeNotes(args, change, true, null).load(); + return new ChangeNotes(args, change, true, null, metaRevId).load(repo); + } + + public ChangeNotes createChecked( + Project.NameKey project, Change.Id changeId, @Nullable ObjectId metaRevId) { + Change change = newChange(project, changeId); + return new ChangeNotes(args, change, true, null, metaRevId).load(); + } + + public ChangeNotes createChecked(Project.NameKey project, Change.Id changeId) { + return createChecked(project, changeId, null); } public static Change newChange(Project.NameKey project, Change.Id changeId) { @@ -344,14 +358,23 @@ public class ChangeNotes extends AbstractChangeNotes { private ImmutableListMultimap approvals; private ImmutableSet commentKeys; - @VisibleForTesting - public ChangeNotes(Args args, Change change, boolean shouldExist, @Nullable RefCache refs) { - super(args, change.getId()); + public ChangeNotes( + Args args, + Change change, + boolean shouldExist, + @Nullable RefCache refs, + @Nullable ObjectId metaSha1) { + super(args, change.getId(), metaSha1); this.change = new Change(change); this.shouldExist = shouldExist; this.refs = refs; } + @VisibleForTesting + public ChangeNotes(Args args, Change change, boolean shouldExist, @Nullable RefCache refs) { + this(args, change, shouldExist, refs, null); + } + public Change getChange() { return change; } diff --git a/java/com/google/gerrit/server/notedb/DraftCommentNotes.java b/java/com/google/gerrit/server/notedb/DraftCommentNotes.java index 9b403e8ce4..49884068a1 100644 --- a/java/com/google/gerrit/server/notedb/DraftCommentNotes.java +++ b/java/com/google/gerrit/server/notedb/DraftCommentNotes.java @@ -59,7 +59,7 @@ public class DraftCommentNotes extends AbstractChangeNotes { } DraftCommentNotes(Args args, Change.Id changeId, Account.Id author, @Nullable Ref ref) { - super(args, changeId); + super(args, changeId, null); this.author = requireNonNull(author); this.ref = ref; if (ref != null) { diff --git a/java/com/google/gerrit/server/notedb/RobotCommentNotes.java b/java/com/google/gerrit/server/notedb/RobotCommentNotes.java index fe0564356e..d53b2caaf6 100644 --- a/java/com/google/gerrit/server/notedb/RobotCommentNotes.java +++ b/java/com/google/gerrit/server/notedb/RobotCommentNotes.java @@ -47,7 +47,7 @@ public class RobotCommentNotes extends AbstractChangeNotes { @Inject RobotCommentNotes(Args args, @Assisted Change change) { - super(args, change.getId()); + super(args, change.getId(), null); this.change = change; } -- cgit v1.2.3