Skip to content

Reduce work in testBulkIndexingRequestSplitting#145188

Merged
DaveCTurner merged 2 commits intoelastic:mainfrom
DaveCTurner:2026/03/30/Netty4IncrementalRequestHandlingIT-testBulkIndexingRequestSplitting
Mar 30, 2026
Merged

Reduce work in testBulkIndexingRequestSplitting#145188
DaveCTurner merged 2 commits intoelastic:mainfrom
DaveCTurner:2026/03/30/Netty4IncrementalRequestHandlingIT-testBulkIndexingRequestSplitting

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner commented Mar 30, 2026

No need to try and index 300 documents if we're splitting each one into
its own bulk, and indeed it can take more than the allowed 10s to do so
many separate bulks. With this change we sometimes try larger
watermarks, and only ever try and index up to roughly twice as many docs
as needed to trigger a split.

Closes #144579

No need to try and index 300 documents if we're splitting each one into
its own bulk, and indeed it can take more than the allowed 10s to do so
many separate bulks. With this change we sometimes try larger
watermarks, and only ever try and index up to roughly twice as many docs
as needed to trigger a split.

Relates elastic#144579
@DaveCTurner DaveCTurner requested a review from nicktindall March 30, 2026 13:22
@DaveCTurner DaveCTurner added >test Issues or PRs that are addressing/adding tests :Distributed/Network Http and internode communication implementations branch:9.2 branch:8.19 v9.4.0 branch:9.3 labels Mar 30, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@DaveCTurner
Copy link
Copy Markdown
Member Author

I'm leaving the logging etc. in place from #144766 for now but I'm pretty sure it's just that we were doing too many separate bulks to fit into the 10s. See e.g. this build scan result which did indeed just take a little over 10s to complete the work, across 240+ separate bulk requests.

@DaveCTurner
Copy link
Copy Markdown
Member Author

I'm leaving the logging etc. in place

Changed my mind, I'm taking it out, see 807f0a8. If it fails again we can reinstate it, but these failures are so rare that we'd have to leave this open for months otherwise and it doesn't seem worth it.

Copy link
Copy Markdown
Contributor

@BrianRothermich BrianRothermich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@DaveCTurner DaveCTurner added the auto-backport Automatically create backport pull requests when merged label Mar 30, 2026
@DaveCTurner DaveCTurner merged commit 78aa896 into elastic:main Mar 30, 2026
36 checks passed
@DaveCTurner DaveCTurner deleted the 2026/03/30/Netty4IncrementalRequestHandlingIT-testBulkIndexingRequestSplitting branch March 30, 2026 19:17
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

💔 Backport failed

Status Branch Result
9.3 Commit could not be cherrypicked due to conflicts
8.19 Commit could not be cherrypicked due to conflicts
9.2 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 145188

DaveCTurner added a commit that referenced this pull request Mar 30, 2026
No need to try and index 300 documents if we're splitting each one into
its own bulk, and indeed it can take more than the allowed 10s to do so
many separate bulks. With this change we sometimes try larger
watermarks, and only ever try and index up to roughly twice as many docs
as needed to trigger a split.

Closes #144579
DaveCTurner added a commit that referenced this pull request Mar 30, 2026
No need to try and index 300 documents if we're splitting each one into
its own bulk, and indeed it can take more than the allowed 10s to do so
many separate bulks. With this change we sometimes try larger
watermarks, and only ever try and index up to roughly twice as many docs
as needed to trigger a split.

Closes #144579
@DaveCTurner
Copy link
Copy Markdown
Member Author

Backports:

Test doesn't exist in 8.19.

mouhc1ne pushed a commit to shmuelhanoch/elasticsearch that referenced this pull request Mar 31, 2026
No need to try and index 300 documents if we're splitting each one into
its own bulk, and indeed it can take more than the allowed 10s to do so
many separate bulks. With this change we sometimes try larger
watermarks, and only ever try and index up to roughly twice as many docs
as needed to trigger a split.

Closes elastic#144579
ncordon pushed a commit to ncordon/elasticsearch that referenced this pull request Apr 1, 2026
No need to try and index 300 documents if we're splitting each one into
its own bulk, and indeed it can take more than the allowed 10s to do so
many separate bulks. With this change we sometimes try larger
watermarks, and only ever try and index up to roughly twice as many docs
as needed to trigger a split.

Closes elastic#144579
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Distributed/Network Http and internode communication implementations Team:Distributed Meta label for distributed team. >test Issues or PRs that are addressing/adding tests v9.2.8 v9.3.3 v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] Netty4IncrementalRequestHandlingIT testBulkIndexingRequestSplitting failing

3 participants