Skip to content

Remove useless StorageDebugging & move mock applying snapshot functions together#1666

Merged
ti-srebot merged 11 commits intopingcap:masterfrom
JaySon-Huang:refactor_mock_region_snapshot
Apr 13, 2021
Merged

Remove useless StorageDebugging & move mock applying snapshot functions together#1666
ti-srebot merged 11 commits intopingcap:masterfrom
JaySon-Huang:refactor_mock_region_snapshot

Conversation

@JaySon-Huang
Copy link
Copy Markdown
Contributor

@JaySon-Huang JaySon-Huang commented Mar 26, 2021

What problem does this PR solve?

This is a PR extracted from #1439.
Extract this PR for easier for reviewing and cherry-pick to release-4.0/release-5.0 branch.

What is changed and how it works?

There are 4 independent changes in this PR, you may check every single commit to better know why that piece of code is changed.
Now merging a PR is expensive (two hours at least, an hour to build and another hour to run the test), and these changes do not involve the logic in the production environment, so I merge these changes into one PR.

  1. Before this PR, we use StorageDebugging for testing delta-merge-test/raft/bugs/FLASH-484.test. But now we have FailPoints and we can elegantly test the case.

  2. Before this PR, debug functions of applying snapshot / pre-applying snapshot are written in different files, making it hard to reuse codes. This PR moves them to dbgFuncMockRaftSnapshot.cpp and aligns the style of naming.

  3. Add test cases for writing raft logs / applying snapshot when the storage is tombstoned.

  4. Fix TiFlash panic during br restore #1691 for a more complicated situation

    1. Apply a snapshot with write cf only
    2. Apply a normal snapshot
      When we applying the normal snapshot, we will try to flush current data into storage and get an exception because there is only write cf. We should also ignore that exception.

Related changes

  • Need to cherry-pick to the release branch: 4.0, 5.0

Check List

Tests

  • Integration test

Release note

  • No release note

@JaySon-Huang
Copy link
Copy Markdown
Contributor Author

/run-all-tests

@JaySon-Huang JaySon-Huang force-pushed the refactor_mock_region_snapshot branch 2 times, most recently from 11d8398 to 3f98bd4 Compare March 29, 2021 05:12
@JaySon-Huang JaySon-Huang self-assigned this Mar 29, 2021
@JaySon-Huang JaySon-Huang added type/testing Issue or PR for testing needs-cherry-pick-release-4.0 PR which needs to be cherry-picked to release-4.0 needs-cherry-pick-release-5.0 PR which needs to be cherry-picked to release-5.0 labels Mar 29, 2021
@JaySon-Huang
Copy link
Copy Markdown
Contributor Author

/run-all-tests

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
…orage is tombstone

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang JaySon-Huang force-pushed the refactor_mock_region_snapshot branch from 3f98bd4 to 33837ff Compare April 1, 2021 10:23
@JaySon-Huang
Copy link
Copy Markdown
Contributor Author

JaySon-Huang commented Apr 1, 2021

Push a commit to fix #1691 for a more complicated situation:

  1. Apply a snapshot with write cf only
  2. Apply a normal snapshot

When we applying the normal snapshot, we will try to flush current data into storage and get an exception because there is only write cf. We should also ignore that exception.

Copy link
Copy Markdown
Contributor

@solotzg solotzg 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-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 1, 2021
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Copy link
Copy Markdown
Contributor Author

/run-all-tests

@JaySon-Huang
Copy link
Copy Markdown
Contributor Author

@zanmato1984 PTAL about the "test cases for writing raft logs / applying snapshot when the storage is tombstoned" part

@JaySon-Huang
Copy link
Copy Markdown
Contributor Author

/run-all-tests

Copy link
Copy Markdown
Contributor

@zanmato1984 zanmato1984 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-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Apr 13, 2021
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Apr 13, 2021
@JaySon-Huang
Copy link
Copy Markdown
Contributor Author

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 13, 2021
@ti-srebot
Copy link
Copy Markdown
Collaborator

/run-all-tests

1 similar comment
@JaySon-Huang
Copy link
Copy Markdown
Contributor Author

/run-all-tests

@JaySon-Huang JaySon-Huang removed the status/can-merge Indicates a PR has been approved by a committer. label Apr 13, 2021
@JaySon-Huang
Copy link
Copy Markdown
Contributor Author

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 13, 2021
@ti-srebot
Copy link
Copy Markdown
Collaborator

/run-all-tests

@ti-srebot
Copy link
Copy Markdown
Collaborator

cherry pick to release-4.0 in PR #1769

@ti-srebot
Copy link
Copy Markdown
Collaborator

cherry pick to release-5.0 in PR #1770

@JaySon-Huang JaySon-Huang deleted the refactor_mock_region_snapshot branch April 13, 2021 12:05
ti-srebot added a commit that referenced this pull request Apr 20, 2021
ti-srebot added a commit that referenced this pull request Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-cherry-pick-release-4.0 PR which needs to be cherry-picked to release-4.0 needs-cherry-pick-release-5.0 PR which needs to be cherry-picked to release-5.0 status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/testing Issue or PR for testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TiFlash panic during br restore

4 participants