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

feat: modify minor compaction to make each ColumnFile in persisted set is sorted #5253

Closed

Conversation

hongyunyan
Copy link
Contributor

@hongyunyan hongyunyan commented Jun 29, 2022

What problem does this PR solve?

Issue Number: ref #5252

Problem Summary:

What is changed and how it works?

Modify minor compaction to make each ColumnFile in persisted set is sorted, thus we can speed up reading under fast mode. Besides, a more ordered delta value space will also improve read performance in normal mode.

Check List

Tests

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

test by dtworkload, environment: M1 Pro 32G
image

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

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jun 29, 2022

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • 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 do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. 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 Jun 29, 2022
@hongyunyan
Copy link
Contributor Author

/rebuild

@hongyunyan
Copy link
Contributor Author

/run-all-tests

@hongyunyan
Copy link
Contributor Author

/run-unit-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Jun 29, 2022

Coverage for changed files

Filename                                 Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Delta/ColumnFilePersistedSet.cpp             258                64    75.19%          19                 0   100.00%         401                53    86.78%         202                60    70.30%
Delta/ColumnFilePersistedSet.h                13                 0   100.00%          10                 0   100.00%          23                 0   100.00%           2                 0   100.00%
Delta/DeltaValueSpace.cpp                    201                82    59.20%          17                 0   100.00%         163                36    77.91%          90                53    41.11%
Delta/DeltaValueSpace.h                      107                21    80.37%          48                 3    93.75%         121                19    84.30%          36                13    63.89%
Delta/MinorCompaction.cpp                     19                 1    94.74%           4                 0   100.00%          44                 1    97.73%          12                 1    91.67%
Delta/MinorCompaction.h                       14                 0   100.00%           6                 0   100.00%          28                 0   100.00%           8                 0   100.00%
DeltaMergeStore.cpp                         1453               535    63.18%          67                 5    92.54%        2043               493    75.87%         840               395    52.98%
tests/gtest_dm_delta_value_space.cpp        1170               170    85.47%          18                 1    94.44%         458                 5    98.91%         362               169    53.31%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                       3235               873    73.01%         189                 9    95.24%        3281               607    81.50%        1552               691    55.48%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18388      9656             47.49%    206760  96715        53.22%

full coverage report (for internal network access only)

@hongyunyan
Copy link
Contributor Author

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Jun 29, 2022

Coverage for changed files

Filename                                 Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Delta/ColumnFilePersistedSet.cpp             258                73    71.71%          19                 0   100.00%         401                57    85.79%         202                63    68.81%
Delta/ColumnFilePersistedSet.h                13                 1    92.31%          10                 1    90.00%          23                 1    95.65%           2                 0   100.00%
Delta/DeltaValueSpace.cpp                    201                91    54.73%          17                 0   100.00%         163                41    74.85%          90                56    37.78%
Delta/DeltaValueSpace.h                      107                21    80.37%          48                 3    93.75%         121                19    84.30%          36                13    63.89%
Delta/MinorCompaction.cpp                     19                 1    94.74%           4                 0   100.00%          44                 1    97.73%          12                 1    91.67%
Delta/MinorCompaction.h                       14                 0   100.00%           6                 0   100.00%          28                 0   100.00%           8                 0   100.00%
DeltaMergeStore.cpp                         1453               536    63.11%          67                 7    89.55%        2043               518    74.65%         840               402    52.14%
tests/gtest_dm_delta_value_space.cpp        1170               170    85.47%          18                 1    94.44%         458                 5    98.91%         362               169    53.31%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                       3235               893    72.40%         189                12    93.65%        3281               642    80.43%        1552               704    54.64%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18402      9635             47.64%    207039  96375        53.45%

full coverage report (for internal network access only)

@hongyunyan hongyunyan changed the title WIP: feat: modify minor compaction to make each ColumnFile in persisted set is sorted feat: modify minor compaction to make each ColumnFile in persisted set is sorted Jul 1, 2022
@ti-chi-bot ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 1, 2022
@hongyunyan
Copy link
Contributor Author

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Jul 1, 2022

