session: Do not run telemetry loops when it's disabled in config (#40156)#41150
session: Do not run telemetry loops when it's disabled in config (#40156)#41150ti-chi-bot merged 3 commits intopingcap:release-6.1from sunxiaoguang:cherry-pick-40156-to-release-6.1
Conversation
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
Signed-off-by: Xiaoguang Sun <sunxiaoguang@gmail.com>
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. DetailsReviewer can indicate their review by submitting an approval review. |
|
Same as #41147 but with merge conflicts fixed. |
session/session.go
Outdated
| if config.GetGlobalConfig().EnableTelemetry { | ||
| // There is no way to turn telemetry on with global variable `tidb_enable_telemetry` | ||
| // when it is disabled in config. See IsTelemetryEnabled function in telemetry/telemetry.go | ||
| go func() { |
There was a problem hiding this comment.
Is adding go func() here like this not changing the behaviour? Is this ok for a bugfix release?
There was a problem hiding this comment.
Hm, it shouldn't affect any feature but just make initialization synchronous and speed up startup when connecting to telemetry server is blocked by network policy.
There was a problem hiding this comment.
The reason to add go here was I realized that starting tidb-server takes about 20 seconds when the hosting environment doesn't have internet access.
There was a problem hiding this comment.
But I think you are right. We should keep changes as small as possible for back port. I have reverted this according to the behavior in 6.1. PTAL
|
/retest |
|
/merge |
|
This pull request has been accepted and is ready to merge. DetailsCommit hash: 82b6dcf |
|
/retest |
What problem does this PR solve?
Issue Number: close #40155
Problem Summary:
This is an automated cherry-pick of #40156
Telemetry can not be enabled by global variable when it's disabled in config file. Therefore we don't need to run telemetry related loops at all when telemetry is disabled from global config.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.