Merge changes I189f6dad,I7a17a318,I8ba34146
* changes:
Allow persistent caches to be not persisted by default
Minor improvements to ProtoCacheSerializers#toByteArray
Align ChangeColumns methods with Change fields
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index ce7adc2..174043d 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -776,7 +776,7 @@
* `"diff_summary"`: default is `1g` (1 GiB of disk space)
+
-If 0, disk storage for the cache is disabled.
+If 0 or negative, disk storage for the cache is disabled.
==== [[cache_names]]Standard Caches
diff --git a/java/com/google/gerrit/server/cache/PersistentCacheBinding.java b/java/com/google/gerrit/server/cache/PersistentCacheBinding.java
index 429f5ab..794d3bb 100644
--- a/java/com/google/gerrit/server/cache/PersistentCacheBinding.java
+++ b/java/com/google/gerrit/server/cache/PersistentCacheBinding.java
@@ -34,7 +34,12 @@
PersistentCacheBinding<K, V> version(int version);
- /** Set the total on-disk limit of the cache */
+ /**
+ * Set the total on-disk limit of the cache.
+ *
+ * <p>If 0 or negative, persistence for the cache is disabled by default, but may still be
+ * overridden in the config.
+ */
PersistentCacheBinding<K, V> diskLimit(long limit);
PersistentCacheBinding<K, V> keySerializer(CacheSerializer<K> keySerializer);
diff --git a/java/com/google/gerrit/server/cache/PersistentCacheProvider.java b/java/com/google/gerrit/server/cache/PersistentCacheProvider.java
index 405de4f..46a9e61 100644
--- a/java/com/google/gerrit/server/cache/PersistentCacheProvider.java
+++ b/java/com/google/gerrit/server/cache/PersistentCacheProvider.java
@@ -39,6 +39,7 @@
CacheModule module, String name, TypeLiteral<K> keyType, TypeLiteral<V> valType) {
super(module, name, keyType, valType);
version = -1;
+ diskLimit = 128 << 20;
}
@Inject(optional = true)
@@ -93,10 +94,7 @@
@Override
public long diskLimit() {
- if (diskLimit > 0) {
- return diskLimit;
- }
- return 128 << 20;
+ return diskLimit;
}
@Override
diff --git a/java/com/google/gerrit/server/cache/ProtoCacheSerializers.java b/java/com/google/gerrit/server/cache/ProtoCacheSerializers.java
index 795df72..a8e7e5f 100644
--- a/java/com/google/gerrit/server/cache/ProtoCacheSerializers.java
+++ b/java/com/google/gerrit/server/cache/ProtoCacheSerializers.java
@@ -23,8 +23,8 @@
/**
* Serializes a proto to a byte array.
*
- * <p>Guarantees deterministic serialization and thus is suitable for use as a persistent cache
- * key. Should be used in preference to {@link MessageLite#toByteArray()}, which is not guaranteed
+ * <p>Guarantees deterministic serialization and thus is suitable for use in persistent caches.
+ * Should be used in preference to {@link MessageLite#toByteArray()}, which is not guaranteed
* deterministic.
*
* @param message the proto message to serialize.
@@ -39,7 +39,7 @@
cout.checkNoSpaceLeft();
return bytes;
} catch (IOException e) {
- throw new IllegalStateException("exception writing to byte array");
+ throw new IllegalStateException("exception writing to byte array", e);
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 5658569..853ed69 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -134,7 +134,7 @@
+ T // readOnlyUntil
+ 1 // isPrivate
+ 1 // workInProgress
- + 1; // hasReviewStarted
+ + 1; // reviewStarted
}
private static int ptr(Object o, int size) {
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index 78734f9..ab7e580 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -95,7 +95,7 @@
@Nullable Timestamp readOnlyUntil,
boolean isPrivate,
boolean workInProgress,
- boolean hasReviewStarted,
+ boolean reviewStarted,
@Nullable Change.Id revertOf) {
checkNotNull(
metaId,
@@ -112,16 +112,16 @@
.lastUpdatedOn(lastUpdatedOn)
.owner(owner)
.branch(branch)
+ .status(status)
.currentPatchSetId(currentPatchSetId)
.subject(subject)
.topic(topic)
.originalSubject(originalSubject)
.submissionId(submissionId)
.assignee(assignee)
- .status(status)
.isPrivate(isPrivate)
- .isWorkInProgress(workInProgress)
- .hasReviewStarted(hasReviewStarted)
+ .workInProgress(workInProgress)
+ .reviewStarted(reviewStarted)
.revertOf(revertOf)
.build())
.pastAssignees(pastAssignees)
@@ -147,7 +147,7 @@
* <p>Notable exceptions include rowVersion and noteDbState, which are only make sense when read
* from NoteDb, so they cannot be cached.
*
- * <p>Fields are in listed column order.
+ * <p>Fields should match the column names in {@link Change}, and are in listed column order.
*/
@AutoValue
abstract static class ChangeColumns {
@@ -162,6 +162,10 @@
// Project not included, as it's not stored anywhere in the meta ref.
abstract String branch();
+ // TODO(dborowitz): Use a sensible default other than null
+ @Nullable
+ abstract Change.Status status();
+
@Nullable
abstract PatchSet.Id currentPatchSetId();
@@ -178,15 +182,12 @@
@Nullable
abstract Account.Id assignee();
- // TODO(dborowitz): Use a sensible default other than null
- @Nullable
- abstract Change.Status status();
abstract boolean isPrivate();
- abstract boolean isWorkInProgress();
+ abstract boolean workInProgress();
- abstract boolean hasReviewStarted();
+ abstract boolean reviewStarted();
@Nullable
abstract Change.Id revertOf();
@@ -219,9 +220,9 @@
abstract Builder isPrivate(boolean isPrivate);
- abstract Builder isWorkInProgress(boolean isWorkInProgress);
+ abstract Builder workInProgress(boolean workInProgress);
- abstract Builder hasReviewStarted(boolean hasReviewStarted);
+ abstract Builder reviewStarted(boolean reviewStarted);
abstract Builder revertOf(@Nullable Change.Id revertOf);
@@ -327,8 +328,8 @@
change.setSubmissionId(c.submissionId());
change.setAssignee(c.assignee());
change.setPrivate(c.isPrivate());
- change.setWorkInProgress(c.isWorkInProgress());
- change.setReviewStarted(c.hasReviewStarted());
+ change.setWorkInProgress(c.workInProgress());
+ change.setReviewStarted(c.reviewStarted());
change.setRevertOf(c.revertOf());
if (!patchSets().isEmpty()) {
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index d974877..b8f544a 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -442,17 +442,17 @@
// Change created in WIP remains in WIP.
RevCommit commit = writeCommit("Update WIP change\n" + "\n" + "Patch-set: 1\n", true);
ChangeNotesState state = newParser(commit).parseAll();
- assertThat(state.columns().hasReviewStarted()).isFalse();
+ assertThat(state.columns().reviewStarted()).isFalse();
// Moving change out of WIP starts review.
commit =
writeCommit("New ready change\n" + "\n" + "Patch-set: 1\n" + "Work-in-progress: false\n");
state = newParser(commit).parseAll();
- assertThat(state.columns().hasReviewStarted()).isTrue();
+ assertThat(state.columns().reviewStarted()).isTrue();
// Change created not in WIP has always been in review started state.
state = assertParseSucceeds("New change that doesn't declare WIP\n" + "\n" + "Patch-set: 1\n");
- assertThat(state.columns().hasReviewStarted()).isTrue();
+ assertThat(state.columns().reviewStarted()).isTrue();
}
@Test
