improve tests

Signed-off-by: Tim Vaillancourt <[email protected]>

Author: timvaillancourt

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

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=<options>                # 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=<options>                # 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

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,25 +12,22 @@ 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")
 		tests.S(t).ExpectEquals(err, ErrCPUProfilingInProgress)
 		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, "")