From 5ec15266da43c0e284eca46aee0811e781bb66a0 Mon Sep 17 00:00:00 2001 From: Jamie Liu Date: Thu, 26 Oct 2023 14:09:03 -0700 Subject: [PATCH] Reduce TestMultiContainerMemoryLeakStress flakiness. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each test_app created by the test deallocates 400-500 MB of memory on exit: ``` I1025 15:41:02.125936 3517 strace.go:561] [ 20: 20] test_app E exit_group(0x0) I1025 15:41:02.125967 3517 strace.go:599] [ 20: 20] test_app X exit_group(0x0) = 0 (0x0) (15.251µs) ... I1025 15:41:02.127646 3517 pgalloc.go:923] MemoryFile 0xc00040e000 has 165408768 reclaimable bytes ... I1025 15:41:02.137618 3517 pgalloc.go:923] MemoryFile 0xc00040e000 has 643493888 reclaimable bytes ``` The rate at which gVisor reclaims deallocated memory is overwhelmingly dominated by the cost of the underlying fallocate() syscall, which is system-dependent; when the sandbox (but not necessarily the containing machine) is idle, the lowest observed reclaim throughput in our testing infrastructure is ~116 MB/s: ``` I1025 15:41:02.289327 3517 pgalloc.go:1343] MemoryFile 0xc00040e000 start reclaiming [0x110000, 0x470000) (3538944 bytes; 1098084352 reclaimable bytes) I1025 15:41:02.345265 3517 pgalloc.go:1367] MemoryFile 0xc00040e000 end reclaiming [0x110000, 0x470000) I1025 15:41:02.345339 3517 pgalloc.go:1343] MemoryFile 0xc00040e000 start reclaiming [0x5f0000, 0x7a0000) (1769472 bytes; 1094545408 reclaimable bytes) I1025 15:41:02.375173 3517 pgalloc.go:1367] MemoryFile 0xc00040e000 end reclaiming [0x5f0000, 0x7a0000) ... I1025 15:41:07.249074 3517 pgalloc.go:1343] MemoryFile 0xc00040e000 start reclaiming [0x2aa65000, 0x2ae05000) (3801088 bytes; 490139648 reclaimable bytes) I1025 15:41:07.289862 3517 pgalloc.go:1367] MemoryFile 0xc00040e000 end reclaiming [0x2aa65000, 0x2ae05000) ``` Thus, 3 seconds is not consistently enough to reclaim 45 test_apps. Reduce the number of test_apps run during the stress phase to 25 (aside from this particular flake, 45 also occasionally causes test OOMs), and modify the test to allow up to 5 seconds to reclaim each test_app. PiperOrigin-RevId: 576982708 --- runsc/container/multi_container_test.go | 61 +++++++++++++++---------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go index b994b9b39..8754169eb 100644 --- a/runsc/container/multi_container_test.go +++ b/runsc/container/multi_container_test.go @@ -2565,8 +2565,12 @@ func TestMultiContainerMemoryLeakStress(t *testing.T) { // Subcontainers will do a lot of filesystem work. Create a lot of them. createFsTree := []string{app, "fsTreeCreate", "--depth=10", "--file-per-level=10", "--file-size=1048576"} - const warmupContainers = 5 - const stressContainers = 45 + const ( + warmupContainers = 5 + stressContainers = 25 + nominalReclaimDurationPerContainer = time.Second + maxReclaimDurationPerContainer = 5 * time.Second + ) cmds := make([][]string, 0, warmupContainers+stressContainers+1) cmds = append(cmds, sleep) for i := 0; i < warmupContainers+stressContainers; i++ { @@ -2603,9 +2607,11 @@ func TestMultiContainerMemoryLeakStress(t *testing.T) { } // Give the reclaimer goroutine some time to reclaim. - time.Sleep(3 * time.Second) + time.Sleep(warmupContainers * nominalReclaimDurationPerContainer) // Measure the memory usage after the warm up. + // It's possible, though unlikely, that reclaiming is unfinished; this is + // harmless because we tolerate newUsage being lower than oldUsage below. oldUsage, err := rootCont[0].Sandbox.Usage(true /* Full */) if err != nil { t.Fatalf("sandbox.Usage failed: %v", err) @@ -2626,28 +2632,37 @@ func TestMultiContainerMemoryLeakStress(t *testing.T) { } } - // Give the reclaimer goroutine some time to reclaim. - time.Sleep(3 * time.Second) - - // Compare memory usage. - newUsage, err := rootCont[0].Sandbox.Usage(true /* Full */) - if err != nil { - t.Fatalf("sandbox.Usage failed: %v", err) - } - // Note that all fields of control.MemoryUsage are exported and uint64. + // Sample memory usage until all fields are no more than 5% greater than + // after warmup. + deadline := time.Now().Add(stressContainers * maxReclaimDurationPerContainer) oldUsageV := reflect.ValueOf(oldUsage) - newUsageV := reflect.ValueOf(newUsage) - numFields := oldUsageV.NumField() - for i := 0; i < numFields; i++ { - name := oldUsageV.Type().Field(i).Name - oldVal := oldUsageV.Field(i).Interface().(uint64) - newVal := newUsageV.Field(i).Interface().(uint64) - if newVal <= oldVal { - continue + for { + newUsage, err := rootCont[0].Sandbox.Usage(true /* Full */) + if err != nil { + t.Fatalf("sandbox.Usage failed: %v", err) } - - if ((newVal-oldVal)*100)/oldVal > 5 { - t.Errorf("%s usage increased by more than 5%%: old=%d, new=%d", name, oldVal, newVal) + allFieldsOk := true + // Note that all fields of control.MemoryUsage are exported and uint64. + newUsageV := reflect.ValueOf(newUsage) + numFields := oldUsageV.NumField() + for i := 0; i < numFields; i++ { + name := oldUsageV.Type().Field(i).Name + oldVal := oldUsageV.Field(i).Interface().(uint64) + newVal := newUsageV.Field(i).Interface().(uint64) + if newVal <= oldVal { + continue + } + if ((newVal-oldVal)*100)/oldVal > 5 { + t.Logf("%s usage increased by more than 5%%: old=%d, new=%d", name, oldVal, newVal) + allFieldsOk = false + } } + if allFieldsOk { + break + } + if time.Now().After(deadline) { + t.Fatalf("Memory usage after stress containers exited did not converge to memory usage after warmup") + } + time.Sleep(time.Second) } }