Skip to content

Conversation

@varunbharadwaj
Copy link
Contributor

@varunbharadwaj varunbharadwaj commented Apr 9, 2025

Description

This PR fixes a bug in the pull-based ingestion flow where some messages can be skipped in some scenarios on shard recovery. The fix is to track the shard pointer on the writer thread and use that as the start point for recovery.

Additionally, this PR also avoids failing entire batch if one record fails and lets the error policy decide when to pause ingestion.

Related Issues

Resolves #17853

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 github-actions bot added bug Something isn't working Indexing Indexing, Bulk Indexing and anything related to indexing labels Apr 9, 2025
@varunbharadwaj varunbharadwaj changed the title [Pull-based ingestion] Use shard pointer tracked by writer for recovery [Pull-based ingestion] Use writer tracked shard pointer tracked for recovery Apr 9, 2025
@varunbharadwaj varunbharadwaj changed the title [Pull-based ingestion] Use writer tracked shard pointer tracked for recovery [Pull-based ingestion] Use writer tracked shard pointer for recovery Apr 9, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 9, 2025

❌ Gradle check result for c4bdc7d: 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
Copy link
Contributor

❌ Gradle check result for 9e0b808: 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
Copy link
Contributor

❌ Gradle check result for 9e0b808: 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
Copy link
Contributor

❌ Gradle check result for 9e0b808: 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
Copy link
Contributor

❌ Gradle check result for 00cf587: 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
Copy link
Contributor

❌ Gradle check result for 00cf587: 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
Copy link
Contributor

✅ Gradle check result for 677b3d9: SUCCESS

@codecov
Copy link

codecov bot commented Apr 11, 2025

Codecov Report

Attention: Patch coverage is 85.71429% with 5 lines in your changes missing coverage. Please review.

Project coverage is 72.43%. Comparing base (1628152) to head (677b3d9).
Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
...rch/indices/pollingingest/DefaultStreamPoller.java 84.84% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #17868      +/-   ##
============================================
- Coverage     72.43%   72.43%   -0.01%     
- Complexity    66789    66815      +26     
============================================
  Files          5449     5452       +3     
  Lines        309085   309198     +113     
  Branches      44979    44988       +9     
============================================
+ Hits         223899   223957      +58     
- Misses        66906    66912       +6     
- Partials      18280    18329      +49     

☔ 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.

@msfroh msfroh merged commit 9bbdd3c into opensearch-project:main Apr 11, 2025
31 checks passed
rgsriram pushed a commit to rgsriram/OpenSearch that referenced this pull request Apr 15, 2025
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
mayanksharma27 pushed a commit to mayanksharma27/OpenSearch that referenced this pull request May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Indexing Indexing, Bulk Indexing and anything related to indexing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Fix corner case in pull-based ingestion where messages can be skipped on shard recovery

4 participants