Skip to content

Commit 26c66bc

Browse files
djshow832djshow832
authored
Make /api/debug/health return error when no backends are available (#692)
1 parent 4243fb5 commit 26c66bc

File tree

14 files changed

+186
-21
lines changed

14 files changed

+186
-21
lines changed

‎pkg/balance/router/router.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ type Router interface {
2929
ConnEventReceiver
3030

3131
GetBackendSelector() BackendSelector
32+
HealthyBackendCount() int
3233
RefreshBackend()
3334
RedirectConnections() error
3435
ConnCount() int

‎pkg/balance/router/router_score.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,22 @@ func (router *ScoreBasedRouter) GetBackendSelector() BackendSelector {
7575
}
7676
}
7777

78+
func (router *ScoreBasedRouter) HealthyBackendCount() int {
79+
router.Lock()
80+
defer router.Unlock()
81+
if router.observeError != nil {
82+
return 0
83+
}
84+
85+
count := 0
86+
for _, backend := range router.backends {
87+
if backend.Healthy() {
88+
count++
89+
}
90+
}
91+
return count
92+
}
93+
7894
func (router *ScoreBasedRouter) getConnWrapper(conn RedirectableConn) *glist.Element[*connWrapper] {
7995
return conn.Value(_routerKey).(*glist.Element[*connWrapper])
8096
}

‎pkg/balance/router/router_score_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,11 +322,14 @@ func TestNoBackends(t *testing.T) {
322322
conn := tester.createConn()
323323
backend := tester.simpleRoute(conn)
324324
require.True(t, backend == nil || reflect.ValueOf(backend).IsNil())
325+
require.Equal(t, 0, tester.router.HealthyBackendCount())
325326
tester.addBackends(1)
327+
require.Equal(t, 1, tester.router.HealthyBackendCount())
326328
tester.addConnections(10)
327329
tester.killBackends(1)
328330
backend = tester.simpleRoute(conn)
329331
require.True(t, backend == nil || reflect.ValueOf(backend).IsNil())
332+
require.Equal(t, 0, tester.router.HealthyBackendCount())
330333
}
331334

332335
// Test that the backends returned by the BackendSelector are complete and different.

‎pkg/balance/router/router_static.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ func (r *StaticRouter) GetBackendSelector() BackendSelector {
4545
}
4646
}
4747

48+
func (r *StaticRouter) HealthyBackendCount() int {
49+
return len(r.backends)
50+
}
51+
4852
func (r *StaticRouter) RefreshBackend() {}
4953

