Skip to content

Commit

Permalink
Merge branch 'v0.x' into luanafragoso_logging_hits_reduce_response_si…
Browse files Browse the repository at this point in the history
…ze_v0x
  • Loading branch information
fragosoluana committed Jan 6, 2025
2 parents d6391fd + 321d299 commit 813e9d9
Show file tree
Hide file tree
Showing 23 changed files with 455 additions and 9 deletions.
1 change: 0 additions & 1 deletion .github/badges/jacoco.svg

This file was deleted.

18 changes: 15 additions & 3 deletions .github/workflows/gradle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ on:
default: v0.x
required: false
type: string
badge_branch:
description: 'The branch which contains the jacoco badge for above branch'
default: badge_v0.x
required: false
type: string

jobs:
build:
Expand All @@ -20,7 +25,13 @@ jobs:
java: ['21']
name: Java ${{ matrix.Java }} build
steps:
- uses: actions/checkout@v2
- name: Checkout
uses: actions/checkout@v2
- name: Checkout badges branch to a badges directory nested inside first checkout
uses: actions/checkout@v2
with:
ref: ${{ inputs.badge_branch }}
path: badges
- name: Setup java
uses: actions/setup-java@v2
with:
Expand All @@ -36,6 +47,7 @@ jobs:
id: jacoco
uses: cicirello/jacoco-badge-generator@v2.1.0
with:
badges-directory: badges
jacoco-csv-file: build/reports/jacoco/test/jacocoTestReport.csv
- name: Log coverage percentage
run: |
Expand All @@ -44,14 +56,14 @@ jobs:
- name: Commit and push badge
if: ${{ github.event_name != 'pull_request' }}
run: |
cd .github/badges
cd badges
if [[ `git status --porcelain *.svg` ]]; then
git config --global user.name github_actions
git add *.svg
git commit -m "Autogenerated JaCoCo coverage badge" *.svg
git pull --rebase origin ${{ inputs.branch }}
git push
fi
cd ..
- name: Upload JaCoCo coverage report
uses: actions/upload-artifact@v4
with:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/gradle_v0.x.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ jobs:
uses: Yelp/nrtsearch/.github/workflows/gradle.yml@v0.x
with:
branch: v0.x
badge_branch: badge_v0.x
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[![Coverage](.github/badges/jacoco.svg)](jacoco.svg) [![Docs](https://readthedocs.org/projects/nrtsearch/badge/?version=latest)](https://nrtsearch.readthedocs.io/en/latest/)
[![Coverage](https://raw.githubusercontent.com/Yelp/nrtsearch/refs/heads/badge_v0.x/jacoco.svg)](jacoco.svg) [![Docs](https://readthedocs.org/projects/nrtsearch/badge/?version=latest)](https://nrtsearch.readthedocs.io/en/latest/)
# nrtSearch
A high performance gRPC server, with optional REST APIs on top of [Apache Lucene](http://lucene.apache.org/) version 8.x source, exposing Lucene's
core functionality over a simple gRPC based API.
Expand Down
4 changes: 4 additions & 0 deletions clientlib/src/main/proto/yelp/nrtsearch/luceneserver.proto
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,8 @@ message LiveSettingsRequest {
int32 defaultSearchTimeoutCheckEvery = 13;
//Terminate after value to use when not specified in the search request.
int32 defaultTerminateAfter = 14;
//Terminate after max recall count value to use when not specified in the search request.
int32 defaultTerminateAfterMaxRecallCount = 15;
}

/* Response from Server to liveSettings */
Expand Down Expand Up @@ -1101,6 +1103,8 @@ message IndexLiveSettings {
google.protobuf.UInt64Value maxMergePreCopyDurationSec = 14;
// Collect and publish additional index metrics, which may be more expensive in terms of volume, memory and/or compute, default: false
google.protobuf.BoolValue verboseMetrics = 15;
// Terminate after max recall count value to use when not specified in the search request, or 0 for none, default: 0
google.protobuf.Int32Value defaultTerminateAfterMaxRecallCount = 18;
}

message IndexStateInfo {
Expand Down
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 = 15;
}

message Hit {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ public class LiveSettingsV2Command implements Callable<Integer> {
description = "Terminate after to use when not provided by the request")
private Integer defaultTerminateAfter;

@CommandLine.Option(
names = {"--defaultTerminateAfterMaxRecallCount"},
description = "Terminate after max recall count to use when not provided by the request")
private Integer defaultTerminateAfterMaxRecallCount;

@CommandLine.Option(
names = {"--maxMergePreCopyDurationSec"},
description = "Maximum time allowed for merge precopy in seconds")
Expand Down Expand Up @@ -181,6 +186,10 @@ public Integer call() throws Exception {
liveSettingsBuilder.setDefaultTerminateAfter(
Int32Value.newBuilder().setValue(defaultTerminateAfter).build());
}
if (defaultTerminateAfterMaxRecallCount != null) {
liveSettingsBuilder.setDefaultTerminateAfterMaxRecallCount(
Int32Value.newBuilder().setValue(defaultTerminateAfterMaxRecallCount).build());
}
if (maxMergePreCopyDurationSec != null) {
liveSettingsBuilder.setMaxMergePreCopyDurationSec(
UInt64Value.newBuilder().setValue(maxMergePreCopyDurationSec));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,9 @@ public abstract IndexWriterConfig getIndexWriterConfig(
/** Get the default terminate after. */
public abstract int getDefaultTerminateAfter();

/** Get the default terminate after max recall count. */
public abstract int getDefaultTerminateAfterMaxRecallCount();

/** Get the default search timeout check every. */
public abstract int getDefaultSearchTimeoutCheckEvery();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@ private LiveSettingsResponse handleAsLiveSettingsV2(
.setValue(liveSettingsRequest.getDefaultTerminateAfter())
.build());
}
if (liveSettingsRequest.getDefaultTerminateAfterMaxRecallCount() >= 0) {
settingsBuilder.setDefaultTerminateAfterMaxRecallCount(
Int32Value.newBuilder()
.setValue(liveSettingsRequest.getDefaultTerminateAfterMaxRecallCount())
.build());
}
try {
updatedSettings = indexStateManager.updateLiveSettings(settingsBuilder.build(), false);
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,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 @@ -91,7 +91,7 @@ private List<String> extractRules(String mappings, String separator) {
return Arrays.asList(mappings.split(separator));
}

static Pattern p = Pattern.compile("(.*)\\s*=>\\s*(.*)\\s*$");
static Pattern p = Pattern.compile("(.*)=>(.*)$");

protected void parseRules(List<String> rules, NormalizeCharMap.Builder builder) {
for (String rule : rules) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.yelp.nrtsearch.server.luceneserver.doc.LoadedDocValues;
import com.yelp.nrtsearch.server.luceneserver.field.properties.RangeQueryable;
import com.yelp.nrtsearch.server.luceneserver.field.properties.Sortable;
import com.yelp.nrtsearch.server.luceneserver.field.properties.TermQueryable;
import java.io.IOException;
import java.time.Instant;
import java.time.LocalDateTime;
Expand All @@ -34,6 +35,7 @@
import java.time.format.DateTimeParseException;
import java.time.format.ResolverStyle;
import java.time.temporal.ChronoField;
import java.util.ArrayList;
import java.util.List;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.LongPoint;
Expand All @@ -50,7 +52,8 @@
import org.apache.lucene.search.SortField;

/** Field class for 'DATE_TIME' field type. */
public class DateTimeFieldDef extends IndexableFieldDef implements Sortable, RangeQueryable {
public class DateTimeFieldDef extends IndexableFieldDef
implements Sortable, RangeQueryable, TermQueryable {
private static final String EPOCH_MILLIS = "epoch_millis";
private static final String STRICT_DATE_OPTIONAL_TIME = "strict_date_optional_time";

Expand Down Expand Up @@ -147,6 +150,28 @@ private long convertDateStringToMillis(String dateString) {
.toEpochMilli();
}

@Override
public Query getTermQueryFromLongValue(long longValue) {
return LongPoint.newExactQuery(getName(), longValue);
}

@Override
public Query getTermInSetQueryFromLongValues(List<Long> longValues) {
return LongPoint.newSetQuery(getName(), longValues);
}

@Override
public Query getTermQueryFromTextValue(String textValue) {
return getTermQueryFromLongValue(getTimeToIndex(textValue));
}

@Override
public Query getTermInSetQueryFromTextValues(List<String> textValues) {
List<Long> longTerms = new ArrayList<>(textValues.size());
textValues.forEach((s) -> longTerms.add(getTimeToIndex(s)));
return getTermInSetQueryFromLongValues(longTerms);
}

private void ensureUpperIsMoreThanLower(RangeQuery rangeQuery, long lower, long upper) {
if (lower > upper) {
throw new IllegalArgumentException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ public class ImmutableIndexState extends IndexState {
.setDefaultSearchTimeoutSec(DoubleValue.newBuilder().setValue(0).build())
.setDefaultSearchTimeoutCheckEvery(Int32Value.newBuilder().setValue(0).build())
.setDefaultTerminateAfter(Int32Value.newBuilder().setValue(0).build())
.setDefaultTerminateAfterMaxRecallCount(Int32Value.newBuilder().setValue(0).build())
.setMaxMergePreCopyDurationSec(UInt64Value.newBuilder().setValue(0))
.setVerboseMetrics(BoolValue.newBuilder().setValue(false).build())
.build();
Expand All @@ -181,6 +182,7 @@ public class ImmutableIndexState extends IndexState {
private final double defaultSearchTimeoutSec;
private final int defaultSearchTimeoutCheckEvery;
private final int defaultTerminateAfter;
private final int defaultTerminateAfterMaxRecallCount;
private final long maxMergePreCopyDurationSec;
private final boolean verboseMetrics;

Expand Down Expand Up @@ -273,6 +275,8 @@ public ImmutableIndexState(
defaultSearchTimeoutCheckEvery =
mergedLiveSettingsWithLocal.getDefaultSearchTimeoutCheckEvery().getValue();
defaultTerminateAfter = mergedLiveSettingsWithLocal.getDefaultTerminateAfter().getValue();
defaultTerminateAfterMaxRecallCount =
mergedLiveSettingsWithLocal.getDefaultTerminateAfterMaxRecallCount().getValue();
maxMergePreCopyDurationSec =
mergedLiveSettingsWithLocal.getMaxMergePreCopyDurationSec().getValue();
verboseMetrics = mergedLiveSettingsWithLocal.getVerboseMetrics().getValue();
Expand Down Expand Up @@ -823,6 +827,11 @@ public int getDefaultTerminateAfter() {
return defaultTerminateAfter;
}

@Override
public int getDefaultTerminateAfterMaxRecallCount() {
return defaultTerminateAfterMaxRecallCount;
}

@Override
public int getDefaultSearchTimeoutCheckEvery() {
return defaultSearchTimeoutCheckEvery;
Expand Down Expand Up @@ -924,6 +933,9 @@ static void validateLiveSettings(IndexLiveSettings liveSettings) {
if (liveSettings.getDefaultTerminateAfter().getValue() < 0) {
throw new IllegalArgumentException("defaultTerminateAfter must be >= 0");
}
if (liveSettings.getDefaultTerminateAfterMaxRecallCount().getValue() < 0) {
throw new IllegalArgumentException("defaultTerminateAfterMaxRecallCount must be >= 0");
}
if (liveSettings.getMaxMergePreCopyDurationSec().getValue() < 0) {
throw new IllegalArgumentException("maxMergePreCopyDurationSec must be >= 0");
}
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();
private final int hitsToLog;

public HitsLoggerFetchTask(LoggingHits loggingHits) {
Expand All @@ -44,7 +47,18 @@ 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();
}

public int getHitsToLog() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ public int getTerminateAfter() {
return terminateAfter;
}

/** Max documents to count beyond terminateAfter. */
public int getTerminateAfterMaxRecallCount() {
return terminateAfterMaxRecallCount;
}

/**
* {@link Collector} implementation that wraps another collector and terminates collection after a
* certain global count of documents is reached.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ <C extends Collector> CollectorManager<? extends Collector, SearcherResult> wrap
int terminateAfterMaxRecallCount =
request.getTerminateAfterMaxRecallCount() > 0
? request.getTerminateAfterMaxRecallCount()
: 0;
: indexState.getDefaultTerminateAfterMaxRecallCount();
if (terminateAfter > 0) {
wrapped =
new TerminateAfterWrapper<>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,8 @@ public void testSetIndexLiveSettings() throws IOException {
IndexLiveSettings.newBuilder()
.setDefaultTerminateAfter(
Int32Value.newBuilder().setValue(1000).build())
.setDefaultTerminateAfterMaxRecallCount(
Int32Value.newBuilder().setValue(1000).build())
.setSegmentsPerTier(Int32Value.newBuilder().setValue(4).build())
.setSliceMaxSegments(Int32Value.newBuilder().setValue(50).build())
.setDefaultSearchTimeoutSec(
Expand All @@ -832,6 +834,7 @@ public void testSetIndexLiveSettings() throws IOException {
IndexLiveSettings expectedSettings =
ImmutableIndexState.DEFAULT_INDEX_LIVE_SETTINGS.toBuilder()
.setDefaultTerminateAfter(Int32Value.newBuilder().setValue(1000).build())
.setDefaultTerminateAfterMaxRecallCount(Int32Value.newBuilder().setValue(1000).build())
.setSegmentsPerTier(Int32Value.newBuilder().setValue(4).build())
.setSliceMaxSegments(Int32Value.newBuilder().setValue(50).build())
.setDefaultSearchTimeoutSec(DoubleValue.newBuilder().setValue(5.1).build())
Expand Down Expand Up @@ -1697,6 +1700,7 @@ public void testLiveSettingsV1All() throws IOException {
.setDefaultSearchTimeoutSec(13.0)
.setDefaultSearchTimeoutCheckEvery(500)
.setDefaultTerminateAfter(5000)
.setDefaultTerminateAfterMaxRecallCount(6000)
.build();

LiveSettingsResponse response = primaryClient.getBlockingStub().liveSettings(request);
Expand All @@ -1715,6 +1719,7 @@ public void testLiveSettingsV1All() throws IOException {
.setDefaultSearchTimeoutSec(DoubleValue.newBuilder().setValue(13.0).build())
.setDefaultSearchTimeoutCheckEvery(Int32Value.newBuilder().setValue(500).build())
.setDefaultTerminateAfter(Int32Value.newBuilder().setValue(5000).build())
.setDefaultTerminateAfterMaxRecallCount(Int32Value.newBuilder().setValue(6000).build())
.setMaxMergePreCopyDurationSec(UInt64Value.newBuilder().setValue(0))
.setVerboseMetrics(BoolValue.newBuilder().setValue(false).build())
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ public void testDifferentSeparator() throws IOException {
assertEquals("abc and d n e'n'f g hi", output);
}

@Test
public void testWhitespaceInRule() throws IOException {
MappingV2CharFilterFactory factory = getFactory(". =>|,=> ");
String output = getFiltered(factory, "this. is, a test, string");
assertEquals("thisis a test string", output);
}

@Test
public void testNoMappings() {
try {
Expand Down
Loading

0 comments on commit 813e9d9

Please sign in to comment.