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

Upgrade zap and fix url #18

Merged
merged 2 commits into from Aug 18, 2021
Merged

Upgrade zap and fix url #18

merged 2 commits into from Aug 18, 2021

Conversation

dragonly
Copy link
Contributor

  1. upgrade zap to 1.19.0
  2. fix unified log format doc url

Copy link
Contributor

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

Thanks for your pull request @dragonly ! May I ask the reason you do the version bump?

@dragonly
Copy link
Contributor Author

dragonly commented Aug 18, 2021

@tisonkun I am currently trying to optimize the general_log performance of tidb-server, and digged into some details of zap. I found some interesting enhancements in the following releases.

For example, in v1.18.0:

And buffering is crucial for high performance logging like the general log feature.

I haven't done a thorough research, but I believe we can do something later to enhance the logging performance of TiDB itself, by leveraging the enhancements of new releases of zap!

@dragonly dragonly requested a review from tisonkun August 18, 2021 09:50
Copy link
Contributor

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

Thanks for your explanation. LGTM.

Though we're using our own WriteSyncer IIRC, we cannot make use of zapcore.BufferedWriteSyncer for free.

@dragonly
Copy link
Contributor Author

dragonly commented Aug 18, 2021

Though we're using our own WriteSyncer IIRC, we cannot make use of zapcore.BufferedWriteSyncer for free.

Actually we can wrap this in a zapcore.BufferedWriteSyncer, and everything seems just work.

And there are actually several different loggers in TiDB, and I can leverage zapcore.BufferedWriteSyncer in general_log first!

@dragonly dragonly merged commit 02d3023 into master Aug 18, 2021
@dragonly dragonly deleted the upgrade-zap-fix-url branch August 18, 2021 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants