feat(elasticsearch sink): fix partial retry logic #22431

Serendo · 2025-02-13T09:14:26Z

Summary

Add a RetryPartial variant to RetryAction. This variant takes a closure which has the information of the response and will be used to modify the request so only sending the failed part of it.
Implemented the RetryPartialFunction for elasticsearch sink, now that request_retry_partial will only send those previously failed requests. those successful requests will not be sent again.

Change Type

Bug fix
New feature
Non-functional (chore, refactoring, docs)
Performance

Is this a breaking change?

Yes. This change will update tower to 0.5 which itself has breaking changes. but it should be transparent to users.
No

How did you test this PR?

I wrote a python server mocking the elasticsearch bulk api. which will give a response in which the first 5 documents are 201 created, and others are 429. I can see that one particular document will get only one 201.

Does this PR include user facing changes?

Yes. Please add a changelog fragment based on our guidelines.
No. A maintainer will apply the "no-changelog" label to this PR.

Checklist

Please read our Vector contributor resources.
- make check-all is a good command to run locally. This check is
  defined here. Some of these
  checks might not be relevant to your PR. For Rust changes, at the very least you should run:
  - cargo fmt --all
  - cargo clippy --workspace --all-targets -- -D warnings
  - cargo nextest run --workspace (alternatively, you can run cargo test --all)
If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
run dd-rust-license-tool write to regenerate the license inventory and commit the changes (if any). More details here.

References

Closes: #140

#14891

- Adjustments of Buffer type definition in Tower 0.5 - As policy is directly modified in Tower 0.5. Removed the policy part in the RetryPolicyFuture. - The change args to mutable for the retry function and clone_request function. It's changed in Tower 0.5. - Changed the RetryPolicyFuture Output to (). This is also changed in Tower 0.5. - Modified some tests as policy is now modified in advance function;

…ls a RetryPartialFunction trait to modify the old request. In the elasticsearch sink. When there are errors in the response, the response's status_codes are keeped in a closure which is later used by the modify_request func in RetryPartialFunction trait.

bits-bot · 2025-02-13T09:14:31Z

All committers have signed the CLA.

pront · 2025-02-13T17:01:03Z

Hi @Serendo, thank you for this PR! I think this is not a breaking change (from the perspective of Vector users). And also we will need a changelog to explain the fix.

Serendo · 2025-02-14T02:42:00Z

Hi @Serendo, thank you for this PR! I think this is not a breaking change (from the perspective of Vector users). And also we will need a changelog to explain the fix.

I have added a changlog fragment for this.

…these stats ranges are related with the test params.

tronboto · 2025-02-24T08:38:17Z

Thank you for this @Serendo - excited to see it land in the next release!

tronboto · 2025-03-11T11:57:52Z

Hey @pront . What's still needed here to get this one across the line? Would be great to see it in the next release.

pront · 2025-03-11T13:46:22Z

Hey @pront . What's still needed here to get this one across the line? Would be great to see it in the next release.

Hi @Serendo and @tronboto, I am taking a look.

changelog.d/elasticsearch_sink_skip_retrying_succeeded_documents.feature.md

src/sinks/util/retries.rs

tests/data/adaptive-concurrency/defers-at-high-concurrency.toml

Cargo.toml

…ts.feature.md

src/sinks/elasticsearch/retry.rs

src/sinks/util/retries.rs

src/sinks/elasticsearch/retry.rs

adamreeve · 2025-04-14T22:27:15Z

Hi @Serendo, are you still working on this? I see you asked for a review last week but there are still outstanding review comments to be addressed. It would be great to see this feature land so if you need help getting this PR finished my team could help (I work on OSS @G-Research, @tronboto is a colleague of mine).

Serendo · 2025-04-15T12:27:09Z

Hi @Serendo, are you still working on this? I see you asked for a review last week but there are still outstanding review comments to be addressed. It would be great to see this feature land so if you need help getting this PR finished my team could help (I work on OSS @G-Research, @tronboto is a colleague of mine).

Hi @adamreeve, I forgot to push the commits and I thought I had pushed them. I'll push them later.

And yes it will be great to get some help from you. Thank you! As you see when modifying the original request, there is a metadata part which is usually generated from the events. But when filtering the requests, I can't use that part of the code as the requests are not in the shape of events now.

pront

Some more nits

src/sinks/elasticsearch/retry.rs

Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com>

pront · 2025-07-30T20:38:49Z

Please take a look at the merge conflicts. We will take a look after those are fixed.

src/sinks/util/retries.rs

Cargo.lock

src/sinks/elasticsearch/retry.rs

This reverts commit 79e7dd3.

This reverts commit e972da4.

src/sinks/util/retries.rs

thomasqueirozb

I think these 2 should be switched, not sure

src/sinks/util/retries.rs

tronboto · 2025-08-08T11:19:48Z

Thanks for all the work on this 😄

Serendo added 2 commits February 13, 2025 15:30

Serendo requested a review from a team as a code owner February 13, 2025 09:14

github-actions bot added the domain: sinks Anything related to the Vector's sinks label Feb 13, 2025

pront changed the title ~~Es retry tower0.5~~ feat(deps): update tower crate to 0.5 and fix partial retry logic Feb 13, 2025

pront changed the title ~~feat(deps): update tower crate to 0.5 and fix partial retry logic~~ feat(deps): update tower crate to v0.5.2 and fix partial retry logic Feb 13, 2025

Serendo added 3 commits February 14, 2025 11:06

add a changelog fragment for elasticsearch sink retry partial

7561898

changed some threshold of the tests. But I dont quite understand how …

