refactor(search): warn on a persisted additive change even when the reopen fails

services/search/pkg/bleve/index.go

@@ -79,24 +79,25 @@ func (r *bleveReconciler) Classify() (searchmapping.Classification, error) {
 
 // ApplyAdditive persists the code mapping and reopens so the live mapping picks
 // it up; otherwise the new fields get indexed dynamically and flip to breaking
-// on the next start. On failure it closes the index and clears the handle.
-func (r *bleveReconciler) ApplyAdditive() error {
+// on the next start. Once SetInternal succeeds it reports persisted=true even if
+// the reopen then fails. On failure it closes the index and clears the handle.
+func (r *bleveReconciler) ApplyAdditive() (bool, error) {
 	if err := r.index.SetInternal([]byte("_mapping"), r.codeB); err != nil {
 		_ = r.index.Close()
 		r.index = nil
-		return fmt.Errorf("failed to store the updated index mapping: %w", err)
+		return false, fmt.Errorf("failed to store the updated index mapping: %w", err)
 	}
 	if err := r.index.Close(); err != nil {
 		r.index = nil
-		return err
+		return true, err
 	}
 	index, err := bleve.OpenUsing(r.destination, openRuntimeConfig)
 	if err != nil {
 		r.index = nil
-		return err
+		return true, err
 	}
 	r.index = index
-	return nil
+	return true, nil
 }
 
 // classifyStoredMapping diffs the stored mapping against NewMapping() and

services/search/pkg/mapping/reconcile.go

@@ -11,7 +11,12 @@ import (
 // them so the verdict-to-action mapping lives in one place for every backend.
 type SchemaReconciler interface {
 	Classify() (Classification, error)
-	ApplyAdditive() error
+	// ApplyAdditive applies an additive change to the live index and reports
+	// whether the schema was persisted. It returns persisted=true even when a
+	// later step then fails (e.g. a bleve reopen), so Reconcile can still warn:
+	// the change is on disk, a subsequent start classifies equal and stays
+	// silent, so this is the only chance to surface the new fields.
+	ApplyAdditive() (persisted bool, err error)
 }
 
 // Reconcile runs the shared schema-verdict flow: an equal schema starts
@@ -27,10 +32,13 @@ func Reconcile(index string, r SchemaReconciler, logger log.Logger) (Classificat
 	case VerdictBreaking:
 		return classification, ManualActionRequiredError(index, classification.Reasons)
 	case VerdictAdditive:
-		if err := r.ApplyAdditive(); err != nil {
+		persisted, err := r.ApplyAdditive()
+		if persisted {
+			logger.Warn().Strs("fields", classification.NewFields).Str("index", index).Msg("extended the search index mapping with new fields; documents indexed before the upgrade do not contain them and queries on these fields will miss those documents until they are re-indexed; to re-index everything run: opencloud search index --all-spaces --force-rescan")
+		}
+		if err != nil {
 			return classification, err
 		}
-		logger.Warn().Strs("fields", classification.NewFields).Str("index", index).Msg("extended the search index mapping with new fields; documents indexed before the upgrade do not contain them and queries on these fields will miss those documents until they are re-indexed; to re-index everything run: opencloud search index --all-spaces --force-rescan")
 	}
 
 	return classification, nil

services/search/pkg/opensearch/index.go

@@ -204,7 +204,8 @@ func (r *osReconciler) Classify() (searchmapping.Classification, error) {
 
 // ApplyAdditive puts the full code properties; the classifier guarantees every
 // existing field already matches the remote state, so this can only add fields.
-func (r *osReconciler) ApplyAdditive() error {
+// The PUT is atomic, so persisted is true only on success.
+func (r *osReconciler) ApplyAdditive() (bool, error) {
 	putResp, err := r.client.Indices.Mapping.Put(r.ctx, opensearchgoAPI.MappingPutReq{
 		Indices: []string{r.name},
 		Body:    strings.NewReader(r.local.Get("mappings").Raw),
@@ -214,13 +215,13 @@ func (r *osReconciler) ApplyAdditive() error {
 	case err != nil && errors.As(err, &putErr) && putErr.Err.Type == "illegal_argument_exception" &&
 		(strings.Contains(putErr.Err.Reason, "cannot be changed") || strings.Contains(putErr.Err.Reason, "Cannot update parameter")):
 		// backstop, should be unreachable after the classification above
-		return searchmapping.ManualActionRequiredError(r.name, []string{putErr.Err.Reason})
+		return false, searchmapping.ManualActionRequiredError(r.name, []string{putErr.Err.Reason})
 	case err != nil:
-		return fmt.Errorf("failed to update mapping of index %s: %w", r.name, err)
+		return false, fmt.Errorf("failed to update mapping of index %s: %w", r.name, err)
 	case !putResp.Acknowledged:
-		return fmt.Errorf("failed to update mapping of index %s: not acknowledged", r.name)
+		return false, fmt.Errorf("failed to update mapping of index %s: not acknowledged", r.name)
 	}
-	return nil
+	return true, nil
 }
 
 // jsonEqual reports whether two raw JSON values are deeply equal. gjson yields