diff --git a/doc/interactive-commands.md b/doc/interactive-commands.md index 19e94306f..47be9e97c 100644 --- a/doc/interactive-commands.md +++ b/doc/interactive-commands.md @@ -17,7 +17,7 @@ Both interfaces may serve at the same time. Both respond to simple text command, - `help`: shows a brief list of available commands - `status`: returns a detailed status summary of migration progress and configuration - `sup`: returns a brief status summary of migration progress -- `cpu-profile`: returns a base64-encoded runtime/pprof CPU profile with options, default '30s'. Comma-separated options 'gzip' and/or 'blocking' (blocking profile) may follow a profile duration +- `cpu-profile`: returns a base64-encoded runtime/pprof CPU profile using a duration, default: `30s`. Comma-separated options `gzip` and/or `block` (blocked profile) may follow the profile duration - `coordinates`: returns recent (though not exactly up to date) binary log coordinates of the inspected server - `applier`: returns the hostname of the applier - `inspector`: returns the hostname of the inspector diff --git a/go/logic/server.go b/go/logic/server.go index 4d77c0e04..24baba4b2 100644 --- a/go/logic/server.go +++ b/go/logic/server.go @@ -25,11 +25,10 @@ import ( "github.com/github/gh-ost/go/base" ) -const defaultCPUProfileDuration = time.Second * 30 - var ( ErrCPUProfilingBadOption = errors.New("unrecognized cpu profiling option") ErrCPUProfilingInProgress = errors.New("cpu profiling already in progress") + defaultCPUProfileDuration = time.Second * 30 ) type printStatusFunc func(PrintStatusRule, io.Writer) @@ -63,15 +62,17 @@ func (this *Server) runCPUProfile(args string) (string, error) { var useGzip bool if args != "" { s := strings.Split(args, ",") + // a duration string must be the 1st field, if any if duration, err = time.ParseDuration(s[0]); err != nil { return "", err } for _, arg := range s[1:] { switch arg { - case "gzip": - useGzip = true case "block", "blocked", "blocking": runtime.SetBlockProfileRate(1) + defer runtime.SetBlockProfileRate(0) + case "gzip": + useGzip = true default: return "", ErrCPUProfilingBadOption } @@ -82,8 +83,7 @@ func (this *Server) runCPUProfile(args string) (string, error) { defer atomic.StoreInt64(&this.isCPUProfiling, 0) var buf bytes.Buffer - var writer io.Writer - writer = &buf + var writer io.Writer = &buf if useGzip { writer = gzip.NewWriter(&buf) } @@ -93,7 +93,7 @@ func (this *Server) runCPUProfile(args string) (string, error) { time.Sleep(duration) pprof.StopCPUProfile() - this.migrationContext.Log.Infof("Captured %d byte runtime/pprof CPU profile", buf.Len()) + this.migrationContext.Log.Infof("Captured %d byte runtime/pprof CPU profile (gzip=%v)", buf.Len(), useGzip) return base64.StdEncoding.EncodeToString(buf.Bytes()), nil } @@ -205,7 +205,7 @@ func (this *Server) applyServerCommand(command string, writer *bufio.Writer) (pr fmt.Fprint(writer, `available commands: status # Print a detailed status message sup # Print a short status message -cpu-profile= # Print a base64-encoded runtime/pprof CPU profile with options, default '30s'. Comma-separated options 'gzip' and/or 'blocking' (blocking profile) may follow a profile duration +cpu-profile= # Print a base64-encoded runtime/pprof CPU profile using a duration, default: 30s. Comma-separated options 'gzip' and/or 'block' (blocked profile) may follow the profile duration coordinates # Print the currently inspected coordinates applier # Print the hostname of the applier inspector # Print the hostname of the inspector diff --git a/go/logic/server_test.go b/go/logic/server_test.go index 9597c8dce..f0dc88065 100644 --- a/go/logic/server_test.go +++ b/go/logic/server_test.go @@ -3,6 +3,7 @@ package logic import ( "encoding/base64" "testing" + "time" "github.com/github/gh-ost/go/base" "github.com/openark/golib/tests" @@ -11,13 +12,6 @@ import ( func TestServerRunCPUProfile(t *testing.T) { t.Parallel() - t.Run("failed bad arg", func(t *testing.T) { - s := &Server{isCPUProfiling: 0} - profile, err := s.runCPUProfile("should-fail") - tests.S(t).ExpectNotNil(err) - tests.S(t).ExpectEquals(profile, "") - }) - t.Run("failed already running", func(t *testing.T) { s := &Server{isCPUProfiling: 1} profile, err := s.runCPUProfile("15ms") @@ -25,11 +19,15 @@ func TestServerRunCPUProfile(t *testing.T) { tests.S(t).ExpectEquals(profile, "") }) - t.Run("failed with bad option", func(t *testing.T) { - s := &Server{ - isCPUProfiling: 0, - migrationContext: base.NewMigrationContext(), - } + t.Run("failed bad duration", func(t *testing.T) { + s := &Server{isCPUProfiling: 0} + profile, err := s.runCPUProfile("should-fail") + tests.S(t).ExpectNotNil(err) + tests.S(t).ExpectEquals(profile, "") + }) + + t.Run("failed bad option", func(t *testing.T) { + s := &Server{isCPUProfiling: 0} profile, err := s.runCPUProfile("10ms,badoption") tests.S(t).ExpectEquals(err, ErrCPUProfilingBadOption) tests.S(t).ExpectEquals(profile, "") @@ -40,7 +38,8 @@ func TestServerRunCPUProfile(t *testing.T) { isCPUProfiling: 0, migrationContext: base.NewMigrationContext(), } - profile, err := s.runCPUProfile("10ms") + defaultCPUProfileDuration = time.Millisecond * 10 + profile, err := s.runCPUProfile("") tests.S(t).ExpectNil(err) tests.S(t).ExpectNotEquals(profile, "")