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

config: disable heartbeat feature #1467

Merged
merged 6 commits into from
Mar 8, 2021

Conversation

lance6716
Copy link
Collaborator

What problem does this PR solve?

close #1423

What is changed and how it works?

overwrite it to false

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

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

@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 Mar 1, 2021
@lance6716 lance6716 added this to the v2.0.3 milestone Mar 1, 2021
@@ -626,7 +626,9 @@ func (c *TaskConfig) SubTaskConfigs(sources map[string]DBConfig) ([]*SubTaskConf
cfg.Mode = c.TaskMode
cfg.CaseSensitive = c.CaseSensitive
cfg.MetaSchema = c.MetaSchema
cfg.EnableHeartbeat = c.EnableHeartbeat
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about set it false explicitly.

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.

rest LGTM

@GMHDBJD
Copy link
Collaborator

GMHDBJD commented Mar 1, 2021

Will there be the same problem when upgrading from v1 to v2 via tiup?

@lance6716 lance6716 added the status/WIP This PR is still work in progress label Mar 1, 2021
@lance6716 lance6716 removed the status/WIP This PR is still work in progress label Mar 3, 2021
@lance6716 lance6716 modified the milestones: v2.0.3, v2.0.2 Mar 3, 2021
@lance6716
Copy link
Collaborator Author

Will there be the same problem when upgrading from v1 to v2 via tiup?

I think the import_v10x test could ensure it

@GMHDBJD
Copy link
Collaborator

GMHDBJD commented Mar 3, 2021

I think the import_v10x test could ensure it

Seems the enable-heartbeat config from get-config still true?

@lance6716
Copy link
Collaborator Author

I think the import_v10x test could ensure it

Seems the enable-heartbeat config from get-config still true?

updated in 453f2b3

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 the status/LGT1 One reviewer already commented LGTM label Mar 3, 2021
@lichunzhu
Copy link
Contributor

/lgtm

Copy link
Contributor

@lichunzhu lichunzhu left a comment

Choose a reason for hiding this comment

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

LGTM

@lichunzhu lichunzhu added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Mar 8, 2021
@lance6716 lance6716 merged commit 139a71b into pingcap:master Mar 8, 2021
ti-srebot pushed a commit to ti-srebot/dm that referenced this pull request Mar 8, 2021
@ti-srebot
Copy link

cherry pick to release-2.0 in PR #1486

@ti-srebot ti-srebot added the already-cherry-pick-2.0 The related PR is already cherry-picked to release-2.0. Add this label once the PR is cherry-picked label Mar 8, 2021
@ti-srebot ti-srebot removed the 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 label Mar 8, 2021
lance6716 pushed a commit that referenced this pull request Mar 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
already-cherry-pick-2.0 The related PR is already cherry-picked to release-2.0. Add this label once the PR is cherry-picked needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated status/LGT2 Two reviewers already commented LGTM, ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

disable heartbeat feature becauase it may block HA
4 participants