Skip to content

Commit b9b4ed4

Browse files
committed
remove timing changes
1 parent ed32b6d commit b9b4ed4

File tree

4 files changed

+15
-9
lines changed

4 files changed

+15
-9
lines changed

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpCache.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,24 +62,27 @@ public String toString() {
6262

6363
@SuppressWarnings("unchecked")
6464
<RESPONSE> RESPONSE putIfAbsent(ProjectId projectId, String ip, String databasePath, Function<String, RESPONSE> retrieveFunction) {
65-
long cacheStart = relativeNanoTimeProvider.getAsLong();
6665
// can't use cache.computeIfAbsent due to the elevated permissions for the jackson (run via the cache loader)
6766
CacheKey cacheKey = new CacheKey(projectId, ip, databasePath);
67+
long cacheStart = relativeNanoTimeProvider.getAsLong();
6868
// intentionally non-locking for simplicity...it's OK if we re-put the same key/value in the cache during a race condition.
6969
Object response = cache.get(cacheKey);
70+
long cacheRequestTime = relativeNanoTimeProvider.getAsLong() - cacheStart;
7071

7172
// populate the cache for this key, if necessary
7273
if (response == null) {
74+
long retrieveStart = relativeNanoTimeProvider.getAsLong();
7375
response = retrieveFunction.apply(ip);
7476
// if the response from the database was null, then use the no-result sentinel value
7577
if (response == null) {
7678
response = NO_RESULT;
7779
}
7880
// store the result or no-result in the cache
7981
cache.put(cacheKey, response);
80-
missesTimeInNanos.add(relativeNanoTimeProvider.getAsLong() - cacheStart);
82+
long databaseRequestAndCachePutTime = relativeNanoTimeProvider.getAsLong() - retrieveStart;
83+
missesTimeInNanos.add(cacheRequestTime + databaseRequestAndCachePutTime);
8184
} else {
82-
hitsTimeInNanos.add(relativeNanoTimeProvider.getAsLong() - cacheStart);
85+
hitsTimeInNanos.add(cacheRequestTime);
8386
}
8487

8588
if (response == NO_RESULT) {

modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/GeoIpCacheTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ public void testGetCacheStats() {
133133
assertThat(cacheStats.evictions(), equalTo(2L));
134134
// There are 3 hits, each taking 1ms:
135135
assertThat(cacheStats.hitsTimeInMillis(), equalTo(3L));
136-
// There are 4 misses. each taking 1ms:
137-
assertThat(cacheStats.missesTimeInMillis(), equalTo(4L));
136+
// There are 4 misses. Each is made up of a cache query, and a database query, each being 1ms:
137+
assertThat(cacheStats.missesTimeInMillis(), equalTo(8L));
138138
}
139139

140140
public void testPurgeCacheEntriesForDatabase() {

x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichCache.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,18 @@ public void computeIfAbsent(
103103
long cacheStart = relativeNanoTimeProvider.getAsLong();
104104
var cacheKey = new CacheKey(projectId, enrichIndex, lookupValue, maxMatches);
105105
List<Map<?, ?>> response = get(cacheKey);
106+
long cacheRequestTime = relativeNanoTimeProvider.getAsLong() - cacheStart;
106107
if (response != null) {
107-
hitsTimeInNanos.add(relativeNanoTimeProvider.getAsLong() - cacheStart);
108+
hitsTimeInNanos.add(cacheRequestTime);
108109
listener.onResponse(response);
109110
} else {
111+
final long retrieveStart = relativeNanoTimeProvider.getAsLong();
110112
searchResponseFetcher.accept(ActionListener.wrap(resp -> {
111113
CacheValue cacheValue = toCacheValue(resp);
112114
put(cacheKey, cacheValue);
113115
List<Map<?, ?>> copy = deepCopy(cacheValue.hits, false);
114-
missesTimeInNanos.add(relativeNanoTimeProvider.getAsLong() - cacheStart);
116+
long databaseQueryAndCachePutTime = relativeNanoTimeProvider.getAsLong() - retrieveStart;
117+
missesTimeInNanos.add(cacheRequestTime + databaseQueryAndCachePutTime);
115118
listener.onResponse(copy);
116119
}, listener::onFailure));
117120
}

x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/EnrichCacheTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ public void testComputeIfAbsent() throws InterruptedException {
140140
assertThat(cacheStats.misses(), equalTo(++expectedMisses));
141141
assertThat(cacheStats.evictions(), equalTo(0L));
142142
assertThat(cacheStats.hitsTimeInMillis(), equalTo(0L));
143-
assertThat(cacheStats.missesTimeInMillis(), equalTo(1L));
143+
assertThat(cacheStats.missesTimeInMillis(), equalTo(2L)); // cache query and enrich query + cache put
144144
}
145145

146146
{
@@ -156,7 +156,7 @@ public void testComputeIfAbsent() throws InterruptedException {
156156
assertThat(cacheStats.misses(), equalTo(expectedMisses));
157157
assertThat(cacheStats.evictions(), equalTo(0L));
158158
assertThat(cacheStats.hitsTimeInMillis(), equalTo(1L));
159-
assertThat(cacheStats.missesTimeInMillis(), equalTo(1L));
159+
assertThat(cacheStats.missesTimeInMillis(), equalTo(2L));
160160
}
161161

162162
{

0 commit comments

Comments
 (0)