Coverage for changed files

Filename                                 Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Delta/ColumnFilePersistedSet.cpp             258                64    75.19%          19                 0   100.00%         401                53    86.78%         202                60    70.30%
Delta/ColumnFilePersistedSet.h                13                 0   100.00%          10                 0   100.00%          23                 0   100.00%           2                 0   100.00%
Delta/DeltaValueSpace.cpp                    201                82    59.20%          17                 0   100.00%         163                36    77.91%          90                53    41.11%
Delta/DeltaValueSpace.h                      107                21    80.37%          48                 3    93.75%         121                19    84.30%          36                13    63.89%
Delta/MinorCompaction.cpp                     19                 1    94.74%           4                 0   100.00%          44                 1    97.73%          12                 1    91.67%
Delta/MinorCompaction.h                       14                 0   100.00%           6                 0   100.00%          28                 0   100.00%           8                 0   100.00%
DeltaMergeStore.cpp                         1453               527    63.73%          67                 7    89.55%        2043               514    74.84%         840               399    52.50%
tests/gtest_dm_delta_value_space.cpp        1170               170    85.47%          18                 1    94.44%         458                 5    98.91%         362               169    53.31%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                       3235               865    73.26%         189                11    94.18%        3281               628    80.86%        1552               695    55.22%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18406      9638             47.64%    207080  96470        53.41%

full coverage report (for internal network access only)

@hongyunyan
Copy link
Contributor Author

/run-integration-tests

@hongyunyan
Copy link
Contributor Author

cc @lidezhu @flowbehappy

@hongyunyan
Copy link
Contributor Author

/run-all-tests

@hongyunyan
Copy link
Contributor Author

/run-unit-tests

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 Jul 5, 2022
@sre-bot
Copy link
Collaborator

sre-bot commented Jul 5, 2022

Coverage for changed files

Filename                                 Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Delta/ColumnFilePersistedSet.cpp             258                64    75.19%          19                 0   100.00%         401                53    86.78%         202                60    70.30%
Delta/ColumnFilePersistedSet.h                13                 0   100.00%          10                 0   100.00%          23                 0   100.00%           2                 0   100.00%
Delta/DeltaValueSpace.cpp                    201                82    59.20%          17                 0   100.00%         163                36    77.91%          90                53    41.11%
Delta/DeltaValueSpace.h                      107                21    80.37%          48                 3    93.75%         121                19    84.30%          36                13    63.89%
Delta/MinorCompaction.cpp                     19                 1    94.74%           4                 0   100.00%          44                 1    97.73%          12                 1    91.67%
Delta/MinorCompaction.h                       14                 0   100.00%           6                 0   100.00%          28                 0   100.00%           8                 0   100.00%
tests/gtest_dm_delta_value_space.cpp        1170               170    85.47%          18                 1    94.44%         458                 5    98.91%         362               169    53.31%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                       1782               338    81.03%         122                 4    96.72%        1238               114    90.79%         712               296    58.43%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18406      9638             47.64%    207079  96399        53.45%

full coverage report (for internal network access only)

@hongyunyan
Copy link
Contributor Author

/run-integration-tests

@hongyunyan
Copy link
Contributor Author

/rebuild

@hongyunyan
Copy link
Contributor Author

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Jul 5, 2022

Coverage for changed files

