-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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.
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
Star tree mapping changes #14261
Star tree mapping changes #14261
Conversation
❌ Gradle check result for 1bb63c7: 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? |
682ba9f
to
79f9970
Compare
❌ Gradle check result for 79f9970: 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? |
❌ Gradle check result for 682ba9f: 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? |
79f9970
to
ccccf2d
Compare
❌ Gradle check result for ccccf2d: 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: Bharathwaj G <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall. Few minor comments only.
server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/compositeindex/datacube/Dimension.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/compositeindex/datacube/DateDimension.java
Outdated
Show resolved
Hide resolved
...n/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeFieldConfiguration.java
Show resolved
Hide resolved
...c/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeIndexSettings.java
Show resolved
Hide resolved
Signed-off-by: Bharathwaj G <[email protected]>
92b8480
to
0a28513
Compare
❌ Gradle check result for 0a28513: 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? |
❌ Gradle check result for 92b8480: 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? |
...st/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeMappingIntegTests.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Bharathwaj G <[email protected]>
❌ Gradle check result for 7ae98db: 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/internalClusterTest/java/org/opensearch/index/mapper/StarTreeMapper1IT.java
Outdated
Show resolved
Hide resolved
7ae98db
to
ed01cc0
Compare
Signed-off-by: Bharathwaj G <[email protected]>
Description
This PR contains the changes for star tree field mapping with feature flag protection. To make the changes extensible to other multi-field/composite/datacube type of indices in future, I've generalized the implementation under 'CompositeIndex'.
Mappings have a new section 'composite' under which multi field mappings such as 'star tree' can be defined. All the fields associated with metrics and dimensions must be present in 'properties' section. [ This is even applicable for update mapping API - but right now , its blocked for star tree - star tree can be only specified during creation of index ]
Min version :
}
And the defaults will be filled for the above fields.
NOTE : We will tune the defaults throughout the development of the star tree index.
Defaults :
Timestamp field intervals = [ Minute, Hour ]
Default Metrics for each metric field = [ SUM, COUNT, AVG, MIN, MAX ]
Complete version :
Validations
Apart from basic validations based on user input :
Open questions
Related Issues
#13875
#14386
Check List
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.