0

I have the following code in GoLang

package main

import (
    "bytes"
    "encoding/json"
    "io/ioutil"
    "log"
    "net/http"
    "time"
)

func httpClient() *http.Client {
    var transport http.RoundTripper = &http.Transport{
        DisableKeepAlives: false,
}
    client := &http.Client{Timeout: 60 * time.Second, Transport: transport}
    return client
}

func sendRequest(client *http.Client, method string) []byte {
    endpoint := "https://httpbin.org/post"
      values := map[string]string{"foo": "baz"}
    jsonData, err := json.Marshal(values)
    req, err := http.NewRequest(method, endpoint, bytes.NewBuffer(jsonData))
    if err != nil {
        log.Fatalf("Error Occurred. %+v", err)
    }

    resp, err:= client.Do(req)
    if err != nil {
        defer resp.Body.Close()
        log.Fatalf("Error sending request to API endpoint. %+v", err)
    }

    // Close the connection to reuse it
    defer resp.Body.Close()
    
    body, err := ioutil.ReadAll(resp.Body)
    if err != nil {
        log.Fatalf("Couldn't parse response body. %+v", err)
    }   
    return body
}

func main() {
  // c should be re-used for further calls
    c := httpClient()
    for i := 1; i <= 60; i++ {
    response := sendRequest(c, http.MethodPost)
    log.Println("Response Body:", string(response))
    response = nil
    time.Sleep(time.Millisecond * 1000)
    }
}

When executed, it keeps the memory size increasing and the growth goes to as much as 90mb in one hour. is the gc not working properly. Even though i am using same httpclient for multiple requests but it still looks like theres something thats increasing the size of memory footprint.

Ahsan
  • 459
  • 8
  • 22
  • how do you measure memory usage ? – davidriod Feb 06 '23 at 10:04
  • The go runtime is quite complex. Based on the code you show here, nothing really stands out as a memory leak, but then the code doesn't really look like something that would be running for 1 hour (you're making 60 requests with a 1 second sleep in between, so this should run in 1 minute + request time). All we essentially know is that you claim to see 90MB of memory usage, but no indication as to what means you're using to assess memory usage. Also: `time.Milisecond * 1000` can be abbreviated to `time.Second` – Elias Van Ootegem Feb 06 '23 at 10:11
  • I am testing the code for 60 seconds obviously for the sake of time saving. Because that timeframe is enough to debug on longer usages. I am obviously running the program in for loop for 3600 in 1hr timeframes. – Ahsan Feb 06 '23 at 10:18
  • @davidriod using sys internals process monitor. – Ahsan Feb 06 '23 at 10:19
  • If using 90M is not causing any memory pressure, and it keeps the garbage collector running smoothly, then there is no problem. The only way to leak memory is via goroutines which are not returning, so a stack trace is the best way see if you actually have a problem. The most concise way to see what the garbage collector is doing (and what memory the process sees as being used) is by viewing the gctrace output. – JimB Feb 06 '23 at 15:21
  • @JimB, will definitely do it but i was trying to avoid it as it was a lengthy process for a small problem. – Ahsan Feb 08 '23 at 07:03
  • There is no lengthy process, send a `SIGQUIT` to dump a full stack trace, use `GODEBUG=gctrace=1` to see the gctrace output. In all likelihood you are chasing a nonexistent problem, and memory usage levels out over time. – JimB Feb 08 '23 at 14:21

1 Answers1

0

I advice you to use tools like pprof, these are very useful at troubleshooting precisely this kind of issues.

You have set DisableKeepAlives field to false, which means that it will keep open connections even after the requests have been made, leading to further memory leaks. You should also call defer resp.Body.Close() after calling ioutil.ReadAll(resp.Body). This is precisely the purpose of the defer keyword - preventing memory leaks. GC does not mean absolute memory safety.

Also, outside of main avoid using log.Fatal. Use leveled logger, like zap or zerolog instead, since log.Fatal calls os.Exit(1) with an immediate effect, which means your defer statements will take no effect, or call plain panic. See Should a Go package ever use log.Fatal and when?

olithad
  • 50
  • 4
  • Aside from debugging, the main purpose of this program is to reuse the existing http connection. – Ahsan Feb 06 '23 at 09:56
  • `defer resp.Body.Close()` is called. It's the statement just before the `ioutil.ReadAll()` call. The defer doesn't have to be added _after_ the `ReadAll` call, it's deferred to occur after the function returns, so the response body will be closed regardless – Elias Van Ootegem Feb 06 '23 at 10:07
  • Got it but doing so has no impact on improvement thus. – Ahsan Feb 06 '23 at 10:20