Add customized result index in data source etc #2212
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kaituo
requested review from
pjfitzgibbons,
ps48,
kavithacm,
derek-ho,
joshuali925,
dai-chen,
YANG-DB,
rupal-bq,
mengweieric,
vmmusings,
Swiddis,
penghuo,
seankao-az,
MaxKsyunz,
Yury-Fridlyand,
anirudha,
forestmvey,
acarbonetto and
GumpacG
as code owners
This PR - Introduce `spark.flint.datasource.name` parameter for data source specification. - Enhance data source creation to allow custom result indices; fallback to default if unavailable. - Include error details in the async result response, sourced from the result index. - Migrate to `org.apache.spark.sql.FlintJob` following updates in OpenSearch-Spark. - Populate query status from result index over EMR-S job status to handle edge cases where jobs may succeed, but queries or mappings fail. Testing done: 1. manual testing including if with or without custom result index async query still works 2. added new unit tests Signed-off-by: Kaituo Li <kaituo@amazon.com>
Codecov Report
@@ Coverage Diff @@
## main #2212 +/- ##
============================================
- Coverage 96.37% 96.36% -0.01%
- Complexity 4722 4727 +5
============================================
Files 439 439
Lines 12650 12686 +36
Branches 869 872 +3
============================================
+ Hits 12191 12225 +34
- Misses 450 452 +2
Partials 9 9
Flags with carried forward coverage won't be shown. Click here to find out more.
|
penghuo
reviewed
Oct 5, 2023
| @@ -8,13 +8,22 @@ | |||
| public class SparkConstants { | |||
There was a problem hiding this comment.
spark/src/main/java/org/opensearch/sql/spark/dispatcher/SparkQueryDispatcher.java
Show resolved
Hide resolved
vmmusings
reviewed
Oct 5, 2023
| @@ -14,4 +14,6 @@ properties: | |||
| keyword: | |||
| type: keyword | |||
| connector: | |||
| type: keyword | |||
| resultIndex: | |||
There was a problem hiding this comment.
yeah, we need to save it and retrieve it later.
There was a problem hiding this comment.
keyword looks good, no full text search required.
Signed-off-by: Kaituo Li <kaituo@amazon.com>
penghuo
reviewed
Oct 5, 2023
| public static final String SPARK_RESPONSE_BUFFER_INDEX_NAME = ".query_execution_result"; | ||
| // TODO should be replaced with mvn jar. | ||
| public static final String FLINT_INTEGRATION_JAR = | ||
| "s3://spark-datasource/flint-spark-integration-assembly-0.1.0-SNAPSHOT.jar"; | ||
| "s3://flint-data-dp-eu-west-1-beta/code/flint/sql-job.jar"; |
There was a problem hiding this comment.
revert this, it is for Spark datasource, not used in flint.
penghuo
previously approved these changes
Oct 5, 2023
vmmusings
previously approved these changes
Oct 5, 2023
| // a job is successful does not mean there is no error in execution. For example, even if result | ||
| // index mapping | ||
| // is incorrect, we still write query result and let the job finish. | ||
| if (result.has(DATA_FIELD)) { |
There was a problem hiding this comment.
We are setting result object only when job status from EMR is success.
In case of index query, EMR JobRUn State is RUNNING, but the status should be succesful in the result.
There was a problem hiding this comment.
Probably we can merge and handle this use case as bug separately in a separate PR.
There was a problem hiding this comment.
let's track in issue #2214
There was a problem hiding this comment.
yeah, we can handle this use case a bug separately.
iiiiii-off-by: Kaituo Li <kaituo@amazon.com>
penghuo
approved these changes
Oct 5, 2023
vmmusings
approved these changes
Oct 5, 2023
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Oct 5, 2023
* Add customized result index in data source etc This PR - Introduce `spark.flint.datasource.name` parameter for data source specification. - Enhance data source creation to allow custom result indices; fallback to default if unavailable. - Include error details in the async result response, sourced from the result index. - Migrate to `org.apache.spark.sql.FlintJob` following updates in OpenSearch-Spark. - Populate query status from result index over EMR-S job status to handle edge cases where jobs may succeed, but queries or mappings fail. Testing done: 1. manual testing including if with or without custom result index async query still works 2. added new unit tests Signed-off-by: Kaituo Li <kaituo@amazon.com> * address comments Signed-off-by: Kaituo Li <kaituo@amazon.com> * revert incorrect change iiiiii-off-by: Kaituo Li <kaituo@amazon.com> --------- Signed-off-by: Kaituo Li <kaituo@amazon.com> (cherry picked from commit 70450e4) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
penghuo
pushed a commit
that referenced
this pull request
Oct 5, 2023
* Add customized result index in data source etc This PR - Introduce `spark.flint.datasource.name` parameter for data source specification. - Enhance data source creation to allow custom result indices; fallback to default if unavailable. - Include error details in the async result response, sourced from the result index. - Migrate to `org.apache.spark.sql.FlintJob` following updates in OpenSearch-Spark. - Populate query status from result index over EMR-S job status to handle edge cases where jobs may succeed, but queries or mappings fail. Testing done: 1. manual testing including if with or without custom result index async query still works 2. added new unit tests * address comments * revert incorrect change iiiiii-off-by: Kaituo Li <kaituo@amazon.com> --------- (cherry picked from commit 70450e4) Signed-off-by: Kaituo Li <kaituo@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR
spark.flint.datasource.nameparameter for data source specification.org.apache.spark.sql.FlintJobfollowing updates in OpenSearch-Spark.Testing done:
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.