Skip to content

Give significance lookups their own home (backport of #57903) #57959

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jun 12, 2020
Merged

Give significance lookups their own home (backport of #57903) #57959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
45e94bc
Give significance lookups their own home (backport of #57903)
nik9000 Jun 10, 2020
3139d03
Merge branch '7.x' into significance_lookup_7_x
nik9000 Jun 11, 2020
ccb9766
Merge branch '7.x' into significance_lookup_7_x
nik9000 Jun 11, 2020
ac33c1e
Merge branch '7.x' into significance_lookup_7_x
nik9000 Jun 11, 2020
99a90f5
Merge branch '7.x' into significance_lookup_7_x
nik9000 Jun 12, 2020
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
22 changes: 14 additions & 8 deletions ...g/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.elasticsearch.search.aggregations.InternalMultiBucketAggregation;
import org.elasticsearch.search.aggregations.LeafBucketCollector;
import org.elasticsearch.search.aggregations.LeafBucketCollectorBase;
import org.elasticsearch.search.aggregations.bucket.terms.SignificanceLookup.BackgroundFrequencyForBytes;
import org.elasticsearch.search.aggregations.bucket.terms.heuristic.SignificanceHeuristic;
import org.elasticsearch.search.aggregations.support.ValuesSource;
import org.elasticsearch.search.internal.SearchContext;
Expand Down Expand Up @@ -753,14 +754,19 @@ class SignificantTermsResults extends ResultStrategy<
SignificantStringTerms.Bucket,
SignificantStringTerms.Bucket> {

// TODO a reference to the factory is weird - probably should be reference to what we need from it.
private final SignificantTermsAggregatorFactory termsAggFactory;
private final BackgroundFrequencyForBytes backgroundFrequencies;
private final long supersetSize;
private final SignificanceHeuristic significanceHeuristic;

private LongArray subsetSizes = context.bigArrays().newLongArray(1, true);

SignificantTermsResults(SignificantTermsAggregatorFactory termsAggFactory, SignificanceHeuristic significanceHeuristic) {
this.termsAggFactory = termsAggFactory;
SignificantTermsResults(
SignificanceLookup significanceLookup,
SignificanceHeuristic significanceHeuristic,
boolean collectsFromSingleBucket
) {
backgroundFrequencies = significanceLookup.bytesLookup(context.bigArrays(), collectsFromSingleBucket);
supersetSize = significanceLookup.supersetSize();
this.significanceHeuristic = significanceHeuristic;
}

Expand Down Expand Up @@ -804,8 +810,8 @@ BucketUpdater<SignificantStringTerms.Bucket> bucketUpdater(long owningBucketOrd)
oversizedCopy(lookupGlobalOrd.apply(globalOrd), spare.termBytes);
spare.subsetDf = docCount;
spare.subsetSize = subsetSize;
spare.supersetDf = termsAggFactory.getBackgroundFrequency(spare.termBytes);
spare.supersetSize = termsAggFactory.getSupersetNumDocs();
spare.supersetDf = backgroundFrequencies.freq(spare.termBytes);
spare.supersetSize = supersetSize;
/*
* During shard-local down-selection we use subset/superset stats
* that are for this shard only. Back at the central reducer these
Expand Down Expand Up @@ -839,7 +845,7 @@ SignificantStringTerms buildResult(long owningBucketOrd, long otherDocCount, Sig
metadata(),
format,
subsetSizes.get(owningBucketOrd),
termsAggFactory.getSupersetNumDocs(),
supersetSize,
significanceHeuristic,
Arrays.asList(topBuckets)
);
Expand All @@ -857,7 +863,7 @@ SignificantStringTerms buildNoValuesResult(long owningBucketOrdinal) {

@Override
public void close() {
Releasables.close(termsAggFactory, subsetSizes);
Releasables.close(backgroundFrequencies, subsetSizes);
}

/**
Expand Down
22 changes: 14 additions & 8 deletions ...ain/java/org/elasticsearch/search/aggregations/bucket/terms/MapStringTermsAggregator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.elasticsearch.search.aggregations.InternalOrder;
import org.elasticsearch.search.aggregations.LeafBucketCollector;
import org.elasticsearch.search.aggregations.LeafBucketCollectorBase;
import org.elasticsearch.search.aggregations.bucket.terms.SignificanceLookup.BackgroundFrequencyForBytes;
import org.elasticsearch.search.aggregations.bucket.terms.heuristic.SignificanceHeuristic;
import org.elasticsearch.search.aggregations.support.ValuesSource;
import org.elasticsearch.search.internal.SearchContext;
Expand Down Expand Up @@ -367,14 +368,19 @@ public void close() {}
* Builds results for the {@code significant_terms} aggregation.
*/
class SignificantTermsResults extends ResultStrategy<SignificantStringTerms, SignificantStringTerms.Bucket> {
// TODO a reference to the factory is weird - probably should be reference to what we need from it.
private final SignificantTermsAggregatorFactory termsAggFactory;
private final BackgroundFrequencyForBytes backgroundFrequencies;
private final long supersetSize;
private final SignificanceHeuristic significanceHeuristic;

private LongArray subsetSizes = context.bigArrays().newLongArray(1, true);

SignificantTermsResults(SignificantTermsAggregatorFactory termsAggFactory, SignificanceHeuristic significanceHeuristic) {
this.termsAggFactory = termsAggFactory;
SignificantTermsResults(
SignificanceLookup significanceLookup,
SignificanceHeuristic significanceHeuristic,
boolean collectsFromSingleBucket
) {
backgroundFrequencies = significanceLookup.bytesLookup(context.bigArrays(), collectsFromSingleBucket);
supersetSize = significanceLookup.supersetSize();
this.significanceHeuristic = significanceHeuristic;
}

Expand Down Expand Up @@ -416,8 +422,8 @@ void updateBucket(SignificantStringTerms.Bucket spare, BytesKeyedBucketOrds.Buck
ordsEnum.readValue(spare.termBytes);
spare.bucketOrd = ordsEnum.ord();
spare.subsetDf = docCount;
spare.supersetDf = termsAggFactory.getBackgroundFrequency(spare.termBytes);
spare.supersetSize = termsAggFactory.getSupersetNumDocs();
spare.supersetDf = backgroundFrequencies.freq(spare.termBytes);
spare.supersetSize = supersetSize;
/*
* During shard-local down-selection we use subset/superset stats
* that are for this shard only. Back at the central reducer these
Expand Down Expand Up @@ -460,7 +466,7 @@ SignificantStringTerms buildResult(long owningBucketOrd, long otherDocCount, Sig
metadata(),
format,
subsetSizes.get(owningBucketOrd),
termsAggFactory.getSupersetNumDocs(),
supersetSize,
significanceHeuristic,
Arrays.asList(topBuckets)
);
Expand All @@ -473,7 +479,7 @@ SignificantStringTerms buildEmptyResult() {

@Override
public void close() {
Releasables.close(termsAggFactory, subsetSizes);
Releasables.close(backgroundFrequencies, subsetSizes);
}
}
}
Expand Down
73 changes: 5 additions & 68 deletions .../main/java/org/elasticsearch/search/aggregations/bucket/terms/NumericTermsAggregator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@
import org.elasticsearch.common.collect.List;
import org.elasticsearch.common.lease.Releasable;
import org.elasticsearch.common.lease.Releasables;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.common.util.LongArray;
import org.elasticsearch.common.util.LongHash;
import org.elasticsearch.index.fielddata.FieldData;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.Aggregator;
Expand All @@ -42,6 +40,7 @@
import org.elasticsearch.search.aggregations.LeafBucketCollectorBase;
import org.elasticsearch.search.aggregations.bucket.terms.IncludeExclude.LongFilter;
import org.elasticsearch.search.aggregations.bucket.terms.LongKeyedBucketOrds.BucketOrdsEnum;
import org.elasticsearch.search.aggregations.bucket.terms.SignificanceLookup.BackgroundFrequencyForLong;
import org.elasticsearch.search.aggregations.bucket.terms.heuristic.SignificanceHeuristic;
import org.elasticsearch.search.aggregations.support.ValuesSource;
import org.elasticsearch.search.internal.ContextIndexSearcher;
Expand Down Expand Up @@ -469,19 +468,18 @@ DoubleTerms buildEmptyResult() {
}

class SignificantLongTermsResults extends ResultStrategy<SignificantLongTerms, SignificantLongTerms.Bucket> {
private final BackgroundFrequencies backgroundFrequencies;
private final BackgroundFrequencyForLong backgroundFrequencies;
private final long supersetSize;
private final SignificanceHeuristic significanceHeuristic;
private LongArray subsetSizes;

SignificantLongTermsResults(
SignificantTermsAggregatorFactory termsAggFactory,
SignificanceLookup significanceLookup,
SignificanceHeuristic significanceHeuristic,
boolean collectsFromSingleBucket
) {
LookupBackgroundFrequencies lookup = new LookupBackgroundFrequencies(termsAggFactory);
backgroundFrequencies = collectsFromSingleBucket ? lookup : new CacheBackgroundFrequencies(lookup, context.bigArrays());
supersetSize = termsAggFactory.getSupersetNumDocs();
backgroundFrequencies = significanceLookup.longLookup(context.bigArrays(), collectsFromSingleBucket);
supersetSize = significanceLookup.supersetSize();
this.significanceHeuristic = significanceHeuristic;
subsetSizes = context.bigArrays().newLongArray(1, true);
}
Expand Down Expand Up @@ -588,66 +586,5 @@ public void close() {
}
}

/**
* Lookup frequencies for terms.
*/
private interface BackgroundFrequencies extends Releasable {
long freq(long term) throws IOException;
}

/**
* Lookup frequencies for terms.
*/
private static class LookupBackgroundFrequencies implements BackgroundFrequencies {
// TODO a reference to the factory is weird - probably should be reference to what we need from it.
private final SignificantTermsAggregatorFactory termsAggFactory;

LookupBackgroundFrequencies(SignificantTermsAggregatorFactory termsAggFactory) {
this.termsAggFactory = termsAggFactory;
}

@Override
public long freq(long term) throws IOException {
return termsAggFactory.getBackgroundFrequency(term);
}

@Override
public void close() {
termsAggFactory.close();
}
}

/**
* Lookup and cache background frequencies for terms.
*/
private static class CacheBackgroundFrequencies implements BackgroundFrequencies {
private final LookupBackgroundFrequencies lookup;
private final BigArrays bigArrays;
private final LongHash termToPosition;
private LongArray positionToFreq;

CacheBackgroundFrequencies(LookupBackgroundFrequencies lookup, BigArrays bigArrays) {
this.lookup = lookup;
this.bigArrays = bigArrays;
termToPosition = new LongHash(1, bigArrays);
positionToFreq = bigArrays.newLongArray(1, false);
}

@Override
public long freq(long term) throws IOException {
long position = termToPosition.add(term);
if (position < 0) {
return positionToFreq.get(-1 - position);
}
long freq = lookup.freq(term);
positionToFreq = bigArrays.grow(positionToFreq, position + 1);
positionToFreq.set(position, freq);
return freq;
}

@Override
public void close() {
Releasables.close(lookup, termToPosition, positionToFreq);
}
}
}
Loading