From ccf8d664d6868d0aa9d972a53685aec0e3e06aef Mon Sep 17 00:00:00 2001 From: Ryan Leung Date: Wed, 7 Aug 2024 15:33:40 +0800 Subject: [PATCH] *: fix the gctuner tests (#6330) close tikv/pd#5953 Signed-off-by: Ryan Leung --- pkg/gctuner/finalizer_test.go | 6 +-- pkg/gctuner/memory_limit_tuner_test.go | 63 ++++++++++++++------------ pkg/gctuner/tuner.go | 4 ++ pkg/gctuner/tuner_calc_test.go | 39 ---------------- pkg/gctuner/tuner_test.go | 41 +++++++++++++---- pkg/gogc/gogc.go | 6 --- 6 files changed, 73 insertions(+), 86 deletions(-) delete mode 100644 pkg/gctuner/tuner_calc_test.go diff --git a/pkg/gctuner/finalizer_test.go b/pkg/gctuner/finalizer_test.go index 64cb308d931..9231ca633e5 100644 --- a/pkg/gctuner/finalizer_test.go +++ b/pkg/gctuner/finalizer_test.go @@ -12,14 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build enable_flaky_tests - package gctuner import ( "runtime" "sync/atomic" "testing" + "time" "github.com/stretchr/testify/require" ) @@ -29,7 +28,7 @@ type testState struct { } func TestFinalizer(t *testing.T) { - maxCount := int32(16) + maxCount := int32(8) state := &testState{} f := newFinalizer(func() { n := atomic.AddInt32(&state.count, 1) @@ -39,6 +38,7 @@ func TestFinalizer(t *testing.T) { }) for i := int32(1); i <= maxCount; i++ { runtime.GC() + time.Sleep(10 * time.Millisecond) require.Equal(t, i, atomic.LoadInt32(&state.count)) } require.Nil(t, f.ref) diff --git a/pkg/gctuner/memory_limit_tuner_test.go b/pkg/gctuner/memory_limit_tuner_test.go index f56a64a7326..5e5f84ccbac 100644 --- a/pkg/gctuner/memory_limit_tuner_test.go +++ b/pkg/gctuner/memory_limit_tuner_test.go @@ -12,8 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build enable_flaky_tests - package gctuner import ( @@ -47,9 +45,9 @@ func (a *mockAllocator) freeAll() { } func TestGlobalMemoryTuner(t *testing.T) { - require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/util/gctuner/testMemoryLimitTuner", "return(true)")) + require.NoError(t, failpoint.Enable("github.com/tikv/pd/pkg/gctuner/testMemoryLimitTuner", "return(true)")) defer func() { - require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/util/gctuner/testMemoryLimitTuner")) + require.NoError(t, failpoint.Disable("github.com/tikv/pd/pkg/gctuner/testMemoryLimitTuner")) }() // Close GOGCTuner gogcTuner := EnableGOGCTuner.Load() @@ -61,11 +59,19 @@ func TestGlobalMemoryTuner(t *testing.T) { GlobalMemoryLimitTuner.UpdateMemoryLimit() require.True(t, GlobalMemoryLimitTuner.isTuning.Load()) defer func() { - time.Sleep(1 * time.Second) // If test.count > 1, wait tuning finished. - require.True(t, GlobalMemoryLimitTuner.isTuning.Load()) - // skip unstable test - // require.False(t, GlobalMemoryLimitTuner.waitingReset.Load()) - require.Equal(t, GlobalMemoryLimitTuner.nextGCTriggeredByMemoryLimit.Load(), false) + // If test.count > 1, wait tuning finished. + require.Eventually(t, func() bool { + //nolint: all_revive + return GlobalMemoryLimitTuner.isTuning.Load() + }, 5*time.Second, 100*time.Millisecond) + require.Eventually(t, func() bool { + //nolint: all_revive + return !GlobalMemoryLimitTuner.waitingReset.Load() + }, 5*time.Second, 100*time.Millisecond) + require.Eventually(t, func() bool { + //nolint: all_revive + return !GlobalMemoryLimitTuner.nextGCTriggeredByMemoryLimit.Load() + }, 5*time.Second, 100*time.Millisecond) }() allocator := &mockAllocator{} @@ -77,44 +83,43 @@ func TestGlobalMemoryTuner(t *testing.T) { } checkNextGCEqualMemoryLimit := func() { runtime.ReadMemStats(r) - // skip unstable test - // nextGC := r.NextGC - // memoryLimit := GlobalMemoryLimitTuner.calcMemoryLimit(GlobalMemoryLimitTuner.GetPercentage()) + nextGC := r.NextGC + memoryLimit := GlobalMemoryLimitTuner.calcMemoryLimit(GlobalMemoryLimitTuner.GetPercentage()) // In golang source, nextGC = memoryLimit - three parts memory. - // require.True(t, nextGC < uint64(memoryLimit)) + require.Less(t, nextGC, uint64(memoryLimit)) } memory600mb := allocator.alloc(600 << 20) gcNum := getNowGCNum() memory210mb := allocator.alloc(210 << 20) - time.Sleep(100 * time.Millisecond) - // skip unstable test - // require.True(t, GlobalMemoryLimitTuner.waitingReset.Load()) - require.True(t, gcNum < getNowGCNum()) + require.Eventually(t, func() bool { + return GlobalMemoryLimitTuner.waitingReset.Load() && gcNum < getNowGCNum() + }, 5*time.Second, 100*time.Millisecond) // Test waiting for reset - time.Sleep(500 * time.Millisecond) - require.Equal(t, GlobalMemoryLimitTuner.calcMemoryLimit(fallbackPercentage), debug.SetMemoryLimit(-1)) + require.Eventually(t, func() bool { + return GlobalMemoryLimitTuner.calcMemoryLimit(fallbackPercentage) == debug.SetMemoryLimit(-1) + }, 5*time.Second, 100*time.Millisecond) gcNum = getNowGCNum() memory100mb := allocator.alloc(100 << 20) - time.Sleep(100 * time.Millisecond) - require.Equal(t, gcNum, getNowGCNum()) // No GC + require.Eventually(t, func() bool { + return gcNum == getNowGCNum() + }, 5*time.Second, 100*time.Millisecond) // No GC allocator.free(memory210mb) allocator.free(memory100mb) runtime.GC() // Trigger GC in 80% again - time.Sleep(500 * time.Millisecond) - // skip unstable test - // require.Equal(t, GlobalMemoryLimitTuner.calcMemoryLimit(GlobalMemoryLimitTuner.GetPercentage()), debug.SetMemoryLimit(-1)) + require.Eventually(t, func() bool { + return GlobalMemoryLimitTuner.calcMemoryLimit(GlobalMemoryLimitTuner.GetPercentage()) == debug.SetMemoryLimit(-1) + }, 5*time.Second, 100*time.Millisecond) time.Sleep(100 * time.Millisecond) - // skip unstable test - // gcNum = getNowGCNum() + gcNum = getNowGCNum() checkNextGCEqualMemoryLimit() memory210mb = allocator.alloc(210 << 20) - time.Sleep(100 * time.Millisecond) - // skip unstable test - // require.True(t, gcNum < getNowGCNum()) + require.Eventually(t, func() bool { + return gcNum < getNowGCNum() + }, 5*time.Second, 100*time.Millisecond) allocator.free(memory210mb) allocator.free(memory600mb) } diff --git a/pkg/gctuner/tuner.go b/pkg/gctuner/tuner.go index 172c6adf326..74932fe174b 100644 --- a/pkg/gctuner/tuner.go +++ b/pkg/gctuner/tuner.go @@ -148,6 +148,10 @@ func (t *tuner) getGCPercent() uint32 { // tuning check the memory inuse and tune GC percent dynamically. // Go runtime ensure that it will be called serially. func (t *tuner) tuning() { + if !EnableGOGCTuner.Load() { + return + } + inuse := readMemoryInuse() threshold := t.getThreshold() log.Debug("tuning", zap.Uint64("inuse", inuse), zap.Uint64("threshold", threshold), diff --git a/pkg/gctuner/tuner_calc_test.go b/pkg/gctuner/tuner_calc_test.go deleted file mode 100644 index 473f5bda67d..00000000000 --- a/pkg/gctuner/tuner_calc_test.go +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright 2023 TiKV Project Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// 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. - -package gctuner - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestCalcGCPercent(t *testing.T) { - const gb = 1024 * 1024 * 1024 - // use default value when invalid params - require.Equal(t, defaultGCPercent, calcGCPercent(0, 0)) - require.Equal(t, defaultGCPercent, calcGCPercent(0, 1)) - require.Equal(t, defaultGCPercent, calcGCPercent(1, 0)) - - require.Equal(t, maxGCPercent.Load(), calcGCPercent(1, 3*gb)) - require.Equal(t, maxGCPercent.Load(), calcGCPercent(gb/10, 4*gb)) - require.Equal(t, maxGCPercent.Load(), calcGCPercent(gb/2, 4*gb)) - require.Equal(t, uint32(300), calcGCPercent(1*gb, 4*gb)) - require.Equal(t, uint32(166), calcGCPercent(1.5*gb, 4*gb)) - require.Equal(t, uint32(100), calcGCPercent(2*gb, 4*gb)) - require.Equal(t, uint32(100), calcGCPercent(3*gb, 4*gb)) - require.Equal(t, minGCPercent.Load(), calcGCPercent(4*gb, 4*gb)) - require.Equal(t, minGCPercent.Load(), calcGCPercent(5*gb, 4*gb)) -} diff --git a/pkg/gctuner/tuner_test.go b/pkg/gctuner/tuner_test.go index 604cd449b35..7018634c5d1 100644 --- a/pkg/gctuner/tuner_test.go +++ b/pkg/gctuner/tuner_test.go @@ -4,7 +4,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http://www.apache.org/licenses/LICENSE-2.0 +// http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, @@ -12,14 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build enable_flaky_tests - package gctuner import ( "runtime" "testing" + "time" + "github.com/docker/go-units" "github.com/stretchr/testify/require" ) @@ -27,7 +27,7 @@ var testHeap []byte func TestTuner(t *testing.T) { EnableGOGCTuner.Store(true) - memLimit := uint64(1000 * 1024 * 1024) // 1000 MB + memLimit := uint64(1000 * units.MiB) // 1000 MB threshold := memLimit / 2 tn := newTuner(threshold) require.Equal(t, threshold, tn.threshold.Load()) @@ -39,7 +39,8 @@ func TestTuner(t *testing.T) { runtime.GC() for i := 0; i < 100; i++ { runtime.GC() - require.Equal(t, maxGCPercent.Load(), tn.getGCPercent()) + require.Eventually(t, func() bool { return maxGCPercent.Load() == tn.getGCPercent() }, + 1*time.Second, 50*time.Microsecond) } // 1/4 threshold @@ -55,8 +56,10 @@ func TestTuner(t *testing.T) { runtime.GC() for i := 0; i < 100; i++ { runtime.GC() - require.GreaterOrEqual(t, tn.getGCPercent(), minGCPercent.Load()) - require.LessOrEqual(t, tn.getGCPercent(), maxGCPercent.Load()/2) + require.Eventually(t, func() bool { return tn.getGCPercent() >= minGCPercent.Load() }, + 1*time.Second, 50*time.Microsecond) + require.Eventually(t, func() bool { return tn.getGCPercent() <= maxGCPercent.Load()/2 }, + 1*time.Second, 50*time.Microsecond) } // 3/4 threshold @@ -64,7 +67,8 @@ func TestTuner(t *testing.T) { runtime.GC() for i := 0; i < 100; i++ { runtime.GC() - require.Equal(t, minGCPercent.Load(), tn.getGCPercent()) + require.Eventually(t, func() bool { return minGCPercent.Load() == tn.getGCPercent() }, + 1*time.Second, 50*time.Microsecond) } // out of threshold @@ -72,6 +76,25 @@ func TestTuner(t *testing.T) { runtime.GC() for i := 0; i < 100; i++ { runtime.GC() - require.Equal(t, minGCPercent.Load(), tn.getGCPercent()) + require.Eventually(t, func() bool { return minGCPercent.Load() == tn.getGCPercent() }, + 1*time.Second, 50*time.Microsecond) } } + +func TestCalcGCPercent(t *testing.T) { + const gb = units.GiB + // use default value when invalid params + require.Equal(t, defaultGCPercent, calcGCPercent(0, 0)) + require.Equal(t, defaultGCPercent, calcGCPercent(0, 1)) + require.Equal(t, defaultGCPercent, calcGCPercent(1, 0)) + + require.Equal(t, maxGCPercent.Load(), calcGCPercent(1, 3*gb)) + require.Equal(t, maxGCPercent.Load(), calcGCPercent(gb/10, 4*gb)) + require.Equal(t, maxGCPercent.Load(), calcGCPercent(gb/2, 4*gb)) + require.Equal(t, uint32(300), calcGCPercent(1*gb, 4*gb)) + require.Equal(t, uint32(166), calcGCPercent(1.5*gb, 4*gb)) + require.Equal(t, uint32(100), calcGCPercent(2*gb, 4*gb)) + require.Equal(t, uint32(100), calcGCPercent(3*gb, 4*gb)) + require.Equal(t, minGCPercent.Load(), calcGCPercent(4*gb, 4*gb)) + require.Equal(t, minGCPercent.Load(), calcGCPercent(5*gb, 4*gb)) +} diff --git a/pkg/gogc/gogc.go b/pkg/gogc/gogc.go index 110b596fb89..2d3039ca244 100644 --- a/pkg/gogc/gogc.go +++ b/pkg/gogc/gogc.go @@ -19,9 +19,6 @@ import ( "runtime/debug" "strconv" "sync/atomic" - - "github.com/pingcap/log" - "go.uber.org/zap" ) var gogcValue int64 @@ -39,9 +36,6 @@ func SetGOGC(val int) int { val = 100 } result := debug.SetGCPercent(val) - if result != val { - log.Info("debug.SetGCPercent", zap.Int("val", val), zap.Int("result", result)) - } atomic.StoreInt64(&gogcValue, int64(val)) return result }