Repository: komoot/photon
Pull request #769 (2024-01-31 14:05:10)

Make ES query timeout configurable
This replaces the hard-coded timeouts for the ES queries with something configurable.

Implements #747.


Patch:
diff --git a/src/main/java/de/komoot/photon/App.java b/src/main/java/de/komoot/photon/App.java
index 8588ab5..f9e35fc 100644
--- a/src/main/java/de/komoot/photon/App.java
+++ b/src/main/java/de/komoot/photon/App.java
@@ -5,6 +5,8 @@ import com.beust.jcommander.ParameterException;
 import de.komoot.photon.elasticsearch.Server;
 import de.komoot.photon.nominatim.NominatimConnector;
 import de.komoot.photon.nominatim.NominatimUpdater;
+import de.komoot.photon.searcher.ReverseHandler;
+import de.komoot.photon.searcher.SearchHandler;
 import de.komoot.photon.utils.CorsFilter;
 import lombok.extern.slf4j.Slf4j;
 import spark.Request;
@@ -162,10 +164,13 @@ public class App {
 
         // setup search API
         String[] langs = dbProperties.getLanguages();
-        get("api", new SearchRequestHandler("api", server.createSearchHandler(langs), langs, args.getDefaultLanguage()));
-        get("api/", new SearchRequestHandler("api/", server.createSearchHandler(langs), langs, args.getDefaultLanguage()));
-        get("reverse", new ReverseSearchRequestHandler("reverse", server.createReverseHandler(), dbProperties.getLanguages(), args.getDefaultLanguage()));
-        get("reverse/", new ReverseSearchRequestHandler("reverse/", server.createReverseHandler(), dbProperties.getLanguages(), args.getDefaultLanguage()));
+        SearchHandler searchHandler = server.createSearchHandler(langs, args.getQueryTimeout());
+        get("api", new SearchRequestHandler("api", searchHandler, langs, args.getDefaultLanguage()));
+        get("api/", new SearchRequestHandler("api/", searchHandler, langs, args.getDefaultLanguage()));
+
+        ReverseHandler reverseHandler = server.createReverseHandler(args.getQueryTimeout());
+        get("reverse", new ReverseSearchRequestHandler("reverse", reverseHandler, dbProperties.getLanguages(), args.getDefaultLanguage()));
+        get("reverse/", new ReverseSearchRequestHandler("reverse/", reverseHandler, dbProperties.getLanguages(), args.getDefaultLanguage()));
 
         if (args.isEnableUpdateApi()) {
             // setup update API
diff --git a/src/main/java/de/komoot/photon/CommandLineArgs.java b/src/main/java/de/komoot/photon/CommandLineArgs.java
index 583a660..0929bac 100644
--- a/src/main/java/de/komoot/photon/CommandLineArgs.java
+++ b/src/main/java/de/komoot/photon/CommandLineArgs.java
@@ -41,6 +41,9 @@ public class CommandLineArgs {
     @Parameter(names = "-synonym-file", description = "file with synonym and classification terms")
     private String synonymFile = null;
 
+    @Parameter(names = "-query-timeout", description = "Time after which to cancel queries to the ES database (in seconds).")
+    private int queryTimeout = 7;
+
     @Parameter(names = "-json", description = "import nominatim database and dump it to a json like files in (useful for developing)")
     private String jsonDump = null;
 
diff --git a/src/main/java/de/komoot/photon/elasticsearch/ElasticsearchReverseHandler.java b/src/main/java/de/komoot/photon/elasticsearch/ElasticsearchReverseHandler.java
index a915e4a..ca2d10b 100644
--- a/src/main/java/de/komoot/photon/elasticsearch/ElasticsearchReverseHandler.java
+++ b/src/main/java/de/komoot/photon/elasticsearch/ElasticsearchReverseHandler.java
@@ -23,9 +23,11 @@ import java.util.List;
  */
 public class ElasticsearchReverseHandler implements ReverseHandler {
     private Client client;
+    private TimeValue queryTimeout;
 
-    public ElasticsearchReverseHandler(Client client) {
+    public ElasticsearchReverseHandler(Client client, int queryTimeoutSec) {
         this.client = client;
+        queryTimeout = TimeValue.timeValueSeconds(queryTimeoutSec);
     }
 
     @Override
@@ -49,10 +51,8 @@ public class ElasticsearchReverseHandler implements ReverseHandler {
 
     private SearchResponse search(QueryBuilder queryBuilder, Integer limit, Point location,
                                  Boolean locationDistanceSort) {
-        TimeValue timeout = TimeValue.timeValueSeconds(7);
-
         SearchRequestBuilder builder = client.prepareSearch(PhotonIndex.NAME).setSearchType(SearchType.QUERY_THEN_FETCH)
-                .setQuery(queryBuilder).setSize(limit).setTimeout(timeout);
+                .setQuery(queryBuilder).setSize(limit).setTimeout(queryTimeout);
 
         if (locationDistanceSort)
             builder.addSort(SortBuilders.geoDistanceSort("coordinate", new GeoPoint(location.getY(), location.getX()))
diff --git a/src/main/java/de/komoot/photon/elasticsearch/ElasticsearchSearchHandler.java b/src/main/java/de/komoot/photon/elasticsearch/ElasticsearchSearchHandler.java
index cff16d7..5f49685 100644
--- a/src/main/java/de/komoot/photon/elasticsearch/ElasticsearchSearchHandler.java
+++ b/src/main/java/de/komoot/photon/elasticsearch/ElasticsearchSearchHandler.java
@@ -20,10 +20,12 @@ public class ElasticsearchSearchHandler implements SearchHandler {
     private final Client client;
     private final String[] supportedLanguages;
     private boolean lastLenient = false;
+    private TimeValue queryTimeout;
 
-    public ElasticsearchSearchHandler(Client client, String[] languages) {
+    public ElasticsearchSearchHandler(Client client, String[] languages, int queryTimeoutSec) {
         this.client = client;
         this.supportedLanguages = languages;
+        queryTimeout = TimeValue.timeValueSeconds(queryTimeoutSec);
     }
 
     @Override
@@ -63,12 +65,11 @@ public class ElasticsearchSearchHandler implements SearchHandler {
     }
 
     private SearchResponse sendQuery(QueryBuilder queryBuilder, Integer limit) {
-        TimeValue timeout = TimeValue.timeValueSeconds(7);
         return client.prepareSearch(PhotonIndex.NAME).
                 setSearchType(SearchType.QUERY_THEN_FETCH).
                 setQuery(queryBuilder).
                 setSize(limit).
-                setTimeout(timeout).
+                setTimeout(queryTimeout).
                 execute().
                 actionGet();
 
diff --git a/src/main/java/de/komoot/photon/elasticsearch/Server.java b/src/main/java/de/komoot/photon/elasticsearch/Server.java
index f3e0c27..c99c808 100644
--- a/src/main/java/de/komoot/photon/elasticsearch/Server.java
+++ b/src/main/java/de/komoot/photon/elasticsearch/Server.java
@@ -273,11 +273,11 @@ public class Server {
         return new de.komoot.photon.elasticsearch.Updater(esClient, languages, extraTags);
     }
 
-    public SearchHandler createSearchHandler(String[] languages) {
-        return new ElasticsearchSearchHandler(esClient, languages);
+    public SearchHandler createSearchHandler(String[] languages, int queryTimeoutSec) {
+        return new ElasticsearchSearchHandler(esClient, languages, queryTimeoutSec);
     }
 
-    public ReverseHandler createReverseHandler() {
-        return new ElasticsearchReverseHandler(esClient);
+    public ReverseHandler createReverseHandler(int queryTimeoutSec) {
+        return new ElasticsearchReverseHandler(esClient, queryTimeoutSec);
     }
 }


Test patch:
diff --git a/src/test/java/de/komoot/photon/elasticsearch/ElasticResultTest.java b/src/test/java/de/komoot/photon/elasticsearch/ElasticResultTest.java
index 71e879f..90b35a6 100644
--- a/src/test/java/de/komoot/photon/elasticsearch/ElasticResultTest.java
+++ b/src/test/java/de/komoot/photon/elasticsearch/ElasticResultTest.java
@@ -74,7 +74,7 @@ public class ElasticResultTest  extends ESBaseTester {
     }
 
     private PhotonResult search(String query) {
-        SearchHandler handler = getServer().createSearchHandler(new String[]{"en", "de", "it"});
+        SearchHandler handler = getServer().createSearchHandler(new String[]{"en", "de", "it"}, 1);
 
         return handler.search(new PhotonRequest(query, "default")).get(0);
     }
diff --git a/src/test/java/de/komoot/photon/query/QueryBasicSearchTest.java b/src/test/java/de/komoot/photon/query/QueryBasicSearchTest.java
index a611d20..e63bced 100644
--- a/src/test/java/de/komoot/photon/query/QueryBasicSearchTest.java
+++ b/src/test/java/de/komoot/photon/query/QueryBasicSearchTest.java
@@ -39,7 +39,7 @@ public class QueryBasicSearchTest extends ESBaseTester {
     }
 
     private List<PhotonResult> search(String query) {
-        return getServer().createSearchHandler(new String[]{"en"}).search(new PhotonRequest(query, "en"));
+        return getServer().createSearchHandler(new String[]{"en"}, 1).search(new PhotonRequest(query, "en"));
     }
 
 
diff --git a/src/test/java/de/komoot/photon/query/QueryByClassificationTest.java b/src/test/java/de/komoot/photon/query/QueryByClassificationTest.java
index 59c962f..8c9acf3 100644
--- a/src/test/java/de/komoot/photon/query/QueryByClassificationTest.java
+++ b/src/test/java/de/komoot/photon/query/QueryByClassificationTest.java
@@ -35,7 +35,7 @@ public class QueryByClassificationTest extends ESBaseTester {
     }
 
     private List<PhotonResult> search(String query) {
-        return getServer().createSearchHandler(new String[]{"en"}).search(new PhotonRequest(query, "en"));
+        return getServer().createSearchHandler(new String[]{"en"}, 1).search(new PhotonRequest(query, "en"));
     }
 
     private void updateClassification(String key, String value, String... terms) {
diff --git a/src/test/java/de/komoot/photon/query/QueryByLanguageTest.java b/src/test/java/de/komoot/photon/query/QueryByLanguageTest.java
index 1ffae89..29e3b8e 100644
--- a/src/test/java/de/komoot/photon/query/QueryByLanguageTest.java
+++ b/src/test/java/de/komoot/photon/query/QueryByLanguageTest.java
@@ -43,7 +43,7 @@ public class QueryByLanguageTest extends ESBaseTester {
     }
 
     private List<PhotonResult> search(String query, String lang) {
-        return getServer().createSearchHandler(languageList).search(new PhotonRequest(query, lang));
+        return getServer().createSearchHandler(languageList, 1).search(new PhotonRequest(query, lang));
     }
 
     @Test
diff --git a/src/test/java/de/komoot/photon/query/QueryFilterLayerTest.java b/src/test/java/de/komoot/photon/query/QueryFilterLayerTest.java
index d83e3ff..552f933 100644
--- a/src/test/java/de/komoot/photon/query/QueryFilterLayerTest.java
+++ b/src/test/java/de/komoot/photon/query/QueryFilterLayerTest.java
@@ -57,7 +57,7 @@ public class QueryFilterLayerTest extends ESBaseTester {
         PhotonRequest request = new PhotonRequest("berlin", "en").setLimit(50);
         request.setLayerFilter(Arrays.stream(layers).collect(Collectors.toSet()));
 
-        return getServer().createSearchHandler(new String[]{"en"}).search(request);
+        return getServer().createSearchHandler(new String[]{"en"}, 1).search(request);
     }
 
     @AfterAll
diff --git a/src/test/java/de/komoot/photon/query/QueryFilterTagValueTest.java b/src/test/java/de/komoot/photon/query/QueryFilterTagValueTest.java
index 0beec64..f26520c 100644
--- a/src/test/java/de/komoot/photon/query/QueryFilterTagValueTest.java
+++ b/src/test/java/de/komoot/photon/query/QueryFilterTagValueTest.java
@@ -67,7 +67,7 @@ public class QueryFilterTagValueTest extends ESBaseTester {
             request.addOsmTagFilter(TagFilter.buildOsmTagFilter(param));
         }
 
-        return getServer().createSearchHandler(new String[]{"en"}).search(request);
+        return getServer().createSearchHandler(new String[]{"en"}, 1).search(request);
     }
 
 
diff --git a/src/test/java/de/komoot/photon/query/QueryRelevanceTest.java b/src/test/java/de/komoot/photon/query/QueryRelevanceTest.java
index 660da60..2653031 100644
--- a/src/test/java/de/komoot/photon/query/QueryRelevanceTest.java
+++ b/src/test/java/de/komoot/photon/query/QueryRelevanceTest.java
@@ -39,11 +39,11 @@ public class QueryRelevanceTest extends ESBaseTester {
     }
 
     private List<PhotonResult> search(String query) {
-        return getServer().createSearchHandler(new String[]{"en"}).search(new PhotonRequest(query, "en"));
+        return getServer().createSearchHandler(new String[]{"en"}, 1).search(new PhotonRequest(query, "en"));
     }
 
     private List<PhotonResult> search(PhotonRequest request) {
-        return getServer().createSearchHandler(new String[]{"en"}).search(request);
+        return getServer().createSearchHandler(new String[]{"en"}, 1).search(request);
     }
 
     @Test
diff --git a/src/test/java/de/komoot/photon/query/QueryReverseFilterLayerTest.java b/src/test/java/de/komoot/photon/query/QueryReverseFilterLayerTest.java
index c5a27fd..d750c5a 100644
--- a/src/test/java/de/komoot/photon/query/QueryReverseFilterLayerTest.java
+++ b/src/test/java/de/komoot/photon/query/QueryReverseFilterLayerTest.java
@@ -54,7 +54,7 @@ public class QueryReverseFilterLayerTest extends ESBaseTester {
         Set<String> layerSet = Arrays.stream(layers).collect(Collectors.toSet());
         ReverseRequest request = new ReverseRequest(pt, "en", 1.0, "", 10, true, layerSet, false);
 
-        return getServer().createReverseHandler().reverse(request);
+        return getServer().createReverseHandler(1).reverse(request);
     }
 
     @Test
diff --git a/src/test/java/de/komoot/photon/query/QueryReverseFilterTagValueTest.java b/src/test/java/de/komoot/photon/query/QueryReverseFilterTagValueTest.java
index 16fed61..cd655fd 100644
--- a/src/test/java/de/komoot/photon/query/QueryReverseFilterTagValueTest.java
+++ b/src/test/java/de/komoot/photon/query/QueryReverseFilterTagValueTest.java
@@ -72,7 +72,7 @@ public class QueryReverseFilterTagValueTest extends ESBaseTester {
         for (String param : params) {
             request.addOsmTagFilter(TagFilter.buildOsmTagFilter(param));
         }
-        return getServer().createReverseHandler().reverse(request);
+        return getServer().createReverseHandler(1).reverse(request);
     }
 
     @ParameterizedTest
diff --git a/src/test/java/de/komoot/photon/query/QueryReverseTest.java b/src/test/java/de/komoot/photon/query/QueryReverseTest.java
index 11dea2a..fbfdfca 100644
--- a/src/test/java/de/komoot/photon/query/QueryReverseTest.java
+++ b/src/test/java/de/komoot/photon/query/QueryReverseTest.java
@@ -47,7 +47,7 @@ public class QueryReverseTest extends ESBaseTester {
     private List<PhotonResult> reverse(double lon, double lat, double radius, int limit) {
         Point pt = FACTORY.createPoint(new Coordinate(lon, lat));
 
-        return getServer().createReverseHandler().reverse(
+        return getServer().createReverseHandler(1).reverse(
             new ReverseRequest(pt, "en", radius, "", limit, true, new HashSet<>(), false)
         );
     }