[Star Tree] [Search] Support for metric aggregations with/without term query #15289

sandeshkr419 · 2024-08-19T03:09:02Z

Disclaimer

These changes are built on top of unmerged/in-review indexing changes, Reviewers kindly ignore this commit while reviewing this change. When the depending changes are merged, will remove the Do not merge from title, and add chngelog to the PR - avoiding adding now to avoid unnecessary rebasing/conflicts.

Description

For an index supporting star-tree composite index, this changes tries to achieve resolving a metric aggregation with/without a numeric terms query with the help of star-tree.

In present state, the PR capture changes for sum, max, min, avg, value-count aggregation. ~~Once the high level changes in sum aggregation are reviewed, will increment with other aggregations.~~

Approach

A new StarTreeQuery is introduced which helps resolve to star-tree documents. This star-tree query is formed (if it can be) at the shard level, this is not done at coordinator level to avoid node to node transportation. Also, all the information is present at shard level and OpenSearch does majority of query rewrite at shard level itself. This star tree query is encapsulated in an OriginalOrStarTreeQuery which helps preserve the original query alongwith the new star tree query. This encapsulation is done so as to preserve both the queries and decision whether to use which query can be taken at a segment level.

High Level Operations (to make code reviewing simple):

Parsing a Search Query, to decide whether or not to use star tree flow: SearchService.java, QueryShardContext.java
Parsed SearchQuery: StarTreeQuery.java, OriginalOrStarTreeQuery.java
Utilities for resolving star-tree given certain predicates (derived from term query of search request), and tree traversal - StarTreeFilter.java
Changes in 5 metric Aggregators - SumAggregator.java, MaxAggregator.java, MinAggregator.java, AvgAggregator.java, ValueCountAggregator.java
Supporting changes - changes in access level of aggregator factory classes so identify correct aggregations during request parsing.

TODO in this PR:

Test cases to be added.
Support for max, min, count, avg metric aggregations.

This PR depends on #14809, therefore the depending unmerged changes have been utilized for now in my private fork to discuss the changes in parallel.

Example query shape:

No query + Agg:

{
    "size": 0,
    "aggs": {
                        "sum_status": {
                            "sum": {
                                "field": "size"
                            }
                        }
                    }
}

Original Response:

{
    "took": 8378,
    "timed_out": false,
    "_shards": {
        "total": 1,
        "successful": 1,
        "skipped": 0,
        "failed": 0
    },
    "hits": {
        "total": {
            "value": 1009,
            "relation": "eq"
        },
        "max_score": null,
        "hits": []
    },
    "aggregations": {
        "sum_status": {
            "value": 615579.0
        }
    }
}

Star Tree Response:

{
    "took": 16394,
    "timed_out": false,
    "_shards": {
        "total": 1,
        "successful": 1,
        "skipped": 0,
        "failed": 0
    },
    "hits": {
        "total": {
            "value": 1,
            "relation": "eq"
        },
        "max_score": null,
        "hits": []
    },
    "aggregations": {
        "sum_status": {
            "value": 615579.0
        }
    }
}

Request:

{
    "query": {
        "term": {
            "status": 200
        }
    },
    "size": 0,
    "aggs": {
                        "sum_status": {
                            "sum": {
                                "field": "size"
                            }
                        }
                    }
}

Original Response:

{
    "took": 4038,
    "timed_out": false,
    "_shards": {
        "total": 1,
        "successful": 1,
        "skipped": 0,
        "failed": 0
    },
    "hits": {
        "total": {
            "value": 42,
            "relation": "eq"
        },
        "max_score": null,
        "hits": []
    },
    "aggregations": {
        "sum_status": {
            "value": 24745.0
        }
    }
}

Star Tree Flow Response:

{
    "took": 21120,
    "timed_out": false,
    "_shards": {
        "total": 1,
        "successful": 1,
        "skipped": 0,
        "failed": 0
    },
    "hits": {
        "total": {
            "value": 1,
            "relation": "eq"
        },
        "max_score": null,
        "hits": []
    },
    "aggregations": {
        "sum_status": {
            "value": 24745.0
        }
    }
}

Multi-aggs in a single request:

{ 
    "query": {
        "term": {
            "status": 201
        }
    },
    "size": 0,
    "aggs": {
                        "sum_status": {
                            "sum": {
                                "field": "size"
                            }
                        },
                        "max_status": {
                            "max": {
                                "field": "size"
                            }
                        },
                        "min_status": {
                            "min": {
                                "field": "size"
                            }
                        },
                        "avg_status": {
                            "avg": {
                                "field": "size"
                            }
                        },
                        "count_status": {
                            "value_count": {
                                "field": "size"
                            }
                        }

                    }
}

