Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consider updating logrus #7986

Closed
siddontang opened this issue Oct 22, 2018 · 1 comment
Closed

consider updating logrus #7986

siddontang opened this issue Oct 22, 2018 · 1 comment
Assignees
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@siddontang
Copy link
Member

Bug Report

In our test, we find there are too many mutex used

268847702 2712 @ 0x473024 0x6f0c40 0x6ef80c 0xf94517 0x1028925 0x1032e46 0x102c0fb 0x102cf3e 0x102c5e9 0x106068c 0x105963e 0x10571b6 0x1055d0e 0x106fce4 0x45bc71
#	0x473023	sync.(*Mutex).Unlock+0x73								/home/qupeng/.local/opt/go/src/sync/mutex.go:201
#	0x6f0c3f	github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus.(*MutexWrap).Unlock+0x3f	/home/qupeng/.go/src/github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus/logger.go:49
#	0x6ef80b	github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus.GetLevel+0x7b			/home/qupeng/.go/src/github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus/exported.go:41
#	0xf94516	github.com/pingcap/tidb/executor.(*ExecStmt).LogSlowQuery+0x36				/home/qupeng/.go/src/github.com/pingcap/tidb/executor/adapter.go:336
#	0x1028924	github.com/pingcap/tidb/session.(*session).CommitTxn+0x124				/home/qupeng/.go/src/github.com/pingcap/tidb/session/session.go:388
#	0x1032e45	github.com/pingcap/tidb/session.runStmt+0x245						/home/qupeng/.go/src/github.com/pingcap/tidb/session/tidb.go:170
#	0x102c0fa	github.com/pingcap/tidb/session.(*session).executeStatement+0x1ea			/home/qupeng/.go/src/github.com/pingcap/tidb/session/session.go:741
#	0x102cf3d	github.com/pingcap/tidb/session.(*session).execute+0x87d				/home/qupeng/.go/src/github.com/pingcap/tidb/session/session.go:812
#	0x102c5e8	github.com/pingcap/tidb/session.(*session).Execute+0x68					/home/qupeng/.go/src/github.com/pingcap/tidb/session/session.go:762
#	0x106068b	github.com/pingcap/tidb/server.(*TiDBContext).Execute+0x7b				/home/qupeng/.go/src/github.com/pingcap/tidb/server/driver_tidb.go:240
#	0x105963d	github.com/pingcap/tidb/server.(*clientConn).handleQuery+0x8d				/home/qupeng/.go/src/github.com/pingcap/tidb/server/conn.go:874
#	0x10571b5	github.com/pingcap/tidb/server.(*clientConn).dispatch+0x685				/home/qupeng/.go/src/github.com/pingcap/tidb/server/conn.go:626
#	0x1055d0d	github.com/pingcap/tidb/server.(*clientConn).Run+0x1bd					/home/qupeng/.go/src/github.com/pingcap/tidb/server/conn.go:470
#	0x106fce3	github.com/pingcap/tidb/server.(*Server).onConn+0x223					/home/qupeng/.go/src/github.com/pingcap/tidb/server/server.go:324

This is cause by the logrus GetLevel function which uses a global mutex. In the latest logrus, it has already switched to using atomic.

I write a simple benchmark https://gist.github.com/siddontang/ee4cee6115b9d8d16286543c734f04d4 and find the atomic has a better performance than mutex.

Luckily, the latest logrus has already used atomic, we can update it directly.

@siddontang siddontang added the type/enhancement The issue or PR belongs to an enhancement. label Oct 22, 2018
@tiancaiamao
Copy link
Contributor

I'm moving to go mod #7922
Maybe I can update logrus after that.
I know update logrus and support module are two different things, but update them once maybe save some time for me. @siddontang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

No branches or pull requests

2 participants