Avoid unnecesarry copy on POST request over HTTP/2

[masaori335]

Prior to this change, all of the received payloads of DATA frames are written in the buffer of Http2Stream (request_buffer). And after HttpTunnel is set up, the data is copied to another buffer on Http2Stream::update_read_request.

It looks like we can directly write the payloads to the buffer which is created by HttpSM/HttpTunnel for the POST request.

Actually, the copy is switching chain of IOBufferBlock, so this doesn't have big performance improvement but this makes code much simpler.

[shinrich]

Agreed that it simplifies the logic significantly if we can always rely the on the vio's buffer.

In the case of the POST data frame, can we be guaranteed that the HttpTunnel VIO is always set up when the DATA frame arrives?

[masaori335]

In the case of the POST data frame, can we be guaranteed that the HttpTunnel VIO is always set up when the DATA frame arrives?

Some DATA frames (65535 bytes, the initial window size) could arrive before the HttpTunnel is set up. But it's not a problem because the payloads of these DATA frames will be written in Http2Stream::request_buffer and copied to the new buffer when HttpSM::do_setup_post_tunnel() is called to set up the tunnel.

trafficserver/proxy/http/HttpSM.cc

Lines 5676 to 5680 in 8a558cb

	// Next order of business if copy the remaining data from the
	// header buffer into new buffer
	client_request_body_bytes = post_buffer->write(ua_buffer_reader, chunked ? ua_buffer_reader->read_avail() : post_bytes);
	ua_buffer_reader->consume(client_request_body_bytes);

So this change just follows what the HTTP/1.1 component does. Http2Stream doesn't need to care about which buffer to write nor copy.

[masaori335]

Looks like the AuTest failures are serious. It reproduced on my local box, I'm figuring out.

[masaori335]

Off-topic, but, consuming nbytes in the while loop looks wrong.

[zwoop]

Cherry-picked to v9.0.x branch.

[zwoop]

@shinrich and I suspect this might be the root cause of the ASAN issues on Docs (and their prod). I'm doing tests right now without this on Docs.

[zizhong]

Why not (read_vio.ntodo() == 0 || this->recv_end_stream) here?

[masaori335]

IIRC, this is for some AuTest failures. This checks the upper layer (HttpSM/HttpTunnel) called do_io_read(this, INT64_MAX, XXXX). Yes, it's a bit unreliable. ( I should have left some comments )