Star Tree Response:

{
    "took": 28,
    "timed_out": false,
    "_shards": {
        "total": 1,
        "successful": 1,
        "skipped": 0,
        "failed": 0
    },
    "hits": {
        "total": {
            "value": 3,
            "relation": "eq"
        },
        "max_score": null,
        "hits": []
    },
    "aggregations": {
        "max_status": {
            "value": 1000.0
        },
        "sum_status": {
            "value": 32154.0
        },
        "count_status": {
            "value": 53
        },
        "avg_status": {
            "value": 606.6792452830189
        },
        "min_status": {
            "value": 200.0
        }
    }
}

Unsupported Metric Operation via star-tree:

{ 
    "query": {
        "term": {
            "status": 201
        }
    },
    "size": 0,
    "aggs": {
                        "sum_status": {
                            "sum": {
                                "field": "size"
                            }
                        },
                        "max_status": {
                            "max": {
                                "field": "size"
                            }
                        },
                        "min_status": {
                            "min": {
                                "field": "size"
                            }
                        },
                        "avg_status": {
                            "avg": {
                                "field": "size"
                            }
                        },
                        "count_status": {
                            "stats": {
                                "field": "size"
                            }
                        }

                    }
}

Defaults to original code-flow (verified by hits reported):

{
    "took": 137433,
    "timed_out": false,
    "_shards": {
        "total": 1,
        "successful": 1,
        "skipped": 0,
        "failed": 0
    },
    "hits": {
        "total": {
            "value": 53,
            "relation": "eq"
        },
        "max_score": null,
        "hits": []
    },
    "aggregations": {
        "max_status": {
            "value": 1000.0
        },
        "sum_status": {
            "value": 32154.0
        },
        "count_status": {
            "count": 53,
            "min": 200.0,
            "max": 1000.0,
            "avg": 606.6792452830189,
            "sum": 32154.0
        },
        "avg_status": {
            "value": 606.6792452830189
        },
        "min_status": {
            "value": 200.0
        }
    }
}

Approach:

The query shape is identified at the shard level (SearchService.java) and the query/aggregation (if can be resolved by star-tree) is parsed to a star-tree query.
The star-tree query is wrapped around OriginalOrStarTreeQuery to preserve the original query - this is because the decision to decide which implementation (default/startree) to use can be taken for a segment level.
If star-tree can be utilized to answer the query, the star-tree document set is then collected by the relevant aggregator/collector. In this POC, I have made changes to SumAggregator to demonstrate the flow of changes.

Related Issues

#15257

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)
Public documentation issue/PR created

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

github-actions · 2024-08-19T03:43:19Z

❌ Gradle check result for ad54ace: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-08-19T18:04:26Z

❌ Gradle check result for 6c6be02: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-08-19T18:25:20Z

❌ Gradle check result for 995db38: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-08-19T18:29:23Z

❌ Gradle check result for 885a383: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-08-19T19:23:36Z

❌ Gradle check result for 9491aae: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-08-19T21:36:53Z

❌ Gradle check result for a01c6e2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

server/src/main/java/org/opensearch/index/query/QueryShardContext.java

server/src/main/java/org/opensearch/search/startree/StarTreeFilter.java

server/src/main/java/org/opensearch/search/startree/StarTreeQuery.java

github-actions · 2024-08-23T03:19:36Z

❌ Gradle check result for e4a270a: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Sandesh Kumar <[email protected]>

github-actions · 2024-10-15T07:22:08Z

❌ Gradle check result for 2d034c8: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-10-15T08:29:21Z

✅ Gradle check result for 2d034c8: SUCCESS

sandeshkr419 · 2024-10-15T23:37:35Z

@msfroh @jainankitk @sarthakaggarwal97 @bharath-techie

Thanks for the comments and suggestions on PR. I have addressed those comments and polished up the PR. Please let me know if there are more comments.

bharath-techie · 2024-10-17T03:38:54Z

...dex/compositeindex/datacube/startree/utils/iterator/SortedNumericStarTreeValuesIterator.java