Filename                                 Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Delta/ColumnFilePersistedSet.cpp             258                73    71.71%          19                 0   100.00%         401                57    85.79%         202                63    68.81%
Delta/ColumnFilePersistedSet.h                13                 1    92.31%          10                 1    90.00%          23                 1    95.65%           2                 0   100.00%
Delta/DeltaValueSpace.cpp                    216                92    57.41%          17                 0   100.00%         174                41    76.44%          96                58    39.58%
Delta/DeltaValueSpace.h                      108                21    80.56%          49                 3    93.88%         122                19    84.43%          36                13    63.89%
Delta/MinorCompaction.cpp                     19                 1    94.74%           4                 0   100.00%          44                 1    97.73%          12                 1    91.67%
Delta/MinorCompaction.h                       14                 0   100.00%           6                 0   100.00%          28                 0   100.00%           8                 0   100.00%
tests/gtest_dm_delta_value_space.cpp        1170               170    85.47%          18                 1    94.44%         458                 5    98.91%         362               169    53.31%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                       1798               358    80.09%         123                 5    95.93%        1250               124    90.08%         718               304    57.66%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18411      9642             47.63%    207204  96528        53.41%

full coverage report (for internal network access only)

@hongyunyan
Copy link
Contributor Author

/run-integration-tests

@hongyunyan
Copy link
Contributor Author

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Jul 6, 2022

Coverage for changed files

Filename                                 Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Delta/ColumnFilePersistedSet.cpp             258                73    71.71%          19                 0   100.00%         401                57    85.79%         202                63    68.81%
Delta/ColumnFilePersistedSet.h                13                 1    92.31%          10                 1    90.00%          23                 1    95.65%           2                 0   100.00%
Delta/DeltaValueSpace.cpp                    216                92    57.41%          17                 0   100.00%         174                41    76.44%          96                58    39.58%
Delta/DeltaValueSpace.h                      108                21    80.56%          49                 3    93.88%         122                19    84.43%          36                13    63.89%
Delta/MinorCompaction.cpp                     19                 1    94.74%           4                 0   100.00%          44                 1    97.73%          12                 1    91.67%
Delta/MinorCompaction.h                       14                 0   100.00%           6                 0   100.00%          28                 0   100.00%           8                 0   100.00%
tests/gtest_dm_delta_value_space.cpp        1170               170    85.47%          18                 1    94.44%         458                 5    98.91%         362               169    53.31%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                       1798               358    80.09%         123                 5    95.93%        1250               124    90.08%         718               304    57.66%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18412      9642             47.63%    207244  96549        53.41%

full coverage report (for internal network access only)

@hongyunyan
Copy link
Contributor Author

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Jul 10, 2022

Coverage for changed files

Filename                                 Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Delta/ColumnFilePersistedSet.cpp             258                73    71.71%          19                 0   100.00%         401                57    85.79%         202                63    68.81%
Delta/ColumnFilePersistedSet.h                13                 1    92.31%          10                 1    90.00%          23                 1    95.65%           2                 0   100.00%
Delta/DeltaValueSpace.cpp                    216                92    57.41%          17                 0   100.00%         174                41    76.44%          96                58    39.58%
Delta/DeltaValueSpace.h                      108                21    80.56%          49                 3    93.88%         122                19    84.43%          36                13    63.89%
Delta/MinorCompaction.cpp                     19                 1    94.74%           4                 0   100.00%          44                 1    97.73%          12                 1    91.67%
Delta/MinorCompaction.h                       14                 0   100.00%           6                 0   100.00%          28                 0   100.00%           8                 0   100.00%
tests/gtest_dm_delta_value_space.cpp        1170               170    85.47%          18                 1    94.44%         458                 5    98.91%         362               169    53.31%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                       1798               358    80.09%         123                 5    95.93%        1250               124    90.08%         718               304    57.66%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18465      9602             48.00%    208034  96407        53.66%

full coverage report (for internal network access only)

@flowbehappy
Copy link
Contributor

/hold
There are some performance regressions. Should be figured out before merging.

@ti-chi-bot ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 13, 2022
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 12, 2022
@ti-chi-bot
Copy link
Member

@hongyunyan: PR needs rebase.

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 kubernetes/test-infra repository.

@hongyunyan
Copy link
Contributor Author

/close

@ti-chi-bot
Copy link
Member

@hongyunyan: Closed this PR.

In response to this:

/close

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 kubernetes/test-infra repository.

@ti-chi-bot ti-chi-bot closed this Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. 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/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants