Logger: Remove stringstream in total #4484

JaySon-Huang · 2022-03-29T10:13:07Z

What problem does this PR solve?

Issue Number: close #4346, close #2179

Problem Summary:

What is changed and how it works?

Remove all stringstream in logging
Add filename and line number for LOG_XXX
Refine some comments in basedaemon

Check List

Tests

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

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

None

Signed-off-by: JaySon-Huang <tshent@qq.com>

ti-chi-bot · 2022-03-29T10:13:08Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

SeaRise
fuzhe1989

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

JaySon-Huang · 2022-03-29T10:19:41Z

/run-all-tests

JaySon-Huang · 2022-03-29T11:08:28Z

/run-all-tests

sre-bot · 2022-03-29T11:35:00Z

Coverage for changed files

Filename                                                Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dbms/src/Client/Connection.cpp                              221               221     0.00%          32                32     0.00%         405               405     0.00%         130               130     0.00%
dbms/src/Storages/DeltaMerge/WriteBatches.h                 107                19    82.24%          14                 0   100.00%         197                12    93.91%          60                18    70.00%
libs/libcommon/include/common/logger_useful.h                12                 0   100.00%           4                 0   100.00%          15                 3    80.00%           6                 0   100.00%
libs/libdaemon/include/daemon/BaseDaemon.h                    9                 7    22.22%           9                 7    22.22%          34                32     5.88%           0                 0         -
libs/libdaemon/include/daemon/OwnPatternFormatter.h           1                 0   100.00%           1                 0   100.00%           1                 0   100.00%           0                 0         -
libs/libdaemon/src/BaseDaemon.cpp                           623               430    30.98%          40                20    50.00%         851               434    49.00%         380               278    26.84%
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                       973               677    30.42%         100                59    41.00%        1503               886    41.05%         576               426    26.04%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
17050      9578             43.82%    191832  97054        49.41%

full coverage report (for internal network access only)

ywqzzy · 2022-03-29T11:34:47Z

libs/libdaemon/src/BaseDaemon.cpp

-        LOG_ERROR(log, "(from thread " << thread_num << ") "
-                                       << "Received signal " << strsignal(sig) << " (" << sig << ")"
-                                       << ".");
+        LOG_FMT_ERROR(log, "########################################");


Maybe merge into one LOG？

This will introduce lots of changes, maybe left it to another PR?

libs/libdaemon/src/tests/gtest_daemon_config.cpp

libs/libcommon/include/common/logger_useful.h

libs/libdaemon/include/daemon/BaseDaemon.h

ywqzzy · 2022-03-30T02:50:58Z

This pr is related to #2179.

JaySon-Huang · 2022-03-30T03:06:10Z

This pr is related to #2179.

Thanks!

JaySon-Huang · 2022-03-30T03:16:31Z

/run-all-tests

JaySon-Huang · 2022-03-30T03:16:41Z

@ywqzzy PTAL again

JaySon-Huang · 2022-03-30T03:41:58Z

/merge

ti-chi-bot · 2022-03-30T03:41:59Z

@JaySon-Huang: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2022-03-30T03:42:05Z

This pull request has been accepted and is ready to merge.

Commit hash: 369e596

sre-bot · 2022-03-30T04:21:40Z

Coverage for changed files

Filename                                                Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dbms/src/Client/Connection.cpp                              221               221     0.00%          32                32     0.00%         405               405     0.00%         130               130     0.00%
dbms/src/Storages/DeltaMerge/WriteBatches.h                 107                19    82.24%          14                 0   100.00%         197                12    93.91%          60                18    70.00%
libs/libcommon/include/common/logger_useful.h                12                 0   100.00%           4                 0   100.00%          15                 3    80.00%           6                 0   100.00%
libs/libdaemon/include/daemon/BaseDaemon.h                    9                 7    22.22%           9                 7    22.22%          34                32     5.88%           0                 0         -
libs/libdaemon/include/daemon/OwnPatternFormatter.h           1                 0   100.00%           1                 0   100.00%           1                 0   100.00%           0                 0         -
libs/libdaemon/src/BaseDaemon.cpp                           623               430    30.98%          40                20    50.00%         851               434    49.00%         380               278    26.84%
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                       973               677    30.42%         100                59    41.00%        1503               886    41.05%         576               426    26.04%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
17069      9589             43.82%    192087  97182        49.41%

