Memory leak when commands are failing

[creker]

rueidis v1.0.17
Tested on Go 1.21.1 and Go 1.20.8

Looks like rueidis leaks memory when pipelining commands that later fail. For example, due to context timeout. I suspect they're not getting recycled properly and retain data inside them. I was able to write an example that consistently reproduces the behaviour.

First, let's start with this example that works:

package main

import (
	"context"
	"fmt"
	"net/http"
	_ "net/http/pprof"
	"time"

	"github.com/redis/rueidis"
)

func work(cl rueidis.Client) {
	for {
		buf := make([]byte, 10000000)

		ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond)

		fmt.Println(cl.Do(ctx, cl.B().Set().Key("test").Value(rueidis.BinaryString(buf)).Build()).Error())

		cancel()

		time.Sleep(time.Second)
	}
}

func main() {
	go func() {
		http.ListenAndServe(":8081", nil)
	}()

	cl, err := rueidis.NewClient(rueidis.ClientOption{
		InitAddress: []string{"localhost:6379"},
	})
	if err != nil {
		panic(err)
	}

	work(cl)
}

This code works and I wasn't able to observe any leaks. It's important that it prints "context deadline exceeded". Without that everything works as expected.

Now, let's make a couple of changes to force the client to pipeline commands. First, we can launch another goroutine in parallel like this:

go work(cl)
work(cl)

Unfortunately, it takes some time. Sometimes 30 seconds, sometimes a couple of minutes. But eventually memory will start ballooning. If we look at the heap profile, it will show that all the allocated memory is in this line:

buf := make([]byte, 10000000)

There's another way to trigger that behaviour. We don't even need to run multiple gorouines. Just enabling AlwaysPipelining option consistently and immediately reproduces the memory leak.

[rueian]

Hi @creker,

Thank you! This will be fixed in the next release.

[creker]

@rueian thank you for such a fast response! I tested the changes and got mixed results.

With AlwaysPipelining=false I no longer observe the leak no matter what I do, so that's great. But I can still see it when AlwaysPipelining=true. You can use the same code as above with 2 or more goroutines but you need to remove the sleep. The memory will start ballooning immediately.

[rueian]

Hi @creker,

Pipelining does use a certain amount of memory, but it will not balloon memory usage indefinitely.

You can reduce that certain amount of memory by lowering the RingScaleEachConn and the PipelineMultiplex options.

For example, the following setting will keep occupying about 1.3GB of memory:

package main

import (
	"context"
	"fmt"
	"time"

	"github.com/redis/rueidis"
)

func work(cl rueidis.Client) {
	for {
		buf := make([]byte, 10000000)
		ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond)
		fmt.Println(cl.Do(ctx, cl.B().Set().Key("test").Value(rueidis.BinaryString(buf)).Build()).Error())
		cancel()
	}
}

func main() {
	cl, err := rueidis.NewClient(rueidis.ClientOption{
		InitAddress:       []string{"localhost:6379"},
		AlwaysPipelining:  true,
		PipelineMultiplex: -1,
		RingScaleEachConn: 6,
	})
	if err != nil {
		panic(err)
	}
	go work(cl)
	work(cl)
}

[creker]

@rueian of course, pipelining uses some memory but I wouldn't expect it to depend so much on the size of the commands. All the commands are failing almost immediately. There's only 2 goroutines and minimal amount of inflight requests but rueidis still holds to the command contents. WIth your example it does stop at 1.3GB but I still think this is a problematic behaviour. All that memory is buf := make([]byte, 10000000), so the initial problem is still there unfortunately.

[rueian]

Those commands are not failing immediately, they are likely queued in the pipeline or halfway through pipelining.

Rueidis will hold command content until it receives the command response from redis because that content is necessary for processing the response properly in many cases.

Unfortunately, there is probably no way to drop those halfway commands except by closing the connection directly. I would suggest you try either removing the context.WithTimeout or lowering the RingScaleEachConn and the PipelineMultiplex options to reduce memory usage.

[creker]

Yeah, bad wording on my part. I can understand the problem. Thank you. At least now I know which knobs to turn in case the problem comes back again. I will close the issue.

[rueian]

I understand. The cause of the problem is not obvious at first glance.

Also there is still room for improving memory efficiency, probably cleaning most commands right after they are written to the network. Contributions are always welcome.