config: add backend cluster schema and multi-port listener config#1103
config: add backend cluster schema and multi-port listener config#1103ti-chi-bot[bot] merged 2 commits intopingcap:mainfrom
Conversation
|
/retest |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1103 +/- ##
=======================================
Coverage ? 67.08%
=======================================
Files ? 141
Lines ? 14814
Branches ? 0
=======================================
Hits ? 9938
Misses ? 4198
Partials ? 678
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
YangKeao
force-pushed
the
pr/02-multi-cluster-config
branch
from
b56b915 to
a09d6c3
Compare
| type BackendCluster struct { | ||
| Name string `yaml:"name,omitempty" toml:"name,omitempty" json:"name,omitempty" reloadable:"true"` | ||
| PDAddrs string `yaml:"pd-addrs,omitempty" toml:"pd-addrs,omitempty" json:"pd-addrs,omitempty" reloadable:"true"` | ||
| NSServers []string `yaml:"ns-servers,omitempty" toml:"ns-servers,omitempty" json:"ns-servers,omitempty" reloadable:"true"` | ||
| } |
There was a problem hiding this comment.
reloadable means it can be updated online. Why not put it in ProxyServerOnline?
Besides, I don't see how it's hot-reloaded.
There was a problem hiding this comment.
Why not put it in ProxyServerOnline?
Done. I've moved it to ProxyServerOnline. Sorry I didn't get the meaning of ProxyServerOnline previously 😢
Besides, I don't see how it's hot-reloaded.
Also through API and watchConfig channel, you can find it in later PR https://github.com/pingcap/tiproxy/pull/1104/changes#diff-3e09c0d7ae18b71320d7c24a83a3be7bc8437ca68f7aaa9de953ef7fbc0a6810R80 .
| if err != nil || portNum < 1 || portNum > 65535 { | ||
| return "", errors.New("port is invalid") | ||
| } | ||
| return net.JoinHostPort(host, strconv.Itoa(portNum)), nil |
There was a problem hiding this comment.
Why not just return server?
There was a problem hiding this comment.
It's also fine to just return server. Re-joining the host and port can avoid some strange cases and return a "normalized value". For example, the 127.0.0.1:0053 will be converted to 127.0.0.1:53 (though 127.0.0.1:0053 is also OK for most of the function calls).
| return nil, errors.New("proxy.port-range must contain exactly two ports") | ||
| } | ||
| start, end := ps.PortRange[0], ps.PortRange[1] | ||
| if start < 1 || start > 65535 || end < 1 || end > 65535 || start > end { |
There was a problem hiding this comment.
What if a port is assigned by BackendIO before it's assigned as a listening port?
There was a problem hiding this comment.
Listeners are not hot-configurable, so it will listen to all ports at the beginning, and no port will be assigned before it listens the port.
The GetSQLAddrs will return a huge list: [127.0.0.1:6000, 127.0.0.1:6001, 127.0.0.1:6002, .... 127.0.0.1:9000] and all of them will be listened at the beginning of the program.
Introduce backend cluster config, compatibility helpers, validation, and SQL listener support for proxy.port-range.
YangKeao
force-pushed
the
pr/02-multi-cluster-config
branch
from
a09d6c3 to
8f52821
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: djshow832 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
What problem does this PR solve?
Issue Number: close #1097
What is changed and how it works:
Add the configuration surface needed for multi-cluster routing:
[[proxy.backend-clusters]]balance.routing-rule = "port"proxy.port-range = [...]This keeps existing single-cluster configuration working while introducing the new schema for:
Check List
Tests
Notable changes
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.