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:19:20 UTC
[GitHub] [trafficserver] masaori335 opened a new pull request #7879: Cleanup: Get rid of HTTP2_SESSION_EVENT_RECV
masaori335 opened a new pull request #7879:
URL: https://github.com/apache/trafficserver/pull/7879
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] maskit commented on pull request #7879: Cleanup: Get rid of HTTP2_SESSION_EVENT_RECV
Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7879:
URL: https://github.com/apache/trafficserver/pull/7879#issuecomment-848552397
I appreciate making small changes, but is it safe to land these separately? The logic is not completely equivalent - zombie event stuff won't be processed and `recursion` is not counted.
--
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 pull request #7879: Cleanup: Get rid of HTTP2_SESSION_EVENT_RECV
Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7879:
URL: https://github.com/apache/trafficserver/pull/7879#issuecomment-849239837
I don't thinks the two are related. I just pointed out two separated things that are currently done in `main_event_handler`.
`ready_to_free()` is checked at the bottom of `main_event_handler` when `recursion` is 0, and `ua_session->free()` is called. Is a similar thing done in somewhere else, or we don't need to do it when we call `rcv_frame`?
--
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 pull request #7879: Cleanup: Get rid of HTTP2_SESSION_EVENT_RECV
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7879:
URL: https://github.com/apache/trafficserver/pull/7879#issuecomment-849259226
[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] masaori335 commented on pull request #7879: Cleanup: Get rid of HTTP2_SESSION_EVENT_RECV
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7879:
URL: https://github.com/apache/trafficserver/pull/7879#issuecomment-849234461
I agree this is not completely equivalent. As you pointed out `recursion` of `Http2ConnectionState` is not incremented. But this change is safe because the `rcv_frame()` call is guarded by `recursion` of Http2ClientSession.
Could you give me a pointer where the zombi event is checking the `recursion`?
--
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 pull request #7879: Cleanup: Get rid of HTTP2_SESSION_EVENT_RECV
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7879:
URL: https://github.com/apache/trafficserver/pull/7879#issuecomment-849258647
https://github.com/apache/trafficserver/blob/050b2dfe9a89348b4105183a2b31e7cc59fc389d/proxy/http2/Http2ConnectionState.cc#L1253-L1254
It looks like when the `main_event_handler` handles `HTTP2_SESSION_EVENT_RECV`, `ua_session->is_recursing()` is always true. Because it only comes from Http2ClientSession SessionHandler.
--
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 removed a comment on pull request #7879: Cleanup: Get rid of HTTP2_SESSION_EVENT_RECV
Posted by GitBox <gi...@apache.org>.
masaori335 removed a comment on pull request #7879:
URL: https://github.com/apache/trafficserver/pull/7879#issuecomment-849259226
[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] maskit commented on pull request #7879: Cleanup: Get rid of HTTP2_SESSION_EVENT_RECV
Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7879:
URL: https://github.com/apache/trafficserver/pull/7879#issuecomment-849280393
> I'm fine with keeping this (and #7878) as a draft and merge with #7845 if reviewers want.
I'm not requesting that, it's an option if other reviews want it though. I was just not sure the points above.
--
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 merged pull request #7879: Cleanup: Get rid of HTTP2_SESSION_EVENT_RECV
Posted by GitBox <gi...@apache.org>.
masaori335 merged pull request #7879:
URL: https://github.com/apache/trafficserver/pull/7879
--
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
[GitHub] [trafficserver] shinrich commented on pull request #7879: Cleanup: Get rid of HTTP2_SESSION_EVENT_RECV
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7879:
URL: https://github.com/apache/trafficserver/pull/7879#issuecomment-868506749
Looks good. Code more clear.
--
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
[GitHub] [trafficserver] masaori335 commented on pull request #7879: Cleanup: Get rid of HTTP2_SESSION_EVENT_RECV
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7879:
URL: https://github.com/apache/trafficserver/pull/7879#issuecomment-849259051
I'm fine with keeping this (and #7878) as a draft and merge with #7845 if reviewers want. This is separated out from it to minimize the 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