Make admins aware that diskLimit cannot be honoured
Due to the very nature of chronicle-map persisted file pre-allocation,
the diskLimit configuration cannot be used to limit the file size on
disk.
Warn the admins with a meaningful log message and document this
behaviour in config.md
Bug: Issue 13458
Change-Id: Id65886dff3f20501d30384066bb6eeada1805d34
diff --git a/config.md b/config.md
index ab189b5..7c8b222 100644
--- a/config.md
+++ b/config.md
@@ -15,6 +15,16 @@
* `refreshAfterWrite`: Duration after which we asynchronously refresh the cached value.
[Gerrit docs](https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#cache.name.refreshAfterWrite)
+* `diskLimit`: Total size in bytes of the keys and values stored on disk.
+Defaults are per-cache and can be found in the relevant documentation:
+[Gerrit docs](https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#cache.name.diskLimit)
+
+ *NOTE*: a per gerrit documentation, a positive value is required to enable disk
+ storage for the cache. However, the provided value cannot be used to limit the
+ size of the file, since that is the result of chronicle-map pre-allocation and
+ it is a function of the number of entries, average sizes and bloat factor,
+ rather than the number of values stored in it.
+
Chronicle-map implementation however might require some additional configuration
## Configuration parameters
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheImpl.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheImpl.java
index c03b1ac..e7adc26 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheImpl.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheImpl.java
@@ -69,11 +69,6 @@
mapBuilder.averageValueSize(config.getAverageValueSize());
mapBuilder.valueMarshaller(new TimedValueMarshaller<>(def.valueSerializer()));
- // TODO: ChronicleMap must have "entries" configured, however cache definition
- // has already the concept of diskLimit. How to reconcile the two when both
- // are defined?
- // Should we honour diskLimit, by computing entries as a function of (avgKeySize +
- // avgValueSize)
mapBuilder.entries(config.getMaxEntries());
mapBuilder.maxBloatFactor(config.getMaxBloatFactor());
@@ -81,6 +76,12 @@
if (config.getPersistedFile() == null || config.getDiskLimit() < 0) {
store = mapBuilder.create();
} else {
+ logger.atWarning().log(
+ "disk storage is enabled for '%s', however chronicle-map "
+ + "cannot honour the diskLimit of %s bytes, since the file size "
+ + "is pre-allocated rather than being a function of the number"
+ + "of entries in the cache",
+ def.name(), def.diskLimit());
store = mapBuilder.createOrRecoverPersistedTo(config.getPersistedFile());
}
