Skip to content

Http stream activity tracker and exceptions handling#119564

Merged
mhl-b merged 3 commits intoelastic:mainfrom
mhl-b:http-stream-activity-tracker-and-exceptions
Jan 7, 2025
Merged

Http stream activity tracker and exceptions handling#119564
mhl-b merged 3 commits intoelastic:mainfrom
mhl-b:http-stream-activity-tracker-and-exceptions

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/Network Http and internode communication implementations Team:Distributed Coordination (obsolete) Meta label for Distributed Coordination team. Obsolete. Please do not use. 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.

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!

@elasticsearchmachine
Copy link
Collaborator

@mhl-b according to this PR's labels, it should not have a changelog YAML, but I can't delete it because it is closed. Please either delete the changelog from the appropriate branch yourself, or adjust the labels.

@mhl-b mhl-b removed the backport label Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Network Http and internode communication implementations >enhancement Team:Distributed Coordination (obsolete) Meta label for Distributed Coordination team. Obsolete. Please do not use. v9.0.0

3 participants