Skip to content

Conversation

@scw00
Copy link
Member

@scw00 scw00 commented Jun 6, 2017

Currently, the post and put request can not work together with server connection retry mechanism.
Because ATS can not replay the post content.

@scw00
Copy link
Member Author

scw00 commented Jun 6, 2017

golang test script

package main

import (
	"net"
	"os"
	"fmt"
)

var closed int = 0
var resp string = "HTTP/1.1 200 OK\r\n" +
	"Cache-Control: max-age=3000\r\n" +
	"Date: Tue, 06 Jun 2017 12:07:49 GMT\r\n" +
	"Content-Length: 10\r\n" +
	"Content-Type: text/plain; charset=utf-8\r\n\r\n" +
	"helloworld"

func main()  {

	ls, err := net.Listen("tcp", os.Args[1])
	if err != nil {
		fmt.Println(err)
		os.Exit(1)
	}


	for {
		conn, err := ls.Accept()
		if err != nil {
			fmt.Println(err)
			os.Exit(1)
		}
		closed ++
		if closed % 2 != 0 {
			conn.Close()
			fmt.Println("times: ", closed, "closed directly")
			continue
		}
	       fmt.Println("times: ", closed)
	       go handleConnection(conn)	
	}
}

func handleConnection(con net.Conn)  {

	data := make([]byte, 40000)
	con.Read(data)

	fmt.Println(string(data))
	con.Write([]byte(resp))
	con.Close()
}

output

times:  1 closed directly
times:  2
POST /x HTTP/1.1
User-Agent: curl/7.35.0
Host: 127.0.0.1:12345
Accept: */*
Content-Length: 3
Content-Type: application/x-www-form-urlencoded
X-Forwarded-For: 127.0.0.1
Via: http/1.1 ubuntu-linux[5af10c97-8570-4476-8480-4db4f7abcc10] (ApacheTrafficServer/8.0.0)

// anther try
times 3
POST /x HTTP/1.1
User-Agent: curl/7.35.0
Host: 127.0.0.1:12345
Accept: */*
Content-Length: 3
Content-Type: application/x-www-form-urlencoded
X-Forwarded-For: 127.0.0.1
Via: http/1.1 ubuntu-linux[5af10c97-8570-4476-8480-4db4f7abcc10] (ApacheTrafficServer/8.0.0)

aaa

That we can see, we send the post request without data in previous request.

In the retry

Breakpoint 3, HttpSM::do_setup_post_tunnel (this=0x7fffeea3f4d0, to_vc_type=HTTP_SERVER_VC) at HttpSM.cc:5699
5699	  bool chunked       = (t_state.client_info.transfer_encoding == HttpTransact::CHUNKED_ENCODING);
(gdb) n
5700	  bool post_redirect = false;
(gdb) 
5701	  int64_t post_bytes = 0;
(gdb) 
5703	  HttpTunnelProducer *p = nullptr;
(gdb) 
5707	  if (t_state.redirect_info.redirect_in_process && enable_redirection &&
(gdb) 
5723	    if (t_state.current.attempts > 0 && t_state.http_config_param->tunnel_faker_enabled) {
(gdb) 
5730	    if (t_state.hdr_info.request_content_length == HTTP_UNDEFINED_CL) {
(gdb) 
5736	      alloc_index = buffer_size_to_index(t_state.hdr_info.request_content_length);
(gdb) 
5738	    MIOBuffer *post_buffer    = new_MIOBuffer(alloc_index);
(gdb) 
5739	    IOBufferReader *buf_start = post_buffer->alloc_reader();
(gdb) 
5740	    post_bytes                = chunked ? INT64_MAX : t_state.hdr_info.request_content_length;
(gdb) 
5748	    client_request_body_bytes = post_buffer->write(ua_buffer_reader, chunked ? ua_buffer_reader->read_avail() : post_bytes);
(gdb) p ua_buffer_reader->read_avail()
$6 = 0
(gdb) p t_state.current.attempts 
$7 = 1
(gdb) 

@oknet
Copy link
Member

oknet commented Jun 6, 2017

[approve ci]

@zwoop Is the autest system support golang script ?

@bryancall
Copy link
Contributor

bryancall commented Jun 6, 2017

POST and PUT should be retryable if we haven't sent any data to the origin. The first conditional checks the method and if there have been any bytes sent. The data should still be around if we haven't tried to write it to the origin.

Something else looks to be broken.

@zwoop
Copy link
Contributor

zwoop commented Jun 6, 2017

This comes down to #947 according to @vmamidi. Should we consider backing it out?

@scw00
Copy link
Member Author

scw00 commented Jun 7, 2017

@bryancall you are right. The reason is that we have already consumed the ua_buffer_reader in do_setup_post_tunnel

    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);

We will got nothing in the next term. It can be fixed simply to remove this consume. And I will push another pr later.

@zwoop
Although # 947 brought us a lot of trouble, I think #947 has nothing to do with this pr.
For #947. IMHO, The best way to solve the problem is to revert #947 and reconsider the logic in iocore .@oknet can do it better than current logic.

@scw00 scw00 closed this Jun 7, 2017
@scw00 scw00 reopened this Jun 7, 2017
@scw00 scw00 changed the title Disable post and put retry Fix post and Jun 7, 2017
@scw00 scw00 changed the title Fix post and Fix post and put retry Jun 7, 2017
@mingzym
Copy link
Member

mingzym commented Jun 7, 2017

considering the trouble made after #947 we definitely should sort it out, no offense because this is the common case when we work in iocore, I am with @zwoop and @scw00

@scw00
Copy link
Member Author

scw00 commented Jun 7, 2017

@oknet @zwoop @bryancall Please review!!

Copy link
Member

@oknet oknet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the server connection retry mechanism of POST & PUT method never works correctly.

ua_buffer_reader->consume(client_request_body_bytes);
// Note: We can not consume the data sice we start retry.
// ua_buffer_reader will be destroy in HttpSessionClient::free
// ua_buffer_reader->consume(client_request_body_bytes);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ua_buffer_reader only keeps the first part of request payload. If the tunnel has read the second part of the request payload, the ATS will only replay the first part of request payload when retry to another server and the second part will be lost.

@scw00
Copy link
Member Author

scw00 commented Jun 9, 2017

#1994
This pr try to fix post and post transform retry.
It is only for test now.

@scw00 scw00 changed the title Fix post and put retry Fix post and post retry retry Jun 9, 2017
@scw00 scw00 changed the title Fix post and post retry retry Fix post and post transform retry Jun 9, 2017
@oknet
Copy link
Member

oknet commented Jun 10, 2017

[approve ci]

@scw00
Copy link
Member Author

scw00 commented Jun 15, 2017

The post request has blocked by #2123 sometimes . It will be fixed when #2123 merge.

Now, that is working well with post request.

Fake vc is running in the same thread. fvc will collect all data before it call sm . If post succeed or post failed entirely, fvc will use fvc_callcont to return state machine in tunnel_handler_fake. If we connect to server and send data successfully, the default handler (tunnel_handler_post)will be used.

If server aborted, we will setup fake vc ->server tunnel directly, and send the data collected by fvc. fvc is freed in tunnel reset or kill tunnel.

we can use 'proxy.config.http.tunnel_faker_enabled' to enable this feature.

@scw00
Copy link
Member Author

scw00 commented Jun 22, 2017

@oknet have another way to solve this problem and that will be better than this one. He will push a new pr ( #2184 )later

@scw00 scw00 closed this Jun 22, 2017
@scw00 scw00 deleted the disable_post_and_put_retry branch July 28, 2017 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants