Skip to content

Commit

Permalink
Re-introduce shardSize member to InternalMultiTerms and InternalMappe…
Browse files Browse the repository at this point in the history 
…dTerms

Signed-off-by: Jay Deng <[email protected]>
  • Loading branch information
@jed326
jed326 authored and Jay Deng committed Aug 9, 2023
1 parent f239aec commit aac2559
Show file tree
Hide file tree
Showing 21 changed files with 95 additions and 13 deletions.
1 change: 1 addition & 0 deletions ...arks/src/main/java/org/opensearch/benchmark/search/aggregations/TermsReduceBenchmark.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ private StringTerms newTerms(Random rand, BytesRef[] dict, boolean withNested) {
BucketOrder.count(false),
Collections.emptyMap(),
DocValueFormat.RAW,
numShards,
true,
0,
buckets,
Expand Down
1 change: 1 addition & 0 deletions ...nsearch/benchmark/search/aggregations/bucket/terms/StringTermsSerializationBenchmark.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ private StringTerms newTerms(boolean withNested) {
BucketOrder.key(true),
null,
DocValueFormat.RAW,
buckets,
false,
100000,
resultBuckets,
Expand Down
14 changes: 13 additions & 1 deletion ...n/java/org/opensearch/search/aggregations/bucket/terms/AbstractStringTermsAggregator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,19 @@ abstract class AbstractStringTermsAggregator extends TermsAggregator {
}

protected StringTerms buildEmptyTermsAggregation() {
return new StringTerms(name, order, order, metadata(), format, showTermDocCountError, 0, emptyList(), 0, bucketCountThresholds);
return new StringTerms(
name,
order,
order,
metadata(),
format,
bucketCountThresholds.getShardSize(),
showTermDocCountError,
0,
emptyList(),
0,
bucketCountThresholds
);
}

protected SignificantStringTerms buildEmptySignificantTermsAggregation(long subsetSize, SignificanceHeuristic significanceHeuristic) {
Expand Down
4 changes: 4 additions & 0 deletions server/src/main/java/org/opensearch/search/aggregations/bucket/terms/DoubleTerms.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ public DoubleTerms(
BucketOrder order,
Map<String, Object> metadata,
DocValueFormat format,
int shardSize,
boolean showTermDocCountError,
long otherDocCount,
List<Bucket> buckets,
Expand All @@ -144,6 +145,7 @@ public DoubleTerms(
order,
metadata,
format,
shardSize,
showTermDocCountError,
otherDocCount,
buckets,
Expand Down Expand Up @@ -172,6 +174,7 @@ public DoubleTerms create(List<Bucket> buckets) {
order,
metadata,
format,
shardSize,
showTermDocCountError,
otherDocCount,
buckets,
Expand Down Expand Up @@ -200,6 +203,7 @@ protected DoubleTerms create(String name, List<Bucket> buckets, BucketOrder redu
order,
getMetadata(),
format,
shardSize,
showTermDocCountError,
otherDocCount,
buckets,
Expand Down
1 change: 1 addition & 0 deletions .../org/opensearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,7 @@ StringTerms buildResult(long owningBucketOrd, long otherDocCount, StringTerms.Bu
order,
metadata(),
format,
bucketCountThresholds.getShardSize(),
showTermDocCountError,
otherDocCount,
Arrays.asList(topBuckets),
Expand Down
3 changes: 2 additions & 1 deletion ...er/src/main/java/org/opensearch/search/aggregations/bucket/terms/InternalMappedTerms.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ protected InternalMappedTerms(
BucketOrder order,
Map<String, Object> metadata,
DocValueFormat format,
int shardSize,
boolean showTermDocCountError,
long otherDocCount,
List<B> buckets,
Expand All @@ -74,7 +75,7 @@ protected InternalMappedTerms(
) {
super(name, reduceOrder, order, bucketCountThresholds, metadata);
this.format = format;
this.shardSize = bucketCountThresholds.getShardSize();
this.shardSize = shardSize;
this.showTermDocCountError = showTermDocCountError;
this.otherDocCount = otherDocCount;
this.docCountError = docCountError;
Expand Down
5 changes: 4 additions & 1 deletion server/src/main/java/org/opensearch/search/aggregations/bucket/terms/InternalMultiTerms.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ public InternalMultiTerms(
BucketOrder reduceOrder,
BucketOrder order,
Map<String, Object> metadata,
int shardSize,
boolean showTermDocCountError,
long otherDocCount,
long docCountError,
Expand All @@ -242,7 +243,7 @@ public InternalMultiTerms(
TermsAggregator.BucketCountThresholds bucketCountThresholds
) {
super(name, reduceOrder, order, bucketCountThresholds, metadata);
this.shardSize = bucketCountThresholds.getShardSize();
this.shardSize = shardSize;
this.showTermDocCountError = showTermDocCountError;
this.otherDocCount = otherDocCount;
this.termFormats = formats;
Expand Down Expand Up @@ -277,6 +278,7 @@ public InternalMultiTerms create(List<Bucket> buckets) {
reduceOrder,
order,
metadata,
shardSize,
showTermDocCountError,
otherDocCount,
docCountError,
Expand Down Expand Up @@ -354,6 +356,7 @@ protected InternalMultiTerms create(
reduceOrder,
order,
metadata,
shardSize,
showTermDocCountError,
otherDocCount,
docCountError,
Expand Down
5 changes: 5 additions & 0 deletions server/src/main/java/org/opensearch/search/aggregations/bucket/terms/LongTerms.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ public LongTerms(
BucketOrder order,
Map<String, Object> metadata,
DocValueFormat format,
int shardSize,
boolean showTermDocCountError,
long otherDocCount,
List<Bucket> buckets,
Expand All @@ -156,6 +157,7 @@ public LongTerms(
order,
metadata,
format,
shardSize,
showTermDocCountError,
otherDocCount,
buckets,
Expand Down Expand Up @@ -184,6 +186,7 @@ public LongTerms create(List<Bucket> buckets) {
order,
metadata,
format,
shardSize,
showTermDocCountError,
otherDocCount,
buckets,
Expand Down Expand Up @@ -212,6 +215,7 @@ protected LongTerms create(String name, List<Bucket> buckets, BucketOrder reduce
order,
getMetadata(),
format,
shardSize,
showTermDocCountError,
otherDocCount,
buckets,
Expand Down Expand Up @@ -287,6 +291,7 @@ static DoubleTerms convertLongTermsToDouble(LongTerms longTerms, DocValueFormat
longTerms.order,
longTerms.metadata,
longTerms.format,
longTerms.shardSize,
longTerms.showTermDocCountError,
longTerms.otherDocCount,
newBuckets,
Expand Down
1 change: 1 addition & 0 deletions ...c/main/java/org/opensearch/search/aggregations/bucket/terms/MapStringTermsAggregator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ StringTerms buildResult(long owningBucketOrd, long otherDocCount, StringTerms.Bu
order,
metadata(),
format,
bucketCountThresholds.getShardSize(),
showTermDocCountError,
otherDocCount,
Arrays.asList(topBuckets),
Expand Down
2 changes: 2 additions & 0 deletions ...r/src/main/java/org/opensearch/search/aggregations/bucket/terms/MultiTermsAggregator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ InternalMultiTerms buildResult(long owningBucketOrd, long otherDocCount, Interna
reduceOrder,
order,
metadata(),
bucketCountThresholds.getShardSize(),
showTermDocCountError,
otherDocCount,
0,
Expand All @@ -199,6 +200,7 @@ public InternalAggregation buildEmptyAggregation() {
order,
order,
metadata(),
bucketCountThresholds.getShardSize(),
showTermDocCountError,
0,
0,
Expand Down
32 changes: 30 additions & 2 deletions ...src/main/java/org/opensearch/search/aggregations/bucket/terms/NumericTermsAggregator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ LongTerms buildResult(long owningBucketOrd, long otherDocCount, LongTerms.Bucket
order,
metadata(),
format,
bucketCountThresholds.getShardSize(),
showTermDocCountError,
otherDocCount,
List.of(topBuckets),
Expand All @@ -407,7 +408,19 @@ LongTerms buildResult(long owningBucketOrd, long otherDocCount, LongTerms.Bucket

@Override
LongTerms buildEmptyResult() {
return new LongTerms(name, order, order, metadata(), format, showTermDocCountError, 0, emptyList(), 0, bucketCountThresholds);
return new LongTerms(
name,
order,
order,
metadata(),
format,
bucketCountThresholds.getShardSize(),
showTermDocCountError,
0,
emptyList(),
0,
bucketCountThresholds
);
}
}

Expand Down Expand Up @@ -464,6 +477,7 @@ DoubleTerms buildResult(long owningBucketOrd, long otherDocCount, DoubleTerms.Bu
order,
metadata(),
format,
bucketCountThresholds.getShardSize(),
showTermDocCountError,
otherDocCount,
List.of(topBuckets),
Expand All @@ -474,7 +488,19 @@ DoubleTerms buildResult(long owningBucketOrd, long otherDocCount, DoubleTerms.Bu

@Override
DoubleTerms buildEmptyResult() {
return new DoubleTerms(name, order, order, metadata(), format, showTermDocCountError, 0, emptyList(), 0, bucketCountThresholds);
return new DoubleTerms(
name,
order,
order,
metadata(),
format,
bucketCountThresholds.getShardSize(),
showTermDocCountError,
0,
emptyList(),
0,
bucketCountThresholds
);
}
}

Expand Down Expand Up @@ -530,6 +556,7 @@ UnsignedLongTerms buildResult(long owningBucketOrd, long otherDocCount, Unsigned
order,
metadata(),
format,
bucketCountThresholds.getShardSize(),
showTermDocCountError,
otherDocCount,
List.of(topBuckets),
Expand All @@ -546,6 +573,7 @@ UnsignedLongTerms buildEmptyResult() {
order,
metadata(),
format,
bucketCountThresholds.getShardSize(),
showTermDocCountError,
0,
emptyList(),
Expand Down
4 changes: 4 additions & 0 deletions server/src/main/java/org/opensearch/search/aggregations/bucket/terms/StringTerms.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ public StringTerms(
BucketOrder order,
Map<String, Object> metadata,
DocValueFormat format,
int shardSize,
boolean showTermDocCountError,
long otherDocCount,
List<Bucket> buckets,
Expand All @@ -148,6 +149,7 @@ public StringTerms(
order,
metadata,
format,
shardSize,
showTermDocCountError,
otherDocCount,
buckets,
Expand Down Expand Up @@ -176,6 +178,7 @@ public StringTerms create(List<Bucket> buckets) {
order,
metadata,
format,
shardSize,
showTermDocCountError,
otherDocCount,
buckets,
Expand Down Expand Up @@ -209,6 +212,7 @@ protected StringTerms create(String name, List<Bucket> buckets, BucketOrder redu
order,
getMetadata(),
format,
shardSize,
showTermDocCountError,
otherDocCount,
buckets,
Expand Down
17 changes: 9 additions & 8 deletions server/src/main/java/org/opensearch/search/aggregations/bucket/terms/TermsAggregator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,13 @@ public boolean equals(Object obj) {
}
}

// BucketCountThresholds type that throws an exception when shardMinDocCount and shardSize are accessed. This is used for
// deserialization on the coordinator during reduce as shardMinDocCount and shardSize should not be accessed this way on the
// coordinator.
/**
* BucketCountThresholds type that throws an exception when shardMinDocCount or shardSize are accessed. This is used for
* deserialization on the coordinator during reduce as shardMinDocCount and shardSize should not be accessed this way on the
* coordinator.
*
* @opensearch.internal
*/
public static class CoordinatorBucketCountThresholds extends BucketCountThresholds {

public CoordinatorBucketCountThresholds(long minDocCount, long shardMinDocCount, int requiredSize, int shardSize) {
Expand All @@ -207,15 +211,12 @@ public CoordinatorBucketCountThresholds(long minDocCount, long shardMinDocCount,

@Override
public long getShardMinDocCount() {
throw new AggregationExecutionException("shard_min_doc_count should not be accessed");
throw new AggregationExecutionException("shard_min_doc_count should not be accessed via CoordinatorBucketCountThresholds");
}

@Override
public int getShardSize() {
if (shardSize < 0) {
throw new AggregationExecutionException("Invalid shard_size accessed");
}
return shardSize;
throw new AggregationExecutionException("shard_size should not be accessed via CoordinatorBucketCountThresholds");
}
}

Expand Down
5 changes: 5 additions & 0 deletions server/src/main/java/org/opensearch/search/aggregations/bucket/terms/UnsignedLongTerms.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ public UnsignedLongTerms(
BucketOrder order,
Map<String, Object> metadata,
DocValueFormat format,
int shardSize,
boolean showTermDocCountError,
long otherDocCount,
List<Bucket> buckets,
Expand All @@ -135,6 +136,7 @@ public UnsignedLongTerms(
order,
metadata,
format,
shardSize,
showTermDocCountError,
otherDocCount,
buckets,
Expand Down Expand Up @@ -163,6 +165,7 @@ public UnsignedLongTerms create(List<Bucket> buckets) {
order,
metadata,
format,
shardSize,
showTermDocCountError,
otherDocCount,
buckets,
Expand Down Expand Up @@ -191,6 +194,7 @@ protected UnsignedLongTerms create(String name, List<Bucket> buckets, BucketOrde
order,
getMetadata(),
format,
shardSize,
showTermDocCountError,
otherDocCount,
buckets,
Expand Down Expand Up @@ -266,6 +270,7 @@ static DoubleTerms convertUnsignedLongTermsToDouble(UnsignedLongTerms unsignedLo
unsignedLongTerms.order,
unsignedLongTerms.metadata,
unsignedLongTerms.format,
unsignedLongTerms.shardSize,
unsignedLongTerms.showTermDocCountError,
unsignedLongTerms.otherDocCount,
newBuckets,
Expand Down
2 changes: 2 additions & 0 deletions server/src/test/java/org/opensearch/search/aggregations/InternalAggregationsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public void testNonFinalReduceTopLevelPipelineAggs() {
BucketOrder.key(true),
Collections.emptyMap(),
DocValueFormat.RAW,
25,
false,
10,
Collections.emptyList(),
Expand All @@ -97,6 +98,7 @@ public void testFinalReduceTopLevelPipelineAggs() {
BucketOrder.key(true),
Collections.emptyMap(),
DocValueFormat.RAW,
25,
false,
10,
Collections.emptyList(),
Expand Down
Loading

0 comments on commit aac2559

Please sign in to comment.