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.

Sign up for GitHub

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

Jump to bottom

storage: unify segment_id for data sharing #5800

Merged
merged 9 commits into from
Sep 7, 2022
Merged

storage: unify segment_id for data sharing #5800

merged 9 commits into from
Sep 7, 2022

Conversation

JaySon-Huang
Copy link
Contributor

@JaySon-Huang JaySon-Huang commented Sep 6, 2022 •
edited
Loading

What problem does this PR solve?

Issue Number: ref #5726

Problem Summary:

What is changed and how it works?

  • Add tracing_id to SegmentReadTaskPool
  • logging with seg_id xxx are unified to be segment_id=xx so that we can simply grep for it to precisely filter out all related log lines
  • Skip NUMA detect on mac
    previous it throw exception and make som trouble for debugging unittest under mac
* Use the mpp query id for `DBGInvoke search_log_for_key` avoid the verbose logging: https://github.com//pull/5715#discussion_r963465875

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

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

None

@JaySon-Huang
Unify the logging style
e0657cf 
Signed-off-by: JaySon-Huang <[email protected]>
@JaySon-Huang
Skip NUMA detect on mac
4e013e6 
Signed-off-by: JaySon-Huang <[email protected]>
@JaySon-Huang
Use segment_id instead of seg_id
abc7659 
Signed-off-by: JaySon-Huang <[email protected]>
@JaySon-Huang
Fix rough set test
a457670 
Signed-off-by: JaySon-Huang <[email protected]>
@JaySon-Huang
Add tracing_id to SegmentReadTaskPool
c915cdf 
Signed-off-by: JaySon-Huang <[email protected]>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Sep 6, 2022 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • breezewish
  • lidezhu

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.

@ti-chi-bot ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 6, 2022
@JaySon-Huang
Fix more
ded1b32 
Signed-off-by: JaySon-Huang <[email protected]>
@JaySon-Huang
Copy link
Contributor Author

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Sep 6, 2022

Coverage for changed files

Filename                                                        Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Debug/dbgFuncMisc.cpp                                                46                46     0.00%           3                 3     0.00%          85                85     0.00%          36                36     0.00%
Storages/DeltaMerge/DeltaMergeStore.cpp                            1707               617    63.85%          70                 5    92.86%        2264               540    76.15%         760               318    58.16%
Storages/DeltaMerge/ReadThread/CPU.cpp                              105                60    42.86%           8                 0   100.00%          76                17    77.63%          40                18    55.00%
Storages/DeltaMerge/ReadThread/ColumnSharingCache.h                  69                 8    88.41%          13                 2    84.62%         118                13    88.98%          30                 8    73.33%
Storages/DeltaMerge/ReadThread/SegmentReadTaskScheduler.cpp          99                 8    91.92%          11                 0   100.00%         161                18    88.82%          50                13    74.00%
Storages/DeltaMerge/ReadThread/SegmentReadTaskScheduler.h             2                 1    50.00%           2                 1    50.00%           7                 3    57.14%           0                 0         -
Storages/DeltaMerge/ReadThread/SegmentReader.cpp                    203                49    75.86%          17                 0   100.00%         154                27    82.47%          60                24    60.00%
Storages/DeltaMerge/ReadThread/SegmentReader.h                        1                 0   100.00%           1                 0   100.00%           4                 0   100.00%           0                 0         -
Storages/DeltaMerge/ReadThread/UnorderedInputStream.h                64                 5    92.19%           7                 0   100.00%          61                19    68.85%          18                 8    55.56%
Storages/DeltaMerge/ReadThread/WorkQueue.h                           25                 5    80.00%           6                 0   100.00%          62                15    75.81%          16                 6    62.50%
Storages/DeltaMerge/SegmentReadTaskPool.cpp                          97                19    80.41%          21                 3    85.71%         185                60    67.57%          56                27    51.79%
Storages/DeltaMerge/SegmentReadTaskPool.h                            39                 1    97.44%          16                 1    93.75%          63                 3    95.24%          10                 1    90.00%
Storages/StorageDeltaMerge.cpp                                      717               276    61.51%          59                21    64.41%        1297               553    57.36%         364               198    45.60%
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                              3174              1095    65.50%         234                36    84.62%        4537              1353    70.18%        1440               657    54.37%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18665      8198             56.08%    215934  84381        60.92%

full coverage report (for internal network access only)

@JaySon-Huang
Copy link
Contributor Author

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Sep 7, 2022

Coverage for changed files

