Skip to content

Commit b6b9bca

Browse files
committed
adding efficient polling to waitForStepsToFinish
The current waitForStepsToFinish implementation is a classic busy-wait. It checks for file existence without any sleep, resulting in a high CPU usage. Adding a profile with a unit test to show that almost all time is spent in system calls with a high total sample count. This led to execssive CPU usage by the sidecar even when just waiting. The function now sleeps 100ms between checks, drastically reducing the frequency. The sidecar now uses minimal CPU while waiting. Signed-off-by: Priti Desai <pdesai@us.ibm.com>
1 parent d504890 commit b6b9bca

File tree

2 files changed

+59
-0
lines changed

2 files changed

+59
-0
lines changed

internal/sidecarlogresults/sidecarlogresults.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"os"
2727
"path/filepath"
2828
"strings"
29+
"time"
2930

3031
"github.com/tektoncd/pipeline/pkg/apis/config"
3132
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
@@ -103,6 +104,7 @@ func waitForStepsToFinish(runDir string) error {
103104
return err
104105
}
105106
}
107+
time.Sleep(100 * time.Millisecond)
106108
}
107109
return nil
108110
}

internal/sidecarlogresults/sidecarlogresults_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ import (
2323
"fmt"
2424
"os"
2525
"path/filepath"
26+
"runtime/pprof"
2627
"sort"
2728
"strings"
2829
"testing"
30+
"time"
2931

3032
"github.com/google/go-cmp/cmp"
3133
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
@@ -608,6 +610,61 @@ func TestExtractStepAndResultFromSidecarResultName_Error(t *testing.T) {
608610
}
609611
}
610612

613+
// TestWaitForStepsToFinish_Profile ensures that waitForStepsToFinish correctly waits for all step output files to appear before returning
614+
// The test creates a file called cpu.prof and starts Go's CPU profiler
615+
// A temporary directory is created to simulate the Tekton step run directory.
616+
// The test creates a large number of subdirectories e.g. step0, step1, ..., each representing a step in a TaskRun
617+
// A goroutine is started that, one by one, writes an out file in each step directory, with a small delay between each
618+
// The test calls the function and waits for it to complete and the profile is saved for later analysis
619+
// This is helpful to compare the impact of code changes, provides a reproducible way to profile and optimize the function waitForStepsToFinish
620+
func TestWaitForStepsToFinish_Profile(t *testing.T) {
621+
if os.Getenv("ENABLE_PROFILING_TESTS") != "true" {
622+
t.Skip("Profiling tests are disabled")
623+
}
624+
f, err := os.Create("cpu.prof")
625+
if err != nil {
626+
t.Fatalf("could not create CPU profile: %v", err)
627+
}
628+
defer func(f *os.File) {
629+
err := f.Close()
630+
if err != nil {
631+
return
632+
}
633+
}(f)
634+
err = pprof.StartCPUProfile(f)
635+
if err != nil {
636+
return
637+
}
638+
defer pprof.StopCPUProfile()
639+
640+
// Setup: create a temp runDir with many fake step files
641+
runDir := t.TempDir()
642+
stepCount := 100
643+
for i := range stepCount {
644+
dir := filepath.Join(runDir, fmt.Sprintf("step%d", i))
645+
err := os.MkdirAll(dir, 0755)
646+
if err != nil {
647+
return
648+
}
649+
}
650+
651+
// Simulate steps finishing one by one with a delay
652+
go func() {
653+
for i := range stepCount {
654+
file := filepath.Join(runDir, fmt.Sprintf("step%d", i), "out")
655+
err := os.WriteFile(file, []byte("done"), 0644)
656+
if err != nil {
657+
return
658+
}
659+
time.Sleep(10 * time.Millisecond)
660+
}
661+
}()
662+
663+
if err := waitForStepsToFinish(runDir); err != nil {
664+
t.Fatalf("waitForStepsToFinish failed: %v", err)
665+
}
666+
}
667+
611668
func TestLookForArtifacts(t *testing.T) {
612669
base := basicArtifacts()
613670
modified := base.DeepCopy()

0 commit comments

Comments
 (0)