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

restore: set configed keepalive for restore #567

Merged
merged 3 commits into from
Nov 2, 2020

Conversation

YuJuncen
Copy link
Collaborator

What problem does this PR solve?

#545 added the config value of keepalive, but it has forgotten the restore part(!).

What is changed and how it works?

Apply the config to restoring.

Check List

Tests

  • Unit test
  • Integration test
    (If the old tests passed, then the default config should be applied.)

Release Note

  • Fix a bug that causes the keepalive configs don't apply to restore.

@@ -102,7 +102,9 @@ func RunRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf
}
defer mgr.Close()

client, err := restore.NewRestoreClient(g, mgr.GetPDClient(), mgr.GetTiKV(), mgr.GetTLSConfig())
keepaliveCfg := GetKeepalive(&cfg.Config)
keepaliveCfg.PermitWithoutStream = true
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 put this line in GetKeepalive function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This function is also used in the backup site, which doesn't needs PermitWithoutStream.

@3pointer
Copy link
Collaborator

/run-integration-tests

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

@kennytm kennytm added the status/LGT1 LGTM1 label Nov 2, 2020
Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

LGTM

@kennytm kennytm merged commit 96ee95b into pingcap:master Nov 2, 2020
3pointer pushed a commit to 3pointer/br that referenced this pull request Nov 6, 2020
* restore: set configed keepalive for restore

* restore: remove SetKeepalive
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #591

kennytm pushed a commit that referenced this pull request Nov 10, 2020
* restore: set configed keepalive for restore

* restore: remove SetKeepalive

Co-authored-by: hillium <yujuncen@pingcap.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants