Repository: minio/minio (language: go)

Issue:
Suspected memory leak in S3Select Limit behavior
## Expected Behavior
Minio can reclaim memory properly and provide normal service.

## Current Behavior
When I repeatedly use the following command:

`mc sql --query "select * from S3Object limit 1000" txy/test/baimi.100w.desc.json`
`mc sql --query "select * from S3Object where protocol='HTTP/1.1' limit 1000" txy/test/baimi.100w.desc.json`
`mc sql --query "select * from S3Object where protocol='HTTP/1.0' limit 1000" txy/test/baimi.100w.desc.json`

When testing the select feature of Minio, I noticed that Minio's memory keeps increasing until the system runs out of memory and is killed by the Linux kernel. Here is a screenshot:

![image](https://github.com/minio/minio/assets/22673808/5fc56dce-29eb-4cf5-a0b6-9491836e51d1)
![image](https://github.com/minio/minio/assets/22673808/69ec7643-4390-43e0-879c-38c6a65c4ee8)

Below is a sample of the log "baimi.100w.desc.json":
```
{"clientip":"222.132.209.57","user":"-","datetime":"15/Jan/2023:01:58:52.340 +0800","method":"GET","request":"/index/login/?gw_address=192.168.13.1&gw_port=2060&gw_id=0539ls200152&mac=50:9f:27:f1:f1:5a&url=http%3A//usr.mb.hd.sohu.com/getts.json","protocol":"HTTP/1.1","status":200,"bytes":1934,"referer":"-","user_agent":"Dalvik/1.6.0 (Linux; U; Android 4.2.2; Y511-U00 Build/HUAWEIY511-U00)","x_forwarded_for":"-"}
{"clientip":"180.107.79.77","user":"-","datetime":"15/Jan/2023:01:58:52.335 +0800","method":"GET","request":"/index/login/?gw_address=192.168.11.1&gw_port=2060&gw_id=0512GS001296&mac=10:48:b1:34:a2:9f&url=http%3A//im13.115.com/chat/r%3Fc%3Db2%26s%3D2c0e12f0e38761ff8b6994392311f13c8eb4b7a4d4482af8ede10daa992da371%26VER%3D2","protocol":"HTTP/1.1","status":200,"bytes":2009,"referer":"-","user_agent":"Dalvik/1.6.0 (Linux; U; Android 4.3; MiTV2 Build/JLS36G)","x_forwarded_for":"-"}
{"clientip":"222.93.71.232","user":"-","datetime":"15/Jan/2023:01:58:52.329 +0800","method":"GET","request":"/index/ping/?gw_id=0512XC000063&sys_uptime=14251&sys_memfree=36892&sys_load=0.00&wifidog_uptime=14116&route_mac=84:82:F4:0B:54:90&route_version=100MSHBHU2S_R_2.0.2","protocol":"HTTP/1.0","status":200,"bytes":5,"referer":"-","user_agent":"WiFiDog 20130917","x_forwarded_for":"-"}
{"clientip":"49.72.224.43","user":"-","datetime":"15/Jan/2023:01:58:52.329 +0800","method":"GET","request":"/index/login/?gw_address=192.168.11.1&gw_port=2060&gw_id=0512GS000010&mac=78:f5:fd:6f:7e:ca&url=http%3A//usr.mb.hd.sohu.com/getts.json","protocol":"HTTP/1.1","status":200,"bytes":1937,"referer":"-","user_agent":"Dalvik/1.6.0 (Linux; U; Android 4.1.1; HUAWEI C8813 Build/HuaweiC8813)","x_forwarded_for":"-"}
{"clientip":"121.11.157.146","user":"-","datetime":"15/Jan/2023:01:58:52.326 +0800","method":"GET","request":"/index/login/?gw_address=192.168.11.1&gw_port=2060&gw_id=0663RC000135&mac=10:68:3f:8b:45:a9&url=http%3A//usr.mb.hd.sohu.com/getts.json","protocol":"HTTP/1.1","status":200,"bytes":1938,"referer":"-","user_agent":"Dalvik/1.6.0 (Linux; U; Android 4.1.2; LG-F240L Build/JZO54K)","x_forwarded_for":"-"}
```

## Possible Solution
<!--- Not obligatory, but suggest a fix/reason for the bug, -->
<!--- or ideas how to implement the addition or change -->

## Steps to Reproduce (for bugs)
<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug. Include code to reproduce, if relevant -->
<!--- and make sure you have followed https://github.com/minio/minio/tree/release/docs/debugging to capture relevant logs -->
I have written a script that executes the above SQL statement every 1 second. Each SQL statement is executed 50 times before switching to the next one. I have observed that the memory of the Minio server keeps increasing over time.

I have already tried using:

`mc admin trace  -v txy`

I have tried command tracing, but I didn't find anything significant. Here is the screenshot:

![image](https://github.com/minio/minio/assets/22673808/dad0dc59-c0f3-4642-bca9-0a481f85a7e1)

## Context
<!--- How has this issue affected you? What are you trying to accomplish? -->
<!--- Providing context helps us come up with a solution that is most useful in the real world -->

## Regression
<!-- Is this issue a regression? (Yes / No) -->
<!-- If Yes, optionally please include minio version or commit id or PR# that caused this regression, if you have these details. -->

## Your Environment
<!--- Include as many relevant details about the environment you experienced the bug in -->
* Version used (`minio --version`):

```
[root@cluster2 ~]# minio --version
minio version RELEASE.2023-02-10T18-48-39Z (commit-id=d0f4cc89a5d0ca45e7ba00fbb7733c2fead3f44e)
Runtime: go1.19.4 linux/amd64
License: GNU AGPLv3 <https://www.gnu.org/licenses/agpl-3.0.html>
Copyright: 2015-2023 MinIO, Inc.
```

* Server setup and configuration:

```
[root@cluster2 ~]# systemctl status minio
  minio.service - MinIO
   Loaded: loaded (/etc/systemd/system/minio.service; disabled; vendor preset: disabled)
   Active: active (running) since Thu 2023-05-18 20:14:31 CST; 9min ago
     Docs: https://docs.min.io
  Process: 215712 ExecStartPre=/bin/bash -c if [ -z "${MINIO_VOLUMES}" ]; then echo "Variable MINIO_VOLUMES not set in /etc/default/minio"; exit 1; fi (code=exited, status=0/SUCCESS)
 Main PID: 215714 (minio)
    Tasks: 10
   Memory: 347.3M
   CGroup: /system.slice/minio.service
           └─215714 /usr/local/bin/minio server --address 172.21.48.12:8000 --console-address :8001 /data/minio

May 18 20:14:31 cluster2 minio[215714]: WARNING: Detected default credentials 'minioadmin:minioadmin', we recommend that you change t...ariables
May 18 20:14:31 cluster2 minio[215714]: MinIO Object Storage Server
May 18 20:14:31 cluster2 minio[215714]: Copyright: 2015-2023 MinIO, Inc.
May 18 20:14:31 cluster2 minio[215714]: License: GNU AGPLv3 <https://www.gnu.org/licenses/agpl-3.0.html>
May 18 20:14:31 cluster2 minio[215714]: Version: RELEASE.2023-02-10T18-48-39Z (go1.19.4 linux/amd64)
May 18 20:14:31 cluster2 minio[215714]: Status:         1 Online, 0 Offline.
May 18 20:14:31 cluster2 minio[215714]: API: http://172.21.48.12:8000
May 18 20:14:31 cluster2 minio[215714]: Console: http://172.21.48.12:8001 http://172.17.0.1:8001 http://127.0.0.1:8001
May 18 20:14:31 cluster2 minio[215714]: Documentation: https://min.io/docs/minio/linux/index.html
May 18 20:14:31 cluster2 minio[215714]: Warning: The standard parity is set to 0. This can lead to data loss.
Hint: Some lines were ellipsized, use -l to show in full.
[root@cluster2 ~]# 
[root@cluster2 ~]# cat /etc/default/minio
MINIO_VOLUMES="/data/minio"
MINIO_OPTS="--address 172.21.48.12:8000 --console-address :8001"
MINIO_ROOT_USER=minioadmin
MINIO_ROOT_PASSWORD=minioadmin
```

* Operating System and version (`uname -a`):

```
[root@cluster2 ~]# cat /etc/default/minio 
MINIO_VOLUMES="/data/minio"
MINIO_OPTS="--address 172.21.48.12:8000 --console-address :8001"
MINIO_ROOT_USER=minioadmin
MINIO_ROOT_PASSWORD=minioadmin
[root@cluster2 ~]# uname -a 
Linux cluster2 3.10.0-1160.59.1.el7.x86_64 #1 SMP Wed Feb 23 16:47:03 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
[root@cluster2 ~]# uname -r 
3.10.0-1160.59.1.el7.x86_64
[root@cluster2 ~]# cat /etc/os-release 
NAME="CentOS Linux"
VERSION="7 (Core)"
ID="centos"
ID_LIKE="rhel fedora"
VERSION_ID="7"
PRETTY_NAME="CentOS Linux 7 (Core)"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:centos:centos:7"
HOME_URL="https://www.centos.org/"
BUG_REPORT_URL="https://bugs.centos.org/"

CENTOS_MANTISBT_PROJECT="CentOS-7"
CENTOS_MANTISBT_PROJECT_VERSION="7"
REDHAT_SUPPORT_PRODUCT="centos"
REDHAT_SUPPORT_PRODUCT_VERSION="7"
```



Discussion / hints:
Let me see if I can reproduce.
Reproed. Seems like returning early (limit 1000) isn't releasing resources.
Great ! looking forward to a fix.

Fix patch:
diff --git a/internal/s3select/simdj/reader.go b/internal/s3select/simdj/reader.go
index 7e1cc63c6faf6..86a72e4332fc3 100644
--- a/internal/s3select/simdj/reader.go
+++ b/internal/s3select/simdj/reader.go
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io"
 	"sync"
+	"sync/atomic"
 
 	"github.com/minio/minio/internal/s3select/json"
 	"github.com/minio/minio/internal/s3select/sql"
@@ -34,8 +35,9 @@ type Reader struct {
 	decoded chan simdjson.Object
 
 	// err will only be returned after decoded has been closed.
-	err        *error
-	readCloser io.ReadCloser
+	err          *error
+	readCloser   io.ReadCloser
+	onReaderExit func()
 
 	exitReader chan struct{}
 	readerWg   sync.WaitGroup
@@ -79,9 +81,8 @@ func (r *Reader) Close() error {
 // If r.input is closed, it is assumed that no more input will come.
 // When this function returns r.readerWg will be decremented and r.decoded will be closed.
 // On errors, r.err will be set. This should only be accessed after r.decoded has been closed.
-func (r *Reader) startReader() {
-	defer r.readerWg.Done()
-	defer close(r.decoded)
+func (r *Reader) startReader(reuse chan<- *simdjson.ParsedJson) {
+	defer r.onReaderExit()
 	var tmpObj simdjson.Object
 	for {
 		var in simdjson.Stream
@@ -143,6 +144,11 @@ func (r *Reader) startReader() {
 				return
 			}
 		}
+		// Don't block if we cannot reuse.
+		select {
+		case reuse <- in.Value:
+		default:
+		}
 		if in.Error == io.EOF {
 			return
 		}
@@ -153,14 +159,25 @@ func (r *Reader) startReader() {
 func NewReader(readCloser io.ReadCloser, args *json.ReaderArgs) *Reader {
 	r := Reader{
 		args:       args,
-		readCloser: readCloser,
+		readCloser: &safeCloser{r: io.Reader(readCloser)},
 		decoded:    make(chan simdjson.Object, 1000),
 		input:      make(chan simdjson.Stream, 2),
 		exitReader: make(chan struct{}),
 	}
-	simdjson.ParseNDStream(readCloser, r.input, nil)
+	r.onReaderExit = func() {
+		close(r.decoded)
+		readCloser.Close()
+		for range r.input {
+			// Read until EOF trickles through.
+			// Otherwise, we risk the decoder hanging.
+		}
+		r.readerWg.Done()
+	}
+
+	reuse := make(chan *simdjson.ParsedJson, 1000)
+	simdjson.ParseNDStream(readCloser, r.input, reuse)
 	r.readerWg.Add(1)
-	go r.startReader()
+	go r.startReader(reuse)
 	return &r
 }
 
@@ -174,15 +191,25 @@ func NewElementReader(ch chan simdjson.Object, err *error, args *json.ReaderArgs
 	}
 }
 
-// NewTapeReaderChan will start a reader that will read input from the provided channel.
-func NewTapeReaderChan(pj chan simdjson.Stream, args *json.ReaderArgs) *Reader {
-	r := Reader{
-		args:       args,
-		decoded:    make(chan simdjson.Object, 1000),
-		input:      pj,
-		exitReader: make(chan struct{}),
+// safeCloser will wrap a Reader as a ReadCloser.
+// It is safe to call Close while the reader is being used.
+type safeCloser struct {
+	closed uint32
+	r      io.Reader
+}
+
+func (s *safeCloser) Read(p []byte) (n int, err error) {
+	if atomic.LoadUint32(&s.closed) == 1 {
+		return 0, io.EOF
 	}
-	r.readerWg.Add(1)
-	go r.startReader()
-	return &r
+	n, err = s.r.Read(p)
+	if atomic.LoadUint32(&s.closed) == 1 {
+		return 0, io.EOF
+	}
+	return n, err
+}
+
+func (s *safeCloser) Close() error {
+	atomic.CompareAndSwapUint32(&s.closed, 0, 1)
+	return nil
 }


Test patch: