Optimization schema memory utilization of ColumnFile in instance level #6589

hongyunyan · 2023-01-05T11:19:35Z

What problem does this PR solve?

Issue Number: ref #6401

Problem Summary:

What is changed and how it works?

Add a new class ColumnFileSchema to store schema information together for ColumnFileInMemory and ColumnFileTiny.
Change the code to share the same ColumnFileSchema when the schema is the same in various ColumnFileInMemory and ColumnFileTiny.

In performance test, after this optimization, the memory usage(static memory) can reduce 10%-30%(which is related with the number of segments for a table)

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

Reduced memory usage by up to 30% in the case without query load

…yunyan_column_file_tiny

…yunyan_column_file_tiny_instance_level

ti-chi-bot · 2023-01-05T11:19:36Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

JaySon-Huang
breezewish

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.

hongyunyan · 2023-01-06T05:51:48Z

/run-all-tests

…yunyan_column_file_tiny_instance_level

hongyunyan · 2023-01-09T07:59:44Z

/run-all-tests

…yunyan_column_file_tiny_instance_level

dbms/src/Storages/DeltaMerge/Delta/ColumnFilePersistedSet.h

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileTiny.cpp

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileTiny.h

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileSchema.h

JaySon-Huang · 2023-02-06T06:01:26Z

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileSchema.h

+            }
+            return true;
+        },
+                                         false);


Suggested change

false);

/*multi*/false, /*interval_ms*/ 60000);

Can make it run less frequently. e.g. every 60 seconds instead of 10 seconds by default

Yes, I agree it don't need run once 10 seconds, but I also not sure how long is an appropriate duration.

JaySon-Huang

Rest LGTM

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileSchema.h

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileTiny.cpp

breezewish

The rest LGTM

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileTiny.cpp

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFile_V2.cpp

dbms/src/Storages/DeltaMerge/Delta/MemTableSet.cpp

hongyunyan · 2023-02-06T10:22:44Z

/run-all-tests

breezewish

LGTM

Some minor suggestions

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileSchema.h

breezewish · 2023-02-06T13:06:26Z

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileSchema.h

+    ColumnFileSchemaPtr find(const Digest & digest)
+    {
+        std::lock_guard<std::mutex> lock(mutex);
+        auto it = column_file_schemas.find(digest);
+        if (it == column_file_schemas.end())
+            return nullptr;
+        return it->second.lock();
+    }


This interface seems to be not needed any more.

It still be used in StorageDeltaMerge.cpp:L966.

dbms/src/Storages/DeltaMerge/tests/gtest_dm_column_file.cpp

hongyunyan · 2023-02-06T13:24:06Z

/run-all-tests

hongyunyan · 2023-02-07T01:49:20Z

/run-all-tests

hongyunyan · 2023-02-07T02:37:06Z

/merge

ti-chi-bot · 2023-02-07T02:37:07Z

@hongyunyan: 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 · 2023-02-07T02:37:09Z

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

Commit hash: 766e29a

pingcap#6589) ref pingcap#6401 Signed-off-by: ywqzzy <[email protected]>

hongyunyan added 14 commits December 21, 2022 11:17

first commit of columnfile memory optimization

24e94df

merge conflict

c15b76b

format

27a580d

format typo

0400dfc

test format

e21392a

add metrics

eb323fe

Merge branch 'master' of https://github.com/pingcap/tiflash into hong…

e79ab6a

…yunyan_column_file_tiny

Merge branch 'master' of https://github.com/pingcap/tiflash into hong…

1ea2949

…yunyan_column_file_tiny

fix restore

151077c

add comments

4d31fde

add comments

7c624cc

for test

c016e41

Merge branch 'master' of https://github.com/pingcap/tiflash into hong…

f82fc9a

…yunyan_column_file_tiny_instance_level

basic implementation of instance level

d91c14a

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 5, 2023

hongyunyan added 7 commits January 5, 2023 19:22

fix typo

e1ce45d

fix typo

b78c821

for compile

109fe8c

for test

f117362

for typo

4e8ccb9

fix test

a24fc30

fix

46d371c

hongyunyan added 2 commits January 6, 2023 15:21

