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

Support migration data from source with different time_zone by default #1670

Merged
merged 20 commits into from
May 13, 2021

Conversation

glorv
Copy link
Collaborator

@glorv glorv commented May 11, 2021

What problem does this PR solve?

Automatically adapt source and target DB time_zone settings so users don't need to explicitly set timezone in dm config.

close #1691

What is changed and how it works?

  • Deprecated the timezone setting in dm config. (maybe we should directly remove this config field)
  • Always set timezone='+00:00' for all db connections in dumpling/loader/syncer.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Code changes

Side effects

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to be included in the release note

@glorv
Copy link
Collaborator Author

glorv commented May 11, 2021

/run-all-tests

@glorv
Copy link
Collaborator Author

glorv commented May 11, 2021

/run-unit-test

@lance6716
Copy link
Collaborator

maybe we should directly remove this config field

we can't remove it because I remember there's a checking about undecoded config items, to fail the user of mistyping. (And I think this also disables smoothly downgrade from a version with more config items)

@ti-chi-bot ti-chi-bot added size/XL and removed size/L labels May 11, 2021
@lance6716
Copy link
Collaborator

we should log a warning if overwrite user's timezone in session variables. rest lgtm

@glorv
Copy link
Collaborator Author

glorv commented May 13, 2021

we should log a warning if overwrite user's timezone in session variables. rest lgtm

done

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • lance6716
  • lichunzhu

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 writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

@ti-chi-bot ti-chi-bot added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels May 13, 2021
@glorv
Copy link
Collaborator Author

glorv commented May 13, 2021

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: a046f1e

@ti-chi-bot ti-chi-bot merged commit 0145c37 into pingcap:master May 13, 2021
@glorv glorv deleted the time_zone branch May 13, 2021 12:43
@lance6716 lance6716 added needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated labels May 21, 2021
@lance6716 lance6716 added this to the v2.0.4 milestone May 21, 2021
@lance6716
Copy link
Collaborator

/cherry-pick release-20.

@lance6716
Copy link
Collaborator

/cherry-pick release-2.0

@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #1714.

@ti-chi-bot
Copy link
Member

@lance6716: cannot checkout release-20.: error checking out release-20.: exit status 1. output: error: pathspec 'release-20.' did not match any file(s) known to git

In response to this:

/cherry-pick release-20.

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
Copy link
Member

In response to a cherrypick label: new pull request could not be created: failed to create pull request against pingcap/dm#release-2.0 from head ti-chi-bot:cherry-pick-1670-to-release-2.0: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for ti-chi-bot:cherry-pick-1670-to-release-2.0."}],"documentation_url":"https://docs.github.com/rest/reference/pulls#create-a-pull-request"}

@ti-chi-bot
Copy link
Member

@lance6716: new pull request could not be created: failed to create pull request against pingcap/dm#release-2.0 from head ti-chi-bot:cherry-pick-1670-to-release-2.0: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for ti-chi-bot:cherry-pick-1670-to-release-2.0."}],"documentation_url":"https://docs.github.com/rest/reference/pulls#create-a-pull-request"}

In response to this:

/cherry-pick release-2.0

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.

@GMHDBJD GMHDBJD added the needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated label Jun 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
first-time-contributor needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 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 size/XXL status/can-merge status/LGT2 Two reviewers already commented LGTM, ready for merge status/PTAL This PR is ready for review. Add this label back after committing new changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data that related to a timezone may be synchronized wrongly
5 participants