Limit 'maxPages' to '100' by deafult
It seems that Lucene implementation has to scan through the large amount
of data in order to reach distant page and results in substantial amount
of memory being used (results in OOO exception). In order to mitigate it
limit 'maxPages' to `100' if no value is set. Note that one can still
set it to unlimited by changing the value to '0'.
Release-Notes: Limit 'index.maxPages' to '100' by default
Bug: Issue 15563
Change-Id: I5a9a50b755d9814fcdd47910702b8ec80b2a029f
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 10baed2..31bb6be 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3092,7 +3092,7 @@
this threshold times the requested limit will result in an error. Set to
0 for no limit.
+
-Defaults to no limit.
+Defaults to 100.
[[index.maxTerms]]index.maxTerms::
+
diff --git a/java/com/google/gerrit/index/IndexConfig.java b/java/com/google/gerrit/index/IndexConfig.java
index 29b8ea6..c93c539 100644
--- a/java/com/google/gerrit/index/IndexConfig.java
+++ b/java/com/google/gerrit/index/IndexConfig.java
@@ -17,6 +17,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
import java.util.function.Consumer;
import java.util.function.IntConsumer;
import org.eclipse.jgit.lib.Config;
@@ -31,6 +32,8 @@
public abstract class IndexConfig {
private static final int DEFAULT_MAX_TERMS = 1024;
+ @VisibleForTesting static final int DEFAULT_MAX_PAGES = 100;
+
public static IndexConfig createDefault() {
return builder().build();
}
@@ -38,14 +41,18 @@
public static Builder fromConfig(Config cfg) {
Builder b = builder();
setIfPresent(cfg, "maxLimit", b::maxLimit);
- setIfPresent(cfg, "maxPages", b::maxPages);
+ setIfPresent(cfg, "maxPages", DEFAULT_MAX_PAGES, b::maxPages);
setIfPresent(cfg, "maxTerms", b::maxTerms);
setTypeOrDefault(cfg, b::type);
return b;
}
private static void setIfPresent(Config cfg, String name, IntConsumer setter) {
- int n = cfg.getInt("index", null, name, 0);
+ setIfPresent(cfg, name, 0, setter);
+ }
+
+ private static void setIfPresent(Config cfg, String name, int def, IntConsumer setter) {
+ int n = cfg.getInt("index", null, name, def);
if (n != 0) {
setter.accept(n);
}
diff --git a/javatests/com/google/gerrit/index/IndexConfigTest.java b/javatests/com/google/gerrit/index/IndexConfigTest.java
new file mode 100644
index 0000000..6d3fcb7
--- /dev/null
+++ b/javatests/com/google/gerrit/index/IndexConfigTest.java
@@ -0,0 +1,58 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.index;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.index.IndexConfig.DEFAULT_MAX_PAGES;
+
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+
+public class IndexConfigTest {
+ @Test
+ public void maxPagesIsLimitedByDefault() {
+ // when
+ IndexConfig config = IndexConfig.fromConfig(new Config()).build();
+
+ // then
+ assertThat(config.maxPages()).isEqualTo(DEFAULT_MAX_PAGES);
+ }
+
+ @Test
+ public void maxPagesCanBeConfigured() {
+ // given
+ Config cfg = new Config();
+ cfg.setInt("index", null, "maxPages", 10);
+
+ // when
+ IndexConfig config = IndexConfig.fromConfig(cfg).build();
+
+ // then
+ assertThat(config.maxPages()).isEqualTo(10);
+ }
+
+ @Test
+ public void maxPagesCanBeConfiguredToUnlimited() {
+ // given
+ Config cfg = new Config();
+ cfg.setInt("index", null, "maxPages", 0);
+
+ // when
+ IndexConfig config = IndexConfig.fromConfig(cfg).build();
+
+ // then
+ assertThat(config.maxPages()).isEqualTo(Integer.MAX_VALUE);
+ }
+}