Merge branch 'master' of https://github.com/pingcap/tiflash into hong…

043c29d

…yunyan_column_file_tiny_instance_level

fix

f82f99d

Merge branch 'master' of https://github.com/pingcap/tiflash into hong…

86cc581

…yunyan_column_file_tiny_instance_level

JaySon-Huang reviewed Feb 6, 2023

View reviewed changes

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileSchema.h Outdated Show resolved Hide resolved

JaySon-Huang reviewed Feb 6, 2023

View reviewed changes

hongyunyan added 2 commits February 6, 2023 14:32

update code

39ca91b

update code

8433058

JaySon-Huang reviewed Feb 6, 2023

View reviewed changes

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileSchema.h Outdated Show resolved Hide resolved

JaySon-Huang approved these changes Feb 6, 2023

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 6, 2023

breezewish reviewed Feb 6, 2023

View reviewed changes

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileTiny.cpp Outdated Show resolved Hide resolved

breezewish reviewed Feb 6, 2023

View reviewed changes

hongyunyan added 2 commits February 6, 2023 17:19

update code

827beb6

add code

f0f2a39

breezewish approved these changes Feb 6, 2023

View reviewed changes

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 Feb 6, 2023

update

2e0b023

Merge branch 'master' into hongyunyan_column_file_tiny_instance_level

766e29a

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 7, 2023

ti-chi-bot merged commit f992ced into pingcap:master Feb 7, 2023

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Feb 7, 2023

ywqzzy pushed a commit to ywqzzy/tiflash_1 that referenced this pull request Feb 13, 2023

Optimization schema memory utilization of ColumnFile in instance level (

6b89097

pingcap#6589) ref pingcap#6401 Signed-off-by: ywqzzy <[email protected]>

JaySon-Huang mentioned this pull request Feb 13, 2023

TSan check broken #6809

Closed

JaySon-Huang mentioned this pull request Aug 23, 2023

Change the implementation of shared_block_schemas with LRUCache #6881

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimization schema memory utilization of ColumnFile in instance level #6589

Optimization schema memory utilization of ColumnFile in instance level #6589

hongyunyan commented Jan 5, 2023 •

edited by flowbehappy

Loading

ti-chi-bot commented Jan 5, 2023 •

edited

Loading

hongyunyan commented Jan 6, 2023

hongyunyan commented Jan 9, 2023

JaySon-Huang Feb 6, 2023

hongyunyan Feb 6, 2023

JaySon-Huang left a comment

breezewish left a comment

hongyunyan commented Feb 6, 2023

breezewish left a comment •

edited

Loading

breezewish Feb 6, 2023

hongyunyan Feb 6, 2023

hongyunyan commented Feb 6, 2023

hongyunyan commented Feb 7, 2023

hongyunyan commented Feb 7, 2023

ti-chi-bot commented Feb 7, 2023

ti-chi-bot commented Feb 7, 2023

Optimization schema memory utilization of ColumnFile in instance level #6589

Optimization schema memory utilization of ColumnFile in instance level #6589

Conversation

hongyunyan commented Jan 5, 2023 • edited by flowbehappy Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jan 5, 2023 • edited Loading

hongyunyan commented Jan 6, 2023

hongyunyan commented Jan 9, 2023

JaySon-Huang Feb 6, 2023

Choose a reason for hiding this comment

hongyunyan Feb 6, 2023

Choose a reason for hiding this comment

JaySon-Huang left a comment

Choose a reason for hiding this comment

breezewish left a comment

Choose a reason for hiding this comment

hongyunyan commented Feb 6, 2023

breezewish left a comment • edited Loading

Choose a reason for hiding this comment

breezewish Feb 6, 2023

Choose a reason for hiding this comment

hongyunyan Feb 6, 2023

Choose a reason for hiding this comment

hongyunyan commented Feb 6, 2023

hongyunyan commented Feb 7, 2023

hongyunyan commented Feb 7, 2023

ti-chi-bot commented Feb 7, 2023

ti-chi-bot commented Feb 7, 2023

hongyunyan commented Jan 5, 2023 •

edited by flowbehappy

Loading

ti-chi-bot commented Jan 5, 2023 •

edited

Loading

breezewish left a comment •

edited

Loading