full coverage report (for internal network access only)

JaySon-Huang added 3 commits March 29, 2022 17:29

temp save

48a8441

Signed-off-by: JaySon-Huang <tshent@qq.com>

Remove KEEP_STRINGSTREAM_FOR_LOG

56354b8

Fix log && some comment

084a977

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 29, 2022

JaySon-Huang requested review from fuzhe1989, ywqzzy, Lloyd-Pottiger and SeaRise March 29, 2022 10:13

Use lambda for generating log msg when we actually need it

0504b75

Fix compile error

3d378ae

ywqzzy reviewed Mar 29, 2022

View reviewed changes

fuzhe1989 approved these changes Mar 29, 2022

View reviewed changes

libs/libcommon/include/common/logger_useful.h Show resolved Hide resolved

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 29, 2022

SeaRise approved these changes Mar 29, 2022

View reviewed changes

libs/libdaemon/include/daemon/BaseDaemon.h Show resolved Hide resolved

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 29, 2022

Lloyd-Pottiger approved these changes Mar 30, 2022

View reviewed changes

ti-chi-bot added the do-not-merge/needs-triage-completed label Mar 30, 2022

JaySon-Huang added 4 commits March 30, 2022 11:07

Add LOG_FATAL

56fb9d8

Add missed comment

825f387

Address comment

da04946

Merge branch 'master' into fix_log_2

369e596

ti-chi-bot removed the do-not-merge/needs-triage-completed label Mar 30, 2022

ywqzzy approved these changes Mar 30, 2022

View reviewed changes

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 30, 2022

Merge branch 'master' into fix_log_2

7dc600f

ti-chi-bot merged commit a1b2035 into pingcap:master Mar 30, 2022

JaySon-Huang deleted the fix_log_2 branch March 30, 2022 04:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Logger: Remove stringstream in total #4484

Logger: Remove stringstream in total #4484

JaySon-Huang commented Mar 29, 2022 •

edited

Loading

ti-chi-bot commented Mar 29, 2022 •

edited

Loading

JaySon-Huang commented Mar 29, 2022

JaySon-Huang commented Mar 29, 2022

sre-bot commented Mar 29, 2022

ywqzzy Mar 29, 2022

JaySon-Huang Mar 30, 2022

ywqzzy commented Mar 30, 2022

JaySon-Huang commented Mar 30, 2022

JaySon-Huang commented Mar 30, 2022

JaySon-Huang commented Mar 30, 2022

JaySon-Huang commented Mar 30, 2022

ti-chi-bot commented Mar 30, 2022

ti-chi-bot commented Mar 30, 2022

sre-bot commented Mar 30, 2022

Logger: Remove stringstream in total #4484

Logger: Remove stringstream in total #4484

Conversation

JaySon-Huang commented Mar 29, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Mar 29, 2022 • edited Loading

JaySon-Huang commented Mar 29, 2022

JaySon-Huang commented Mar 29, 2022

sre-bot commented Mar 29, 2022

Coverage for changed files

Coverage summary

ywqzzy Mar 29, 2022

Choose a reason for hiding this comment

JaySon-Huang Mar 30, 2022

Choose a reason for hiding this comment

ywqzzy commented Mar 30, 2022

JaySon-Huang commented Mar 30, 2022

JaySon-Huang commented Mar 30, 2022

JaySon-Huang commented Mar 30, 2022

JaySon-Huang commented Mar 30, 2022

ti-chi-bot commented Mar 30, 2022

ti-chi-bot commented Mar 30, 2022

sre-bot commented Mar 30, 2022

Coverage for changed files

Coverage summary

JaySon-Huang commented Mar 29, 2022 •

edited

Loading

ti-chi-bot commented Mar 29, 2022 •

edited

Loading