diff --git a/services/search/pkg/bleve/index.go b/services/search/pkg/bleve/index.go index d01bc17d9d..1e95641349 100644 --- a/services/search/pkg/bleve/index.go +++ b/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 diff --git a/services/search/pkg/mapping/reconcile.go b/services/search/pkg/mapping/reconcile.go index 8f02604d8c..a812128c2b 100644 --- a/services/search/pkg/mapping/reconcile.go +++ b/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 diff --git a/services/search/pkg/opensearch/index.go b/services/search/pkg/opensearch/index.go index 4acc840ff5..00df4b1ef3 100644 --- a/services/search/pkg/opensearch/index.go +++ b/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