From 2c2e985fd4708c6079b012e5b436f0bde804980d Mon Sep 17 00:00:00 2001
From: imalasong <2879499479@qq.com>
Date: Wed, 3 Apr 2024 18:07:06 +0800
Subject: [PATCH 1/3] GNUmakefile: add .PHONY: all and help

Signed-off-by: xiaochangbai <704566072@qq.com>
---
 GNUmakefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/GNUmakefile b/GNUmakefile
index aa7c8441c3d8..326fb5ebcb15 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -11,10 +11,11 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
 # implied. See the License for the specific language governing
 # permissions and limitations under the License.
-
+.PHONY: all
 all: build
 	$(MAKE) help
 
+.PHONY: help
 help:
 	@echo
 	@echo "Tip: use ./dev instead of 'make'."

From 49b995da5c1114f0fd41da23409040e876089920 Mon Sep 17 00:00:00 2001
From: Yahor Yuzefovich <yahor@cockroachlabs.com>
Date: Thu, 4 Apr 2024 19:25:04 -0700
Subject: [PATCH 2/3] server/profiler: remove unnecessary code

This commit reverts d879eab805d6c1104c5f314ac3ea53febfa5f065 which was
needed for compatibility with files generated by 20.2-betas. I think
it's a pretty safe bet that such files shouldn't be around anymore. If
there are still present, a warning will get logged.

Release note: None
---
 pkg/server/profiler/profilestore.go      | 9 ---------
 pkg/server/profiler/profilestore_test.go | 4 ----
 2 files changed, 13 deletions(-)

diff --git a/pkg/server/profiler/profilestore.go b/pkg/server/profiler/profilestore.go
index d9cc4aa93eb2..62ce63f6042b 100644
--- a/pkg/server/profiler/profilestore.go
+++ b/pkg/server/profiler/profilestore.go
@@ -160,15 +160,6 @@ func (s *profileStore) parseFileName(
 		// Not for us. Silently ignore.
 		return
 	}
-	if len(parts[2]) < 3 {
-		// At some point in the v20.2 cycle the timestamps were generated
-		// with format .999, which caused the trailing zeroes to be
-		// omitted. During parsing, they must be present, so re-add them
-		// here.
-		//
-		// TODO(knz): Remove this code in v21.1.
-		parts[2] += "000"[:3-len(parts[2])]
-	}
 	maybeTimestamp := parts[1] + "." + parts[2]
 	var err error
 	timestamp, err = time.Parse(timestampFormat, maybeTimestamp)
diff --git a/pkg/server/profiler/profilestore_test.go b/pkg/server/profiler/profilestore_test.go
index bcd6b905181b..7895ce450d8e 100644
--- a/pkg/server/profiler/profilestore_test.go
+++ b/pkg/server/profiler/profilestore_test.go
@@ -58,10 +58,6 @@ func TestParseFileName(t *testing.T) {
 
 		// New format.
 		{"memprof.2020-06-15T13_19_19.543.123456", time.Date(2020, 6, 15, 13, 19, 19, 543000000, time.UTC), 123456, false},
-		// v20.2 transition formats.
-		// TODO(knz): Remove in v21.1.
-		{"memprof.2020-06-15T13_19_19.54.123456", time.Date(2020, 6, 15, 13, 19, 19, 540000000, time.UTC), 123456, false},
-		{"memprof.2020-06-15T13_19_19.5.123456", time.Date(2020, 6, 15, 13, 19, 19, 500000000, time.UTC), 123456, false},
 	}
 
 	s := profileStore{prefix: heapFileNamePrefix}

From 1678e78ea6e2ee91b050711f5e6caec1b3a84de1 Mon Sep 17 00:00:00 2001
From: Yahor Yuzefovich <yahor@cockroachlabs.com>
Date: Thu, 4 Apr 2024 19:38:15 -0700
Subject: [PATCH 3/3] server/profiler: introduce separate limit for each
 profiler

