From 7fb16c783dd8717f6bd82459ac3158dc088f1e89 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Thu, 12 Jun 2014 12:09:52 +0200 Subject: [PATCH] Added caching support to geohash_filter Caching is turned off by default. Closes #6478 --- .../filters/geohash-cell-filter.asciidoc | 10 +++ .../index/query/GeohashCellFilter.java | 45 ++++++++++++- .../search/geo/GeoFilterTests.java | 66 +++++++++++++++---- 3 files changed, 106 insertions(+), 15 deletions(-) diff --git a/docs/reference/query-dsl/filters/geohash-cell-filter.asciidoc b/docs/reference/query-dsl/filters/geohash-cell-filter.asciidoc index 34409020be1..15e602681d0 100644 --- a/docs/reference/query-dsl/filters/geohash-cell-filter.asciidoc +++ b/docs/reference/query-dsl/filters/geohash-cell-filter.asciidoc @@ -60,3 +60,13 @@ next to the given cell. } } -------------------------------------------------- + +[float] +==== Caching + +coming[1.3.0] + +The result of the filter is not cached by default. The +`_cache` parameter can be set to `true` to turn caching on. +By default the filter uses the resulting geohash cells as a cache key. +This can be changed by using the `_cache_key` option. diff --git a/src/main/java/org/elasticsearch/index/query/GeohashCellFilter.java b/src/main/java/org/elasticsearch/index/query/GeohashCellFilter.java index 36c85da1ad8..79ad952a8aa 100644 --- a/src/main/java/org/elasticsearch/index/query/GeohashCellFilter.java +++ b/src/main/java/org/elasticsearch/index/query/GeohashCellFilter.java @@ -32,6 +32,7 @@ import org.elasticsearch.common.unit.DistanceUnit; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser.Token; +import org.elasticsearch.index.cache.filter.support.CacheKeyFilter; import org.elasticsearch.index.mapper.FieldMapper; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.core.StringFieldMapper; @@ -61,6 +62,8 @@ public class GeohashCellFilter { public static final String NAME = "geohash_cell"; public static final String NEIGHBORS = "neighbors"; public static final String PRECISION = "precision"; + public static final String CACHE = "_cache"; + public static final String CACHE_KEY = "_cache_key"; /** * Create a new geohash filter for a given set of geohashes. In general this method @@ -100,6 +103,9 @@ public class GeohashCellFilter { private String geohash; private int levels = -1; private boolean neighbors; + private Boolean cache; + private String cacheKey; + public Builder(String field) { this(field, null, false); @@ -155,6 +161,19 @@ public class GeohashCellFilter { return this; } + /** + * Should the filter be cached or not. Defaults to false. + */ + public Builder cache(boolean cache) { + this.cache = cache; + return this; + } + + public Builder cacheKey(String cacheKey) { + this.cacheKey = cacheKey; + return this; + } + @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); @@ -164,6 +183,12 @@ public class GeohashCellFilter { if(levels > 0) { builder.field(PRECISION, levels); } + if (cache != null) { + builder.field(CACHE, cache); + } + if (cacheKey != null) { + builder.field(CACHE_KEY, cacheKey); + } builder.field(field, geohash); builder.endObject(); @@ -189,6 +214,9 @@ public class GeohashCellFilter { String geohash = null; int levels = -1; boolean neighbors = false; + boolean cache = false; + CacheKeyFilter.Key cacheKey = null; + XContentParser.Token token; if ((token = parser.currentToken()) != Token.START_OBJECT) { @@ -210,6 +238,12 @@ public class GeohashCellFilter { } else if (NEIGHBORS.equals(field)) { parser.nextToken(); neighbors = parser.booleanValue(); + } else if (CACHE.equals(field)) { + parser.nextToken(); + cache = parser.booleanValue(); + } else if (CACHE_KEY.equals(field)) { + parser.nextToken(); + cacheKey = new CacheKeyFilter.Key(parser.text()); } else { fieldName = field; token = parser.nextToken(); @@ -254,11 +288,18 @@ public class GeohashCellFilter { geohash = geohash.substring(0, len); } + Filter filter; if (neighbors) { - return create(parseContext, geoMapper, geohash, GeoHashUtils.addNeighbors(geohash, new ArrayList(8))); + filter = create(parseContext, geoMapper, geohash, GeoHashUtils.addNeighbors(geohash, new ArrayList(8))); } else { - return create(parseContext, geoMapper, geohash, null); + filter = create(parseContext, geoMapper, geohash, null); } + + if (cache) { + filter = parseContext.cacheFilter(filter, cacheKey); + } + + return filter; } } } diff --git a/src/test/java/org/elasticsearch/search/geo/GeoFilterTests.java b/src/test/java/org/elasticsearch/search/geo/GeoFilterTests.java index ca218402db5..335ce733491 100644 --- a/src/test/java/org/elasticsearch/search/geo/GeoFilterTests.java +++ b/src/test/java/org/elasticsearch/search/geo/GeoFilterTests.java @@ -34,6 +34,7 @@ import org.elasticsearch.action.bulk.BulkItemResponse; import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.Priority; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.geo.GeoHashUtils; import org.elasticsearch.common.geo.GeoPoint; @@ -44,6 +45,7 @@ import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.query.FilterBuilders; +import org.elasticsearch.index.query.GeohashCellFilter; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.search.SearchHit; import org.elasticsearch.test.ElasticsearchIntegrationTest; @@ -54,10 +56,7 @@ import java.io.ByteArrayOutputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Iterator; -import java.util.Random; +import java.util.*; import java.util.zip.GZIPInputStream; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -500,27 +499,68 @@ public class GeoFilterTests extends ElasticsearchIntegrationTest { client().admin().indices().prepareRefresh("locations").execute().actionGet(); - // Result of this geohash search should contain the geohash only - SearchResponse results1 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setPostFilter("{\"geohash_cell\": {\"pin\": \"" + geohash + "\", \"neighbors\": false}}").execute().actionGet(); - assertHitCount(results1, 1); + Map expectedCounts = new HashMap<>(); + Map expectedResults = new HashMap<>(); + Map cacheKeys = new HashMap<>(); - // test the same, just with the builder - results1 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setPostFilter(geoHashCellFilter("pin", geohash, false)).execute().actionGet(); - assertHitCount(results1, 1); + expectedCounts.put(geoHashCellFilter("pin", geohash, false), 1L); - // Result of the parent query should contain the parent it self, its neighbors, the child and all its neighbors - SearchResponse results2 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setPostFilter("{\"geohash_cell\": {\"pin\": \"" + geohash.substring(0, geohash.length() - 1) + "\", \"neighbors\": true}}").execute().actionGet(); - assertHitCount(results2, 2 + neighbors.size() + parentNeighbors.size()); + expectedCounts.put(geoHashCellFilter("pin", geohash.substring(0, geohash.length() - 1), true), 2L + neighbors.size() + parentNeighbors.size()); // Testing point formats and precision GeoPoint point = GeoHashUtils.decode(geohash); int precision = geohash.length(); + expectedCounts.put(geoHashCellFilter("pin", point).neighbors(true).precision(precision), 1L + neighbors.size()); + + logger.info("random testing of setting"); + + List filterBuilders = new ArrayList<>(expectedCounts.keySet()); + for (int j = filterBuilders.size() * 2 * randomIntBetween(1, 5); j > 0; j--) { + Collections.shuffle(filterBuilders, getRandom()); + for (GeohashCellFilter.Builder builder : filterBuilders) { + if (randomBoolean()) { + builder.cache(randomBoolean()); + } + if (randomBoolean()) { + String cacheKey = cacheKeys.get(builder); + if (cacheKey == null) { + cacheKey = randomUnicodeOfLength(6); + cacheKeys.put(builder, cacheKey); + } + builder.cacheKey(cacheKey); + } else { + builder.cacheKey(null); + } + try { + SearchResponse response = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setPostFilter(builder).get(); + assertHitCount(response, expectedCounts.get(builder)); + String[] expectedIds = expectedResults.get(builder); + if (expectedIds == null) { + ArrayList ids = new ArrayList<>(); + for (SearchHit hit : response.getHits()) { + ids.add(hit.id()); + } + expectedResults.put(builder, ids.toArray(Strings.EMPTY_ARRAY)); + continue; + } + + assertSearchHits(response, expectedIds); + + } catch (AssertionError error) { + throw new AssertionError(error.getMessage() + "\n geohash_cell filter:" + builder, error); + } + + + } + } + logger.info("Testing lat/lon format"); String pointTest1 = "{\"geohash_cell\": {\"pin\": {\"lat\": " + point.lat() + ",\"lon\": " + point.lon() + "},\"precision\": " + precision + ",\"neighbors\": true}}"; SearchResponse results3 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setPostFilter(pointTest1).execute().actionGet(); assertHitCount(results3, neighbors.size() + 1); + logger.info("Testing String format"); String pointTest2 = "{\"geohash_cell\": {\"pin\": \"" + point.lat() + "," + point.lon() + "\",\"precision\": " + precision + ",\"neighbors\": true}}"; SearchResponse results4 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setPostFilter(pointTest2).execute().actionGet();