ddl: fix wait fts ready in ddl add index

Project: pingcap/tidb | Language: Go | Concepts: error-handling, api-design, testing-strategy | Difficulty: mid
Link: https://github.com/pingcap/tidb/pull/70799

The problem

When adding a FULLTEXT index via DDL, TiDB waited for the wrong readiness field from TiKV's status API, causing timeouts or incorrect behavior. The system checked vector-index-ready instead of fts-index-ready, which is specific to FULLTEXT indexes. This led to DDL jobs hanging or failing to recognize when a FULLTEXT index was actually ready.

Why it's non-trivial

A naive fix might just swap which field is checked, but:

• The TiKV API may not always include the fts-index-ready field (older versions).
• The DDL code must distinguish between index types (FULLTEXT vs VECTOR) and select the correct field dynamically.
• Error handling is needed if the expected field is missing.
• Tests must cover all cases, including missing fields and zero values.

The approach

1. DDL now checks the index type and passes it to the progress calculator

-	progress, err := infosync.CalculateColumnarIndexProgress(tbl.Meta().ID, indexID, tikvStores)
+	indexInfo := tbl.Meta().FindIndexByID(indexID)
+	if indexInfo == nil {
+		return false, 0, errors.Errorf("could not find indexInfo by %d", indexID)
+	}
+	columnarIndexType := indexInfo.GetColumnarIndexType()
+	if columnarIndexType == model.ColumnarIndexTypeNA {
+		return false, 0, errors.Trace(dbterror.ErrUnsupportedAddColumnarIndex.GenWithStackByArgs("Columnar does not support index types."))
+	}
+	progress, err := infosync.CalculateColumnarIndexProgress(tbl.Meta().ID, indexID, columnarIndexType, tikvStores)

This change ensures that the DDL logic is aware of the specific index type (FULLTEXT, VECTOR, etc.) and passes this information to the progress calculation function. It also adds error handling for unsupported or missing index types.

2. Progress calculation distinguishes between FULLTEXT and VECTOR indexes

-func CalculateColumnarIndexProgress(tableID, indexID int64, tikvStores map[int64]pdhttp.StoreInfo) (float64, error) {
+func CalculateColumnarIndexProgress(tableID, indexID int64, columnarIndexType model.ColumnarIndexType, tikvStores map[int64]pdhttp.StoreInfo) (float64, error) {
...
-		indexReady += columnarStatus.VectorIndexReady
+		switch columnarIndexType {
+		case model.ColumnarIndexTypeFulltext:
+			if !columnarStatus.HasFtsIndexReady {
+				return 0, errors.Errorf("fts-index-ready not found in TiKV columnar_status response from %s (store %d); please check TiKV version", addr, storeStat.Store.ID)
+			}
+			indexReady += columnarStatus.FtsIndexReady
+		default:
+			indexReady += columnarStatus.VectorIndexReady
+		}

This change selects the correct readiness field based on the index type. If the field for FULLTEXT indexes is missing, it returns an error, helping users identify compatibility issues.

3. The status response parser detects presence of the FULLTEXT field

 type ColumnarStatusResp struct {
 	Ready            uint `json:"ready"`
 	VectorIndexReady uint `json:"vector-index-ready"`
+	FtsIndexReady    uint `json:"fts-index-ready"`
 	Total            uint `json:"total"`
+	// HasFtsIndexReady reports whether the JSON payload contains "fts-index-ready".
+	HasFtsIndexReady bool `json:"-"`
 }
...
-	err = json.Unmarshal(body, &columnarStatus)
+	type columnarStatusPayload struct {
+		Ready            uint  `json:"ready"`
+		VectorIndexReady uint  `json:"vector-index-ready"`
+		FtsIndexReady    *uint `json:"fts-index-ready"`
+		Total            uint  `json:"total"`
+	}
+	var payload columnarStatusPayload
+	err = json.Unmarshal(body, &payload)
 	if err != nil {
 		return columnarStatus, errors.Trace(err)
 	}
+	columnarStatus.Ready = payload.Ready
+	columnarStatus.VectorIndexReady = payload.VectorIndexReady
+	columnarStatus.Total = payload.Total
+	if payload.FtsIndexReady != nil {
+		columnarStatus.HasFtsIndexReady = true
+		columnarStatus.FtsIndexReady = *payload.FtsIndexReady
+	}

The parser now checks if the fts-index-ready field is present in the JSON response. This allows the code to distinguish between a missing field (older TiKV) and a field present with a value of zero.

4. Unit tests cover all cases: present, zero, and missing FULLTEXT field

func TestCollectColumnarStatusFTSIndexReady(t *testing.T) {
	testCases := []struct {
		name             string
		response         string
		ftsIndexReady    uint
		hasFtsIndexReady bool
	}{
		{
			name:             "present",
			response:         `{"ready":3,"vector-index-ready":2,"fts-index-ready":1,"total":4}`,
			ftsIndexReady:    1,
			hasFtsIndexReady: true,
		},
		{
			name:             "present zero",
			response:         `{"ready":3,"vector-index-ready":2,"fts-index-ready":0,"total":4}`,
			ftsIndexReady:    0,
			hasFtsIndexReady: true,
		},
		{
			name:             "missing on older TiKV",
			response:         `{"ready":3,"vector-index-ready":2,"total":4}`,
			ftsIndexReady:    0,
			hasFtsIndexReady: false,
		},
	}
	...
}

This test ensures that the parser and logic behave correctly whether the FULLTEXT field is present, present and zero, or missing entirely.

From the review thread

No notable review discussion on this PR.

Takeaways

• APIs may evolve; always detect and handle missing or optional fields explicitly, not just by value.
• When adding support for new types or features, ensure all code paths (including error handling and tests) are aware of and check for the new cases.
• Test for all permutations of optional fields: present, zero, and missing, to ensure robust parsing and error handling.