@@ -29,4 +29,12 @@ public SortedNumericStarTreeValuesIterator(DocIdSetIterator docIdSetIterator) {
 public long nextValue() throws IOException {
 return ((SortedNumericDocValues) docIdSetIterator).nextValue();
 }
+
+ public int valuesCount() throws IOException {


nit : can we have 1-1 mapping , lets have the same name docValueCount

These are "star tree values", though, right?

I wanted to avoid docValue within the method name.

We also didn't had 1-1 mapping in the parent class as well.

public int entryId() { return docIdSetIterator.docID(); } public int nextEntry() throws IOException { return docIdSetIterator.nextDoc(); }

valuesCount() looked kind of okay to me, but okay to rename otherwise.

My bad on not giving context . I mean we can have 'entryValueCount' going by that convention - the thing is we have getValueCount and getDocValueCount for sortedSetDocValues - so just valuesCount will have issues.

/** * Retrieves the number of unique ords for the current document. This must always be greater than * zero. It is illegal to call this method after {@link #advanceExact(int)} returned {@code * false}. */ public abstract int docValueCount(); /** * Returns the number of unique values. * * @return number of unique values in this SortedDocValues. This is also equivalent to one plus * the maximum ordinal. */ public abstract long getValueCount();

bharath-techie

Thanks for the changes. LGTM

...in/java/org/opensearch/index/compositeindex/datacube/startree/utils/StarTreeQueryHelper.java

msfroh · 2024-10-17T20:37:48Z

...in/java/org/opensearch/index/compositeindex/datacube/startree/utils/StarTreeQueryHelper.java

+ if (!supportedDimensions.contains(field)) {
+ return null;
+ }
+ long inputQueryVal = Long.parseLong(tq.value().toString());


@sandeshkr419, what happens if you pass "6" as your value to the TermQueryBuilder in your integ test?

I'm pretty sure it will successfully create the TermQueryBuilder and doToQuery will convert it to a Lucene query correctly. (Technically, it won't create a TermQuery, but rather a PointRangeQuery over the range [6,6].)

msfroh · 2024-10-17T20:38:51Z

...dex/compositeindex/datacube/startree/utils/iterator/SortedNumericStarTreeValuesIterator.java

@@ -29,4 +29,12 @@ public SortedNumericStarTreeValuesIterator(DocIdSetIterator docIdSetIterator) {
 public long nextValue() throws IOException {
 return ((SortedNumericDocValues) docIdSetIterator).nextValue();
 }
+
+ public int valuesCount() throws IOException {


These are "star tree values", though, right?

server/src/main/java/org/opensearch/search/startree/StarTreeFilter.java

Signed-off-by: Sandesh Kumar <[email protected]>

github-actions · 2024-10-19T05:28:39Z

❌ Gradle check result for bdabc51: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-10-19T06:26:02Z

❌ Gradle check result for bdabc51: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-10-19T07:24:47Z

✅ Gradle check result for bdabc51: SUCCESS

sandeshkr419 changed the title ~~St0~~ Star Tree Request/Response structure Aug 19, 2024

sandeshkr419 mentioned this pull request Aug 19, 2024

Star Tree Request/Response structure sandeshkr419/OpenSearch#227

Closed

8 tasks

sandeshkr419 force-pushed the st0 branch from ad54ace to 6c6be02 Compare August 19, 2024 17:57

sandeshkr419 force-pushed the st0 branch 2 times, most recently from 995db38 to 885a383 Compare August 19, 2024 18:18

sandeshkr419 force-pushed the st0 branch from 885a383 to 9491aae Compare August 19, 2024 18:45

sandeshkr419 force-pushed the st0 branch from 9491aae to a01c6e2 Compare August 19, 2024 21:05

bharath-techie reviewed Aug 21, 2024

View reviewed changes

server/src/main/java/org/opensearch/index/query/QueryShardContext.java Outdated Show resolved Hide resolved

bharath-techie reviewed Aug 21, 2024

View reviewed changes

server/src/main/java/org/opensearch/search/startree/StarTreeFilter.java Outdated Show resolved Hide resolved

bharath-techie reviewed Aug 21, 2024

View reviewed changes

server/src/main/java/org/opensearch/search/startree/StarTreeQuery.java Outdated Show resolved Hide resolved

bharath-techie reviewed Aug 21, 2024

View reviewed changes

server/src/main/java/org/opensearch/search/startree/StarTreeQuery.java Outdated Show resolved Hide resolved

sandeshkr419 force-pushed the st0 branch from a01c6e2 to e4a270a Compare August 23, 2024 03:09

sandeshkr419 force-pushed the st0 branch 2 times, most recently from d812134 to addb332 Compare August 25, 2024 09:59

sandeshkr419 marked this pull request as ready for review August 25, 2024 10:01

sandeshkr419 requested review from anasalkouz, andrross, ashking94, Bukhtawar, CEHENKLE, dblock, dbwiddis and gbbafna as code owners August 25, 2024 10:01

sandeshkr419 added 13 commits October 14, 2024 23:39

temp temp

8cc1604

Signed-off-by: Sandesh Kumar <[email protected]>

temp temp temp

8d6af54

Signed-off-by: Sandesh Kumar <[email protected]>

fix initial values in aggs

908bf19

Signed-off-by: Sandesh Kumar <[email protected]>

adding tests

eeca2f8

Signed-off-by: Sandesh Kumar <[email protected]>

refactoring

5970199

Signed-off-by: Sandesh Kumar <[email protected]>

max/min agg fix, spotless, test fixes

422c6e6

Signed-off-by: Sandesh Kumar <[email protected]>

spotless fix, test refactoring

5eb56f5

Signed-off-by: Sandesh Kumar <[email protected]>

avg aggregator fix, iterators refactoring

48b067f

Signed-off-by: Sandesh Kumar <[email protected]>

making StarTreeFieldType back to final

c8e0d47

Signed-off-by: Sandesh Kumar <[email protected]>

move value cache to star tree context + other comments

a08aa9f

Signed-off-by: Sandesh Kumar <[email protected]>

refactor cache map to cache array

66876e8

Signed-off-by: Sandesh Kumar <[email protected]>

refactor star tree filter

fc2ea8b

Signed-off-by: Sandesh Kumar <[email protected]>

Rebasing & adding star tree filter tests

2d034c8

Signed-off-by: Sandesh Kumar <[email protected]>

sandeshkr419 force-pushed the st0 branch from b711f22 to 2d034c8 Compare October 15, 2024 06:40

sandeshkr419 closed this Oct 15, 2024

sandeshkr419 reopened this Oct 15, 2024

bharath-techie reviewed Oct 17, 2024

View reviewed changes

bharath-techie approved these changes Oct 17, 2024

View reviewed changes

sarthakaggarwal97 approved these changes Oct 17, 2024

View reviewed changes

msfroh reviewed Oct 17, 2024

View reviewed changes

Merge branch 'main' into st0

bdabc51

Signed-off-by: Sandesh Kumar <[email protected]>

sandeshkr419 closed this Oct 19, 2024

sandeshkr419 reopened this Oct 19, 2024

opensearch-ci-bot mentioned this pull request Oct 19, 2024

[AUTOCUT] Gradle Check Flaky Test Report for SearchOnlyReplicaIT #15812

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Star Tree] [Search] Support for metric aggregations with/without term query #15289

[Star Tree] [Search] Support for metric aggregations with/without term query #15289

sandeshkr419 commented Aug 19, 2024 •

edited

Loading

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 23, 2024

github-actions bot commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

sandeshkr419 commented Oct 15, 2024

bharath-techie Oct 17, 2024

msfroh Oct 17, 2024

sandeshkr419 Oct 18, 2024

bharath-techie Oct 18, 2024 •

edited

Loading

bharath-techie left a comment

msfroh Oct 17, 2024

msfroh Oct 17, 2024

github-actions bot commented Oct 19, 2024

github-actions bot commented Oct 19, 2024

github-actions bot commented Oct 19, 2024

[Star Tree] [Search] Support for metric aggregations with/without term query #15289

Are you sure you want to change the base?

[Star Tree] [Search] Support for metric aggregations with/without term query #15289

Conversation

sandeshkr419 commented Aug 19, 2024 • edited Loading

Disclaimer

Description

Approach

Approach:

Related Issues

Check List

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 23, 2024

github-actions bot commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

sandeshkr419 commented Oct 15, 2024

bharath-techie Oct 17, 2024

Choose a reason for hiding this comment

msfroh Oct 17, 2024

Choose a reason for hiding this comment

sandeshkr419 Oct 18, 2024

Choose a reason for hiding this comment

bharath-techie Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

bharath-techie left a comment

Choose a reason for hiding this comment

msfroh Oct 17, 2024

Choose a reason for hiding this comment

msfroh Oct 17, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 19, 2024

github-actions bot commented Oct 19, 2024

github-actions bot commented Oct 19, 2024

sandeshkr419 commented Aug 19, 2024 •

edited

Loading

bharath-techie Oct 18, 2024 •

edited

Loading