Failfast when elasticsearch module has paginationType set to NONE
The paginationType NONE option needs to be honoured by the indexing
backend (see change 377794).
The elasticsearch backend doesn't support this configuration therefore
it throws ProvisisionException during the startup when such
configuration is detected. Here is the exception message:
The 'index.paginationType = NONE' configuration is not supported
by Elasticsearch
Notes:
* `ElasticConfigurationTest` was modified to ahere to modified
constructor and to verify that `ProvisionException` is thrown when
`NONE` pagination type is configured
Bug: Issue 291106652
Change-Id: I47c6b7f972e2b1d443cc5748386290dc10556f2a
diff --git a/BUILD b/BUILD
index 4070a77..94ce436 100644
--- a/BUILD
+++ b/BUILD
@@ -90,7 +90,8 @@
exclude = ["src/test/java/**/Elastic*Query*" + SUFFIX],
),
tags = ["elastic"],
- deps = PLUGIN_TEST_DEPS + [
+ deps = ELASTICSEARCH_DEPS + PLUGIN_TEST_DEPS + [
+ ":elasticsearch_test_utils",
":index-elasticsearch__plugin",
],
)
diff --git a/src/main/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java b/src/main/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
index 2a5fcde..0379ed9 100644
--- a/src/main/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
+++ b/src/main/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
@@ -18,6 +18,8 @@
import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.index.IndexConfig;
+import com.google.gerrit.index.PaginationType;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import com.google.inject.ProvisionException;
@@ -70,7 +72,12 @@
final String prefix;
@Inject
- ElasticConfiguration(@GerritServerConfig Config cfg) {
+ ElasticConfiguration(@GerritServerConfig Config cfg, IndexConfig indexConfig) {
+ if (PaginationType.NONE == indexConfig.paginationType()) {
+ throw new ProvisionException(
+ "The 'index.paginationType = NONE' configuration is not supported by Elasticsearch");
+ }
+
this.cfg = cfg;
this.password = cfg.getString(SECTION_ELASTICSEARCH, null, KEY_PASSWORD);
this.username =
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 927d229..1954cb0 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -10,6 +10,16 @@
value is not configured during site initialization, defaults to 10000, which is the default value
of `index.max_result_window` in Elasticsearch.
+### index.paginationType
+
+The pagination type to use when index queries are repeated to obtain the next set of results.
+Supported values are: `OFFSET` and `SEARCH_AFTER`. For more information, refer to
+[`index.paginationType`](https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#index.paginationType).
+
+Defaults to `OFFSET`.
+Note: paginationType `NONE` is not supported and Gerrit will not start if it is configured (results
+in `ProvisionException`).
+
## Section elasticsearch
WARNING: Support for Elasticsearch is still experimental and is not recommended for production
diff --git a/src/test/java/com/google/gerrit/elasticsearch/ElasticConfigurationTest.java b/src/test/java/com/google/gerrit/elasticsearch/ElasticConfigurationTest.java
index 7e044c3..9d20edc 100644
--- a/src/test/java/com/google/gerrit/elasticsearch/ElasticConfigurationTest.java
+++ b/src/test/java/com/google/gerrit/elasticsearch/ElasticConfigurationTest.java
@@ -25,6 +25,7 @@
import static java.util.stream.Collectors.toList;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.index.IndexConfig;
import com.google.inject.ProvisionException;
import java.util.Arrays;
import org.apache.http.HttpHost;
@@ -35,7 +36,7 @@
@Test
public void singleServerNoOtherConfig() throws Exception {
Config cfg = newConfig();
- ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ ElasticConfiguration esCfg = newElasticConfig(cfg);
assertHosts(esCfg, "http://elastic:1234");
assertThat(esCfg.username).isNull();
assertThat(esCfg.password).isNull();
@@ -46,7 +47,7 @@
public void serverWithoutPortSpecified() throws Exception {
Config cfg = new Config();
cfg.setString(SECTION_ELASTICSEARCH, null, KEY_SERVER, "http://elastic");
- ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ ElasticConfiguration esCfg = newElasticConfig(cfg);
assertHosts(esCfg, "http://elastic:9200");
}
@@ -54,7 +55,7 @@
public void prefix() throws Exception {
Config cfg = newConfig();
cfg.setString(SECTION_ELASTICSEARCH, null, KEY_PREFIX, "myprefix");
- ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ ElasticConfiguration esCfg = newElasticConfig(cfg);
assertThat(esCfg.prefix).isEqualTo("myprefix");
}
@@ -63,7 +64,7 @@
Config cfg = newConfig();
cfg.setString(SECTION_ELASTICSEARCH, null, KEY_USERNAME, "myself");
cfg.setString(SECTION_ELASTICSEARCH, null, KEY_PASSWORD, "s3kr3t");
- ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ ElasticConfiguration esCfg = newElasticConfig(cfg);
assertThat(esCfg.username).isEqualTo("myself");
assertThat(esCfg.password).isEqualTo("s3kr3t");
}
@@ -72,7 +73,7 @@
public void withAuthenticationPasswordOnlyUsesDefaultUsername() throws Exception {
Config cfg = newConfig();
cfg.setString(SECTION_ELASTICSEARCH, null, KEY_PASSWORD, "s3kr3t");
- ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ ElasticConfiguration esCfg = newElasticConfig(cfg);
assertThat(esCfg.username).isEqualTo(DEFAULT_USERNAME);
assertThat(esCfg.password).isEqualTo("s3kr3t");
}
@@ -85,20 +86,20 @@
null,
KEY_SERVER,
ImmutableList.of("http://elastic1:1234", "http://elastic2:1234"));
- ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ ElasticConfiguration esCfg = newElasticConfig(cfg);
assertHosts(esCfg, "http://elastic1:1234", "http://elastic2:1234");
}
@Test
public void noServers() throws Exception {
- assertProvisionException(new Config());
+ assertProvisionException(new Config(), "No valid Elasticsearch servers configured");
}
@Test
public void singleServerInvalid() throws Exception {
Config cfg = new Config();
cfg.setString(SECTION_ELASTICSEARCH, null, KEY_SERVER, "foo");
- assertProvisionException(cfg);
+ assertProvisionException(cfg, "No valid Elasticsearch servers configured");
}
@Test
@@ -106,24 +107,35 @@
Config cfg = new Config();
cfg.setStringList(
SECTION_ELASTICSEARCH, null, KEY_SERVER, ImmutableList.of("http://elastic1:1234", "foo"));
- ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+ ElasticConfiguration esCfg = newElasticConfig(cfg);
assertHosts(esCfg, "http://elastic1:1234");
}
+ @Test
+ public void unsupportedPaginationTypeNone() {
+ Config cfg = new Config();
+ cfg.setString("index", null, "paginationType", "NONE");
+ assertProvisionException(
+ cfg, "The 'index.paginationType = NONE' configuration is not supported by Elasticsearch");
+ }
+
private static Config newConfig() {
Config config = new Config();
config.setString(SECTION_ELASTICSEARCH, null, KEY_SERVER, "http://elastic:1234");
return config;
}
+ private static ElasticConfiguration newElasticConfig(Config cfg) {
+ return new ElasticConfiguration(cfg, IndexConfig.fromConfig(cfg).build());
+ }
+
private void assertHosts(ElasticConfiguration cfg, Object... hostURIs) throws Exception {
assertThat(Arrays.asList(cfg.getHosts()).stream().map(HttpHost::toURI).collect(toList()))
.containsExactly(hostURIs);
}
- private void assertProvisionException(Config cfg) {
- ProvisionException thrown =
- assertThrows(ProvisionException.class, () -> new ElasticConfiguration(cfg));
- assertThat(thrown).hasMessageThat().contains("No valid Elasticsearch servers configured");
+ private void assertProvisionException(Config cfg, String msg) {
+ ProvisionException thrown = assertThrows(ProvisionException.class, () -> newElasticConfig(cfg));
+ assertThat(thrown).hasMessageThat().contains(msg);
}
}
