Fixed ingester ReadOnly state related bugs (#6208)

Signed-off-by: Alex Le <[email protected]>
cortexproject · Sep 12, 2024 · f74b4cd · f74b4cd
1 parent 2490796
commit f74b4cd
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 2 deletions.
diff --git a/pkg/ingester/http_admin.go b/pkg/ingester/http_admin.go
@@ -43,7 +43,7 @@ const tpl = `
 					{{ range .Stats }}
 					<tr>
 						<td>{{ .UserID }}</td>
-						<td align='right'>{{ .UserStats.LoadBlocks }}</td>
+						<td align='right'>{{ .UserStats.LoadedBlocks }}</td>
 						<td align='right'>{{ .UserStats.NumSeries }}</td>
 						<td align='right'>{{ .UserStats.ActiveSeries }}</td>
 						<td align='right'>{{ printf "%.2f" .UserStats.IngestionRate }}</td>

diff --git a/pkg/ingester/http_admin_test.go b/pkg/ingester/http_admin_test.go
@@ -0,0 +1,37 @@
+package ingester
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestUserStatsPageRendered(t *testing.T) {
+	req := httptest.NewRequest(http.MethodGet, "/ingester/all_user_stats", nil)
+	res := httptest.NewRecorder()
+	userStats := []UserIDStats{
+		{
+			UserID: "123",
+			UserStats: UserStats{
+				IngestionRate:     11.11,
+				NumSeries:         2222,
+				APIIngestionRate:  33.33,
+				RuleIngestionRate: 44.44,
+				ActiveSeries:      5555,
+				LoadedBlocks:      6666,
+			},
+		},
+	}
+	AllUserStatsRender(res, req, userStats, 3)
+	assert.Equal(t, http.StatusOK, res.Code)
+	body := res.Body.String()
+	assert.Regexp(t, "<td.+123.+/td>", body)
+	assert.Regexp(t, "<td.+11.11.+/td>", body)
+	assert.Regexp(t, "<td.+2222.+/td>", body)
+	assert.Regexp(t, "<td.+33.33.+/td>", body)
+	assert.Regexp(t, "<td.+44.44.+/td>", body)
+	assert.Regexp(t, "<td.+5555.+/td>", body)
+	assert.Regexp(t, "<td.+6666.+/td>", body)
+}
diff --git a/pkg/ingester/ingester_test.go b/pkg/ingester/ingester_test.go
@@ -5206,20 +5206,23 @@ func Test_Ingester_ModeHandler(t *testing.T) {
 		mode             string
 		expectedState    ring.InstanceState
 		expectedResponse int
+		expectedIsReady  bool
 	}{
 		"should change to READONLY mode": {
 			method:           "POST",
 			initialState:     ring.ACTIVE,
 			requestUrl:       "/mode?mode=reAdOnLy",
 			expectedState:    ring.READONLY,
 			expectedResponse: http.StatusOK,
+			expectedIsReady:  true,
 		},
 		"should change mode on GET method": {
 			method:           "GET",
 			initialState:     ring.ACTIVE,
 			requestUrl:       "/mode?mode=READONLY",
 			expectedState:    ring.READONLY,
 			expectedResponse: http.StatusOK,
+			expectedIsReady:  true,
 		},
 		"should change mode on POST method via body": {
 			method:           "POST",
@@ -5228,53 +5231,61 @@ func Test_Ingester_ModeHandler(t *testing.T) {
 			requestBody:      strings.NewReader("mode=readonly"),
 			expectedState:    ring.READONLY,
 			expectedResponse: http.StatusOK,
+			expectedIsReady:  true,
 		},
 		"should change to ACTIVE mode": {
 			method:           "POST",
 			initialState:     ring.READONLY,
 			requestUrl:       "/mode?mode=active",
 			expectedState:    ring.ACTIVE,
 			expectedResponse: http.StatusOK,
+			expectedIsReady:  true,
 		},
 		"should fail to unknown mode": {
 			method:           "POST",
 			initialState:     ring.ACTIVE,
 			requestUrl:       "/mode?mode=NotSupported",
 			expectedState:    ring.ACTIVE,
 			expectedResponse: http.StatusBadRequest,
+			expectedIsReady:  true,
 		},
 		"should maintain in readonly": {
 			method:           "POST",
 			initialState:     ring.READONLY,
 			requestUrl:       "/mode?mode=READONLY",
 			expectedState:    ring.READONLY,
 			expectedResponse: http.StatusOK,
+			expectedIsReady:  true,
 		},
 		"should maintain in active": {
 			method:           "POST",
 			initialState:     ring.ACTIVE,
 			requestUrl:       "/mode?mode=ACTIVE",
 			expectedState:    ring.ACTIVE,
 			expectedResponse: http.StatusOK,
+			expectedIsReady:  true,
 		},
 		"should fail mode READONLY if LEAVING state": {
 			method:           "POST",
 			initialState:     ring.LEAVING,
 			requestUrl:       "/mode?mode=READONLY",
 			expectedState:    ring.LEAVING,
 			expectedResponse: http.StatusBadRequest,
+			expectedIsReady:  false,
 		},
 		"should fail with malformatted request": {
 			method:           "GET",
 			initialState:     ring.ACTIVE,
 			requestUrl:       "/mode?mod;e=READONLY",
 			expectedResponse: http.StatusBadRequest,
+			expectedIsReady:  true,
 		},
 	}
 
 	for testName, testData := range tests {
 		t.Run(testName, func(t *testing.T) {
 			cfg := defaultIngesterTestConfig(t)
+			cfg.LifecyclerConfig.MinReadyDuration = 0
 			i, err := prepareIngesterWithBlocksStorage(t, cfg, prometheus.NewRegistry())
 			require.NoError(t, err)
 			require.NoError(t, services.StartAndAwaitRunning(context.Background(), i))
@@ -5304,6 +5315,13 @@ func Test_Ingester_ModeHandler(t *testing.T) {
 
 			require.Equal(t, testData.expectedResponse, response.Code)
 			require.Equal(t, testData.expectedState, i.lifecycler.GetState())
+
+			err = i.CheckReady(context.Background())
+			if testData.expectedIsReady {
+				require.NoError(t, err)
+			} else {
+				require.NotNil(t, err)
+			}
 		})
 	}
 }

diff --git a/pkg/ring/model.go b/pkg/ring/model.go
@@ -153,7 +153,7 @@ func (i *InstanceDesc) IsReady(storageLastUpdated time.Time, heartbeatTimeout ti
 	if !i.IsHeartbeatHealthy(heartbeatTimeout, storageLastUpdated) {
 		return fmt.Errorf("instance %s past heartbeat timeout", i.Addr)
 	}
-	if i.State != ACTIVE {
+	if i.State != ACTIVE && i.State != READONLY {
 		return fmt.Errorf("instance %s in state %v", i.Addr, i.State)
 	}
 	return nil

diff --git a/pkg/ring/model_test.go b/pkg/ring/model_test.go
@@ -275,6 +275,20 @@ func TestDesc_Ready(t *testing.T) {
 	if err := r.IsReady(now, 10*time.Second); err != nil {
 		t.Fatal("expected ready, got", err)
 	}
+
+	r = &Desc{
+		Ingesters: map[string]InstanceDesc{
+			"ing1": {
+				Tokens:    []uint32{100, 200, 300},
+				State:     READONLY,
+				Timestamp: now.Unix(),
+			},
+		},
+	}
+
+	if err := r.IsReady(now, 10*time.Second); err != nil {
+		t.Fatal("expected readonly ingester as ready, but got", err)
+	}
 }
 
 func TestDesc_getTokensByZone(t *testing.T) {