review comments

admin/worker/deployments_health_check.go

@@ -183,26 +183,26 @@ func (w *Worker) deploymentHealthCheck(ctx context.Context, d *database.Deployme
  // log metrics view errors
  logger := w.logger.WithLazy(f...)
  if health.ParseErrorCount > 0 || health.ReconcileErrorCount > 0 {
- logger.Error("deployment health check: project has parse/reconcile errors", zap.Int32("parse_errors", health.ParseErrorCount), zap.Int32("reconcile_errors", health.ReconcileErrorCount))
+ logger.Warn("deployment health check: project has parse/reconcile errors", zap.Int32("parse_errors", health.ParseErrorCount), zap.Int32("reconcile_errors", health.ReconcileErrorCount))
  }
  for d, err := range health.MetricsViewErrors {
- logger.Error("deployment health check: dashboard error", zap.String("dashboard", d), zap.String("error", err))
+ logger.Warn("deployment health check: metrics view error", zap.String("metrics_view", d), zap.String("error", err))
  }
 
- onlyUnhealthyDash := true
+ logError := false
  if health.OlapError != "" {
- onlyUnhealthyDash = false
+ logError = true
  f = append(f, zap.String("olap_error", health.OlapError))
  }
  if health.ControllerError != "" {
- onlyUnhealthyDash = false
+ logError = true
  f = append(f, zap.String("controller_error", health.ControllerError))
  }
  if health.RepoError != "" {
- onlyUnhealthyDash = false
+ logError = true
  f = append(f, zap.String("repo_error", health.RepoError))
  }
- if onlyUnhealthyDash {
+ if !logError {
  continue
  }
  w.logger.Error("deployment health check: runtime instance is unhealthy", f...)

runtime/health.go

@@ -15,18 +15,26 @@ type Health struct {
  InstancesHealth map[string]*InstanceHealth
 }
 
+// InstanceHealth contains health information for a single instance.
+// The information about OLAP and metrics views is cached in the catalog.
+// We want to avoid hitting the underlying OLAP engine when OLAP engine can scale to zero when no queries are generated within TTL.
+// We do not want to keep it running just to check health. In such cases, we use the cached health information.
 type InstanceHealth struct {
- // always recomputed
- Controller string
- Repo string
- ParseErrCount int
- ReconcileErrCount int
-
- // cached
- OLAP string
- MetricsViews map[string]metricsViewHealth
+ // always recomputed on every health check
+ Controller string `json:"-"`
+ Repo string `json:"-"`
+ ParseErrCount int `json:"-"`
+ ReconcileErrCount int `json:"-"`
+
+ // cached health check information can be used if controller version is same and metrics view spec is same
+ OLAP string `json:"olap"`
+ MetricsViews map[string]InstanceHealthMetricsViewError `json:"metrics_views"`
+ ControllerVersion int64 `json:"controller_version"`
+}
 
- ControllerVersion int64
+type InstanceHealthMetricsViewError struct {
+ Err string `json:"err"`
+ Version int64 `json:"version"`
 }
 
 func (r *Runtime) Health(ctx context.Context) (*Health, error) {
@@ -91,22 +99,26 @@ func (r *Runtime) InstanceHealth(ctx context.Context, instanceID string) (*Insta
  release()
  }
 
- // run queries against metrics views
- resources, err := ctrl.List(ctx, ResourceKindMetricsView, "", false)
+ // check resources with reconcile errors
+ resources, err := ctrl.List(ctx, "", "", false)
  if err != nil {
  return nil, err
  }
- res.MetricsViews = make(map[string]metricsViewHealth, len(resources))
+ for _, r := range resources {
+ if r.Meta.ReconcileError != "" {
+ res.ReconcileErrCount++
+ }
+ }
+
+ // run queries against metrics views
+ res.MetricsViews = make(map[string]InstanceHealthMetricsViewError, len(resources))
  for _, mv := range resources {
- if mv.GetMetricsView().State.ValidSpec == nil {
- if mv.Meta.ReconcileError != "" {
- res.ReconcileErrCount++
- }
+ if mv.GetMetricsView() == nil || mv.GetMetricsView().State.ValidSpec == nil {
  continue
  }
  olap, release, err := r.OLAP(ctx, instanceID, mv.GetMetricsView().State.ValidSpec.Connector)
  if err != nil {
- res.MetricsViews[mv.Meta.Name.Name] = metricsViewHealth{err: err.Error()}
+ res.MetricsViews[mv.Meta.Name.Name] = InstanceHealthMetricsViewError{Err: err.Error()}
  release()
  continue
  }
@@ -130,17 +142,15 @@ func (r *Runtime) InstanceHealth(ctx context.Context, instanceID string) (*Insta
  Claims: &SecurityClaims{SkipChecks: true},
  })
 
- mvHealth := metricsViewHealth{Version: mv.Meta.StateVersion}
+ mvHealth := InstanceHealthMetricsViewError{
+ Version: mv.Meta.StateVersion,
+ }
  if err != nil {
- mvHealth.err = err.Error()
+ mvHealth.Err = err.Error()
  }
  res.MetricsViews[mv.Meta.Name.Name] = mvHealth
  }
 
- if !canScaleToZero {
- return res, nil
- }
-
  // save to cache
  res.ControllerVersion = ctrl.catalog.version
  bytes, err := json.Marshal(res)
@@ -156,7 +166,7 @@ func (r *Runtime) InstanceHealth(ctx context.Context, instanceID string) (*Insta
 
  err = catalog.UpsertInstanceHealth(ctx, &drivers.InstanceHealth{
  InstanceID: instanceID,
- Health:  bytes,
+ HealthJSON: bytes,
  })
  if err != nil {
  return nil, err
@@ -178,7 +188,7 @@ func (r *Runtime) cachedInstanceHealth(ctx context.Context, instanceID string, c
  }
 
  c := &InstanceHealth{}
- err = json.Unmarshal(cached.Health, c)
+ err = json.Unmarshal(cached.HealthJSON, c)
  if err != nil || ctrlVersion != c.ControllerVersion {
  return nil, false
  }
@@ -219,14 +229,9 @@ func (h *InstanceHealth) To() *runtimev1.InstanceHealth {
  }
  r.MetricsViewErrors = make(map[string]string, len(h.MetricsViews))
  for k, v := range h.MetricsViews {
- if v.err != "" {
- r.MetricsViewErrors[k] = v.err
+ if v.Err != "" {
+ r.MetricsViewErrors[k] = v.Err
  }
  }
  return r
 }
-
-type metricsViewHealth struct {
- err string
- Version int64
-}