Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

config, loader: auto-remove imported dump file #770

Merged
merged 28 commits into from
Jul 10, 2020
Merged

config, loader: auto-remove imported dump file #770

merged 28 commits into from
Jul 10, 2020

Conversation

lance6716
Copy link
Collaborator

@lance6716 lance6716 commented Jun 30, 2020 •
edited
Loading

What problem does this PR solve?

fix #698

What is changed and how it works?

A task configuration clean-dump-file is added and propagated to subtask configuraion, to enabled

for data file: after writing checkpoint, check if load is finished. If finised, remove data file

for structure file: remove if whole task successfully finish

for metadata: 1) full_mode: remove if whole task successfully finish 2) all_mode: remove after syncer saves metadata to DB

Check List

Tests

  • modify case all_mode full_mode load_interruptionto check if dump files have been deleted. also, in dm_syncer incremental_mode check turn off this option to make use of metadata file.

Code changes

Side effects

Related changes

  • Need to update the documentation

@ti-srebot ti-srebot added the contribution For contributor label Jun 30, 2020
@codecov
Copy link

codecov bot commented Jun 30, 2020 •
edited
Loading

Codecov Report

Merging #770 into master will decrease coverage by 0.0171%.
The diff coverage is 50.4249%.

@@               Coverage Diff                @@
##             master       #770        +/-   ##
================================================
- Coverage   57.0981%   57.0809%   -0.0172%     
================================================
  Files           205        206         +1     
  Lines         21104      21473       +369     
================================================
+ Hits          12050      12257       +207     
- Misses         7890       8031       +141     
- Partials       1164       1185        +21

@WangXiangUSTC WangXiangUSTC removed the contribution For contributor label Jun 30, 2020
@lance6716 lance6716 changed the title [WIP]config, loader: auto-remove imported dump file [DNM]config, loader: auto-remove imported dump file Jun 30, 2020
@lance6716 lance6716 added the status/WIP This PR is still work in progress label Jul 1, 2020
@lance6716 lance6716 changed the title [DNM]config, loader: auto-remove imported dump file config, loader: auto-remove imported dump file Jul 1, 2020
@lance6716 lance6716 added needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated priority/normal Minor change, requires approval from ≥1 primary reviewer status/PTAL This PR is ready for review. Add this label back after committing new changes type/feature New feature and removed status/WIP This PR is still work in progress labels Jul 1, 2020
@GMHDBJD
Copy link
Collaborator

GMHDBJD commented Jul 1, 2020

You need to add the case name in tests/others_integration.txt when add new integration test directory for CI

@lance6716 lance6716 added status/DNM Do not merge, test is failing or blocked by another PR and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Jul 1, 2020
@lance6716 lance6716 added status/PTAL This PR is ready for review. Add this label back after committing new changes status/DNM Do not merge, test is failing or blocked by another PR and removed status/DNM Do not merge, test is failing or blocked by another PR status/PTAL This PR is ready for review. Add this label back after committing new changes labels Jul 1, 2020
@lance6716
Copy link
Collaborator Author

/run-all-tests

@lance6716 lance6716 added status/PTAL This PR is ready for review. Add this label back after committing new changes and removed status/DNM Do not merge, test is failing or blocked by another PR labels Jul 1, 2020
@lance6716
Copy link
Collaborator Author

/run-all-tests

1 similar comment
@lance6716
Copy link
Collaborator Author

/run-all-tests

@lance6716
Copy link
Collaborator Author

test of this feature working as expected is added in 0c4a60f, in load_interrupt check files does generated and removed @WangXiangUSTC

WangXiangUSTC
WangXiangUSTC reviewed Jul 9, 2020
View reviewed changes
Copy link
Contributor

@WangXiangUSTC WangXiangUSTC left a comment

Choose a reason for hiding this comment

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

LGTM

@WangXiangUSTC WangXiangUSTC added status/LGT1 One reviewer already commented LGTM and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Jul 9, 2020
GMHDBJD
GMHDBJD approved these changes Jul 10, 2020
View reviewed changes
Copy link
Collaborator

@GMHDBJD GMHDBJD left a comment

Choose a reason for hiding this comment

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

LGTM

@GMHDBJD GMHDBJD added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Jul 10, 2020
@lance6716 lance6716 merged commit d23a0f8 into pingcap:master Jul 10, 2020
@lance6716 lance6716 deleted the fix698 branch July 10, 2020 01:26
@csuzhangxc csuzhangxc added this to the v2.0.0 RC milestone Jul 15, 2020
@lance6716 lance6716 added the needs-cherry-pick-release-1.0 This PR should be cherry-picked to release-1.0. Remove this label after cherry-picked to release-1.0 label Jul 15, 2020
@lance6716
Copy link
Collaborator Author

/run-cherry-picker

@ti-srebot
Copy link

cherry pick to release-1.0 failed

1 similar comment
@ti-srebot
Copy link

cherry pick to release-1.0 failed

lance6716 added a commit to lance6716/dm that referenced this pull request Jul 15, 2020
@lance6716
config, loader: auto-remove imported dump file (pingcap#770)
ebcc218
@lance6716 lance6716 mentioned this pull request Jul 15, 2020
Merged
lance6716 added a commit that referenced this pull request Jul 16, 2020
@lance6716
config, loader: auto-remove imported dump file (#770) (#804)
63e3388
@lance6716 lance6716 added already-cherry-pick-1.0 The related PR is already cherry-picked to release-1.0. Add this label once the PR is cherry-picked and removed needs-cherry-pick-release-1.0 This PR should be cherry-picked to release-1.0. Remove this label after cherry-picked to release-1.0 labels Jul 17, 2020
@lance6716 lance6716 mentioned this pull request Jul 21, 2020
Merged
2 tasks
@lance6716 lance6716 added already-update-docs The docs related to this PR already updated. Add this label once the docs are updated needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated and removed needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated labels Jul 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@WangXiangUSTC WangXiangUSTC WangXiangUSTC left review comments

@csuzhangxc csuzhangxc csuzhangxc left review comments

@GMHDBJD GMHDBJD GMHDBJD approved these changes

Assignees
No one assigned
Labels
already-cherry-pick-1.0 The related PR is already cherry-picked to release-1.0. Add this label once the PR is cherry-picked already-update-docs The docs related to this PR already updated. Add this label once the docs are updated priority/normal Minor change, requires approval from ≥1 primary reviewer status/LGT2 Two reviewers already commented LGTM, ready for merge type/feature New feature
Projects
None yet
Milestone
v2.0.0 RC
Development

Successfully merging this pull request may close these issues.

remove dumped data after load finished
5 participants
@lance6716 @GMHDBJD @ti-srebot @csuzhangxc @WangXiangUSTC