Fix pageutils will truncate the file if write failed #4925

jiaqizho · 2022-05-18T13:56:48Z

What problem does this PR solve?

Issue Number: ref #3594

Problem Summary:

If PageStorage failed to write, Then PageUtils will truncate it into offset.
It will make trouble in V3, Because we are perform random write in V3 BlobFile.

What is changed and how it works?

If write failed. Do not truncate the file into offset.

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

ti-chi-bot · 2022-05-18T13:56:49Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

CalvinNeo
JaySon-Huang

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.

dbsid · 2022-05-18T14:09:36Z

/run-build-arm64 comment=true

jiaqizho · 2022-05-18T14:10:04Z

/run-all-tests

CalvinNeo

lgtm

jiaqizho · 2022-05-18T14:47:16Z

/run-all-tests

JaySon-Huang · 2022-05-18T15:05:55Z

dbms/src/Storages/Page/PageUtil.h

+                int truncate_res = 0;
+                // If write failed in V3, Don't do truncate
+                if (truncate_if_failed)
+                {
+                    // If error occurs, apply `ftruncate` try to truncate the broken bytes we have written.
+                    // Note that the result of this ftruncate is ignored, there is nothing we can do to
+                    // handle ftruncate error. The errno may change after ftruncate called.
+                    truncate_res = ::ftruncate(file->getFd(), offset);
+                }


We introduce this ftruncate in #1934 for PageStorage V2 to handle error caused by incomplete write.
But for PageStorage V3, we perform random write in BlobFile, so we should not do ftruncate for V3

This PR keeps the behavior not changed for V2, but disable the truncate after error happen for V3

…cate

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

JaySon-Huang

LGTM

JaySon-Huang · 2022-05-18T15:20:17Z

/merge

ti-chi-bot · 2022-05-18T15:20:19Z

@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-05-18T15:20:20Z

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

Commit hash: 254e351

JaySon-Huang · 2022-05-18T17:26:36Z

/merge

ti-chi-bot · 2022-05-18T17:26:37Z

@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-05-18T17:26:39Z

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

Commit hash: f7c8d8a

sre-bot · 2022-05-18T18:12:28Z

Coverage for changed files

Filename                         Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
PageUtil.h                           100                16    84.00%           8                 0   100.00%         147                16    89.12%          64                20    68.75%
V2/PageFile.cpp                      576               198    65.62%          57                 2    96.49%        1080               258    76.11%         390               152    61.03%
V2/tests/gtest_page_util.cpp          83                23    72.29%           3                 0   100.00%          52                 4    92.31%          22                 9    59.09%
V3/BlobFile.cpp                       29                 4    86.21%           6                 0   100.00%          84                10    88.10%          16                 5    68.75%
V3/BlobStore.cpp                     547               153    72.03%          53                 1    98.11%        1123               235    79.07%         346               123    64.45%
V3/LogFile/LogWriter.cpp              50                 1    98.00%           8                 1    87.50%         116                 8    93.10%          32                 0   100.00%
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                               1385               395    71.48%         135                 4    97.04%        2602               531    79.59%         870               309    64.48%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18201      9784             46.24%    203692  97886        51.94%

full coverage report (for internal network access only)

JaySon-Huang · 2022-05-18T18:21:43Z

/merge

ti-chi-bot · 2022-05-18T18:21:44Z

@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-05-18T18:21:46Z

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

Commit hash: 4ed52a9

sre-bot · 2022-05-18T18:55:38Z

Coverage for changed files

Filename                         Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
PageUtil.h                           100                16    84.00%           8                 0   100.00%         147                16    89.12%          64                20    68.75%
V2/PageFile.cpp                      576               198    65.62%          57                 2    96.49%        1080               258    76.11%         390               152    61.03%
V2/tests/gtest_page_util.cpp          83                23    72.29%           3                 0   100.00%          52                 4    92.31%          22                 9    59.09%
V3/BlobFile.cpp                       29                 4    86.21%           6                 0   100.00%          84                10    88.10%          16                 5    68.75%
V3/BlobStore.cpp                     547               153    72.03%          53                 1    98.11%        1123               235    79.07%         346               123    64.45%
V3/LogFile/LogWriter.cpp              50                 1    98.00%           8                 1    87.50%         116                 8    93.10%          32                 0   100.00%
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                               1385               395    71.48%         135                 4    97.04%        2602               531    79.59%         870               309    64.48%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18201      9783             46.25%    203692  97877        51.95%

full coverage report (for internal network access only)

sre-bot · 2022-05-18T19:09:42Z

Coverage for changed files

Filename                         Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
PageUtil.h                           100                16    84.00%           8                 0   100.00%         147                16    89.12%          64                20    68.75%
V2/PageFile.cpp                      576               198    65.62%          57                 2    96.49%        1080               258    76.11%         390               152    61.03%
V2/tests/gtest_page_util.cpp          83                23    72.29%           3                 0   100.00%          52                 4    92.31%          22                 9    59.09%
V3/BlobFile.cpp                       29                 4    86.21%           6                 0   100.00%          84                10    88.10%          16                 5    68.75%
V3/BlobStore.cpp                     547               153    72.03%          53                 1    98.11%        1123               235    79.07%         346               123    64.45%
V3/LogFile/LogWriter.cpp              50                 1    98.00%           8                 1    87.50%         116                 8    93.10%          32                 0   100.00%
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                               1385               395    71.48%         135                 4    97.04%        2602               531    79.59%         870               309    64.48%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18218      9785             46.29%    203972  97923        51.99%

