From a3452842ec2c86a84c5f4db4ba5fd21a7df1de3e Mon Sep 17 00:00:00 2001
From: Cam Saul <cammsaul@gmail.com>
Date: Mon, 8 Apr 2019 13:10:28 -0700
Subject: [PATCH] Better error handling when saving TaskHistory fails :warning:
---
src/metabase/models/task_history.clj | 26 +++++++++++++---------
src/metabase/sync/util.clj | 25 ++++++++++++---------
src/metabase/task/task_history_cleanup.clj | 2 +-
3 files changed, 31 insertions(+), 22 deletions(-)
diff --git a/src/metabase/models/task_history.clj b/src/metabase/models/task_history.clj
index 7fe870220b1..0e058eccb49 100644
--- a/src/metabase/models/task_history.clj
+++ b/src/metabase/models/task_history.clj
@@ -1,8 +1,10 @@
(ns metabase.models.task-history
- (:require [metabase.models.interface :as i]
+ (:require [clojure.tools.logging :as log]
+ [metabase.models.interface :as i]
[metabase.util :as u]
[metabase.util
[date :as du]
+ [i18n :refer [trs]]
[schema :as su]]
[schema.core :as s]
[toucan
@@ -20,9 +22,9 @@
;; the date that task finished, it deletes everything after that. As we continue to add TaskHistory entries, this
;; ensures we'll have a good amount of history for debugging/troubleshooting, but not grow too large and fill the
;; disk.
- (when-let [clean-before-date (db/select-one-field :ended_at TaskHistory {:limit 1
- :offset num-rows-to-keep
- :order-by [[:ended_at :desc]]})]
+ (when-let [clean-before-date (db/select-one-field :ended_at TaskHistory {:limit 1
+ :offset num-rows-to-keep
+ :order-by [[:ended_at :desc]]})]
(db/simple-delete! TaskHistory :ended_at [:<= clean-before-date])))
(u/strict-extend (class TaskHistory)
@@ -36,7 +38,7 @@
(s/defn all
"Return all TaskHistory entries, applying `limit` and `offset` if not nil"
- [limit :- (s/maybe su/IntGreaterThanZero)
+ [limit :- (s/maybe su/IntGreaterThanZero)
offset :- (s/maybe su/IntGreaterThanOrEqualToZero)]
(db/select TaskHistory (merge {:order-by [[:ended_at :desc]]}
(when limit
@@ -58,11 +60,14 @@
(defn- save-task-history! [start-time-ms info]
(let [end-time-ms (System/currentTimeMillis)
duration-ms (- end-time-ms start-time-ms)]
- (db/insert! TaskHistory
- (assoc info
- :started_at (du/->Timestamp start-time-ms)
- :ended_at (du/->Timestamp end-time-ms)
- :duration duration-ms))))
+ (try
+ (db/insert! TaskHistory
+ (assoc info
+ :started_at (du/->Timestamp start-time-ms)
+ :ended_at (du/->Timestamp end-time-ms)
+ :duration duration-ms))
+ (catch Throwable e
+ (log/warn e (trs "Error saving task history"))))))
(s/defn do-with-task-history
"Impl for `with-task-history` macro; see documentation below."
@@ -85,6 +90,7 @@
"Execute `body`, recording a TaskHistory entry when the task completes; if it failed to complete, records an entry
containing information about the Exception. `info` should contain at least a name for the task (conventionally
lisp-cased) as `:task`; see the `TaskHistoryInfo` schema in this namespace for other optional keys.
+
(with-task-history {:task \"send-pulses\"}
...)"
{:style/indent 1}
diff --git a/src/metabase/sync/util.clj b/src/metabase/sync/util.clj
index 89260d16608..12a561fe521 100644
--- a/src/metabase/sync/util.clj
+++ b/src/metabase/sync/util.clj
@@ -411,23 +411,26 @@
[task-name :- su/NonBlankString
database :- i/DatabaseInstance
{:keys [start-time end-time]} :- SyncOperationOrStepRunMetadata]
- {:task task-name
- :db_id (u/get-id database)
+ {:task task-name
+ :db_id (u/get-id database)
:started_at (du/->Timestamp start-time)
- :ended_at (du/->Timestamp end-time)
- :duration (du/calculate-duration start-time end-time)})
+ :ended_at (du/->Timestamp end-time)
+ :duration (du/calculate-duration start-time end-time)})
(s/defn ^:private store-sync-summary!
[operation :- s/Str
database :- i/DatabaseInstance
{:keys [steps] :as sync-md} :- SyncOperationMetadata]
- (db/insert-many! TaskHistory
- (cons (create-task-history operation database sync-md)
- (for [[step-name step-info] steps
- :let [task-details (dissoc step-info :start-time :end-time :log-summary-fn)]]
- (assoc (create-task-history step-name database step-info)
- :task_details (when (seq task-details)
- task-details))))))
+ (try
+ (db/insert-many! TaskHistory
+ (cons (create-task-history operation database sync-md)
+ (for [[step-name step-info] steps
+ :let [task-details (dissoc step-info :start-time :end-time :log-summary-fn)]]
+ (assoc (create-task-history step-name database step-info)
+ :task_details (when (seq task-details)
+ task-details)))))
+ (catch Throwable e
+ (log/warn e (trs "Error saving task history")))))
(s/defn run-sync-operation
"Run `sync-steps` and log a summary message"
diff --git a/src/metabase/task/task_history_cleanup.clj b/src/metabase/task/task_history_cleanup.clj
index 45a5f6dfb77..53762c57029 100644
--- a/src/metabase/task/task_history_cleanup.clj
+++ b/src/metabase/task/task_history_cleanup.clj
@@ -9,7 +9,7 @@
[metabase.util.i18n :refer [trs]]))
(def ^:private history-rows-to-keep
- "Maximum number of TaskHistory rows. This is not a `const` so that we can redef it in tests"
+ "Maximum number of TaskHistory rows."
100000)
(defn- task-history-cleanup! []
--
GitLab