http: fix upstream_rq stat increment #4055
Conversation
Signed-off-by: Rama <rama.rao@salesforce.com>
|
@mattklein123 PTAL. Fixed upstream_rq increment incase of connection failure in http1. For http2 it seems to work correctly, so just added test validation. |
mattklein123
left a comment
There was a problem hiding this comment.
Thanks for working on this!
| host_->stats().cx_connect_fail_.inc(); | ||
|
|
||
| // Increment the total requests also. | ||
| host_->cluster().stats().upstream_rq_total_.inc(); |
There was a problem hiding this comment.
I think it would be better to consolidate this logic and only increment these stats in 1 place. Can you take a look at the other place we do this and move the increment to when the connection pool is first asked for a stream?
Signed-off-by: Rama <rama.rao@salesforce.com>
|
@mattklein123 consolidated the logic. PTAL when you get a chance? |
|
|
||
| StreamEncoderWrapper::inner_.getStream().addCallbacks(*this); | ||
| parent_.parent_.host_->cluster().stats().upstream_rq_total_.inc(); | ||
| parent_.parent_.host_->cluster().stats().upstream_rq_active_.inc(); |
There was a problem hiding this comment.
Arguably rq_active should move also, though, the logic to keep that consistent will be more complicated and I'm not sure it's worth it. Thoughts?
There was a problem hiding this comment.
Yes.It may not be worth the complication.
Signed-off-by: Rama rama.rao@salesforce.com
Description: Increment upstream_rq in case of connection failures also for http1.
Risk Level: Low
Testing: Added Automated tests
Docs Changes: N/A
Release Notes: N/A
Fixes #3950