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

backup: dump stats to json should not be fatal (#682) #686

Merged
merged 3 commits into from
Jan 12, 2021

Conversation

ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Jan 11, 2021

cherry-pick #682 to release-4.0
You can switch your code base to this Pull Request by using git-extras:

# In br repo:
git pr 686

After apply modifications, you can push your change to this PR via:

git push [email protected]:ti-srebot/br.git pr/686:ti-srebot:release-4.0-5abd5baa56e6

also improved some logs

What problem does this PR solve?

Part 1 of #679.

What is changed and how it works?

If the stats is broken, we just log and skip it instead of bailing out.

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

  • Need to cherry-pick to the release branch

Release Note

  • Failing to backup statistics is no longer fatal.

Copy link
Collaborator

@3pointer 3pointer 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 LGTM1 label Jan 11, 2021
@overvenus
Copy link
Member

/run-all-tests

2 similar comments
@lichunzhu
Copy link
Contributor

/run-all-tests

@overvenus
Copy link
Member

/run-all-tests

@kennytm
Copy link
Collaborator

kennytm commented Jan 11, 2021

/run-all-tests

🤔 https://internal.pingcap.net/idc-jenkins/blue/organizations/jenkins/br_ghpr_unit_and_integration_test/detail/br_ghpr_unit_and_integration_test/4558/pipeline

[2021-01-11T07:40:47.288Z] + rm -rf /tmp/backup_restore_test
[2021-01-11T07:40:47.288Z] rm: cannot remove '/tmp/backup_restore_test': Directory not empty

@lichunzhu
Copy link
Contributor

/run-all-tests

@overvenus
Copy link
Member

/run-all-tests

@overvenus
Copy link
Member

LGTM

@ti-srebot ti-srebot added status/LGT2 LGTM2 and removed status/LGT1 LGTM1 labels Jan 11, 2021
@overvenus
Copy link
Member

/merge

@ti-srebot
Copy link
Contributor Author

/run-all-tests

1 similar comment
@3pointer
Copy link
Collaborator

/run-all-tests

@kennytm
Copy link
Collaborator

kennytm commented Jan 11, 2021

(Hound is broken)

@kennytm kennytm closed this Jan 11, 2021
@kennytm kennytm reopened this Jan 11, 2021
@3pointer
Copy link
Collaborator

/run-all-tests

1 similar comment
@3pointer
Copy link
Collaborator

/run-all-tests

@ti-srebot ti-srebot added status/LGT3 LGTM3. This PR looks very good to our bot. and removed status/LGT2 LGTM2 labels Jan 12, 2021
@kennytm kennytm merged commit c411e3e into pingcap:release-4.0 Jan 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component/backup status/LGT3 LGTM3. This PR looks very good to our bot. type/bug-fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants