You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2020/09/11 18:46:26 UTC
[GitHub] [trafficserver] shinrich opened a new pull request #7182: Remove obsolete cdn_ HttpTransact vars
shinrich opened a new pull request #7182:
URL: https://github.com/apache/trafficserver/pull/7182
Noticed this while working through the origin connect code to restructure it for multiple protocols.
First I observed that cdn_remap_complete is only set to true, so the true check clause in HttpTransact::OSDNSLookup is obsolete. Once that clause removed, cdn_saved_next_action is only set in how_to_open_connection and used as a return value. So it is easy enough to replace the member variable with a local variable.
Finally cdn_saved_transact_return_point and first_dns_lookup are only ever initialized and never used.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on a change in pull request #7182: Remove obsolete cdn_ HttpTransact vars
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #7182:
URL: https://github.com/apache/trafficserver/pull/7182#discussion_r487585485
##########
File path: proxy/http/HttpTransact.cc
##########
@@ -757,15 +757,15 @@ how_to_open_connection(HttpTransact::State *s)
break;
}
- s->cdn_saved_next_action = HttpTransact::SM_ACTION_ORIGIN_SERVER_OPEN;
+ HttpTransact::StateMachineAction_t connect_next_action = HttpTransact::SM_ACTION_ORIGIN_SERVER_OPEN;
// Setting up a direct CONNECT tunnel enters OriginServerRawOpen. We always do that if we
// are not forwarding CONNECT and are not going to a parent proxy.
if (s->method == HTTP_WKSIDX_CONNECT) {
if (s->txn_conf->forward_connect_method == 1 || s->parent_result.result == PARENT_SPECIFIED) {
- s->cdn_saved_next_action = HttpTransact::SM_ACTION_ORIGIN_SERVER_OPEN;
+ connect_next_action = HttpTransact::SM_ACTION_ORIGIN_SERVER_OPEN;
Review comment:
Isn't it already that value? Shouldn't this check only for having to change to `SM_ACTION_ORIGIN_SERVER_RAW_OPEN`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich commented on a change in pull request #7182: Remove obsolete cdn_ HttpTransact vars
Posted by GitBox <gi...@apache.org>.
shinrich commented on a change in pull request #7182:
URL: https://github.com/apache/trafficserver/pull/7182#discussion_r488020558
##########
File path: proxy/http/HttpTransact.cc
##########
@@ -757,15 +757,15 @@ how_to_open_connection(HttpTransact::State *s)
break;
}
- s->cdn_saved_next_action = HttpTransact::SM_ACTION_ORIGIN_SERVER_OPEN;
+ HttpTransact::StateMachineAction_t connect_next_action = HttpTransact::SM_ACTION_ORIGIN_SERVER_OPEN;
// Setting up a direct CONNECT tunnel enters OriginServerRawOpen. We always do that if we
// are not forwarding CONNECT and are not going to a parent proxy.
if (s->method == HTTP_WKSIDX_CONNECT) {
if (s->txn_conf->forward_connect_method == 1 || s->parent_result.result == PARENT_SPECIFIED) {
- s->cdn_saved_next_action = HttpTransact::SM_ACTION_ORIGIN_SERVER_OPEN;
+ connect_next_action = HttpTransact::SM_ACTION_ORIGIN_SERVER_OPEN;
Review comment:
Yes, I was trying to minimize code change to simplify verification that the code change was not a functional change. But you are correct, this could be further simplified.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich commented on pull request #7182: Remove obsolete cdn_ HttpTransact vars
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7182:
URL: https://github.com/apache/trafficserver/pull/7182#issuecomment-691258388
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] randall commented on pull request #7182: Remove obsolete cdn_ HttpTransact vars
Posted by GitBox <gi...@apache.org>.
randall commented on pull request #7182:
URL: https://github.com/apache/trafficserver/pull/7182#issuecomment-691280671
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich commented on pull request #7182: Remove obsolete cdn_ HttpTransact vars
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7182:
URL: https://github.com/apache/trafficserver/pull/7182#issuecomment-691681245
[approve ci autest]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] randall commented on pull request #7182: Remove obsolete cdn_ HttpTransact vars
Posted by GitBox <gi...@apache.org>.
randall commented on pull request #7182:
URL: https://github.com/apache/trafficserver/pull/7182#issuecomment-691280671
[approve ci autest]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] randall commented on pull request #7182: Remove obsolete cdn_ HttpTransact vars
Posted by GitBox <gi...@apache.org>.
randall commented on pull request #7182:
URL: https://github.com/apache/trafficserver/pull/7182#issuecomment-691280671
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich commented on pull request #7182: Remove obsolete cdn_ HttpTransact vars
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7182:
URL: https://github.com/apache/trafficserver/pull/7182#issuecomment-691258388
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich merged pull request #7182: Remove obsolete cdn_ HttpTransact vars
Posted by GitBox <gi...@apache.org>.
shinrich merged pull request #7182:
URL: https://github.com/apache/trafficserver/pull/7182
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich commented on pull request #7182: Remove obsolete cdn_ HttpTransact vars
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7182:
URL: https://github.com/apache/trafficserver/pull/7182#issuecomment-692137160
Pushed another commit to address @SolidWallOfCode's comment
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich commented on pull request #7182: Remove obsolete cdn_ HttpTransact vars
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7182:
URL: https://github.com/apache/trafficserver/pull/7182#issuecomment-691258388
[approve ci autest]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org