Skip to content

[Pull-based Ingestion] disable push-API for indexing in ingestionEngine #17768

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

Merged
merged 11 commits into from
Apr 8, 2025

Conversation

yupeng9
Copy link
Contributor

@yupeng9 yupeng9 commented Apr 2, 2025

Description

The indexing API call will fail for ingestion engine. For example

curl -X PUT -H 'Content-Type: application/json' http://localhost:9200/my-index/_doc/1 --data '
{
  "name": "John Doe",
  "age": 24
}
'

returns

{"error":{"root_cause":[{"type":"unsupported_operation_exception","reason":"push-based indexing is not supported in ingestion engine, use streaming source instead"}],"type":"unsupported_operation_exception","reason":"push-based indexing is not supported in ingestion engine, use streaming source instead"},"status":400}%

Related Issues

#16930

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.

@andrross
Copy link
Member

andrross commented Apr 2, 2025

{"error":{"root_cause":[{"type":"unsupported_encoding_exception","reason":"push-based indexing is not supported in ingestion engine, use streaming source instead"}],"type":"unsupported_encoding_exception","reason":"push-based indexing is not supported in ingestion engine, use streaming source instead"},"status":500}%

I think this should be a 4xx error code, not a 500, right?

Copy link
Contributor

github-actions bot commented Apr 2, 2025

❕ Gradle check result for 04595e1: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Copy link

codecov bot commented Apr 2, 2025

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.

Project coverage is 72.40%. Comparing base (c91cdcb) to head (c7ee374).
Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
...nsearch/index/engine/IngestionEngineException.java 40.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #17768      +/-   ##
============================================
- Coverage     72.46%   72.40%   -0.07%     
+ Complexity    66502    66458      -44     
============================================
  Files          5408     5409       +1     
  Lines        308080   308192     +112     
  Branches      44720    44746      +26     
============================================
- Hits         223239   223133     -106     
- Misses        66536    66732     +196     
- Partials      18305    18327      +22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yupeng9
Copy link
Contributor Author

yupeng9 commented Apr 3, 2025

500

right, ideally it shall be 4xx to indicate client-side error. is it possible to catch the thrown exception and change error code somewhere?

Copy link
Contributor

github-actions bot commented Apr 8, 2025

✅ Gradle check result for 0d1d406: SUCCESS

Copy link
Contributor

github-actions bot commented Apr 8, 2025

❌ Gradle check result for c97ee5b: 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?

Copy link
Contributor

github-actions bot commented Apr 8, 2025

✅ Gradle check result for 5d16874: SUCCESS

yupeng9 added 10 commits April 8, 2025 13:31
Signed-off-by: Yupeng Fu <[email protected]>
Signed-off-by: Yupeng Fu <[email protected]>
Signed-off-by: Yupeng Fu <[email protected]>
Signed-off-by: Yupeng Fu <[email protected]>
Signed-off-by: Yupeng Fu <[email protected]>
Signed-off-by: Yupeng Fu <[email protected]>
Signed-off-by: Yupeng Fu <[email protected]>
Signed-off-by: Yupeng Fu <[email protected]>
Signed-off-by: Yupeng Fu <[email protected]>
Copy link
Contributor

github-actions bot commented Apr 8, 2025

❌ Gradle check result for da32ad0: 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: Yupeng Fu <[email protected]>
Copy link
Contributor

github-actions bot commented Apr 8, 2025

✅ Gradle check result for c7ee374: SUCCESS

@msfroh msfroh merged commit 5ec6e9c into opensearch-project:main Apr 8, 2025
30 of 31 checks passed
Harsh-87 pushed a commit to Harsh-87/OpenSearch that referenced this pull request May 7, 2025
Harsh-87 pushed a commit to Harsh-87/OpenSearch that referenced this pull request May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants