opendev/gerrit
Code Issues Proposed changes

CommentJsonMigrator: Improve error handling if comment note is too large

Browse Source 
Schema 169 migrates comments in NoteDb to JSON. To check whether
comments need to be migrated the note blob with the comments needs to be
loaded. Loading the note blob failed if the blob was larger than 25 MB
and the schema migration failed with LargeObjectException. The 25 MB
limit was hard-coded. This change removes this hard-coded limit.
Loading the blob could still fail with LargeObjectException if the
object is too large to fit into a byte array. To prevent this we check
whether the object is too large before attempting to load it. If it's
too large we fail with an IOException. This is better than failing with
a LargeObjectException since it results in a proper log that says for
which change the problem occurred. After logging the IOException it is
ignored and the migration is attempted despite of it. It's not unlikely
that the migration still fails for note blobs that are larger than 25 MB
because the 25 MB limit for note blobs is also checked in other places
(e.g. in RevisionNote#parse()), but at least this change improves the
error reporting and should reveal if there are other places during the
migration that have a hard-coded limit.

Change-Id: Idaef9a65b7fcbd9e593ffbd7c097cbffd105a916
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-12-27 14:29:32 +01:00
parent b60e7643dc
commit cb2ba3e7f9
1 changed files with 6 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
View File

@@ -15,7 +15,6 @@
package com.google.gerrit.server.notedb;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.server.notedb.RevisionNote.MAX_NOTE_SZ;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import com.google.common.collect.ImmutableList;
@@ -39,6 +38,7 @@ import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectLoader;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -220,7 +220,11 @@ public class CommentJsonMigrator {
NoteMap noteMap = NoteMap.read(reader, c);
for (Note note : noteMap) {
// Match pre-parsing logic in RevisionNote#parse().
byte[] raw = reader.open(note.getData(), OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
ObjectLoader objectLoader = reader.open(note.getData(), OBJ_BLOB);
if (objectLoader.isLarge()) {
throw new IOException(String.format("Comment note %s is too large", note.name()));
}
byte[] raw = objectLoader.getCachedBytes();
MutableInteger p = new MutableInteger();
RevisionNote.trimLeadingEmptyLines(raw, p);
if (!ChangeRevisionNote.isJson(raw, p.value)) {