Skip to content

Commit 71a7472

Browse files
jed326Jay Deng
authored andcommitted
Correctly calculate doc_count_error at the slice level for concurrent segment search. Change slice_size heuristic to be equal to shard_size.
Signed-off-by: Jay Deng <[email protected]>
1 parent 10be2ef commit 71a7472

File tree

7 files changed

+403
-21
lines changed

7 files changed

+403
-21
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
192192
- Restore support for Java 8 for RestClient ([#11562](https://github.com/opensearch-project/OpenSearch/pull/11562))
193193
- Add deleted doc count in _cat/shards ([#11678](https://github.com/opensearch-project/OpenSearch/pull/11678))
194194
- Capture information for additional query types and aggregation types ([#11582](https://github.com/opensearch-project/OpenSearch/pull/11582))
195+
- Use slice_size == shard_size heuristic in terms aggs for concurrent segment search and properly calculate the doc_count_error ([#11732](https://github.com/opensearch-project/OpenSearch/pull/11732))
195196

196197
### Deprecated
197198

server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/ShardSizeTermsIT.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ public void testShardSizeEqualsSizeString() throws Exception {
8686
terms("keys").field("key")
8787
.size(3)
8888
.shardSize(3)
89+
.showTermDocCountError(true)
8990
.collectMode(randomFrom(SubAggCollectionMode.values()))
9091
.order(BucketOrder.count(false))
9192
)
@@ -98,8 +99,11 @@ public void testShardSizeEqualsSizeString() throws Exception {
9899
expected.put("1", 8L);
99100
expected.put("3", 8L);
100101
expected.put("2", 4L);
102+
Long expectedDocCount;
101103
for (Terms.Bucket bucket : buckets) {
102-
assertThat(bucket.getDocCount(), equalTo(expected.get(bucket.getKeyAsString())));
104+
expectedDocCount = expected.get(bucket.getKeyAsString());
105+
// Doc count can vary when using concurrent segment search. See https://github.com/opensearch-project/OpenSearch/issues/11680
106+
assertTrue((bucket.getDocCount() == expectedDocCount) || bucket.getDocCount() + bucket.getDocCountError() >= expectedDocCount);
103107
}
104108
}
105109

@@ -221,6 +225,7 @@ public void testShardSizeEqualsSizeLong() throws Exception {
221225
terms("keys").field("key")
222226
.size(3)
223227
.shardSize(3)
228+
.showTermDocCountError(true)
224229
.collectMode(randomFrom(SubAggCollectionMode.values()))
225230
.order(BucketOrder.count(false))
226231
)
@@ -233,8 +238,11 @@ public void testShardSizeEqualsSizeLong() throws Exception {
233238
expected.put(1, 8L);
234239
expected.put(3, 8L);
235240
expected.put(2, 4L);
241+
Long expectedDocCount;
236242
for (Terms.Bucket bucket : buckets) {
237-
assertThat(bucket.getDocCount(), equalTo(expected.get(bucket.getKeyAsNumber().intValue())));
243+
expectedDocCount = expected.get(bucket.getKeyAsNumber().intValue());
244+
// Doc count can vary when using concurrent segment search. See https://github.com/opensearch-project/OpenSearch/issues/11680
245+
assertTrue((bucket.getDocCount() == expectedDocCount) || bucket.getDocCount() + bucket.getDocCountError() >= expectedDocCount);
238246
}
239247
}
240248

@@ -355,6 +363,7 @@ public void testShardSizeEqualsSizeDouble() throws Exception {
355363
terms("keys").field("key")
356364
.size(3)
357365
.shardSize(3)
366+
.showTermDocCountError(true)
358367
.collectMode(randomFrom(SubAggCollectionMode.values()))
359368
.order(BucketOrder.count(false))
360369
)
@@ -367,8 +376,11 @@ public void testShardSizeEqualsSizeDouble() throws Exception {
367376
expected.put(1, 8L);
368377
expected.put(3, 8L);
369378
expected.put(2, 4L);
379+
Long expectedDocCount;
370380
for (Terms.Bucket bucket : buckets) {
371-
assertThat(bucket.getDocCount(), equalTo(expected.get(bucket.getKeyAsNumber().intValue())));
381+
expectedDocCount = expected.get(bucket.getKeyAsNumber().intValue());
382+
// Doc count can vary when using concurrent segment search. See https://github.com/opensearch-project/OpenSearch/issues/11680
383+
assertTrue((bucket.getDocCount() == expectedDocCount) || bucket.getDocCount() + bucket.getDocCountError() >= expectedDocCount);
372384
}
373385
}
374386

server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/TermsDocCountErrorIT.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,16 @@ public void setupSuiteScopeCluster() throws Exception {
225225
}
226226

227227
indexRandom(true, builders);
228-
indexRandomForMultipleSlices("idx");
229228
ensureSearchable();
229+
230+
// Force merge each shard down to 1 segment to verify results are the same between concurrent and non-concurrent search paths, else
231+
// for concurrent segment search there will be additional error introduced during the slice level reduce and thus different buckets,
232+
// doc_counts, and doc_count_errors may be returned. This test serves to verify that the doc_count_error is the same between
233+
// concurrent and non-concurrent search in the 1 slice case. TermsFixedDocCountErrorIT verifies that the doc count error is
234+
// correctly calculated for concurrent segment search at the slice level.
235+
// See https://github.com/opensearch-project/OpenSearch/issues/11680"
236+
forceMerge(1);
237+
Thread.sleep(5000); // Sleep 5s to ensure force merge completes
230238
}
231239

232240
private void assertDocCountErrorWithinBounds(int size, SearchResponse accurateResponse, SearchResponse testResponse) {

0 commit comments

Comments
 (0)