Add things to know for concurrent segment search #6362
Conversation
jed326
requested review from
hdhalter,
kolchfa-aws,
Naarcha-AWS,
vagimeli,
AMoo-Miki,
natebower and
dlvenable
as code owners
hdhalter
added
3 - Tech review
Signed-off-by: Jay Deng <[email protected]>
jed326
force-pushed
the
cs-things-to-know
branch
from
1fcb205 to
07686f3
Compare
Co-authored-by: kolchfa-aws <[email protected]> Signed-off-by: Jay Deng <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>
There was a problem hiding this comment.
@jed326 @kolchfa-aws Please see my comments and changes and let me know if you have any questions. Thanks!
| Parent aggregations on [join]({{site.url}}{{site.baseurl}}/field-types/supported-field-types/join/) fields do not support the concurrent search model. Thus, if a search request contains a parent aggregation, the aggregation will be executed using the non-concurrent path even if concurrent segment search is enabled at the cluster level. | ||
| The following aggregations do not support the concurrent search model. If a search request contains one of these aggregations, the request will be executed using the non-concurrent path even if concurrent segment search is enabled at the cluster level or index level. | ||
| - Parent aggregations on [join]({{site.url}}{{site.baseurl}}/field-types/supported-field-types/join/) fields. See [GitHub issue](https://github.com/opensearch-project/OpenSearch/issues/9316). | ||
| - Sampler and Diversified Sampler aggregations. See [GitHub issue](https://github.com/opensearch-project/OpenSearch/issues/110750). |
There was a problem hiding this comment.
Confirm that "Diversified Sampler" is meant to be capitalized and should not be formatted differently.
There was a problem hiding this comment.
It should probably just be "Diversified sampler"
There was a problem hiding this comment.
Or is it a common noun and should just be "diversified sampler"?
There was a problem hiding this comment.
The name of the aggregation type is "Diversified sampler". Similar to "Significant terms" vs. "Terms" aggregations.
See: https://opensearch.org/docs/latest/aggregations/bucket/diversified-sampler/
There was a problem hiding this comment.
Got it, it should be in code font, then.
|
|
||
| ### Terms aggregations | ||
|
|
||
| When performing concurrent segment search, the `shard_size` parameter will be applied at the segment slice level. This may introduce additional document count error, which is correctly calculated by the `doc_count_error_upper_bound` response parameter for such cases. |
There was a problem hiding this comment.
Is "additional" necessary here, or should it just be "a document count error"? I don't believe we've referenced an initial error.
There was a problem hiding this comment.
@jed326 Could you address this question?
There was a problem hiding this comment.
For non-concurrent search doc count error is introduced when the shards eliminate candidate buckets and we just added some more detailed documentation for this in #6355.
The "additional" here refers to how in additional to the doc count error introduced by the shards, there is (potentially) additional document count error introduced by the slices for concurrent segment search.
There was a problem hiding this comment.
Ok, then if we want to keep the word "additional," we need to explain that, or the reader may ask "Where was the first error?"
|
|
||
| When performing concurrent segment search, the `shard_size` parameter will be applied at the segment slice level. This may introduce additional document count error, which is correctly calculated by the `doc_count_error_upper_bound` response parameter for such cases. | ||
|
|
||
| For more details on how `shard_size` can affect both `doc_count_error_upper_bound` and collect buckets, see [GitHub issue](https://github.com/opensearch-project/OpenSearch/issues/11680#issuecomment-1885882985). |
There was a problem hiding this comment.
Please replace "collect" with the appropriate word. Collector? Collection?
There was a problem hiding this comment.
@jed326 Could you address this question?
There was a problem hiding this comment.
It should be "collected"
Co-authored-by: Nathan Bower <[email protected]> Signed-off-by: kolchfa-aws <[email protected]>
Signed-off-by: Jay Deng <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>
hdhalter
added
3 - Done
Description
Add "things to know" section for concurrent segment search.
Checklist
For more information on following Developer Certificate of Origin and signing off your commits, please check here.