pingcap / pingcap/tidb-operator

PD url in discovery service need to be fix

Open
#4,933 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
1.3k
Forks
540
Avg merge
3d 2h
Merged PRs (30d)
18

Description

Bug Report

What version of Kubernetes are you using?

Client Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.17", GitCommit:"a7736eaf34d823d7652415337ac0ad06db9167fc", GitTreeState:"clean", BuildDate:"2022-12-08T11:41:04Z", GoVersion:"go1.16.15", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"23+", GitVersion:"v1.23.16-airbnb0", GitCommit:"3051216402e96974819f485e35143cbe2fdf43a8", GitTreeState:"clean", BuildDate:"2023-02-07T20:29:26Z", GoVersion:"go1.19.5", Compiler:"gc", Platform:"linux/amd64"}

What version of TiDB Operator are you using?

TiDB Operator Version: version.Info{GitVersion:"v0.0.0-master+$Format:%h$", GitCommit:"d9f634343ef71926b43c85df17458fb55dc955b8", GitTreeState:"clean", BuildDate:"2023-02-15T05:18:23Z", GoVersion:"go1.18.10", Compiler:"gc", Platform:"linux/amd64"}

What storage classes exist in the Kubernetes cluster and what are used for PD/TiKV pods?

N/A
What's the status of the TiDB cluster pods?

N/A
What did you do?

https://github.com/pingcap/tidb-operator/blob/master/pkg/discovery/discovery.go#L271 need to be fixed and include not only PeerMembers but Members as well.

And for https://github.com/pingcap/tidb-operator/blob/master/pkg/discovery/discovery.go#L260, it needs to actually connect to the pd and fetch the member instead of just construct a url from spec

What did you expect to see?

What did you see instead?
In the discovery.go when it's generating the list of PD urls it only includes the PeerMembers but not local member. We are deploying tidb across 3 k8s clusters. Does it make sense to include the local PD in the VerifyPDEndpoint response?

var returnPDMember string
returnPDMembers := []string{pdURL}
for _, peerPDMember := range tc.Status.PD.PeerMembers {
if peerPDMember.Health {
if len(pdEndpoint.scheme) == 0 {
peerPDEndpoint := parsePDURL(peerPDMember.ClientURL)
returnPDMember = fmt.Sprintf("%s:%s", peerPDEndpoint.pdMemberName, peerPDEndpoint.pdMemberPort)
} else {
returnPDMember = peerPDMember.ClientURL
}
returnPDMembers = append(returnPDMembers, returnPDMember)
}
}

Also we have Heterogeneous tidb and tikv cluster, currently discovery is generating to url from spec, this gave us wrong pd url in our use case. Does it make sense to actually connect to PD and fetch the PD member?

// if local pd doesn't exist, return target cluster pd peer addr
if tc.Heterogeneous() && tc.WithoutLocalPD() {
addr := controller.PDPeerFullyDomain(tc.Spec.Cluster.Name, tc.Spec.Cluster.Namespace, tc.Spec.Cluster.ClusterDomain)
if pdEndpoint.scheme != "" {
addr = fmt.Sprintf("%s://%s", pdEndpoint.scheme, addr)
}
addr = addr + ":" + pdEndpoint.pdMemberPort
return addr, nil
}

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read pkg/discovery/discovery.go around lines 260 and 271, including VerifyPDEndpoint and the PD URL construction paths. Trace how PD members are obtained for heterogeneous clusters, then verify that the response includes the local Members entry and uses the connected PD member rather than only the spec-derived URL.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, kubernetes
Domain
databases, distributed-systems, infrastructure
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.