This commit introduces a separate cluster setting for each profiler that
writes into `heapprofiler` directory. There are five of these (heap, mem
monitoring, jemalloc, mem stats, and active query), and previously all
of them use the same limit for the combined size (that was initially
introduced for the heap profiles). However, `DumpStore.GC` logic only
looks at the files owned by each profiler when calculating the
footprint, so each profiler would get up to 256MiB of space. This commit
makes the following limits the default:
- heap - 256MiB
- mem monitor - 4MiB (these files are usually in single KiBs range)
- jemalloc - 32MiB (don't have a good intuition about this one)
- mem stats - 32MiB (these files seems like an order of magnitude
smaller than heap profiles
- active query - 64MiB (these files seem smaller than heap profiles, but
preserving longer history might be beneficial in investigations).

Note that the heap profile limit was increased from 128MiB to 256MiB
when the mem monitoring profiler was introduced under incorrect
understanding that the limit was on the total size of the `heapprofiler`
directory. We could consider reducing it back down to 128MiB.

Release note: None
---
 pkg/server/profiler/BUILD.bazel               |  1 +
 pkg/server/profiler/activequeryprofiler.go    | 10 ++++++-
 .../profiler/activequeryprofiler_test.go      | 10 +++++--
 pkg/server/profiler/cgoprofiler.go            | 10 ++++++-
 pkg/server/profiler/heapprofiler.go           | 18 ++++++++++++
 .../profiler/memory_monitoring_profiler.go    |  9 +++++-
 pkg/server/profiler/profilestore.go           | 29 ++++---------------
 pkg/server/profiler/profilestore_test.go      |  5 ++++
 pkg/server/profiler/statsprofiler.go          | 10 ++++++-
 9 files changed, 72 insertions(+), 30 deletions(-)

diff --git a/pkg/server/profiler/BUILD.bazel b/pkg/server/profiler/BUILD.bazel
index 841942720435..da13a4b44878 100644
--- a/pkg/server/profiler/BUILD.bazel
+++ b/pkg/server/profiler/BUILD.bazel
@@ -49,6 +49,7 @@ go_test(
         "//pkg/clusterversion",
         "//pkg/server/dumpstore",
         "//pkg/settings/cluster",
+        "//pkg/util/log",
         "//pkg/util/mon",
         "//pkg/util/timeutil",
         "@com_github_cockroachdb_redact//:redact",
diff --git a/pkg/server/profiler/activequeryprofiler.go b/pkg/server/profiler/activequeryprofiler.go
index ce6ea7b13d27..7d7b00ca21f0 100644
--- a/pkg/server/profiler/activequeryprofiler.go
+++ b/pkg/server/profiler/activequeryprofiler.go
@@ -17,6 +17,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/server/debug"
 	"github.com/cockroachdb/cockroach/pkg/server/dumpstore"
+	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/util/cgroups"
@@ -63,6 +64,13 @@ const (
 	QueryFileNameSuffix = ".csv"
 )
 
+var activeQueryCombinedFileSize = settings.RegisterByteSizeSetting(
+	settings.ApplicationLevel,
+	"server.active_query.total_dump_size_limit",
+	"maximum combined disk size of preserved active query profiles",
+	64<<20, // 64MiB
+)
+
 // NewActiveQueryProfiler creates a NewQueryProfiler. dir is the directory in which
 // profiles are to be stored.
 func NewActiveQueryProfiler(
@@ -72,7 +80,7 @@ func NewActiveQueryProfiler(
 		return nil, errors.AssertionFailedf("need to specify dir for NewQueryProfiler")
 	}
 
-	dumpStore := dumpstore.NewStore(dir, maxCombinedFileSize, st)
+	dumpStore := dumpstore.NewStore(dir, activeQueryCombinedFileSize, st)
 
 	maxMem, warn, err := memLimitFn()
 	if err != nil {
diff --git a/pkg/server/profiler/activequeryprofiler_test.go b/pkg/server/profiler/activequeryprofiler_test.go
index 635d4e0d4778..6e445d6d5bbe 100644
--- a/pkg/server/profiler/activequeryprofiler_test.go
+++ b/pkg/server/profiler/activequeryprofiler_test.go
@@ -18,6 +18,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/server/dumpstore"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/stretchr/testify/require"
 )
 
@@ -53,7 +54,7 @@ func TestNewActiveQueryProfiler(t *testing.T) {
 			profiler: &ActiveQueryProfiler{
 				profiler: makeProfiler(
 					newProfileStore(
-						dumpstore.NewStore(heapProfilerDirName, maxCombinedFileSize, nil),
+						dumpstore.NewStore(heapProfilerDirName, activeQueryCombinedFileSize, nil),
 						QueryFileNamePrefix,
 						QueryFileNameSuffix,
 						nil),
@@ -81,12 +82,13 @@ func TestNewActiveQueryProfiler(t *testing.T) {
 			test.profiler.resetInterval = nil
 			profiler.resetInterval = nil
 			require.Equal(t, test.profiler, profiler)
-			require.Equal(t, test.profiler, profiler)
 		})
 	}
 }
 
 func TestShouldDump(t *testing.T) {
+	defer log.Scope(t).Close(t)
+
 	ctx := context.Background()
 	createSettingFn := func(settingEnabled bool) *cluster.Settings {
 		s := &cluster.Settings{}
@@ -100,7 +102,7 @@ func TestShouldDump(t *testing.T) {
 	profiler := &ActiveQueryProfiler{
 		profiler: profiler{
 			store: newProfileStore(
-				dumpstore.NewStore(heapProfilerDirName, maxCombinedFileSize, nil),
+				dumpstore.NewStore(heapProfilerDirName, activeQueryCombinedFileSize, nil),
 				QueryFileNamePrefix,
 				QueryFileNameSuffix,
 				nil),
@@ -205,6 +207,8 @@ func TestShouldDump(t *testing.T) {
 }
 
 func TestMaybeDumpQueries_PanicHandler(t *testing.T) {
+	defer log.Scope(t).Close(t)
+
 	ctx := context.Background()
 	memLimitFn = cgroupFnWithReturn(mbToBytes(256), "", nil)
 	s := &cluster.Settings{}
diff --git a/pkg/server/profiler/cgoprofiler.go b/pkg/server/profiler/cgoprofiler.go
index f129ec1bc92e..b52cc0509734 100644
--- a/pkg/server/profiler/cgoprofiler.go
+++ b/pkg/server/profiler/cgoprofiler.go
@@ -14,6 +14,7 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/server/dumpstore"
+	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
@@ -38,6 +39,13 @@ const jemallocFileNamePrefix = "jeprof"
 // jemallocFileNameSuffix is the file name extension of jemalloc profile dumps.
 const jemallocFileNameSuffix = ".jeprof"
 
+var jemallCombinedFileSize = settings.RegisterByteSizeSetting(
+	settings.ApplicationLevel,
+	"server.jemalloc.total_dump_size_limit",
+	"maximum combined disk size of preserved jemalloc profiles",
+	32<<20, // 32MiB
+)
+
 // NewNonGoAllocProfiler creates a NonGoAllocProfiler. dir is the
 // directory in which profiles are to be stored.
 func NewNonGoAllocProfiler(
@@ -47,7 +55,7 @@ func NewNonGoAllocProfiler(
 		return nil, errors.AssertionFailedf("need to specify dir for NewHeapProfiler")
 	}
 
-	dumpStore := dumpstore.NewStore(dir, maxCombinedFileSize, st)
+	dumpStore := dumpstore.NewStore(dir, jemallCombinedFileSize, st)
 
 	hp := &NonGoAllocProfiler{
 		profiler: makeProfiler(
diff --git a/pkg/server/profiler/heapprofiler.go b/pkg/server/profiler/heapprofiler.go
index d57172cb0fc8..eb0fdf1ab1ea 100644
--- a/pkg/server/profiler/heapprofiler.go
+++ b/pkg/server/profiler/heapprofiler.go
@@ -16,6 +16,7 @@ import (
 	"runtime/pprof"
 
 	"github.com/cockroachdb/cockroach/pkg/server/dumpstore"
+	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
@@ -39,6 +40,23 @@ const heapFileNamePrefix = "memprof"
 // heapFileNameSuffix is the suffix of files containing pprof data.
 const heapFileNameSuffix = ".pprof"
 
+var maxCombinedFileSize = settings.RegisterByteSizeSetting(
+	settings.ApplicationLevel,
+	"server.mem_profile.total_dump_size_limit",
+	"maximum combined disk size of preserved memory profiles",
+	256<<20, // 256MiB
+)
+
+func init() {
+	_ = settings.RegisterByteSizeSetting(
+		settings.ApplicationLevel,
+		"server.heap_profile.total_dump_size_limit",
+		"use server.mem_profile.total_dump_size_limit instead",
+		256<<20, // 256MiB
+		settings.Retired,
+	)
+}
+
 // NewHeapProfiler creates a HeapProfiler. dir is the directory in which
 // profiles are to be stored.
 func NewHeapProfiler(ctx context.Context, dir string, st *cluster.Settings) (*HeapProfiler, error) {
diff --git a/pkg/server/profiler/memory_monitoring_profiler.go b/pkg/server/profiler/memory_monitoring_profiler.go
index a8c17cb774af..46bb130b383e 100644
--- a/pkg/server/profiler/memory_monitoring_profiler.go
+++ b/pkg/server/profiler/memory_monitoring_profiler.go
@@ -44,6 +44,13 @@ type MemoryMonitoringProfiler struct {
 const memMonitoringFileNamePrefix = "memmonitoring"
 const memMonitoringFileNameSuffix = ".txt"
 
+var memMonitoringCombinedFileSize = settings.RegisterByteSizeSetting(
+	settings.ApplicationLevel,
+	"server.mem_monitoring.total_dump_size_limit",
+	"maximum combined disk size of preserved mem monitoring profiles",
+	4<<20, // 4MiB
+)
+
 // NewMemoryMonitoringProfiler returns a new MemoryMonitoringProfiler. dir is
 // the directory in which memory monitoring dumps are to be stored.
 func NewMemoryMonitoringProfiler(
@@ -53,7 +60,7 @@ func NewMemoryMonitoringProfiler(
 		return nil, errors.AssertionFailedf("need to specify dir for MemoryMonitoringProfiler")
 	}
 
-	dumpStore := dumpstore.NewStore(dir, maxCombinedFileSize, st)
+	dumpStore := dumpstore.NewStore(dir, memMonitoringCombinedFileSize, st)
 	mmp := &MemoryMonitoringProfiler{
 		profiler: makeProfiler(
 			newProfileStore(dumpStore, memMonitoringFileNamePrefix, memMonitoringFileNameSuffix, st),
diff --git a/pkg/server/profiler/profilestore.go b/pkg/server/profiler/profilestore.go
index 62ce63f6042b..0685a1b89e33 100644
--- a/pkg/server/profiler/profilestore.go
+++ b/pkg/server/profiler/profilestore.go
@@ -26,21 +26,12 @@ import (
 	"github.com/cockroachdb/errors"
 )
 
-var (
-	maxProfiles = settings.RegisterIntSetting(
-		settings.ApplicationLevel,
-		"server.mem_profile.max_profiles",
-		"maximum number of profiles to be kept per ramp-up of memory usage. "+
-			"A ramp-up is defined as a sequence of profiles with increasing usage.",
-		5,
-	)
-
-	maxCombinedFileSize = settings.RegisterByteSizeSetting(
-		settings.ApplicationLevel,
-		"server.mem_profile.total_dump_size_limit",
-		"maximum combined disk size of preserved memory profiles",
-		256<<20, // 256MiB
-	)
+var maxProfiles = settings.RegisterIntSetting(
+	settings.ApplicationLevel,
+	"server.mem_profile.max_profiles",
+	"maximum number of profiles to be kept per ramp-up of memory usage. "+
+		"A ramp-up is defined as a sequence of profiles with increasing usage.",
+	5,
 )
 
 func init() {
@@ -48,14 +39,6 @@ func init() {
 		settings.ApplicationLevel,
 		"server.heap_profile.max_profiles", "use server.mem_profile.max_profiles instead", 5,
 		settings.Retired)
-
-	_ = settings.RegisterByteSizeSetting(
-		settings.ApplicationLevel,
-		"server.heap_profile.total_dump_size_limit",
-		"use server.mem_profile.total_dump_size_limit instead",
-		256<<20, // 256MiB
-		settings.Retired,
-	)
 }
 
 // profileStore represents the directory where heap profiles are stored.
diff --git a/pkg/server/profiler/profilestore_test.go b/pkg/server/profiler/profilestore_test.go
index 7895ce450d8e..9a8ec78e6ea4 100644
--- a/pkg/server/profiler/profilestore_test.go
+++ b/pkg/server/profiler/profilestore_test.go
@@ -20,6 +20,7 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/server/dumpstore"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -42,6 +43,8 @@ func TestMakeFileName(t *testing.T) {
 }
 
 func TestParseFileName(t *testing.T) {
+	defer log.Scope(t).Close(t)
+
 	z := time.Time{}
 	testData := []struct {
 		f         string
@@ -73,6 +76,8 @@ func TestParseFileName(t *testing.T) {
 }
 
 func TestCleanupLastRampup(t *testing.T) {
+	defer log.Scope(t).Close(t)
+
 	testData := []struct {
 		startFiles []string
 		maxP       int64
diff --git a/pkg/server/profiler/statsprofiler.go b/pkg/server/profiler/statsprofiler.go
index 45a38099ee81..cf4c62d293ba 100644
--- a/pkg/server/profiler/statsprofiler.go
+++ b/pkg/server/profiler/statsprofiler.go
@@ -19,6 +19,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/server/dumpstore"
 	"github.com/cockroachdb/cockroach/pkg/server/status"
+	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
@@ -43,6 +44,13 @@ const statsFileNamePrefix = "memstats"
 // statsFileNameSuffix is the suffix of memory stats dumps.
 const statsFileNameSuffix = ".txt"
 
+var memStatsCombinedFileSize = settings.RegisterByteSizeSetting(
+	settings.ApplicationLevel,
+	"server.mem_stats.total_dump_size_limit",
+	"maximum combined disk size of preserved memstats profiles",
+	32<<20, // 32MiB
+)
+
 // NewStatsProfiler creates a StatsProfiler. dir is the
 // directory in which profiles are to be stored.
 func NewStatsProfiler(
@@ -52,7 +60,7 @@ func NewStatsProfiler(
 		return nil, errors.AssertionFailedf("need to specify dir for NewStatsProfiler")
 	}
 
-	dumpStore := dumpstore.NewStore(dir, maxCombinedFileSize, st)
+	dumpStore := dumpstore.NewStore(dir, memStatsCombinedFileSize, st)
 
 	hp := &StatsProfiler{
 		profiler: makeProfiler(