From d050bd259207fada9374d8d1e2c5f9c23b346f47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Egelund-M=C3=BCller?= Date: Fri, 24 Jul 2026 13:06:30 +0200 Subject: [PATCH 1/2] Prevent transient table lookup errors from triggering full model resets --- runtime/drivers/clickhouse/model_manager.go | 9 ++++++++- runtime/drivers/duckdb/model_manager.go | 9 ++++++++- runtime/metricsview/executor/executor_validate.go | 2 +- runtime/reconcilers/model.go | 13 ++++++++++++- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/runtime/drivers/clickhouse/model_manager.go b/runtime/drivers/clickhouse/model_manager.go index 5f9906f719ae..0a1e167cf2eb 100644 --- a/runtime/drivers/clickhouse/model_manager.go +++ b/runtime/drivers/clickhouse/model_manager.go @@ -2,6 +2,7 @@ package clickhouse import ( "context" + "errors" "fmt" "strings" @@ -282,7 +283,13 @@ func (c *Connection) Exists(ctx context.Context, res *drivers.ModelResult) (bool } _, err := olap.InformationSchema().Lookup(ctx, c.config.Database, "", res.Table) - return err == nil, nil + if err != nil { + if errors.Is(err, drivers.ErrNotFound) { + return false, nil + } + return false, err + } + return true, nil } func (c *Connection) Delete(ctx context.Context, res *drivers.ModelResult) error { diff --git a/runtime/drivers/duckdb/model_manager.go b/runtime/drivers/duckdb/model_manager.go index 77d773c33f25..6133c1e671bf 100644 --- a/runtime/drivers/duckdb/model_manager.go +++ b/runtime/drivers/duckdb/model_manager.go @@ -2,6 +2,7 @@ package duckdb import ( "context" + "errors" "fmt" "strings" @@ -137,7 +138,13 @@ func (c *connection) Exists(ctx context.Context, res *drivers.ModelResult) (bool } _, err := olap.InformationSchema().Lookup(ctx, "", "", res.Table) - return err == nil, nil + if err != nil { + if errors.Is(err, drivers.ErrNotFound) { + return false, nil + } + return false, err + } + return true, nil } func (c *connection) Delete(ctx context.Context, res *drivers.ModelResult) error { diff --git a/runtime/metricsview/executor/executor_validate.go b/runtime/metricsview/executor/executor_validate.go index 9a0ef88325cd..a44d2af05741 100644 --- a/runtime/metricsview/executor/executor_validate.go +++ b/runtime/metricsview/executor/executor_validate.go @@ -75,7 +75,7 @@ func (e *Executor) ValidateAndNormalizeMetricsView(ctx context.Context) (*Valida res.OtherErrs = append(res.OtherErrs, fmt.Errorf("table %q does not exist", mv.Table)) return res, nil } - return nil, fmt.Errorf("could not find table %q: %w", mv.Table, err) + return nil, fmt.Errorf("failed to look up table %q: %w", mv.Table, err) } // Populate empty database/databaseSchema from table metadata for StarRocks only. diff --git a/runtime/reconcilers/model.go b/runtime/reconcilers/model.go index b88bba67b491..b2b6852f5df1 100644 --- a/runtime/reconcilers/model.go +++ b/runtime/reconcilers/model.go @@ -277,8 +277,19 @@ func (r *ModelReconciler) Reconcile(ctx context.Context, n *runtimev1.ResourceNa var exists bool if prevManager != nil { exists, err = prevManager.Exists(ctx, prevResult) + // If the check errored, we can't know if the output is still there. + // Treating it as missing could trigger a destructive full reset on a transient failure, + // so we return the error instead and let the next reconcile retry. + // Exception: a manually triggered full refresh is an explicit instruction to rebuild, + // so we let it proceed as the escape hatch for a persistently failing check. + if err != nil && !model.Spec.TriggerFull { + return runtime.ReconcileResult{ + Err: fmt.Errorf("failed to check if model output exists (trigger a full refresh to rebuild anyway): %w", err), + Retrigger: refreshOn, + } + } if err != nil { - r.C.Logger.Warn("failed to check if model output exists", zap.String("model", n.Name), zap.Error(err), observability.ZapCtx(ctx)) + r.C.Logger.Warn("failed to check if model output exists, proceeding because a full refresh was manually triggered", zap.String("model", n.Name), zap.Error(err), observability.ZapCtx(ctx)) } } From d56a69bbc9cb58de3de08cdf50f6031c9b2f6684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Egelund-M=C3=BCller?= Date: Fri, 24 Jul 2026 13:11:19 +0200 Subject: [PATCH 2/2] Review --- runtime/reconcilers/model.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/runtime/reconcilers/model.go b/runtime/reconcilers/model.go index b2b6852f5df1..5a4750367739 100644 --- a/runtime/reconcilers/model.go +++ b/runtime/reconcilers/model.go @@ -282,13 +282,13 @@ func (r *ModelReconciler) Reconcile(ctx context.Context, n *runtimev1.ResourceNa // so we return the error instead and let the next reconcile retry. // Exception: a manually triggered full refresh is an explicit instruction to rebuild, // so we let it proceed as the escape hatch for a persistently failing check. - if err != nil && !model.Spec.TriggerFull { - return runtime.ReconcileResult{ - Err: fmt.Errorf("failed to check if model output exists (trigger a full refresh to rebuild anyway): %w", err), - Retrigger: refreshOn, - } - } if err != nil { + if !model.Spec.TriggerFull { + return runtime.ReconcileResult{ + Err: fmt.Errorf("failed to check if model output exists (trigger a full refresh to rebuild anyway): %w", err), + Retrigger: refreshOn, + } + } r.C.Logger.Warn("failed to check if model output exists, proceeding because a full refresh was manually triggered", zap.String("model", n.Name), zap.Error(err), observability.ZapCtx(ctx)) } }