*: add S3 quick start and few enhancement of log (#202)

* README, docker: add quick start about S3 storage

Signed-off-by: Neil Shen <overvenus@gmail.com>

* pkg/summary: make sure to output correct summary

Signed-off-by: Neil Shen <overvenus@gmail.com>

* cmd, tests: log to terminal if BR_LOG_TO_TERM is set

Signed-off-by: Neil Shen <overvenus@gmail.com>

* Update pkg/task/common.go

Co-Authored-By: kennytm <kennytm@gmail.com>

* address comments

Signed-off-by: Neil Shen <overvenus@gmail.com>

* address comments

Signed-off-by: Neil Shen <overvenus@gmail.com>

* tests: cat log if br fails

Signed-off-by: Neil Shen <overvenus@gmail.com>

Co-authored-by: kennytm <kennytm@gmail.com>

README.md

@@ -56,7 +56,7 @@ go-ycsb load mysql -p workload=core \
 mysql -uroot -htidb -P4000 -E -e "SELECT COUNT(*) FROM test.usertable"
 
 # Build BR and backup!
-make release && \
+make build && \
 bin/br backup full --pd pd0:2379 --storage "local:///data/backup/full" \
     --log-file "/logs/br_backup.log"
 
@@ -69,6 +69,20 @@ bin/br restore full --pd pd0:2379 --storage "local:///data/backup/full" \
 
 # How many rows do we get again? Expected to be 100000 rows.
 mysql -uroot -htidb -P4000 -E -e "SELECT COUNT(*) FROM test.usertable"
+
+# Test S3 compatible storage (MinIO).
+# Create a bucket to save backup by mc (a MinIO Client).
+mc config host add minio $S3_ENDPOINT $MINIO_ACCESS_KEY $MINIO_SECRET_KEY && \
+mc mb minio/mybucket
+
+# Backup to S3 compatible storage.
+bin/br backup full --pd pd0:2379 --storage "s3://mybucket/full" \
+    --s3.endpoint="$S3_ENDPOINT"
+
+# Drop database and restore!
+mysql -uroot -htidb -P4000 -E -e "DROP DATABASE test; SHOW DATABASES;" && \
+bin/br restore full --pd pd0:2379 --storage "s3://mybucket/full" \
+    --s3.endpoint="$S3_ENDPOINT"
 ```
 
 ## Contributing

cmd/cmd.go

@@ -25,10 +25,11 @@ import (
 )
 
 var (
-	initOnce       = sync.Once{}
-	defaultContext context.Context
-	hasLogFile     uint64
-	tidbGlue       = gluetidb.Glue{}
+	initOnce        = sync.Once{}
+	defaultContext  context.Context
+	hasLogFile      uint64
+	tidbGlue        = gluetidb.Glue{}
+	envLogToTermKey = "BR_LOG_TO_TERM"
 )
 
 const (
@@ -46,7 +47,7 @@ const (
 )
 
 func timestampLogFileName() string {
-	return filepath.Join(os.TempDir(), "br-"+time.Now().Format(time.RFC3339))
+	return filepath.Join(os.TempDir(), "br.log."+time.Now().Format(time.RFC3339))
 }
 
 // AddFlags adds flags to the given cmd.
@@ -81,11 +82,15 @@ func Init(cmd *cobra.Command) (err error) {
 		if err != nil {
 			return
 		}
+		_, outputLogToTerm := os.LookupEnv(envLogToTermKey)
+		if outputLogToTerm {
+			// Log to term if env `BR_LOG_TO_TERM` is set.
+			conf.File.Filename = ""
+		}
 		if len(conf.File.Filename) != 0 {
 			atomic.StoreUint64(&hasLogFile, 1)
 			summary.InitCollector(true)
-		} else {
-			cmd.Printf("log file: %s\n", conf.File.Filename)
+			cmd.Printf("Detial BR log in %s\n", conf.File.Filename)
 		}
 		lg, p, e := log.InitLogger(conf)
 		if e != nil {

docker-compose.yaml

@@ -1,6 +1,6 @@
 ---
 # Source: tidb-docker-compose/templates/docker-compose.yml
-version: '2.1'
+version: '3.2'
 
 services:
   control:
@@ -10,11 +10,13 @@ services:
       dockerfile: ./docker/Dockerfile
     volumes:
       - ./docker/data:/data
-      - ./docker/logs:/logs
+      - ./docker/logs:/tmp
     command: -c "/usr/bin/tail -f /dev/null"
     depends_on:
       - "tidb"
     restart: on-failure
+    env_file:
+      - ./docker/minio.env
 
   pd0:
     image: pingcap/pd:latest
@@ -64,6 +66,8 @@ services:
     #     soft: 1000000
     #     hard: 1000000
     restart: on-failure
+    env_file:
+      - ./docker/minio.env
 
   tikv1:
     image: pingcap/tikv:latest
@@ -87,6 +91,8 @@ services:
     #     soft: 1000000
     #     hard: 1000000
     restart: on-failure
+    env_file:
+      - ./docker/minio.env
 
   tikv2:
     image: pingcap/tikv:latest
@@ -110,6 +116,8 @@ services:
     #     soft: 1000000
     #     hard: 1000000
     restart: on-failure
+    env_file:
+      - ./docker/minio.env
 
   tikv3:
     image: pingcap/tikv:latest
@@ -133,6 +141,8 @@ services:
     #     soft: 1000000
     #     hard: 1000000
     restart: on-failure
+    env_file:
+      - ./docker/minio.env
 
   tikv4:
     image: pingcap/tikv:latest
@@ -156,6 +166,8 @@ services:
     #     soft: 1000000
     #     hard: 1000000
     restart: on-failure
+    env_file:
+      - ./docker/minio.env
 
   tidb:
     image: pingcap/tidb:latest
@@ -185,6 +197,16 @@ services:
     #     hard: 1000000
     restart: on-failure
 
+  minio:
+    image: minio/minio
+    ports:
+      - 24927:24927
+    volumes:
+      - ./docker/data/s3:/data/s3
+    command: server --address=:24927 /data/s3
+    env_file:
+      - ./docker/minio.env
+
   tidb-vision:
     image: pingcap/tidb-vision:latest
     environment:

docker/Dockerfile

@@ -1,11 +1,13 @@
-FROM golang:1.13.8-buster as builder
-
 # For loading data to TiDB
+FROM golang:1.13.8-buster as go-ycsb-builder
 WORKDIR /go/src/github.com/pingcap/
 RUN git clone https://github.com/pingcap/go-ycsb.git && \
     cd go-ycsb && \
     make
 
+# For operating minio S3 compatible storage
+FROM minio/mc as mc-builder
+
 FROM golang:1.13.8-buster
 
 RUN apt-get update && apt-get install -y --no-install-recommends \
@@ -19,6 +21,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
 WORKDIR /go/src/github.com/pingcap/br
 COPY . .
 
-COPY --from=builder /go/src/github.com/pingcap/go-ycsb/bin/go-ycsb /go/bin/go-ycsb
+COPY --from=go-ycsb-builder /go/src/github.com/pingcap/go-ycsb/bin/go-ycsb /go/bin/go-ycsb
+COPY --from=mc-builder /usr/bin/mc /usr/bin/mc
 
 ENTRYPOINT ["/bin/bash"]

docker/minio.env

@@ -0,0 +1,6 @@
+MINIO_ACCESS_KEY=brs3accesskey
+MINIO_SECRET_KEY=brs3secretkey
+MINIO_BROWSER=off
+AWS_ACCESS_KEY_ID=brs3accesskey
+AWS_SECRET_ACCESS_KEY=brs3secretkey
+S3_ENDPOINT=http://minio:24927

pkg/summary/collector.go

@@ -35,6 +35,8 @@ type LogCollector interface {
 
 	CollectInt(name string, t int)
 
+	SetSuccessStatus(success bool)
+
 	Summary(name string)
 }
 
@@ -43,7 +45,9 @@ type logFunc func(msg string, fields ...zap.Field)
 var collector LogCollector = newLogCollector(log.Info)
 
 // InitCollector initilize global collector instance.
-func InitCollector(hasLogFile bool) {
+func InitCollector( // revive:disable-line:flag-parameter
+	hasLogFile bool,
+) {
 	logF := log.L().Info
 	if hasLogFile {
 		conf := new(log.Config)
@@ -69,6 +73,7 @@ type logCollector struct {
 	failureReasons   map[string]error
 	durations        map[string]time.Duration
 	ints             map[string]int
+	successStatus    bool
 
 	log logFunc
 }
@@ -134,6 +139,12 @@ func (tc *logCollector) CollectInt(name string, t int) {
 	tc.ints[name] += t
 }
 
+func (tc *logCollector) SetSuccessStatus(success bool) {
+	tc.mu.Lock()
+	defer tc.mu.Unlock()
+	tc.successStatus = success
+}
+
 func (tc *logCollector) Summary(name string) {
 	tc.mu.Lock()
 	defer func() {
@@ -162,7 +173,7 @@ func (tc *logCollector) Summary(name string) {
 		logFields = append(logFields, zap.Int(key, val))
 	}
 
-	if len(tc.failureReasons) != 0 {
+	if len(tc.failureReasons) != 0 || !tc.successStatus {
 		for unitName, reason := range tc.failureReasons {
 			logFields = append(logFields, zap.String("unitName", unitName), zap.Error(reason))
 		}

pkg/summary/collector_test.go

@@ -30,6 +30,7 @@ func (suit *testCollectorSuite) TestSumDurationInt(c *C) {
 	col.CollectDuration("b", time.Second)
 	col.CollectInt("c", 2)
 	col.CollectInt("c", 2)
+	col.SetSuccessStatus(true)
 	col.Summary("foo")
 
 	c.Assert(len(fields), Equals, 3)

pkg/summary/summary.go

@@ -29,6 +29,11 @@ func CollectInt(name string, t int) {
 	collector.CollectInt(name, t)
 }
 
+// SetSuccessStatus sets final success status
+func SetSuccessStatus(success bool) {
+	collector.SetSuccessStatus(success)
+}
+
 // Summary outputs summary log
 func Summary(name string) {
 	collector.Summary(name)

pkg/task/backup.go

@@ -214,6 +214,9 @@ func RunBackup(c context.Context, g glue.Glue, cmdName string, cfg *BackupConfig
 	if err != nil {
 		return err
 	}
+
+	// Set task summary to success status.
+	summary.SetSuccessStatus(true)
 	return nil
 }
 

pkg/task/backup_raw.go

@@ -141,5 +141,8 @@ func RunBackupRaw(c context.Context, g glue.Glue, cmdName string, cfg *RawKvConf
 	if err != nil {
 		return err
 	}
+
+	// Set task summary to success status.
+	summary.SetSuccessStatus(true)
 	return nil
 }

pkg/task/common.go

@@ -95,7 +95,7 @@ type Config struct {
 // DefineCommonFlags defines the flags common to all BRIE commands.
 func DefineCommonFlags(flags *pflag.FlagSet) {
 	flags.BoolP(flagSendCreds, "c", true, "Whether send credentials to tikv")
-	flags.StringP(flagStorage, "s", "", `specify the url where backup storage, eg, "s3:///path/to/save"`)
+	flags.StringP(flagStorage, "s", "", `specify the url where backup storage, eg, "s3://bucket/path/prefix"`)
 	flags.StringSliceP(flagPD, "u", []string{"127.0.0.1:2379"}, "PD address")
 	flags.String(flagCA, "", "CA certificate path for TLS connection")
 	flags.String(flagCert, "", "Certificate path for TLS connection")

pkg/task/restore.go

@@ -251,6 +251,8 @@ func RunRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf
 	}
 	updateCh.Close()
 
+	// Set task summary to success status.
+	summary.SetSuccessStatus(true)
 	return nil
 }
 
@@ -415,5 +417,7 @@ func RunRestoreTiflashReplica(c context.Context, g glue.Glue, cmdName string, cf
 	updateCh.Close()
 	summary.CollectInt("recover tables", len(tables))
 
+	// Set task summary to success status.
+	summary.SetSuccessStatus(true)
 	return nil
 }

pkg/task/restore_raw.go

@@ -126,5 +126,7 @@ func RunRestoreRaw(c context.Context, g glue.Glue, cmdName string, cfg *RestoreR
 	// Restore has finished.
 	updateCh.Close()
 
+	// Set task summary to success status.
+	summary.SetSuccessStatus(true)
 	return nil
 }

tests/br_full_ddl/run.sh

@@ -36,7 +36,10 @@ done
 
 # backup full
 echo "backup start..."
-run_br --pd $PD_ADDR backup full -s "local://$TEST_DIR/$DB" --ratelimit 5 --concurrency 4 --log-file $LOG
+# Do not log to terminal
+unset BR_LOG_TO_TERM
+run_br --pd $PD_ADDR backup full -s "local://$TEST_DIR/$DB" --ratelimit 5 --concurrency 4 --log-file $LOG || cat $LOG
+BR_LOG_TO_TERM=1
 
 checksum_count=$(cat $LOG | grep "fast checksum success" | wc -l | xargs)
 

tests/br_full_index/run.sh

@@ -36,7 +36,10 @@ done
 
 # backup full
 echo "backup start..."
-run_br --pd $PD_ADDR backup full -s "local://$TEST_DIR/$DB" --ratelimit 5 --concurrency 4 --log-file $LOG
+# Do not log to terminal
+unset BR_LOG_TO_TERM
+run_br --pd $PD_ADDR backup full -s "local://$TEST_DIR/$DB" --ratelimit 5 --concurrency 4 --log-file $LOG || cat $LOG
+BR_LOG_TO_TERM=1
 
 checksum_count=$(cat $LOG | grep "fast checksum success" | wc -l | xargs)
 

tests/run.sh

@@ -46,5 +46,6 @@ for script in tests/${TEST_NAME-*}/run.sh; do
     TIKV_ADDR="$TIKV_ADDR" \
     PATH="tests/_utils:bin:$PATH" \
     TEST_NAME="$(basename "$(dirname "$script")")" \
+    BR_LOG_TO_TERM=1 \
     bash "$script"
 done