From f12984bd96cd531bdfe8df365bd8b265a06ec50f Mon Sep 17 00:00:00 2001 From: Luana Fragoso Date: Mon, 3 Feb 2025 14:45:42 -0700 Subject: [PATCH] Add some conditions to skip hits logging (#817) * Skip logging when warming * Added unit test --- .../server/handler/SearchHandler.java | 2 +- .../server/logging/HitsLoggerFetchTask.java | 21 +++++++----- .../server/search/SearchContext.java | 12 +++++++ .../server/search/SearchRequestProcessor.java | 6 ++-- .../nrtsearch/server/grpc/TimeoutTest.java | 3 +- .../server/logging/HitsLoggerTest.java | 34 +++++++++++++++++++ .../server/query/QueryNodeMapperTest.java | 2 +- 7 files changed, 66 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/yelp/nrtsearch/server/handler/SearchHandler.java b/src/main/java/com/yelp/nrtsearch/server/handler/SearchHandler.java index 9b966a523..1bcf9be2d 100644 --- a/src/main/java/com/yelp/nrtsearch/server/handler/SearchHandler.java +++ b/src/main/java/com/yelp/nrtsearch/server/handler/SearchHandler.java @@ -169,7 +169,7 @@ public SearchResponse handle(IndexState indexState, SearchRequest searchRequest) searchContext = SearchRequestProcessor.buildContextForRequest( - searchRequest, indexState, shardState, s, diagnostics, profileResultBuilder); + searchRequest, indexState, shardState, s, diagnostics, profileResultBuilder, warming); long searchStartTime = System.nanoTime(); diff --git a/src/main/java/com/yelp/nrtsearch/server/logging/HitsLoggerFetchTask.java b/src/main/java/com/yelp/nrtsearch/server/logging/HitsLoggerFetchTask.java index 9bd8715bd..ca7c34ac7 100644 --- a/src/main/java/com/yelp/nrtsearch/server/logging/HitsLoggerFetchTask.java +++ b/src/main/java/com/yelp/nrtsearch/server/logging/HitsLoggerFetchTask.java @@ -47,17 +47,20 @@ public HitsLoggerFetchTask(LoggingHits loggingHits) { */ @Override public void processAllHits(SearchContext searchContext, List hits) { - long startTime = System.nanoTime(); + if (!searchContext.isWarming() && searchContext.getHitsToLog() > 0) { + long startTime = System.nanoTime(); - // hits list can contain extra hits that don't need to be logged, otherwise, pass all hits that - // can be logged - if (searchContext.getHitsToLog() < hits.size()) { - hitsLogger.log(searchContext, hits.subList(0, searchContext.getHitsToLog())); - } else { - hitsLogger.log(searchContext, hits); - } + // hits list can contain extra hits that don't need to be logged, otherwise, pass all hits + // that + // can be logged + if (searchContext.getHitsToLog() < hits.size()) { + hitsLogger.log(searchContext, hits.subList(0, searchContext.getHitsToLog())); + } else { + hitsLogger.log(searchContext, hits); + } - timeTakenMs.add(((System.nanoTime() - startTime) / TEN_TO_THE_POWER_SIX)); + timeTakenMs.add(((System.nanoTime() - startTime) / TEN_TO_THE_POWER_SIX)); + } } /** diff --git a/src/main/java/com/yelp/nrtsearch/server/search/SearchContext.java b/src/main/java/com/yelp/nrtsearch/server/search/SearchContext.java index 7e5763c75..6e6ea5e3d 100644 --- a/src/main/java/com/yelp/nrtsearch/server/search/SearchContext.java +++ b/src/main/java/com/yelp/nrtsearch/server/search/SearchContext.java @@ -58,6 +58,7 @@ public enum VectorScoringMode { } private final boolean explain; + private final boolean warming; private SearchContext(Builder builder, boolean validate) { this.indexState = builder.indexState; @@ -78,6 +79,7 @@ private SearchContext(Builder builder, boolean validate) { this.extraContext = builder.extraContext; this.queryNestedPath = builder.queryNestedPath; this.explain = builder.explain; + this.warming = builder.warming; if (validate) { validate(); @@ -192,6 +194,10 @@ public boolean isExplain() { return explain; } + public boolean isWarming() { + return warming; + } + /** Get new context builder instance * */ public static Builder newBuilder() { return new Builder(); @@ -249,6 +255,7 @@ public static class Builder { private Map extraContext; private String queryNestedPath; private boolean explain; + private boolean warming; private Builder() {} @@ -360,6 +367,11 @@ public Builder setExplain(boolean explain) { return this; } + public Builder setWarming(boolean warming) { + this.warming = warming; + return this; + } + /** * Use builder to create new search context. Skipping validation is possible, but mainly * intended for tests that do not require a complete context. diff --git a/src/main/java/com/yelp/nrtsearch/server/search/SearchRequestProcessor.java b/src/main/java/com/yelp/nrtsearch/server/search/SearchRequestProcessor.java index 034807cf1..79fa00a71 100644 --- a/src/main/java/com/yelp/nrtsearch/server/search/SearchRequestProcessor.java +++ b/src/main/java/com/yelp/nrtsearch/server/search/SearchRequestProcessor.java @@ -116,7 +116,8 @@ public static SearchContext buildContextForRequest( ShardState shardState, SearcherTaxonomyManager.SearcherAndTaxonomy searcherAndTaxonomy, SearchResponse.Diagnostics.Builder diagnostics, - ProfileResult.Builder profileResult) + ProfileResult.Builder profileResult, + boolean warming) throws IOException { SearchContext.Builder contextBuilder = SearchContext.newBuilder(); @@ -130,7 +131,8 @@ public static SearchContext buildContextForRequest( .setTimestampSec(System.currentTimeMillis() / 1000) .setStartHit(searchRequest.getStartHit()) .setTopHits(searchRequest.getTopHits()) - .setExplain(searchRequest.getExplain()); + .setExplain(searchRequest.getExplain()) + .setWarming(warming); Map queryVirtualFields = getVirtualFields(indexState, searchRequest); Map queryRuntimeFields = getRuntimeFields(indexState, searchRequest); diff --git a/src/test/java/com/yelp/nrtsearch/server/grpc/TimeoutTest.java b/src/test/java/com/yelp/nrtsearch/server/grpc/TimeoutTest.java index 4e11e69c5..925722e42 100644 --- a/src/test/java/com/yelp/nrtsearch/server/grpc/TimeoutTest.java +++ b/src/test/java/com/yelp/nrtsearch/server/grpc/TimeoutTest.java @@ -469,7 +469,8 @@ private TopDocs queryWithFunction(SearchRequest request, Function