From 262e23aa56b538f501aba5a6c283034bc8d8c492 Mon Sep 17 00:00:00 2001 From: Oleg Zaytsev Date: Fri, 6 May 2022 12:56:00 +0200 Subject: [PATCH 1/8] Push back `forget` logic to rings. I don't want to give http handler the access to perform random CAS on the rings, especially since I want to export the http handler and that would mean that I need to export CasRing(), and that's something I want to avoid. Signed-off-by: Oleg Zaytsev --- ring/basic_lifecycler.go | 8 ++++---- ring/http.go | 19 +++---------------- ring/lifecycler.go | 8 ++++---- ring/ring.go | 8 ++++---- ring/util.go | 14 ++++++++++++++ 5 files changed, 29 insertions(+), 28 deletions(-) diff --git a/ring/basic_lifecycler.go b/ring/basic_lifecycler.go index 1bb95c083..e89fb9f29 100644 --- a/ring/basic_lifecycler.go +++ b/ring/basic_lifecycler.go @@ -498,10 +498,6 @@ func (l *BasicLifecycler) run(fn func() error) error { } } -func (l *BasicLifecycler) casRing(ctx context.Context, f func(in interface{}) (out interface{}, retry bool, err error)) error { - return l.store.CAS(ctx, l.ringKey, f) -} - func (l *BasicLifecycler) getRing(ctx context.Context) (*Desc, error) { obj, err := l.store.Get(ctx, l.ringKey) if err != nil { @@ -511,6 +507,10 @@ func (l *BasicLifecycler) getRing(ctx context.Context) (*Desc, error) { return GetOrCreateRingDesc(obj), nil } +func (l *BasicLifecycler) forget(ctx context.Context, id string) error { + return forget(ctx, l.store, l.ringKey, id) +} + func (l *BasicLifecycler) ServeHTTP(w http.ResponseWriter, req *http.Request) { newRingPageHandler(l, l.cfg.HeartbeatPeriod).handle(w, req) } diff --git a/ring/http.go b/ring/http.go index bcf3d1cc8..bb423086d 100644 --- a/ring/http.go +++ b/ring/http.go @@ -48,8 +48,8 @@ type ingesterDesc struct { } type ringAccess interface { - casRing(ctx context.Context, f func(in interface{}) (out interface{}, retry bool, err error)) error - getRing(context.Context) (*Desc, error) + getRing(ctx context.Context) (*Desc, error) + forget(ctx context.Context, id string) error } type ringPageHandler struct { @@ -67,7 +67,7 @@ func newRingPageHandler(r ringAccess, heartbeatPeriod time.Duration) *ringPageHa func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { if req.Method == http.MethodPost { ingesterID := req.FormValue("forget") - if err := h.forget(req.Context(), ingesterID); err != nil { + if err := h.r.forget(req.Context(), ingesterID); err != nil { http.Error( w, fmt.Errorf("error forgetting instance '%s': %w", ingesterID, err).Error(), @@ -147,19 +147,6 @@ func renderHTTPResponse(w http.ResponseWriter, v httpResponse, t *template.Templ } } -func (h *ringPageHandler) forget(ctx context.Context, id string) error { - unregister := func(in interface{}) (out interface{}, retry bool, err error) { - if in == nil { - return nil, false, fmt.Errorf("found empty ring when trying to unregister") - } - - ringDesc := in.(*Desc) - ringDesc.RemoveIngester(id) - return ringDesc, true, nil - } - return h.r.casRing(ctx, unregister) -} - // WriteJSONResponse writes some JSON as a HTTP response. func writeJSONResponse(w http.ResponseWriter, v httpResponse) { w.Header().Set("Content-Type", "application/json") diff --git a/ring/lifecycler.go b/ring/lifecycler.go index 9db0a7e6b..ea02d5318 100644 --- a/ring/lifecycler.go +++ b/ring/lifecycler.go @@ -851,10 +851,6 @@ func (i *Lifecycler) processShutdown(ctx context.Context) { time.Sleep(i.cfg.FinalSleep) } -func (i *Lifecycler) casRing(ctx context.Context, f func(in interface{}) (out interface{}, retry bool, err error)) error { - return i.KVStore.CAS(ctx, i.RingKey, f) -} - func (i *Lifecycler) getRing(ctx context.Context) (*Desc, error) { obj, err := i.KVStore.Get(ctx, i.RingKey) if err != nil { @@ -864,6 +860,10 @@ func (i *Lifecycler) getRing(ctx context.Context) (*Desc, error) { return GetOrCreateRingDesc(obj), nil } +func (i *Lifecycler) forget(ctx context.Context, id string) error { + return forget(ctx, i.KVStore, i.RingKey, id) +} + func (i *Lifecycler) ServeHTTP(w http.ResponseWriter, req *http.Request) { newRingPageHandler(i, i.cfg.HeartbeatPeriod).handle(w, req) } diff --git a/ring/ring.go b/ring/ring.go index be78fee59..b3243b100 100644 --- a/ring/ring.go +++ b/ring/ring.go @@ -815,10 +815,6 @@ func (r *Ring) CleanupShuffleShardCache(identifier string) { } } -func (r *Ring) casRing(ctx context.Context, f func(in interface{}) (out interface{}, retry bool, err error)) error { - return r.KVClient.CAS(ctx, r.key, f) -} - func (r *Ring) getRing(ctx context.Context) (*Desc, error) { r.mtx.RLock() defer r.mtx.RUnlock() @@ -828,6 +824,10 @@ func (r *Ring) getRing(ctx context.Context) (*Desc, error) { return ringDesc, nil } +func (r *Ring) forget(ctx context.Context, id string) error { + return forget(ctx, r.KVClient, r.key, id) +} + func (r *Ring) ServeHTTP(w http.ResponseWriter, req *http.Request) { newRingPageHandler(r, r.cfg.HeartbeatTimeout).handle(w, req) } diff --git a/ring/util.go b/ring/util.go index b39f2f26e..04f351da3 100644 --- a/ring/util.go +++ b/ring/util.go @@ -13,6 +13,7 @@ import ( "github.com/go-kit/log/level" "github.com/grafana/dskit/backoff" + "github.com/grafana/dskit/kv" ) // GenerateTokens make numTokens unique random tokens, none of which clash @@ -222,3 +223,16 @@ func filterIPs(addrs []net.Addr) string { } return ipAddr } + +func forget(ctx context.Context, client kv.Client, ringKey, id string) error { + unregister := func(in interface{}) (out interface{}, retry bool, err error) { + if in == nil { + return nil, false, fmt.Errorf("found empty ring when trying to unregister") + } + + ringDesc := in.(*Desc) + ringDesc.RemoveIngester(id) + return ringDesc, true, nil + } + return client.CAS(ctx, ringKey, unregister) +} From fd029f9fe22b1d0cc84305e49406ee599c1662fe Mon Sep 17 00:00:00 2001 From: Oleg Zaytsev Date: Fri, 6 May 2022 13:02:34 +0200 Subject: [PATCH 2/8] Rename getRing to describe Since this operation returns a Desc, IMO that's a better name. Signed-off-by: Oleg Zaytsev --- ring/basic_lifecycler.go | 2 +- ring/http.go | 4 ++-- ring/lifecycler.go | 2 +- ring/ring.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ring/basic_lifecycler.go b/ring/basic_lifecycler.go index e89fb9f29..ea8df71d7 100644 --- a/ring/basic_lifecycler.go +++ b/ring/basic_lifecycler.go @@ -498,7 +498,7 @@ func (l *BasicLifecycler) run(fn func() error) error { } } -func (l *BasicLifecycler) getRing(ctx context.Context) (*Desc, error) { +func (l *BasicLifecycler) describe(ctx context.Context) (*Desc, error) { obj, err := l.store.Get(ctx, l.ringKey) if err != nil { return nil, err diff --git a/ring/http.go b/ring/http.go index bb423086d..c1f1d8842 100644 --- a/ring/http.go +++ b/ring/http.go @@ -48,7 +48,7 @@ type ingesterDesc struct { } type ringAccess interface { - getRing(ctx context.Context) (*Desc, error) + describe(ctx context.Context) (*Desc, error) forget(ctx context.Context, id string) error } @@ -88,7 +88,7 @@ func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { return } - ringDesc, err := h.r.getRing(req.Context()) + ringDesc, err := h.r.describe(req.Context()) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/ring/lifecycler.go b/ring/lifecycler.go index ea02d5318..f8afdb711 100644 --- a/ring/lifecycler.go +++ b/ring/lifecycler.go @@ -851,7 +851,7 @@ func (i *Lifecycler) processShutdown(ctx context.Context) { time.Sleep(i.cfg.FinalSleep) } -func (i *Lifecycler) getRing(ctx context.Context) (*Desc, error) { +func (i *Lifecycler) describe(ctx context.Context) (*Desc, error) { obj, err := i.KVStore.Get(ctx, i.RingKey) if err != nil { return nil, err diff --git a/ring/ring.go b/ring/ring.go index b3243b100..83fde9337 100644 --- a/ring/ring.go +++ b/ring/ring.go @@ -815,7 +815,7 @@ func (r *Ring) CleanupShuffleShardCache(identifier string) { } } -func (r *Ring) getRing(ctx context.Context) (*Desc, error) { +func (r *Ring) describe(ctx context.Context) (*Desc, error) { r.mtx.RLock() defer r.mtx.RUnlock() From bac3d25dc369113510feea6cdd37a3a7ec4b035a Mon Sep 17 00:00:00 2001 From: Oleg Zaytsev Date: Fri, 6 May 2022 13:04:41 +0200 Subject: [PATCH 3/8] Export Operator that is needed for an httpHandler Signed-off-by: Oleg Zaytsev --- ring/basic_lifecycler.go | 4 ++-- ring/http.go | 16 +++++++++------- ring/lifecycler.go | 4 ++-- ring/ring.go | 4 ++-- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/ring/basic_lifecycler.go b/ring/basic_lifecycler.go index ea8df71d7..fff4076f9 100644 --- a/ring/basic_lifecycler.go +++ b/ring/basic_lifecycler.go @@ -498,7 +498,7 @@ func (l *BasicLifecycler) run(fn func() error) error { } } -func (l *BasicLifecycler) describe(ctx context.Context) (*Desc, error) { +func (l *BasicLifecycler) Describe(ctx context.Context) (*Desc, error) { obj, err := l.store.Get(ctx, l.ringKey) if err != nil { return nil, err @@ -507,7 +507,7 @@ func (l *BasicLifecycler) describe(ctx context.Context) (*Desc, error) { return GetOrCreateRingDesc(obj), nil } -func (l *BasicLifecycler) forget(ctx context.Context, id string) error { +func (l *BasicLifecycler) Forget(ctx context.Context, id string) error { return forget(ctx, l.store, l.ringKey, id) } diff --git a/ring/http.go b/ring/http.go index c1f1d8842..752d2e1e0 100644 --- a/ring/http.go +++ b/ring/http.go @@ -47,17 +47,19 @@ type ingesterDesc struct { Ownership float64 `json:"-"` } -type ringAccess interface { - describe(ctx context.Context) (*Desc, error) - forget(ctx context.Context, id string) error +// Operator allows external entities to perform generic operations on the ring, +// like describing it or force-forgetting one of the members. +type Operator interface { + Describe(ctx context.Context) (*Desc, error) + Forget(ctx context.Context, id string) error } type ringPageHandler struct { - r ringAccess + r Operator heartbeatPeriod time.Duration } -func newRingPageHandler(r ringAccess, heartbeatPeriod time.Duration) *ringPageHandler { +func newRingPageHandler(r Operator, heartbeatPeriod time.Duration) *ringPageHandler { return &ringPageHandler{ r: r, heartbeatPeriod: heartbeatPeriod, @@ -67,7 +69,7 @@ func newRingPageHandler(r ringAccess, heartbeatPeriod time.Duration) *ringPageHa func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { if req.Method == http.MethodPost { ingesterID := req.FormValue("forget") - if err := h.r.forget(req.Context(), ingesterID); err != nil { + if err := h.r.Forget(req.Context(), ingesterID); err != nil { http.Error( w, fmt.Errorf("error forgetting instance '%s': %w", ingesterID, err).Error(), @@ -88,7 +90,7 @@ func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { return } - ringDesc, err := h.r.describe(req.Context()) + ringDesc, err := h.r.Describe(req.Context()) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/ring/lifecycler.go b/ring/lifecycler.go index f8afdb711..5183c797b 100644 --- a/ring/lifecycler.go +++ b/ring/lifecycler.go @@ -851,7 +851,7 @@ func (i *Lifecycler) processShutdown(ctx context.Context) { time.Sleep(i.cfg.FinalSleep) } -func (i *Lifecycler) describe(ctx context.Context) (*Desc, error) { +func (i *Lifecycler) Describe(ctx context.Context) (*Desc, error) { obj, err := i.KVStore.Get(ctx, i.RingKey) if err != nil { return nil, err @@ -860,7 +860,7 @@ func (i *Lifecycler) describe(ctx context.Context) (*Desc, error) { return GetOrCreateRingDesc(obj), nil } -func (i *Lifecycler) forget(ctx context.Context, id string) error { +func (i *Lifecycler) Forget(ctx context.Context, id string) error { return forget(ctx, i.KVStore, i.RingKey, id) } diff --git a/ring/ring.go b/ring/ring.go index 83fde9337..999660a03 100644 --- a/ring/ring.go +++ b/ring/ring.go @@ -815,7 +815,7 @@ func (r *Ring) CleanupShuffleShardCache(identifier string) { } } -func (r *Ring) describe(ctx context.Context) (*Desc, error) { +func (r *Ring) Describe(ctx context.Context) (*Desc, error) { r.mtx.RLock() defer r.mtx.RUnlock() @@ -824,7 +824,7 @@ func (r *Ring) describe(ctx context.Context) (*Desc, error) { return ringDesc, nil } -func (r *Ring) forget(ctx context.Context, id string) error { +func (r *Ring) Forget(ctx context.Context, id string) error { return forget(ctx, r.KVClient, r.key, id) } From 25fac49e3b183b1bbfd026f901f50fc9c05bf267 Mon Sep 17 00:00:00 2001 From: Oleg Zaytsev Date: Fri, 6 May 2022 13:06:14 +0200 Subject: [PATCH 4/8] Export data rendered in the template Signed-off-by: Oleg Zaytsev --- ring/http.go | 16 ++++++++-------- ring/status.gohtml | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ring/http.go b/ring/http.go index 752d2e1e0..36bff2fc6 100644 --- a/ring/http.go +++ b/ring/http.go @@ -29,13 +29,13 @@ var defaultPageTemplate = template.Must(template.New("webpage").Funcs(template.F "durationSince": func(t time.Time) string { return time.Since(t).Truncate(time.Millisecond).String() }, }).Parse(defaultPageContent)) -type httpResponse struct { - Ingesters []ingesterDesc `json:"shards"` +type StatusPageData struct { + Ingesters []IngesterDesc `json:"shards"` Now time.Time `json:"now"` ShowTokens bool `json:"-"` } -type ingesterDesc struct { +type IngesterDesc struct { ID string `json:"id"` State string `json:"state"` Address string `json:"address"` @@ -104,7 +104,7 @@ func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { sort.Strings(ingesterIDs) now := time.Now() - var ingesters []ingesterDesc + var ingesters []IngesterDesc for _, id := range ingesterIDs { ing := ringDesc.Ingesters[id] state := ing.State.String() @@ -112,7 +112,7 @@ func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { state = "UNHEALTHY" } - ingesters = append(ingesters, ingesterDesc{ + ingesters = append(ingesters, IngesterDesc{ ID: id, State: state, Address: ing.Addr, @@ -127,7 +127,7 @@ func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { tokensParam := req.URL.Query().Get("tokens") - renderHTTPResponse(w, httpResponse{ + renderHTTPResponse(w, StatusPageData{ Ingesters: ingesters, Now: now, ShowTokens: tokensParam == "true", @@ -136,7 +136,7 @@ func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { // RenderHTTPResponse either responds with json or a rendered html page using the passed in template // by checking the Accepts header -func renderHTTPResponse(w http.ResponseWriter, v httpResponse, t *template.Template, r *http.Request) { +func renderHTTPResponse(w http.ResponseWriter, v StatusPageData, t *template.Template, r *http.Request) { accept := r.Header.Get("Accept") if strings.Contains(accept, "application/json") { writeJSONResponse(w, v) @@ -150,7 +150,7 @@ func renderHTTPResponse(w http.ResponseWriter, v httpResponse, t *template.Templ } // WriteJSONResponse writes some JSON as a HTTP response. -func writeJSONResponse(w http.ResponseWriter, v httpResponse) { +func writeJSONResponse(w http.ResponseWriter, v StatusPageData) { w.Header().Set("Content-Type", "application/json") if err := json.NewEncoder(w).Encode(v); err != nil { diff --git a/ring/status.gohtml b/ring/status.gohtml index 80e5b6a8f..00d852a92 100644 --- a/ring/status.gohtml +++ b/ring/status.gohtml @@ -1,4 +1,4 @@ -{{- /*gotype: github.com/grafana/dskit/ring.httpResponse */ -}} +{{- /*gotype: github.com/grafana/dskit/ring.StatusPageData */ -}} From b61dfe5a650f8254d07afe626e830eeacfa88e76 Mon Sep 17 00:00:00 2001 From: Oleg Zaytsev Date: Fri, 6 May 2022 13:06:55 +0200 Subject: [PATCH 5/8] Move template init to the end of the file I don't want to start the file with that, since it's not something needed for users coming to see how to use this. Signed-off-by: Oleg Zaytsev --- ring/http.go | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/ring/http.go b/ring/http.go index 36bff2fc6..1c4b92d3e 100644 --- a/ring/http.go +++ b/ring/http.go @@ -13,22 +13,6 @@ import ( "time" ) -//go:embed status.gohtml -var defaultPageContent string -var defaultPageTemplate = template.Must(template.New("webpage").Funcs(template.FuncMap{ - "mod": func(i, j int) bool { return i%j == 0 }, - "humanFloat": func(f float64) string { - return fmt.Sprintf("%.2g", f) - }, - "timeOrEmptyString": func(t time.Time) string { - if t.IsZero() { - return "" - } - return t.Format(time.RFC3339Nano) - }, - "durationSince": func(t time.Time) string { return time.Since(t).Truncate(time.Millisecond).String() }, -}).Parse(defaultPageContent)) - type StatusPageData struct { Ingesters []IngesterDesc `json:"shards"` Now time.Time `json:"now"` @@ -157,3 +141,19 @@ func writeJSONResponse(w http.ResponseWriter, v StatusPageData) { http.Error(w, err.Error(), http.StatusInternalServerError) } } + +//go:embed status.gohtml +var defaultPageContent string +var defaultPageTemplate = template.Must(template.New("webpage").Funcs(template.FuncMap{ + "mod": func(i, j int) bool { return i%j == 0 }, + "humanFloat": func(f float64) string { + return fmt.Sprintf("%.2g", f) + }, + "timeOrEmptyString": func(t time.Time) string { + if t.IsZero() { + return "" + } + return t.Format(time.RFC3339Nano) + }, + "durationSince": func(t time.Time) string { return time.Since(t).Truncate(time.Millisecond).String() }, +}).Parse(defaultPageContent)) From 69bc4ee4f69953da26387bcc93dfa6cb1ff461fe Mon Sep 17 00:00:00 2001 From: Oleg Zaytsev Date: Fri, 6 May 2022 13:20:00 +0200 Subject: [PATCH 6/8] Export ring status http handler It also allows configuring the template used to be rendered. Signed-off-by: Oleg Zaytsev --- ring/basic_lifecycler.go | 2 +- ring/http.go | 41 +++++++++++++++++++++++------------- ring/lifecycler.go | 2 +- ring/replication_set_test.go | 2 +- ring/ring.go | 2 +- 5 files changed, 30 insertions(+), 19 deletions(-) diff --git a/ring/basic_lifecycler.go b/ring/basic_lifecycler.go index fff4076f9..797c80d25 100644 --- a/ring/basic_lifecycler.go +++ b/ring/basic_lifecycler.go @@ -512,5 +512,5 @@ func (l *BasicLifecycler) Forget(ctx context.Context, id string) error { } func (l *BasicLifecycler) ServeHTTP(w http.ResponseWriter, req *http.Request) { - newRingPageHandler(l, l.cfg.HeartbeatPeriod).handle(w, req) + NewHTTPStatusHandler(l, defaultPageTemplate, l.cfg.HeartbeatPeriod).ServeHTTP(w, req) } diff --git a/ring/http.go b/ring/http.go index 1c4b92d3e..97345cbf4 100644 --- a/ring/http.go +++ b/ring/http.go @@ -14,13 +14,18 @@ import ( ) type StatusPageData struct { - Ingesters []IngesterDesc `json:"shards"` - Now time.Time `json:"now"` - ShowTokens bool `json:"-"` + // Ingesters is the list of the ingesters found in the ring. + Ingesters []IngesterDesc `json:"shards"` + // ShowTokens is true if user requested to see show the tokens. + // Tokens are always provided in the IngesterDesc struct, regardless of this param. + ShowTokens bool `json:"-"` + // Now is the current time (time when template was rendered) + Now time.Time `json:"now"` } type IngesterDesc struct { - ID string `json:"id"` + ID string `json:"id"` + // State can be: "ACTIVE", "LEAVING", "PENDING", "JOINING", "LEFT" or "UNHEALTHY" State string `json:"state"` Address string `json:"address"` HeartbeatTimestamp time.Time `json:"timestamp"` @@ -28,7 +33,8 @@ type IngesterDesc struct { Zone string `json:"zone"` Tokens []uint32 `json:"tokens"` NumTokens int `json:"-"` - Ownership float64 `json:"-"` + // Ownership represents the percentage (0-100) of the tokens owned by this instance. + Ownership float64 `json:"-"` } // Operator allows external entities to perform generic operations on the ring, @@ -38,19 +44,24 @@ type Operator interface { Forget(ctx context.Context, id string) error } -type ringPageHandler struct { - r Operator - heartbeatPeriod time.Duration -} - -func newRingPageHandler(r Operator, heartbeatPeriod time.Duration) *ringPageHandler { - return &ringPageHandler{ +// NewHTTPStatusHandler will use the provided Operator to build an http.Handler to inspect the ring status over http. +// It will render the provided template (unless Accept: application/json header is provided, in which case it will return a JSON response). +// The handler provided also can force forgetting members of the ring by sending a POST request with the ID in the `forget` field. +func NewHTTPStatusHandler(r Operator, tpl *template.Template, heartbeatPeriod time.Duration) HTTPStatusHandler { + return HTTPStatusHandler{ r: r, heartbeatPeriod: heartbeatPeriod, + template: tpl, } } -func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { +type HTTPStatusHandler struct { + r Operator + template *template.Template + heartbeatPeriod time.Duration +} + +func (h HTTPStatusHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { if req.Method == http.MethodPost { ingesterID := req.FormValue("forget") if err := h.r.Forget(req.Context(), ingesterID); err != nil { @@ -115,10 +126,10 @@ func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { Ingesters: ingesters, Now: now, ShowTokens: tokensParam == "true", - }, defaultPageTemplate, req) + }, h.template, req) } -// RenderHTTPResponse either responds with json or a rendered html page using the passed in template +// renderHTTPResponse either responds with json or a rendered html page using the passed in template // by checking the Accepts header func renderHTTPResponse(w http.ResponseWriter, v StatusPageData, t *template.Template, r *http.Request) { accept := r.Header.Get("Accept") diff --git a/ring/lifecycler.go b/ring/lifecycler.go index 5183c797b..7a6ef11d5 100644 --- a/ring/lifecycler.go +++ b/ring/lifecycler.go @@ -865,7 +865,7 @@ func (i *Lifecycler) Forget(ctx context.Context, id string) error { } func (i *Lifecycler) ServeHTTP(w http.ResponseWriter, req *http.Request) { - newRingPageHandler(i, i.cfg.HeartbeatPeriod).handle(w, req) + NewHTTPStatusHandler(i, defaultPageTemplate, i.cfg.HeartbeatPeriod).ServeHTTP(w, req) } // unregister removes our entry from consul. diff --git a/ring/replication_set_test.go b/ring/replication_set_test.go index 42ecc0f11..2694dbae7 100644 --- a/ring/replication_set_test.go +++ b/ring/replication_set_test.go @@ -157,7 +157,7 @@ func TestReplicationSet_Do(t *testing.T) { expectedError: errFailure, }, { - name: "max errors = 1, should handle context canceled", + name: "max errors = 1, should ServeHTTP context canceled", instances: []InstanceDesc{{}, {}, {}}, maxErrors: 1, f: func(c context.Context, id *InstanceDesc) (interface{}, error) { diff --git a/ring/ring.go b/ring/ring.go index 999660a03..e65305c30 100644 --- a/ring/ring.go +++ b/ring/ring.go @@ -829,7 +829,7 @@ func (r *Ring) Forget(ctx context.Context, id string) error { } func (r *Ring) ServeHTTP(w http.ResponseWriter, req *http.Request) { - newRingPageHandler(r, r.cfg.HeartbeatTimeout).handle(w, req) + NewHTTPStatusHandler(r, defaultPageTemplate, r.cfg.HeartbeatTimeout).ServeHTTP(w, req) } // Operation describes which instances can be included in the replica set, based on their state. From 84e189189268b7df02b8912cee53b72be5c2cf11 Mon Sep 17 00:00:00 2001 From: Oleg Zaytsev Date: Fri, 6 May 2022 13:33:10 +0200 Subject: [PATCH 7/8] Update CHANGELOG.md Signed-off-by: Oleg Zaytsev --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d52e3f1db..308e1dd4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ * [ENHANCEMENT] Lifecycler: It's now possible to change default value of lifecycler's `final-sleep`. #121 * [ENHANCEMENT] Memberlist: Update to latest fork of memberlist. #160 * [ENHANCEMENT] Memberlist: extracted HTTP status page handler to `memberlist.HTTPStatusHandler` which now can be instantiated with a custom template. #163 +* [ENHANCEMENT] Ring: extracted HTTP status page handler to `ring.HTTPStatusHandler` which now can be instantiated with a custom template. #166 * [BUGFIX] spanlogger: Support multiple tenant IDs. #59 * [BUGFIX] Memberlist: fixed corrupted packets when sending compound messages with more than 255 messages or messages bigger than 64KB. #85 * [BUGFIX] Ring: `ring_member_ownership_percent` and `ring_tokens_owned` metrics are not updated on scale down. #109 From 4e74f5bcdd5aa0a3fd9e3925da6c8558b2d28686 Mon Sep 17 00:00:00 2001 From: Oleg Zaytsev Date: Fri, 6 May 2022 14:22:14 +0200 Subject: [PATCH 8/8] Move IsHealthy to ring.Operator Instead of asking for the timeout when instantiating the http status handler, move the logic of determining that to the rings implemeantion. This makes it easier to instantiate in downstream projects (we don't need to lookup the config). Signed-off-by: Oleg Zaytsev --- ring/basic_lifecycler.go | 6 +++++- ring/http.go | 16 ++++++++-------- ring/lifecycler.go | 6 +++++- ring/ring.go | 2 +- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/ring/basic_lifecycler.go b/ring/basic_lifecycler.go index 797c80d25..807228a9e 100644 --- a/ring/basic_lifecycler.go +++ b/ring/basic_lifecycler.go @@ -511,6 +511,10 @@ func (l *BasicLifecycler) Forget(ctx context.Context, id string) error { return forget(ctx, l.store, l.ringKey, id) } +func (l *BasicLifecycler) IsHealthy(instance *InstanceDesc, op Operation, now time.Time) bool { + return instance.IsHealthy(op, l.cfg.HeartbeatPeriod, now) +} + func (l *BasicLifecycler) ServeHTTP(w http.ResponseWriter, req *http.Request) { - NewHTTPStatusHandler(l, defaultPageTemplate, l.cfg.HeartbeatPeriod).ServeHTTP(w, req) + NewHTTPStatusHandler(l, defaultPageTemplate).ServeHTTP(w, req) } diff --git a/ring/http.go b/ring/http.go index 97345cbf4..ead7a8ae6 100644 --- a/ring/http.go +++ b/ring/http.go @@ -42,23 +42,23 @@ type IngesterDesc struct { type Operator interface { Describe(ctx context.Context) (*Desc, error) Forget(ctx context.Context, id string) error + + IsHealthy(instance *InstanceDesc, op Operation, now time.Time) bool } // NewHTTPStatusHandler will use the provided Operator to build an http.Handler to inspect the ring status over http. // It will render the provided template (unless Accept: application/json header is provided, in which case it will return a JSON response). // The handler provided also can force forgetting members of the ring by sending a POST request with the ID in the `forget` field. -func NewHTTPStatusHandler(r Operator, tpl *template.Template, heartbeatPeriod time.Duration) HTTPStatusHandler { +func NewHTTPStatusHandler(r Operator, tpl *template.Template) HTTPStatusHandler { return HTTPStatusHandler{ - r: r, - heartbeatPeriod: heartbeatPeriod, - template: tpl, + r: r, + template: tpl, } } type HTTPStatusHandler struct { - r Operator - template *template.Template - heartbeatPeriod time.Duration + r Operator + template *template.Template } func (h HTTPStatusHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { @@ -103,7 +103,7 @@ func (h HTTPStatusHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { for _, id := range ingesterIDs { ing := ringDesc.Ingesters[id] state := ing.State.String() - if !ing.IsHealthy(Reporting, h.heartbeatPeriod, now) { + if !h.r.IsHealthy(&ing, Reporting, now) { state = "UNHEALTHY" } diff --git a/ring/lifecycler.go b/ring/lifecycler.go index 7a6ef11d5..d40e5074e 100644 --- a/ring/lifecycler.go +++ b/ring/lifecycler.go @@ -864,8 +864,12 @@ func (i *Lifecycler) Forget(ctx context.Context, id string) error { return forget(ctx, i.KVStore, i.RingKey, id) } +func (i *Lifecycler) IsHealthy(instance *InstanceDesc, op Operation, now time.Time) bool { + return instance.IsHealthy(op, i.cfg.HeartbeatPeriod, now) +} + func (i *Lifecycler) ServeHTTP(w http.ResponseWriter, req *http.Request) { - NewHTTPStatusHandler(i, defaultPageTemplate, i.cfg.HeartbeatPeriod).ServeHTTP(w, req) + NewHTTPStatusHandler(i, defaultPageTemplate).ServeHTTP(w, req) } // unregister removes our entry from consul. diff --git a/ring/ring.go b/ring/ring.go index e65305c30..77e32d213 100644 --- a/ring/ring.go +++ b/ring/ring.go @@ -829,7 +829,7 @@ func (r *Ring) Forget(ctx context.Context, id string) error { } func (r *Ring) ServeHTTP(w http.ResponseWriter, req *http.Request) { - NewHTTPStatusHandler(r, defaultPageTemplate, r.cfg.HeartbeatTimeout).ServeHTTP(w, req) + NewHTTPStatusHandler(r, defaultPageTemplate).ServeHTTP(w, req) } // Operation describes which instances can be included in the replica set, based on their state.