Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

print lightning log to local file #313

Merged
merged 13 commits into from
May 11, 2020
Merged

Conversation

lichunzhu
Copy link
Contributor

@lichunzhu lichunzhu commented May 11, 2020

What problem does this PR solve?

#307
Now lightning print logs directly to console if the user doesn't set log path. The stack info will confuse the users. For example:

[2020/05/09 22:44:18.681 +08:00] [ERROR] [main.go:76] ["tidb lightning encountered error"] [error="Error 1062: Duplicate entry '120' for key 'gen_idx'"] [errorVerbose="Error 1062: Duplicate entry '120' for key 'gen_idx'\ng
ithub.com/pingcap/errors.AddStack\n\t/home/pingcap/goPath/pkg/mod/github.com/pingcap/errors@v0.11.5-0.20190809092503-95897b64e011/errors.go:174\ngithub.com/pingcap/errors.Trace\n\t/home/pingcap/goPath/pkg/mod/github.com/pi
ngcap/errors@v0.11.5-0.20190809092503-95897b64e011/juju_adaptor.go:15\ngithub.com/pingcap/tidb-lightning/lightning/restore.(*chunkRestore).restore\n\t/home/pingcap/goPath/src/github.com/pingcap/tidb-lightning/lightning/res
tore/restore.go:1824\ngithub.com/pingcap/tidb-lightning/lightning/restore.(*TableRestore).restoreEngine.func1\n\t/home/pingcap/goPath/src/github.com/pingcap/tidb-lightning/lightning/restore/restore.go:934\nruntime.goexit\n
\t/usr/local/go/src/runtime/asm_amd64.s:1357"]

What is changed and how it works?

Redirect logs to log file. If it's not given, will save log to ./logs/tidb-lightning-timestamp.log. Only print necessary error message in console.

The testing result is:

Verbose debug logs will be written to /home/root/bin/logs/tidb-lightning-2020-05-11-09-52-43.log.

Welcome to lightning
Release Version: v3.0.14-4-g632582e
Git Commit Hash: 632582e34007807bd9f6d3656cb5094574a2c67c
Git Branch: fixLogStack
UTC Build Time: 2020-05-11 01:51:59
Go Version: go version go1.13.4 linux/amd64

loaded task's cfg: {"id":1589161963977751803,"lightning":{"table-concurrency":16,"index-concurrency":16,"region-concurrency":16,"io-concurrency":5,"check-requirements":false},"tidb":{"host":"127.0.0.1","port":4000,"user":"
root","status-port":10080,"pd-addr":"/tmp/tidb","sql-mode":"ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION","tls":"false","security
":{"ca-path":"","cert-path":"","key-path":""},"max-allowed-packet":67108864,"distsql-scan-concurrency":100,"build-stats-concurrency":20,"index-serial-scan-concurrency":20,"checksum-table-concurrency":16},"checkpoint":{"ena
ble":true,"schema":"tidb_lightning_checkpoint","driver":"file","keep-after-success":false},"mydumper":{"read-block-size":65536,"batch-size":107374182400,"batch-import-ratio":0,"no-schema":false,"character-set":"auto","csv":{"separator":",","delimiter":"\"","header":true,"trim-last-separator":false,"not-null":false,"null":"\\N","backslash-escape":tru
e,"max-region-size":0},"case-sensitive":false,"strict-format":false,"max-region-size":268435456},"black-white-list":{"do-tables":null,"do-dbs":null,"ignore-tables":null,"ignore-dbs":["mysql","information_schema","performan
ce_schema","sys"]},"tikv-importer":{"addr":"","backend":"tidb","on-duplicate":"error","max-kv-pairs":32},"post-restore":{"level-1-compact":false,"compact":false,"checksum":true,"analyze":true},"cron":{"switch-mode":"5m0s",
"log-progress":"5m0s"},"routes":null,"security":{"ca-path":"","cert-path":"","key-path":""}}

Error: restore table `all_mode`.`t1` failed: Error 1062: Duplicate entry '120' for key 'gen_idx'
tidb lightning encountered error:  restore table `all_mode`.`t1` failed: Error 1062: Duplicate entry '120' for key 'gen_idx'

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)

Related changes

  • Need to be included in the release note

@lichunzhu lichunzhu added the status/PTAL This PR is ready for review. Add this label back after committing new changes label May 11, 2020
@lichunzhu lichunzhu requested a review from kennytm May 11, 2020 02:18
@CLAassistant
Copy link

CLAassistant commented May 11, 2020

CLA assistant check
All committers have signed the CLA.

@lichunzhu lichunzhu linked an issue May 11, 2020 that may be closed by this pull request
@kennytm
Copy link
Collaborator

kennytm commented May 11, 2020

Some printf will affect DM who uses Lightning as a library.

lightning/common/version.go Outdated Show resolved Hide resolved
Comment on lines 181 to 189
if *logFilePath != "" {
cfg.App.Config.File = *logFilePath
} else {
configFile, err := log.AdjustLogFilePath()
if err != nil {
return nil, errors.Annotate(err, "Create debug logs directory failed")
}
cfg.App.Config.File = configFile
fmt.Fprintf(os.Stdout, "Verbose debug logs will be written to %s.\n\n", configFile)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should still be a way to explicitly use stderr as log output.

i suggest using BR's approach of making the default value of the CLI parameter to be the log path.

lightning/lightning.go Outdated Show resolved Hide resolved
lightning/restore/restore.go Outdated Show resolved Hide resolved
@lichunzhu
Copy link
Contributor Author

Some printf will affect DM who uses Lightning as a library.

I move all Fprintf to main.go.

Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest LGTM

} else {
logger.Info("tidb lightning exit")
fmt.Fprintln(os.Stdout, "tidb lightning exit", err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fmt.Fprintln(os.Stdout, "tidb lightning exit", err)
fmt.Fprintln(os.Stdout, "tidb lightning exit")

Copy link
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lichunzhu lichunzhu merged commit 3c3fa29 into pingcap:master May 11, 2020
@lichunzhu lichunzhu deleted the fixLogStack branch May 11, 2020 10:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status/PTAL This PR is ready for review. Add this label back after committing new changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refine error info, avoid redundant stack info for users
4 participants