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 2021/05/26 07:03:56 UTC

[GitHub] [trafficserver] masaori335 opened a new pull request #7878: Cleanup: Get rid of HTTP2_SESSION_EVENT_INIT

masaori335 opened a new pull request #7878:
URL: https://github.com/apache/trafficserver/pull/7878


   Part of #7852. The definition will be removed by #7845 with other deprecated events.


-- 
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] masaori335 commented on a change in pull request #7878: Cleanup: Get rid of HTTP2_SESSION_EVENT_INIT

Posted by GitBox <gi...@apache.org>.
masaori335 commented on a change in pull request #7878:
URL: https://github.com/apache/trafficserver/pull/7878#discussion_r639463956



##########
File path: proxy/http2/Http2ConnectionState.cc
##########
@@ -1100,33 +1125,6 @@ Http2ConnectionState::main_event_handler(int event, void *edata)
   }
   ++recursion;
   switch (event) {
-  // Initialize HTTP/2 Connection
-  case HTTP2_SESSION_EVENT_INIT: {
-    ink_assert(this->ua_session == nullptr);
-    this->ua_session = static_cast<Http2ClientSession *>(edata);
-    REMEMBER(event, this->recursion);

Review comment:
       Fair enough, I'll back REMEMBER.




-- 
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] masaori335 commented on a change in pull request #7878: Cleanup: Get rid of HTTP2_SESSION_EVENT_INIT

Posted by GitBox <gi...@apache.org>.
masaori335 commented on a change in pull request #7878:
URL: https://github.com/apache/trafficserver/pull/7878#discussion_r639463956



##########
File path: proxy/http2/Http2ConnectionState.cc
##########
@@ -1100,33 +1125,6 @@ Http2ConnectionState::main_event_handler(int event, void *edata)
   }
   ++recursion;
   switch (event) {
-  // Initialize HTTP/2 Connection
-  case HTTP2_SESSION_EVENT_INIT: {
-    ink_assert(this->ua_session == nullptr);
-    this->ua_session = static_cast<Http2ClientSession *>(edata);
-    REMEMBER(event, this->recursion);

Review comment:
       Fair enough, I'll put it back for now. 
   
   We might need to revisit which function call should be recorded with the #7845 change.




-- 
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] maskit commented on a change in pull request #7878: Cleanup: Get rid of HTTP2_SESSION_EVENT_INIT

Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7878:
URL: https://github.com/apache/trafficserver/pull/7878#discussion_r639458292



##########
File path: proxy/http2/Http2ConnectionState.cc
##########
@@ -1100,33 +1125,6 @@ Http2ConnectionState::main_event_handler(int event, void *edata)
   }
   ++recursion;
   switch (event) {
-  // Initialize HTTP/2 Connection
-  case HTTP2_SESSION_EVENT_INIT: {
-    ink_assert(this->ua_session == nullptr);
-    this->ua_session = static_cast<Http2ClientSession *>(edata);
-    REMEMBER(event, this->recursion);

Review comment:
       We've been losing hints by removing calls for REMEMBER(). Isn't it worth keeping it? I'm fine with remove it temporally but want to put them back before we release 9.2.x (assuming we don't have this change on 9.1).




-- 
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] maskit commented on a change in pull request #7878: Cleanup: Get rid of HTTP2_SESSION_EVENT_INIT

Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7878:
URL: https://github.com/apache/trafficserver/pull/7878#discussion_r639493881



##########
File path: proxy/http2/Http2ConnectionState.cc
##########
@@ -1100,33 +1125,6 @@ Http2ConnectionState::main_event_handler(int event, void *edata)
   }
   ++recursion;
   switch (event) {
-  // Initialize HTTP/2 Connection
-  case HTTP2_SESSION_EVENT_INIT: {
-    ink_assert(this->ua_session == nullptr);
-    this->ua_session = static_cast<Http2ClientSession *>(edata);
-    REMEMBER(event, this->recursion);

Review comment:
       Yeah, we should reconsider where to put it on the restructuring. We may remove this one after all, but current places were considered as places that may help debugging. It would be nice to keep for future discussion.




-- 
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] bryancall merged pull request #7878: Cleanup: Get rid of HTTP2_SESSION_EVENT_INIT

Posted by GitBox <gi...@apache.org>.
bryancall merged pull request #7878:
URL: https://github.com/apache/trafficserver/pull/7878


   


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org