Filename                                               Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
DeltaMerge/DeltaMergeStore.cpp                            1707               617    63.85%          70                 5    92.86%        2264               540    76.15%         760               318    58.16%
DeltaMerge/ReadThread/CPU.cpp                              105                60    42.86%           8                 0   100.00%          76                17    77.63%          40                18    55.00%
DeltaMerge/ReadThread/ColumnSharingCache.h                  69                 8    88.41%          13                 2    84.62%         118                13    88.98%          30                 8    73.33%
DeltaMerge/ReadThread/SegmentReadTaskScheduler.cpp          99                 8    91.92%          11                 0   100.00%         161                18    88.82%          50                13    74.00%
DeltaMerge/ReadThread/SegmentReadTaskScheduler.h             2                 1    50.00%           2                 1    50.00%           7                 3    57.14%           0                 0         -
DeltaMerge/ReadThread/SegmentReader.cpp                    203                49    75.86%          17                 0   100.00%         154                27    82.47%          60                24    60.00%
DeltaMerge/ReadThread/SegmentReader.h                        1                 0   100.00%           1                 0   100.00%           4                 0   100.00%           0                 0         -
DeltaMerge/ReadThread/UnorderedInputStream.h                64                 5    92.19%           7                 0   100.00%          61                19    68.85%          18                 8    55.56%
DeltaMerge/ReadThread/WorkQueue.h                           25                 5    80.00%           6                 0   100.00%          62                15    75.81%          16                 6    62.50%
DeltaMerge/SegmentReadTaskPool.cpp                          97                19    80.41%          21                 3    85.71%         185                60    67.57%          56                27    51.79%
DeltaMerge/SegmentReadTaskPool.h                            39                 1    97.44%          16                 1    93.75%          63                 3    95.24%          10                 1    90.00%
StorageDeltaMerge.cpp                                      711               276    61.18%          59                21    64.41%        1300               555    57.31%         364               198    45.60%
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                     3122              1049    66.40%         231                33    85.71%        4455              1270    71.49%        1404               621    55.77%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18665      8198             56.08%    215936  84315        60.95%

full coverage report (for internal network access only)

lidezhu
lidezhu approved these changes Sep 7, 2022
View reviewed changes
Copy link
Contributor

@lidezhu lidezhu left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 7, 2022
@ti-chi-bot 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 Sep 7, 2022
@breezewish
Copy link
Member

/merge

@ti-chi-bot
Copy link
Member

@breezewish: 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
Copy link
Member

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

Commit hash: 4e16592

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 7, 2022
@sre-bot
Copy link
Collaborator

sre-bot commented Sep 7, 2022

Coverage for changed files

Filename                                               Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
DeltaMerge/DeltaMergeStore.cpp                            1717               625    63.60%          70                 5    92.86%        2274               548    75.90%         768               323    57.94%
DeltaMerge/ReadThread/CPU.cpp                              105                60    42.86%           8                 0   100.00%          76                17    77.63%          40                18    55.00%
DeltaMerge/ReadThread/ColumnSharingCache.h                  69                 8    88.41%          13                 2    84.62%         118                13    88.98%          30                 8    73.33%
DeltaMerge/ReadThread/SegmentReadTaskScheduler.cpp          99                 8    91.92%          11                 0   100.00%         161                18    88.82%          50                13    74.00%
DeltaMerge/ReadThread/SegmentReadTaskScheduler.h             2                 1    50.00%           2                 1    50.00%           7                 3    57.14%           0                 0         -
DeltaMerge/ReadThread/SegmentReader.cpp                    203                49    75.86%          17                 0   100.00%         154                27    82.47%          60                24    60.00%
DeltaMerge/ReadThread/SegmentReader.h                        1                 0   100.00%           1                 0   100.00%           4                 0   100.00%           0                 0         -
DeltaMerge/ReadThread/UnorderedInputStream.h                64                 5    92.19%           7                 0   100.00%          61                19    68.85%          18                 8    55.56%
DeltaMerge/ReadThread/WorkQueue.h                           25                 5    80.00%           6                 0   100.00%          62                15    75.81%          16                 6    62.50%
DeltaMerge/SegmentReadTaskPool.cpp                          97                19    80.41%          21                 3    85.71%         185                60    67.57%          56                27    51.79%
DeltaMerge/SegmentReadTaskPool.h                            39                 1    97.44%          16                 1    93.75%          63                 3    95.24%          10                 1    90.00%
StorageDeltaMerge.cpp                                      711               276    61.18%          59                21    64.41%        1300               555    57.31%         364               198    45.60%
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                     3132              1057    66.25%         231                33    85.71%        4465              1278    71.38%        1412               626    55.67%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18666      8199             56.08%    215952  84405        60.91%

full coverage report (for internal network access only)

@ti-chi-bot ti-chi-bot merged commit e89c6c4 into pingcap:master Sep 7, 2022
@JaySon-Huang JaySon-Huang deleted the align_read_thread_log branch September 8, 2022 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lidezhu lidezhu lidezhu approved these changes

@breezewish breezewish breezewish approved these changes

@JinheLin JinheLin Awaiting requested review from JinheLin

Assignees
No one assigned
Labels
release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JaySon-Huang @ti-chi-bot @sre-bot @breezewish @lidezhu