Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backward compatibility of #794 #795

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clientlib/src/main/proto/yelp/nrtsearch/search.proto
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,7 @@ message SearchResponse {
double rescoreTimeMs = 10;
map<string, double> rescorersTimeMs = 11;
map<string, Diagnostics> innerHitsDiagnostics = 12;
double loggingHitsTimeMs = 13;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to use the same field number as the main branch, to keep the clientlib compatible

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for pointing this out!

}

message Hit {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,10 @@ public SearchResponse handle(IndexState indexState, SearchRequest searchRequest)
diagnostics.setHighlightTimeMs(
searchContext.getFetchTasks().getHighlightFetchTask().getTimeTakenMs());
}
if (searchContext.getFetchTasks().getHitsLoggerFetchTask() != null) {
diagnostics.setLoggingHitsTimeMs(
searchContext.getFetchTasks().getHitsLoggerFetchTask().getTimeTakenMs());
}
if (searchContext.getFetchTasks().getInnerHitFetchTaskList() != null) {
diagnostics.putAllInnerHitsDiagnostics(
searchContext.getFetchTasks().getInnerHitFetchTaskList().stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,16 @@
import com.yelp.nrtsearch.server.luceneserver.search.SearchContext;
import com.yelp.nrtsearch.server.plugins.HitsLoggerPlugin;
import java.util.List;
import java.util.concurrent.atomic.DoubleAdder;

/**
* Implementation of {@link FetchTask} which holds the required context to be able to log hits for a
* search request.
*/
public class HitsLoggerFetchTask implements FetchTask {
private static final double TEN_TO_THE_POWER_SIX = Math.pow(10, 6);
private final HitsLogger hitsLogger;
private final DoubleAdder timeTakenMs = new DoubleAdder();

public HitsLoggerFetchTask(LoggingHits loggingHits) {
this.hitsLogger = HitsLoggerCreator.getInstance().createHitsLogger(loggingHits);
Expand All @@ -42,6 +45,17 @@ public HitsLoggerFetchTask(LoggingHits loggingHits) {
*/
@Override
public void processAllHits(SearchContext searchContext, List<SearchResponse.Hit.Builder> hits) {
long startTime = System.nanoTime();
hitsLogger.log(searchContext, hits);
timeTakenMs.add(((System.nanoTime() - startTime) / TEN_TO_THE_POWER_SIX));
}

/**
* Get the total time taken so far to logging hits.
*
* @return Total time taken to logging hits in ms.
*/
public double getTimeTakenMs() {
return timeTakenMs.doubleValue();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.yelp.nrtsearch.server.luceneserver.logging;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

import com.google.protobuf.Struct;
import com.google.protobuf.Value;
Expand Down Expand Up @@ -192,4 +193,34 @@ public void testCustomHitsLoggerWithoutParam() {

assertEquals(expectedLogMessage, HitsLoggerTest.logMessage);
}

@Test
public void testLoggingTimeTaken() {
SearchRequest request =
SearchRequest.newBuilder()
.setTopHits(1)
.setStartHit(0)
.setIndexName(DEFAULT_TEST_INDEX)
.addRetrieveFields("doc_id")
.setQuery(
Query.newBuilder()
.setTermQuery(
TermQuery.newBuilder()
.setField("vendor_name")
.setTextValue("vendor")
.build())
.build())
.setLoggingHits(
LoggingHits.newBuilder()
.setName("custom_logger")
.setParams(
Struct.newBuilder()
.putFields(
"external_value", Value.newBuilder().setStringValue("abc").build()))
.build())
.build();
SearchResponse response = getGrpcServer().getBlockingStub().search(request);

assertTrue(response.getDiagnostics().getLoggingHitsTimeMs() > 0);
}
}
Loading