5054
func (r *StaticRouter) RedirectConnections() error {

‎pkg/manager/namespace/manager.go‎

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,19 @@ import (
2222
"go.uber.org/zap"
2323
)
2424

25-
type NamespaceManager struct {
25+
type NamespaceManager interface {
26+
Init(logger *zap.Logger, nscs []*config.Namespace, tpFetcher observer.TopologyFetcher,
27+
promFetcher metricsreader.PromInfoFetcher, httpCli *http.Client, cfgMgr *mconfig.ConfigManager,
28+
metricsReader metricsreader.MetricsReader) error
29+
CommitNamespaces(nss []*config.Namespace, nssDelete []bool) error
30+
GetNamespace(nm string) (*Namespace, bool)
31+
GetNamespaceByUser(user string) (*Namespace, bool)
32+
RedirectConnections() []error
33+
Ready() bool
34+
Close() error
35+
}
36+
37+
type namespaceManager struct {
2638
sync.RWMutex
2739
nsm map[string]*Namespace
2840
tpFetcher observer.TopologyFetcher
@@ -33,17 +45,17 @@ type NamespaceManager struct {
3345
cfgMgr *mconfig.ConfigManager
3446
}
3547

36-
func NewNamespaceManager() *NamespaceManager {
37-
return &NamespaceManager{}
48+
func NewNamespaceManager() *namespaceManager {
49+
return &namespaceManager{}
3850
}
3951

40-
func (mgr *NamespaceManager) buildNamespace(cfg *config.Namespace) (*Namespace, error) {
52+
func (mgr *namespaceManager) buildNamespace(cfg *config.Namespace) (*Namespace, error) {
4153
logger := mgr.logger.With(zap.String("namespace", cfg.Namespace))
4254

4355
// init BackendFetcher
4456
var fetcher observer.BackendFetcher
4557
healthCheckCfg := config.NewDefaultHealthCheckConfig()
46-
if !reflect.ValueOf(mgr.tpFetcher).IsNil() {
58+
if mgr.tpFetcher != nil && !reflect.ValueOf(mgr.tpFetcher).IsNil() {
4759
fetcher = observer.NewPDFetcher(mgr.tpFetcher, logger.Named("be_fetcher"), healthCheckCfg)
4860
} else {
4961
fetcher = observer.NewStaticFetcher(cfg.Backend.Instances)
@@ -65,7 +77,7 @@ func (mgr *NamespaceManager) buildNamespace(cfg *config.Namespace) (*Namespace,
6577
}, nil
6678
}
6779

68-
func (mgr *NamespaceManager) CommitNamespaces(nss []*config.Namespace, nss_delete []bool) error {
80+
func (mgr *namespaceManager) CommitNamespaces(nss []*config.Namespace, nssDelete []bool) error {
6981
nsm := make(map[string]*Namespace)
7082
mgr.RLock()
7183
for k, v := range mgr.nsm {
@@ -74,7 +86,7 @@ func (mgr *NamespaceManager) CommitNamespaces(nss []*config.Namespace, nss_delet
7486
mgr.RUnlock()
7587

7688
for i, nsc := range nss {
77-
if nss_delete != nil && nss_delete[i] {
89+
if nssDelete != nil && nssDelete[i] {
7890
delete(nsm, nsc.Namespace)
7991
continue
8092
}
@@ -92,7 +104,7 @@ func (mgr *NamespaceManager) CommitNamespaces(nss []*config.Namespace, nss_delet
92104
return nil
93105
}
94106

95-
func (mgr *NamespaceManager) Init(logger *zap.Logger, nscs []*config.Namespace, tpFetcher observer.TopologyFetcher,
107+
func (mgr *namespaceManager) Init(logger *zap.Logger, nscs []*config.Namespace, tpFetcher observer.TopologyFetcher,
96108
promFetcher metricsreader.PromInfoFetcher, httpCli *http.Client, cfgMgr *mconfig.ConfigManager,
97109
metricsReader metricsreader.MetricsReader) error {
98110
mgr.Lock()
@@ -106,15 +118,15 @@ func (mgr *NamespaceManager) Init(logger *zap.Logger, nscs []*config.Namespace,
106118
return mgr.CommitNamespaces(nscs, nil)
107119
}
108120

109-
func (mgr *NamespaceManager) GetNamespace(nm string) (*Namespace, bool) {
121+
func (mgr *namespaceManager) GetNamespace(nm string) (*Namespace, bool) {
110122
mgr.RLock()
111123
defer mgr.RUnlock()
112124

113125
ns, ok := mgr.nsm[nm]
114126
return ns, ok
115127
}
116128

117-
func (mgr *NamespaceManager) GetNamespaceByUser(user string) (*Namespace, bool) {
129+
func (mgr *namespaceManager) GetNamespaceByUser(user string) (*Namespace, bool) {
118130
mgr.RLock()
119131
defer mgr.RUnlock()
120132

@@ -126,7 +138,7 @@ func (mgr *NamespaceManager) GetNamespaceByUser(user string) (*Namespace, bool)
126138
return nil, false
127139
}
128140

129-
func (mgr *NamespaceManager) RedirectConnections() []error {
141+
func (mgr *namespaceManager) RedirectConnections() []error {
130142
mgr.RLock()
131143
defer mgr.RUnlock()
132144

@@ -140,7 +152,21 @@ func (mgr *NamespaceManager) RedirectConnections() []error {
140152
return errs
141153
}
142154

143-
func (mgr *NamespaceManager) Close() error {
155+
func (mgr *namespaceManager) Ready() bool {
156+
mgr.RLock()
157+
defer mgr.RUnlock()
158+
if len(mgr.nsm) == 0 {
159+
return false
160+
}
161+
for _, ns := range mgr.nsm {
162+
if ns.GetRouter().HealthyBackendCount() <= 0 {
163+
return false
164+
}
165+
}
166+
return true
167+
}
168+
169+
func (mgr *namespaceManager) Close() error {
144170
mgr.RLock()
145171
for _, ns := range mgr.nsm {
146172
ns.Close()

‎pkg/manager/namespace/manager_test.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright 2024 PingCAP, Inc.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package namespace
5+
6+
import (
7+
"testing"
8+
9+
"github.com/pingcap/tiproxy/pkg/balance/router"
10+
"github.com/stretchr/testify/require"
11+
"go.uber.org/zap"
12+
)
13+
14+
func TestReady(t *testing.T) {
15+
nsMgr := NewNamespaceManager()
16+
require.NoError(t, nsMgr.Init(zap.NewNop(), nil, nil, nil, nil, nil, nil))
17+
require.False(t, nsMgr.Ready())
18+
19+
rt := router.NewStaticRouter([]string{})
20+
nsMgr.nsm = map[string]*Namespace{
21+
"test": {
22+
router: rt,
23+
},
24+
}
25+
require.False(t, nsMgr.Ready())
26+
27+
rt = router.NewStaticRouter([]string{"127.0.0.1:4000"})
28+
ns, ok := nsMgr.GetNamespace("test")
29+
require.True(t, ok)
30+
ns.router = rt
31+
require.True(t, nsMgr.Ready())
32+
}

‎pkg/proxy/backend/handshake_handler.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,10 @@ type HandshakeHandler interface {
5252
}
5353

5454
type DefaultHandshakeHandler struct {
55-
nsManager *namespace.NamespaceManager
55+
nsManager namespace.NamespaceManager
5656
}
5757

58-
func NewDefaultHandshakeHandler(nsManager *namespace.NamespaceManager) *DefaultHandshakeHandler {
58+
func NewDefaultHandshakeHandler(nsManager namespace.NamespaceManager) *DefaultHandshakeHandler {
5959
return &DefaultHandshakeHandler{
6060
nsManager: nsManager,
6161
}

‎pkg/server/api/debug.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313

1414
func (h *Server) DebugHealth(c *gin.Context) {
1515
status := http.StatusOK
16-
if h.isClosing.Load() {
16+
if h.isClosing.Load() || !h.mgr.NsMgr.Ready() {
1717
status = http.StatusBadGateway
1818
}
1919
c.JSON(status, config.HealthInfo{

‎pkg/server/api/debug_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ func TestDebug(t *testing.T) {
2424
require.Equal(t, http.StatusOK, r.StatusCode)
2525
})
2626

27+
server.mgr.NsMgr.(*mockNamespaceManager).success.Store(false)
28+
doHTTP(t, http.MethodGet, "/api/debug/health", httpOpts{}, func(t *testing.T, r *http.Response) {
29+
require.Equal(t, http.StatusBadGateway, r.StatusCode)
30+
})
31+
32+
server.mgr.NsMgr.(*mockNamespaceManager).success.Store(true)
2733
doHTTP(t, http.MethodGet, "/api/debug/health", httpOpts{}, func(t *testing.T, r *http.Response) {
2834
require.Equal(t, http.StatusOK, r.StatusCode)
2935
})

‎pkg/server/api/mock_test.go‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
// Copyright 2024 PingCAP, Inc.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package api
5+
6+
import (
7+
"context"
8+
"errors"
9+
"sync/atomic"
10+
11+
"github.com/pingcap/tiproxy/lib/config"
12+
"github.com/pingcap/tiproxy/pkg/balance/metricsreader"
13+
"github.com/pingcap/tiproxy/pkg/balance/observer"
14+
mconfig "github.com/pingcap/tiproxy/pkg/manager/config"
15+
"github.com/pingcap/tiproxy/pkg/manager/namespace"
16+
"github.com/pingcap/tiproxy/pkg/util/http"
17+
"go.uber.org/zap"
18+
)
19+
20+
var _ namespace.NamespaceManager = (*mockNamespaceManager)(nil)
21+
22+
type mockNamespaceManager struct {
23+
success atomic.Bool
24+
}
25+
26+
func newMockNamespaceManager() *mockNamespaceManager {
27+
mgr := &mockNamespaceManager{}
28+
mgr.success.Store(true)
29+
return mgr
30+
}
31+
32+
func (m *mockNamespaceManager) Init(_ *zap.Logger, _ []*config.Namespace, _ observer.TopologyFetcher,
33+
_ metricsreader.PromInfoFetcher, _ *http.Client, _ *mconfig.ConfigManager, _ metricsreader.MetricsReader) error {
34+
return nil
35+
}
36+
37+
func (m *mockNamespaceManager) GetNamespace(_ string) (*namespace.Namespace, bool) {
38+
return nil, false
39+
}
40+
41+
func (m *mockNamespaceManager) GetNamespaceByUser(_ string) (*namespace.Namespace, bool) {
42+
return nil, false
43+
}
44+
45+
func (m *mockNamespaceManager) SetNamespace(_ context.Context, _ string, _ *config.Namespace) error {
46+
if m.success.Load() {
47+
return nil
48+
}
49+
return errors.New("mock error")
50+
}
51+
52+
func (m *mockNamespaceManager) GetConfigChecksum() string {
53+
return ""
54+
}
55+
56+
func (m *mockNamespaceManager) Ready() bool {
57+
return m.success.Load()
58+
}
59+
60+
func (m *mockNamespaceManager) RedirectConnections() []error {
61+
if m.success.Load() {
62+
return nil
63+
}
64+
return []error{errors.New("mock error")}
65+
}
66+
67+
func (m *mockNamespaceManager) Close() error {
68+
return nil
69+
}
70+
71+
func (m *mockNamespaceManager) CommitNamespaces(_ []*config.Namespace, _ []bool) error {
72+
if m.success.Load() {
73+
return nil
74+
}
75+
return errors.New("mock error")
76+
}

0 commit comments

Comments
 (0)