e02e6ad

…these stats ranges are related with the test params.

add newline to changelog end

454d3a2

pront added 2 commits March 11, 2025 12:58

Merge remote-tracking branch 'origin/master' into es-retry-tower0.5

20f2829

cargo.lock update

6edf77f

pront reviewed Mar 11, 2025

View reviewed changes

changelog.d/elasticsearch_sink_skip_retrying_succeeded_documents.feature.md Outdated Show resolved Hide resolved

src/sinks/util/retries.rs Show resolved Hide resolved

tests/data/adaptive-concurrency/defers-at-high-concurrency.toml Show resolved Hide resolved

pront reviewed Mar 24, 2025

View reviewed changes

Cargo.toml Show resolved Hide resolved

pront closed this Mar 24, 2025

pront reopened this Mar 24, 2025

pront mentioned this pull request Mar 24, 2025

chore(deps): Bump the tower group with 2 updates #22714

Closed

Update changelog.d/elasticsearch_sink_skip_retrying_succeeded_documen…

3b241d1

…ts.feature.md

pront requested changes Mar 25, 2025

View reviewed changes

Merge branch 'vectordotdev:master' into es-retry-tower0.5

a6badfc

Serendo requested a review from pront April 5, 2025 03:51

pront added the meta: awaiting author Pull requests that are awaiting their author. label Apr 11, 2025

remove unwrap for safety.

4a7ab30

pront changed the title ~~feat(deps): update tower crate to v0.5.2 and fix partial retry logic~~ feat(elastic_search sink): fix partial retry logic Jul 29, 2025

pront reviewed Jul 29, 2025

View reviewed changes

src/sinks/elasticsearch/retry.rs Outdated Show resolved Hide resolved

src/sinks/elasticsearch/retry.rs Show resolved Hide resolved

src/sinks/elasticsearch/retry.rs Outdated Show resolved Hide resolved

src/sinks/elasticsearch/retry.rs Show resolved Hide resolved

Update src/sinks/elasticsearch/retry.rs

fb5b597

Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com>

github-actions bot removed the meta: awaiting author Pull requests that are awaiting their author. label Jul 30, 2025

pront added the meta: awaiting author Pull requests that are awaiting their author. label Jul 30, 2025

Merge remote-tracking branch 'origin/master' into es-retry-tower0.5

3b1cac7

github-actions bot removed the meta: awaiting author Pull requests that are awaiting their author. label Jul 31, 2025

as pront suggested

522feb6

Serendo changed the title ~~feat(elastic_search sink): fix partial retry logic~~ feat(elasticsearch sink): fix partial retry logic Jul 31, 2025

Merge branch 'master' into es-retry-tower0.5

b6edcf7

thomasqueirozb reviewed Jul 31, 2025

View reviewed changes

src/sinks/util/retries.rs Show resolved Hide resolved

thomasqueirozb added 2 commits August 6, 2025 15:47

Remove downcast ref and trait usage from partial retry logic

e972da4

Fix make check-clippy errors

79e7dd3

pront reviewed Aug 6, 2025

View reviewed changes

Cargo.lock Outdated Show resolved Hide resolved

src/sinks/elasticsearch/retry.rs Show resolved Hide resolved

thomasqueirozb added 4 commits August 6, 2025 16:09

Revert "Fix make check-clippy errors"

abeb520

This reverts commit 79e7dd3.

Revert "Remove downcast ref and trait usage from partial retry logic"

3bd9c0d

This reverts commit e972da4.

Merge remote-tracking branch 'origin/master' into es-retry-tower0.5

5eb44e1

Revert cmake bump in Cargo.lock

1b64e9e

thomasqueirozb requested a review from pront August 6, 2025 20:18

thomasqueirozb mentioned this pull request Aug 6, 2025

chore(sinks): remove downcast and trait in sink retry code #23543

Merged

8 tasks

cargo fmt

ce35e6d

pront reviewed Aug 6, 2025

View reviewed changes

src/sinks/util/retries.rs Outdated Show resolved Hide resolved

src/sinks/util/retries.rs Show resolved Hide resolved

src/sinks/util/retries.rs Outdated Show resolved Hide resolved

thomasqueirozb reviewed Aug 6, 2025

View reviewed changes

src/sinks/util/retries.rs Outdated Show resolved Hide resolved

src/sinks/util/retries.rs Outdated Show resolved Hide resolved

pront approved these changes Aug 6, 2025

View reviewed changes

Address PR feedback

2b401f4

pront added this pull request to the merge queue Aug 7, 2025

Merged via the queue into vectordotdev:master with commit d9bef7e Aug 7, 2025
182 of 210 checks passed

feat(elasticsearch sink): fix partial retry logic #22431

feat(elasticsearch sink): fix partial retry logic #22431

Uh oh!

Conversation

Serendo commented Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Change Type

Is this a breaking change?

How did you test this PR?

Does this PR include user facing changes?

Checklist

References

Uh oh!

bits-bot commented Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pront commented Feb 13, 2025

Uh oh!

Serendo commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tronboto commented Feb 24, 2025

Uh oh!

tronboto commented Mar 11, 2025

Uh oh!

pront commented Mar 11, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adamreeve commented Apr 14, 2025

Uh oh!

Serendo commented Apr 15, 2025

Uh oh!

pront left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pront commented Jul 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thomasqueirozb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tronboto commented Aug 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Serendo commented Feb 13, 2025 •

edited

Loading

bits-bot commented Feb 13, 2025 •

edited

Loading

Serendo commented Feb 14, 2025 •

edited

Loading