summaryrefslogtreecommitdiffhomepage
path: root/pkg
diff options
context:
space:
mode:
authorAdin Scannell <ascannell@google.com>2021-01-05 13:20:12 -0800
committergVisor bot <gvisor-bot@google.com>2021-01-05 13:21:54 -0800
commitb06e5bc5b0913d3740b435d8753a2569220e0a33 (patch)
tree63c4a862ed11526c76aa7cf8c54d9b640d679df8 /pkg
parent93b38bddba90f54bfdc166322f6e83e5f012e4cb (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.go5
-rw-r--r--pkg/sentry/control/pprof.go56
-rw-r--r--pkg/test/dockerutil/container.go32
-rw-r--r--pkg/test/dockerutil/profile.go16
-rw-r--r--pkg/urpc/urpc.go19
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()