full coverage report (for internal network access only)

JaySon-Huang · 2022-05-18T20:02:39Z

/run-integration-test

ti-chi-bot · 2022-05-19T00:18:26Z

@jiaqizho: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

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.

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.

sre-bot · 2022-05-19T00:46:59Z

Coverage for changed files

Filename                         Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
PageUtil.h                           100                16    84.00%           8                 0   100.00%         147                16    89.12%          64                20    68.75%
V2/PageFile.cpp                      576               198    65.62%          57                 2    96.49%        1080               258    76.11%         390               152    61.03%
V2/tests/gtest_page_util.cpp          83                23    72.29%           3                 0   100.00%          52                 4    92.31%          22                 9    59.09%
V3/BlobFile.cpp                       29                 4    86.21%           6                 0   100.00%          84                10    88.10%          16                 5    68.75%
V3/BlobStore.cpp                     547               153    72.03%          53                 1    98.11%        1123               235    79.07%         346               123    64.45%
V3/LogFile/LogWriter.cpp              50                 1    98.00%           8                 1    87.50%         116                 8    93.10%          32                 0   100.00%
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                               1385               395    71.48%         135                 4    97.04%        2602               531    79.59%         870               309    64.48%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18218      9785             46.29%    203973  97914        52.00%

full coverage report (for internal network access only)

Fix pageutils will truncate the file if write failed

2b5aac6

ti-chi-bot added the release-note-none Denotes a PR that doesn't merit a release note. label May 18, 2022

jiaqizho requested a review from JaySon-Huang May 18, 2022 13:56

ti-chi-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 18, 2022

update

bfb6735

CalvinNeo approved these changes May 18, 2022

View reviewed changes

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

change to arg

f99eec1

ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 18, 2022

jiaqizho added 2 commits May 18, 2022 22:32

add note

e2f8a7f

add todo

fba6d31

JaySon-Huang reviewed May 18, 2022

View reviewed changes

JaySon-Huang added 2 commits May 18, 2022 23:18

Merge remote-tracking branch 'upstream/master' into fix-pageutil-trun…

596cf64

…cate

Add more log for error

254e351

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

JaySon-Huang approved these changes May 18, 2022

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 May 18, 2022

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

ti-chi-bot and others added 3 commits May 19, 2022 00:18

Merge branch 'master' into fix-pageutil-truncate

7bd0f51

Merge branch 'master' into fix-pageutil-truncate

350f337

Fix ut

f7c8d8a

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label May 18, 2022

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

Fix lint

4ed52a9

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label May 18, 2022

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

Merge branch 'master' into fix-pageutil-truncate

5167571

Merge branch 'master' into fix-pageutil-truncate

19ecafd

ti-chi-bot merged commit aedee1c into pingcap:master May 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix pageutils will truncate the file if write failed #4925

Fix pageutils will truncate the file if write failed #4925

jiaqizho commented May 18, 2022 •

edited by JaySon-Huang

Loading

ti-chi-bot commented May 18, 2022 •

edited

Loading

dbsid commented May 18, 2022

jiaqizho commented May 18, 2022

CalvinNeo left a comment

jiaqizho commented May 18, 2022

JaySon-Huang May 18, 2022

JaySon-Huang May 18, 2022

JaySon-Huang left a comment

JaySon-Huang commented May 18, 2022

ti-chi-bot commented May 18, 2022

ti-chi-bot commented May 18, 2022

JaySon-Huang commented May 18, 2022

ti-chi-bot commented May 18, 2022

ti-chi-bot commented May 18, 2022

sre-bot commented May 18, 2022

JaySon-Huang commented May 18, 2022

ti-chi-bot commented May 18, 2022

ti-chi-bot commented May 18, 2022

sre-bot commented May 18, 2022

sre-bot commented May 18, 2022

JaySon-Huang commented May 18, 2022

ti-chi-bot commented May 19, 2022

sre-bot commented May 19, 2022

Fix pageutils will truncate the file if write failed #4925

Fix pageutils will truncate the file if write failed #4925

Conversation

jiaqizho commented May 18, 2022 • edited by JaySon-Huang Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented May 18, 2022 • edited Loading

dbsid commented May 18, 2022

jiaqizho commented May 18, 2022

CalvinNeo left a comment

Choose a reason for hiding this comment

jiaqizho commented May 18, 2022

JaySon-Huang May 18, 2022

Choose a reason for hiding this comment

JaySon-Huang May 18, 2022

Choose a reason for hiding this comment

JaySon-Huang left a comment

Choose a reason for hiding this comment

JaySon-Huang commented May 18, 2022

ti-chi-bot commented May 18, 2022

ti-chi-bot commented May 18, 2022

JaySon-Huang commented May 18, 2022

ti-chi-bot commented May 18, 2022

ti-chi-bot commented May 18, 2022

sre-bot commented May 18, 2022

Coverage for changed files

Coverage summary

JaySon-Huang commented May 18, 2022

ti-chi-bot commented May 18, 2022

ti-chi-bot commented May 18, 2022

sre-bot commented May 18, 2022

Coverage for changed files

Coverage summary

sre-bot commented May 18, 2022

Coverage for changed files

Coverage summary

JaySon-Huang commented May 18, 2022

ti-chi-bot commented May 19, 2022

sre-bot commented May 19, 2022

Coverage for changed files

Coverage summary

jiaqizho commented May 18, 2022 •

edited by JaySon-Huang

Loading

ti-chi-bot commented May 18, 2022 •

edited

Loading