-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[SMALLFIX] Fix logging message template in AlluxioMasterProcess #14683
Conversation
Hi @xwzbupt, thanks for your contribution! In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. |
You did it @xwzbupt! Thank you for signing the Contribution License Agreement. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
@jiacheliu3 please merge this once the unit tests pass.
known flakey test:
|
alluxio-bot, merge this please |
### What changes are proposed in this pull request? Modify the placeholder from "%s" to "{}" in the LOG statement. ### Why are the changes needed? Please clarify why the changes are needed. For instance, 1. The performance of `{}` is better than `%s`,considering `%s` will cause memory overhead and generate temporary objects. ### Does this PR introduce any user facing changes? No. Fixes Alluxio/Community#617 pr-link: Alluxio#14683 change-id: cid-7ef21fd8246ae2c0b952aabb82f043050356fb6c
What changes are proposed in this pull request?
Modify the placeholder from "%s" to "{}" in the LOG statement.
Why are the changes needed?
Please clarify why the changes are needed. For instance,
{}
is better than%s
,considering%s
will cause memory overhead and generate temporary objects.Does this PR introduce any user facing changes?
No.
Fixes Alluxio/Community#617