Github user shinrich commented on the pull request:
https://github.com/apache/trafficserver/pull/237#issuecomment-117186342
The timeout logic looks good. And I agree with your comment to Brian that
it could probably moved into a separate commit. Not a lot of code, but this is
all tricky logic, so separation might be worth it.
The main thing here seems to be the logic to ensure that any data sent
along with the last handshake record is processed immediately. What you have
seems to do the trick.
However, I wonder if we could get by with fewer additional mechanisms
(buffers and bits) by using the trigger bit? When the client session starts
(all them them, Spdy, H2, and Http), we set read.vio.triggered to true. The
first non-zero do_io_read will add the vc to the read_ready_list and set
read_vio.enabled to true. The next event poll will have a timeout of 0 and
return immediately. The new VC will be processed when the read_ready_list is
walked. Since enabled and triggered will be true a READ_READY event will be
sent. At least that is my reading of things.
The downside is that in many cases you would be issuing a useless
READ_READY at the start of each session. Perhaps one could do a pending check
before setting the trigger bit.
Also due to the nesting of SPDY/H2 and Http clients, you might set the
trigger bit unnecessarily on the nested Http clients. Any lingering SSL data
would have been dealt with in the SPDY/H2 client session.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
