Skip to content
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

Http stream activity tracker and exceptions handling #119564

Merged

Conversation

mhl-b
Copy link
Contributor

@mhl-b mhl-b commented Jan 5, 2025

Pulling larger PR apart #117787.

Add HTTP body stream activity tracker when chunk is buffered. Also catch all (Throwable) exceptions when running ChunkConsumer in the channel event-loop. Some exceptions, for example OOM graceful shutdown, can slip through and never terminate stream and channel, making it hanging indefinitely. This causes issues during shutdown.

@mhl-b mhl-b requested a review from DaveCTurner January 5, 2025 03:41
@mhl-b mhl-b marked this pull request as ready for review January 5, 2025 03:41
@elasticsearchmachine elasticsearchmachine added the needs:triage Requires assignment of a team area label label Jan 5, 2025
@mhl-b mhl-b added >enhancement :Distributed Coordination/Network Http and internode communication implementations Team:Distributed Coordination Meta label for Distributed Coordination team and removed needs:triage Requires assignment of a team area label labels Jan 5, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Hi @mhl-b, I've created a changelog YAML for you.

@@ -594,7 +594,7 @@ record Ctx(String testName, String nodeName, Bootstrap clientBootstrap, Channel
@Override
public void close() throws Exception {
safeGet(clientChannel.close());
safeGet(clientBootstrap.config().group().shutdownGracefully());
safeGet(clientBootstrap.config().group().shutdownGracefully(0, 0, TimeUnit.SECONDS));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A speedup for integ test suite.

Copy link
Contributor

@nicktindall nicktindall left a comment

Choose a reason for hiding this comment

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

LGTM with 1 minor query

@mhl-b
Copy link
Contributor Author

mhl-b commented Jan 7, 2025

Thanks Nick!

@mhl-b mhl-b merged commit 17ece0a into elastic:main Jan 7, 2025
16 checks passed
mhl-b added a commit to mhl-b/elasticsearch that referenced this pull request Jan 8, 2025
mhl-b added a commit to mhl-b/elasticsearch that referenced this pull request Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport pending :Distributed Coordination/Network Http and internode communication implementations >enhancement Team:Distributed Coordination Meta label for Distributed Coordination team v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants