stmcginnis / gofish

Gofish is a Golang client library for DMTF Redfish and SNIA Swordfish interaction.
BSD 3-Clause "New" or "Revised" License
204 stars 109 forks source link

http requests to keep using the same TCP connection #349

Closed valdemarpavesi closed 3 weeks ago

valdemarpavesi commented 4 weeks ago

using context and setting IdleConnTimeout and DisableKeepAlives and req.Close = false

still release the TCP for every new request.

We wanna keep the same TCP connection Have you accomplished it?

        ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
    defer cancel() // Ensure the context is canceled to free resources

    defaultTransport := http.DefaultTransport.(*http.Transport)
    transport := &http.Transport{
        Proxy: defaultTransport.Proxy,
        //DialContext:           defaultTransport.DialContext,
        DialContext: (&net.Dialer{
            Timeout:   60 * time.Second,
            KeepAlive: 15 * time.Second,
            DualStack: true,
        }).DialContext,
        MaxIdleConns:          defaultTransport.MaxIdleConns,
        IdleConnTimeout:       10 * time.Minute, //defaultTransport.IdleConnTimeout
        ExpectContinueTimeout: defaultTransport.ExpectContinueTimeout,
        TLSHandshakeTimeout:   30 * time.Second,
        DisableKeepAlives:     false, // default true, if true, disables HTTP keep-alives and will only use the connection to the server for a single HTTP request, tcp rst for every send.
        TLSClientConfig: &tls.Config{
            InsecureSkipVerify: true,
        },
    }

    client_HTTPClient := &http.Client{Transport: transport}

    config := gofish.ClientConfig{
        Endpoint:   "test",
        Username:   "test",
        Password:   "test",
        Insecure:   true,
        HTTPClient: client_HTTPClient,
    }

    c, err := gofish.ConnectContext(ctx, config)
    if err != nil {
        panic(err)
    }

    service := c.Service
    chassis, err := service.Chassis()
    if err != nil {
        panic(err)
    }

    for _, chass := range chassis {
        fmt.Printf("Chassis: %#v\n\n", chass)
    }

    // The client and its underlying connections can be reused for subsequent requests
    // The connections will be kept open and reused according to the transport's configuration
    fmt.Println("Press any key to continue...")
    var input string
    fmt.Scanln(&input)
stmcginnis commented 4 weeks ago

I have not tried this, but you could make a local modification and see if it would work. I think you would just need to change this line:

https://github.com/stmcginnis/gofish/blob/main/client.go#L503

valdemarpavesi commented 3 weeks ago

Thanks @stmcginnis, yes it helps

   transport
   DisableKeepAlives:     false
   IdleConnTimeout:       1 * time.Minute

  and

req.Close = false
req.Header.Add("Connection", "keep-alive")

vendor1 tcp fin/ack 11 times vendor2 tcp fin/ack 37

with req.Close = true

vendor1 tcp fin/ack 169 times vendor2 tcp fin/ack 37

vendor2 make no difference.

Thanks.

regards! Valdemar

stmcginnis commented 3 weeks ago

Awesome - does this mean it works how you would like it to with that change? I'm wondering if we need to expose a config flag or something to make this easier. Or if there is value in trying to make this the default behavior.

valdemarpavesi commented 3 weeks ago

Hello @stmcginnis Yes, if can expose it into the config will be better.

regards! Valdemar

stmcginnis commented 3 weeks ago

If you're able to test with #351, that would be great to have someone else verify it improves things.