Skip to content

Move network stats marking into InboundPipeline#54393

Merged
Tim-Brooks merged 7 commits intoelastic:masterfrom
Tim-Brooks:move_stats_inside_pipeline
Mar 31, 2020
Merged

Move network stats marking into InboundPipeline#54393
Tim-Brooks merged 7 commits intoelastic:masterfrom
Tim-Brooks:move_stats_inside_pipeline

Conversation

@Tim-Brooks
Copy link
Copy Markdown
Contributor

This is a follow-up to #48263. It moves the inbound stats tracking
inside of the InboundPipeline.

@Tim-Brooks Tim-Brooks added >non-issue :Distributed/Network Http and internode communication implementations v8.0.0 v7.8.0 labels Mar 30, 2020
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
Contributor

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

Looks good, just one issue with the time supplier

@@ -676,9 +682,6 @@ public void inboundMessage(TcpChannel channel, InboundMessage message) {
}

public void inboundDecodeException(TcpChannel channel, Tuple<Header, Exception> tuple) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we remove this method now?

Copy link
Copy Markdown
Contributor Author

@Tim-Brooks Tim-Brooks Mar 30, 2020

Choose a reason for hiding this comment

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

There will be a follow up related to exception handling. Specifically CircuitBreaking will move inside the pipeline. But circuit breaking generates an exception that has to be handled by generating a message to send back.

@Tim-Brooks
Copy link
Copy Markdown
Contributor Author

I have updated this PR for @original-brownbear changes. And will address @ywelsch comments in a follow-up.

Copy link
Copy Markdown
Contributor

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

LGTM

@Tim-Brooks Tim-Brooks merged commit 9d861bf into elastic:master Mar 31, 2020
Tim-Brooks added a commit to Tim-Brooks/elasticsearch that referenced this pull request Apr 7, 2020
This is a follow-up to elastic#48263. It moves the inbound stats tracking
inside of the InboundPipeline.
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 >non-issue v7.8.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants