diff options
author | Adin Scannell <ascannell@google.com> | 2021-01-05 13:20:12 -0800 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-01-05 13:21:54 -0800 |
commit | b06e5bc5b0913d3740b435d8753a2569220e0a33 (patch) | |
tree | 63c4a862ed11526c76aa7cf8c54d9b640d679df8 /pkg | |
parent | 93b38bddba90f54bfdc166322f6e83e5f012e4cb (diff) |
Add benchmarks targets to BuildKite.
This includes minor fix-ups:
* Handle SIGTERM in runsc debug, to exit gracefully.
* Fix cmd.debug.go opening all profiles as RDONLY.
* Fix the test name in fio_test.go, and encode the block size in the test.
PiperOrigin-RevId: 350205718
Diffstat (limited to 'pkg')
-rw-r--r-- | pkg/control/server/server.go | 5 | ||||
-rw-r--r-- | pkg/sentry/control/pprof.go | 56 | ||||
-rw-r--r-- | pkg/test/dockerutil/container.go | 32 | ||||
-rw-r--r-- | pkg/test/dockerutil/profile.go | 16 | ||||
-rw-r--r-- | pkg/urpc/urpc.go | 19 |
5 files changed, 83 insertions, 45 deletions
diff --git a/pkg/control/server/server.go b/pkg/control/server/server.go index 41abe1f2d..629dae8f4 100644 --- a/pkg/control/server/server.go +++ b/pkg/control/server/server.go @@ -67,9 +67,10 @@ func (s *Server) Wait() { // and the server should not be used afterwards. func (s *Server) Stop() { s.socket.Close() - s.wg.Wait() + s.Wait() - // This will cause existing clients to be terminated safely. + // This will cause existing clients to be terminated safely. If the + // registered handlers have a Stop callback, it will be called. s.server.Stop() } diff --git a/pkg/sentry/control/pprof.go b/pkg/sentry/control/pprof.go index b78e29416..2f3664c57 100644 --- a/pkg/sentry/control/pprof.go +++ b/pkg/sentry/control/pprof.go @@ -50,17 +50,17 @@ type Profile struct { done chan struct{} } -// NewProfile returns a new Profile object, and a stop callback. -// -// The stop callback should be used at most once. -func NewProfile(k *kernel.Kernel) (*Profile, func()) { - p := &Profile{ +// NewProfile returns a new Profile object. +func NewProfile(k *kernel.Kernel) *Profile { + return &Profile{ kernel: k, done: make(chan struct{}), } - return p, func() { - close(p.done) - } +} + +// Stop implements urpc.Stopper.Stop. +func (p *Profile) Stop() { + close(p.done) } // CPUProfileOpts contains options specifically for CPU profiles. @@ -70,9 +70,6 @@ type CPUProfileOpts struct { // Duration is the duration of the profile. Duration time.Duration `json:"duration"` - - // Hz is the rate, which may be zero. - Hz int `json:"hz"` } // CPU is an RPC stub which collects a CPU profile. @@ -81,19 +78,13 @@ func (p *Profile) CPU(o *CPUProfileOpts, _ *struct{}) error { return nil // Allowed. } - output, err := fd.NewFromFile(o.FilePayload.Files[0]) - if err != nil { - return err - } + output := o.FilePayload.Files[0] defer output.Close() p.cpuMu.Lock() defer p.cpuMu.Unlock() // Returns an error if profiling is already started. - if o.Hz != 0 { - runtime.SetCPUProfileRate(o.Hz) - } if err := pprof.StartCPUProfile(output); err != nil { return err } @@ -112,6 +103,11 @@ func (p *Profile) CPU(o *CPUProfileOpts, _ *struct{}) error { type HeapProfileOpts struct { // FilePayload is the destination for the profiling output. urpc.FilePayload + + // Delay is the sleep time, similar to Duration. This may + // not affect the data collected however, as the heap will + // continue only the memory associated with the last alloc. + Delay time.Duration `json:"delay"` } // Heap generates a heap profile. @@ -123,7 +119,16 @@ func (p *Profile) Heap(o *HeapProfileOpts, _ *struct{}) error { output := o.FilePayload.Files[0] defer output.Close() - runtime.GC() // Get up-to-date statistics. + // Wait for the given delay. + select { + case <-time.After(o.Delay): + case <-p.done: + } + + // Get up-to-date statistics. + runtime.GC() + + // Write the given profile. return pprof.WriteHeapProfile(output) } @@ -170,8 +175,12 @@ func (p *Profile) Block(o *BlockProfileOpts, _ *struct{}) error { defer p.blockMu.Unlock() // Always set the rate. We then wait to collect a profile at this rate, - // and disable when we're done. - rate := 1 + // and disable when we're done. Note that the default here is 10%, which + // will record a stacktrace 10% of the time when blocking occurs. Since + // these events should not be super frequent, we expect this to achieve + // a reasonable balance between collecting the data we need and imposing + // a high performance cost (e.g. skewing even the CPU profile). + rate := 10 if o.Rate != 0 { rate = o.Rate } @@ -211,8 +220,9 @@ func (p *Profile) Mutex(o *MutexProfileOpts, _ *struct{}) error { p.mutexMu.Lock() defer p.mutexMu.Unlock() - // Always set the fraction. - fraction := 1 + // Always set the fraction. Like the block rate above, we use + // a default rate of 10% for the same reasons. + fraction := 10 if o.Fraction != 0 { fraction = o.Fraction } diff --git a/pkg/test/dockerutil/container.go b/pkg/test/dockerutil/container.go index 7b5fcef9c..7bacb70d3 100644 --- a/pkg/test/dockerutil/container.go +++ b/pkg/test/dockerutil/container.go @@ -125,9 +125,7 @@ func makeContainer(ctx context.Context, logger testutil.Logger, runtime string) // // Containers will check flags for profiling requests. func MakeContainer(ctx context.Context, logger testutil.Logger) *Container { - c := makeContainer(ctx, logger, *runtime) - c.profileInit() - return c + return makeContainer(ctx, logger, *runtime) } // MakeNativeContainer constructs a suitable Container object. @@ -141,7 +139,7 @@ func MakeNativeContainer(ctx context.Context, logger testutil.Logger) *Container // Spawn is analogous to 'docker run -d'. func (c *Container) Spawn(ctx context.Context, r RunOpts, args ...string) error { - if err := c.create(ctx, c.config(r, args), c.hostConfig(r), nil); err != nil { + if err := c.create(ctx, r.Image, c.config(r, args), c.hostConfig(r), nil); err != nil { return err } return c.Start(ctx) @@ -154,7 +152,7 @@ func (c *Container) SpawnProcess(ctx context.Context, r RunOpts, args ...string) config.Tty = true config.OpenStdin = true - if err := c.CreateFrom(ctx, config, hostconf, netconf); err != nil { + if err := c.CreateFrom(ctx, r.Image, config, hostconf, netconf); err != nil { return Process{}, err } @@ -181,7 +179,7 @@ func (c *Container) SpawnProcess(ctx context.Context, r RunOpts, args ...string) // Run is analogous to 'docker run'. func (c *Container) Run(ctx context.Context, r RunOpts, args ...string) (string, error) { - if err := c.create(ctx, c.config(r, args), c.hostConfig(r), nil); err != nil { + if err := c.create(ctx, r.Image, c.config(r, args), c.hostConfig(r), nil); err != nil { return "", err } @@ -193,8 +191,6 @@ func (c *Container) Run(ctx context.Context, r RunOpts, args ...string) (string, return "", err } - c.stopProfiling() - return c.Logs(ctx) } @@ -210,16 +206,21 @@ func (c *Container) MakeLink(target string) string { } // CreateFrom creates a container from the given configs. -func (c *Container) CreateFrom(ctx context.Context, conf *container.Config, hostconf *container.HostConfig, netconf *network.NetworkingConfig) error { - return c.create(ctx, conf, hostconf, netconf) +func (c *Container) CreateFrom(ctx context.Context, profileImage string, conf *container.Config, hostconf *container.HostConfig, netconf *network.NetworkingConfig) error { + return c.create(ctx, profileImage, conf, hostconf, netconf) } // Create is analogous to 'docker create'. func (c *Container) Create(ctx context.Context, r RunOpts, args ...string) error { - return c.create(ctx, c.config(r, args), c.hostConfig(r), nil) + return c.create(ctx, r.Image, c.config(r, args), c.hostConfig(r), nil) } -func (c *Container) create(ctx context.Context, conf *container.Config, hostconf *container.HostConfig, netconf *network.NetworkingConfig) error { +func (c *Container) create(ctx context.Context, profileImage string, conf *container.Config, hostconf *container.HostConfig, netconf *network.NetworkingConfig) error { + if c.runtime != "" { + // Use the image name as provided here; which normally represents the + // unmodified "basic/alpine" image name. This should be easy to grok. + c.profileInit(profileImage) + } cont, err := c.client.ContainerCreate(ctx, conf, hostconf, nil, c.Name) if err != nil { return err @@ -428,6 +429,7 @@ func (c *Container) Status(ctx context.Context) (types.ContainerState, error) { // Wait waits for the container to exit. func (c *Container) Wait(ctx context.Context) error { + defer c.stopProfiling() statusChan, errChan := c.client.ContainerWait(ctx, c.id, container.WaitConditionNotRunning) select { case err := <-errChan: @@ -489,14 +491,16 @@ func (c *Container) WaitForOutputSubmatch(ctx context.Context, pattern string, t func (c *Container) stopProfiling() { if c.profile != nil { if err := c.profile.Stop(c); err != nil { - c.logger.Logf("profile.Stop failed: %v", err) + // This most likely means that the runtime for the container + // was too short to connect and actually get a profile. + c.logger.Logf("warning: profile.Stop failed: %v", err) } } } // Kill kills the container. func (c *Container) Kill(ctx context.Context) error { - c.stopProfiling() + defer c.stopProfiling() return c.client.ContainerKill(ctx, c.id, "") } diff --git a/pkg/test/dockerutil/profile.go b/pkg/test/dockerutil/profile.go index f1103eb6e..5cad3e959 100644 --- a/pkg/test/dockerutil/profile.go +++ b/pkg/test/dockerutil/profile.go @@ -38,12 +38,19 @@ type profile struct { } // profileInit initializes a profile object, if required. -func (c *Container) profileInit() { +// +// N.B. The profiling filename initialized here will use the *image* +// name, and not the unique container name. This is intentional. Most +// of the time, profiling will be used for benchmarks. Benchmarks will +// be run iteratively until a sufficiently large N is reached. It is +// useful in this context to overwrite previous runs, and generate a +// single profile result for the final test. +func (c *Container) profileInit(image string) { if !*pprofBlock && !*pprofCPU && !*pprofMutex && !*pprofHeap { return // Nothing to do. } c.profile = &profile{ - BasePath: filepath.Join(*pprofBaseDir, c.runtime, c.Name), + BasePath: filepath.Join(*pprofBaseDir, c.runtime, c.logger.Name(), image), Duration: *pprofDuration, } if *pprofCPU { @@ -83,6 +90,7 @@ func (p *profile) createProcess(c *Container) error { args = append(args, fmt.Sprintf("--profile-%s=%s", profileArg, outputPath)) } args = append(args, fmt.Sprintf("--duration=%s", p.Duration)) // Or until container exits. + args = append(args, fmt.Sprintf("--delay=%s", p.Duration)) // Ditto. args = append(args, c.ID()) // Best effort wait until container is running. @@ -104,8 +112,6 @@ func (p *profile) createProcess(c *Container) error { } // killProcess kills the process, if running. -// -// Precondition: mu must be held. func (p *profile) killProcess() error { if p.cmd != nil && p.cmd.Process != nil { return p.cmd.Process.Signal(syscall.SIGTERM) @@ -114,8 +120,6 @@ func (p *profile) killProcess() error { } // waitProcess waits for the process, if running. -// -// Precondition: mu must be held. func (p *profile) waitProcess() error { defer func() { p.cmd = nil }() if p.cmd != nil { diff --git a/pkg/urpc/urpc.go b/pkg/urpc/urpc.go index dfd23032c..0e9a829f6 100644 --- a/pkg/urpc/urpc.go +++ b/pkg/urpc/urpc.go @@ -170,6 +170,9 @@ type Server struct { // methods is the set of server methods. methods map[string]registeredMethod + // stoppers are all registered stoppers. + stoppers []Stopper + // clients is a map of clients. clients map[*unet.Socket]clientState @@ -195,6 +198,12 @@ func NewServerWithCallback(afterRPCCallback func()) *Server { } } +// Stopper is an optional interface, that when implemented, allows an object +// to have a callback executed when the server is shutting down. +type Stopper interface { + Stop() +} + // Register registers the given object as an RPC receiver. // // This functions is the same way as the built-in RPC package, but it does not @@ -206,6 +215,7 @@ func (s *Server) Register(obj interface{}) { defer s.mu.Unlock() typ := reflect.TypeOf(obj) + stopper, hasStop := obj.(Stopper) // If we got a pointer, deref it to the underlying object. We need this to // obtain the name of the underlying type. @@ -221,6 +231,10 @@ func (s *Server) Register(obj interface{}) { // Can't be anonymous. panic("type not named.") } + if hasStop && method.Name == "Stop" { + s.stoppers = append(s.stoppers, stopper) + continue // Legal stop method. + } prettyName := typDeref.Name() + "." + method.Name if _, ok := s.methods[prettyName]; ok { @@ -448,6 +462,11 @@ func (s *Server) Stop() { // Wait for all outstanding requests. defer s.wg.Wait() + // Call any Stop callbacks. + for _, stopper := range s.stoppers { + stopper.Stop() + } + // Close all known clients. s.mu.Lock() defer s.mu.Unlock() |