Merge pull request #2570 from metabase/slow-card-messages

Slow dashboard card warnings

frontend/src/components/LoadingSpinner.jsx

@@ -4,18 +4,17 @@ import "./LoadingSpinner.css";
 
 export default class LoadingSpinner extends Component {
     static defaultProps = {
-        width: '32px',
-        height: '32px',
-        borderWidth: '4px',
+        size: 32,
+        borderWidth: 4,
         fill: 'currentcolor',
         spinnerClassName: 'LoadingSpinner'
     };
 
     render() {
-        var { width, height, borderWidth, className, spinnerClassName } = this.props;
+        var { size, borderWidth, className, spinnerClassName } = this.props;
         return (
             <div className={className}>
-                <div className={spinnerClassName} style={{ width, height, borderWidth }}></div>
+                <div className={spinnerClassName} style={{ width: size, height: size, borderWidth }}></div>
             </div>
         );
     }

frontend/src/components/SaveStatus.jsx

@@ -35,7 +35,7 @@ export default class SaveStatus extends Component {
 
     render() {
         if (this.state.saving) {
-            return (<div className="SaveStatus mx2 px2 border-right"><LoadingSpinner width="24" height="24" /></div>);
+            return (<div className="SaveStatus mx2 px2 border-right"><LoadingSpinner size={24} /></div>);
         } else if (this.state.error) {
             return (<div className="SaveStatus mx2 px2 border-right text-error">Error: {this.state.error}</div>)
         } else if (this.state.recentlySavedTimeout != null) {

frontend/src/css/dashboard.css

@@ -106,6 +106,10 @@
     height: 100%;
 }
 
+.DashCard .Card.Card--slow {
+    border-color: var(--gold-color);
+}
+
 .Dash--editing .DashCard .Card {
     pointer-events: none;
     transition: border .3s, background-color .3s;

frontend/src/dashboard/actions.js

@@ -6,12 +6,12 @@ import { normalize, Schema, arrayOf } from "normalizr";
 
 import moment from "moment";
 import { augmentDatabase } from "metabase/lib/table";
-import { delay } from "metabase/lib/promise";
 
 import MetabaseAnalytics from "metabase/lib/analytics";
 import { getPositionForNewDashCard } from "metabase/lib/dashboard_grid";
 
-const DATASET_TIMEOUT = 60;
+const DATASET_SLOW_TIMEOUT   = 15 * 1000;
+const DATASET_USUALLY_FAST_THRESHOLD = 15 * 1000;
 
 // normalizr schemas
 const dashcard = new Schema('dashcard');
@@ -40,6 +40,7 @@ export const SET_DASHCARD_ATTRIBUTES = 'SET_DASHCARD_ATTRIBUTES';
 export const SAVE_DASHCARD = 'SAVE_DASHCARD';
 
 export const FETCH_CARD_DATA = 'FETCH_CARD_DATA';
+export const FETCH_CARD_DURATION = 'FETCH_CARD_DURATION';
 export const FETCH_REVISIONS = 'FETCH_REVISIONS';
 export const REVERT_TO_REVISION = 'REVERT_TO_REVISION';
 
@@ -49,7 +50,7 @@ export const FETCH_DATABASE_METADATA = 'FETCH_DATABASE_METADATA';
 
 // resource wrappers
 const DashboardApi = new AngularResourceProxy("Dashboard", ["get", "update", "delete", "reposition_cards", "addcard", "removecard"]);
-const MetabaseApi = new AngularResourceProxy("Metabase", ["dataset", "db_metadata"]);
+const MetabaseApi = new AngularResourceProxy("Metabase", ["dataset", "dataset_duration", "db_metadata"]);
 const CardApi = new AngularResourceProxy("Card", ["list", "update", "delete"]);
 const RevisionApi = new AngularResourceProxy("Revision", ["list", "revert"]);
 
@@ -99,17 +100,28 @@ export const removeCardFromDashboard = createAction(REMOVE_CARD_FROM_DASH);
 
 export const fetchCardData = createThunkAction(FETCH_CARD_DATA, function(card) {
     return async function(dispatch, getState) {
-        let timeout = delay(DATASET_TIMEOUT * 1000);
-        try {
-            let result = await MetabaseApi.dataset({ timeout }, card.dataset_query);
-            return { id: card.id, result };
-        } catch (error) {
-            if (error && error.status === 0) {
-                throw "Card took longer than " + DATASET_TIMEOUT + " seconds to load.";
-            } else {
-                throw error;
+        let result = null;
+        let slowCardTimer = setTimeout(() => {
+            if (result === null) {
+                dispatch(fetchCardDuration(card));
             }
-        }
+        }, DATASET_SLOW_TIMEOUT);
+        result = await MetabaseApi.dataset(card.dataset_query);
+        clearTimeout(slowCardTimer);
+        return { id: card.id, result };
+    };
+});
+
+export const fetchCardDuration = createThunkAction(FETCH_CARD_DURATION, function(card) {
+    return async function(dispatch, getState) {
+        let result = await MetabaseApi.dataset_duration(card.dataset_query);
+        return {
+            id: card.id,
+            result: {
+                fast_threshold: DATASET_USUALLY_FAST_THRESHOLD,
+                ...result
+            }
+        };
     };
 });
 

frontend/src/dashboard/components/DashCard.jsx

@@ -30,7 +30,7 @@ export default class DashCard extends Component {
     };
 
     async componentDidMount() {
-        const { dashcard } = this.props;
+        const { dashcard, markNewCardSeen, fetchCardData } = this.props;
 
         this.visibilityTimer = window.setInterval(this.updateVisibility, 2000);
         window.addEventListener("scroll", this.updateVisibility, false);
@@ -38,15 +38,12 @@ export default class DashCard extends Component {
         // HACK: way to scroll to a newly added card
         if (dashcard.justAdded) {
             ReactDOM.findDOMNode(this).scrollIntoView();
-            this.props.markNewCardSeen(dashcard.id);
+            markNewCardSeen(dashcard.id);
         }
 
+        let cards = [dashcard.card].concat(...(dashcard.series || []));
         try {
-            await Promise.all([
-                this.props.fetchCardData(dashcard.card)
-            ].concat(
-                dashcard.series && dashcard.series.map(this.props.fetchCardData)
-            ));
+            await Promise.all(cards.map(fetchCardData));
         } catch (error) {
             console.error("DashCard error", error)
             this.setState({ error });
@@ -73,7 +70,7 @@ export default class DashCard extends Component {
     }
 
     render() {
-        const { dashcard, cardData, isEditing, onAddSeries, onRemove } = this.props;
+        const { dashcard, cardData, cardDurations, isEditing, onAddSeries, onRemove } = this.props;
 
         const cards = [dashcard.card].concat(dashcard.series || []);
         const series = cards
@@ -81,7 +78,14 @@ export default class DashCard extends Component {
                 card: card,
                 data: cardData[card.id] && cardData[card.id].data,
                 error: cardData[card.id] && cardData[card.id].error,
+                duration: cardDurations[card.id]
             }));
+
+        const loading = !(series.length > 0 && _.every(series, (s) => s.data));
+        const expectedDuration = Math.max(...series.map((s) => s.duration ? s.duration.average : 0));
+        const usuallyFast = _.every(series, (s) => s.duration && s.duration.average < s.duration.fast_threshold);
+        const isSlow = loading && _.some(series, (s) => s.duration) && (usuallyFast ? "usually-fast" : "usually-slow");
+
         const errors = series.map(s => s.error).filter(e => e);
         const error = errors[0] || this.state.error;
 
@@ -100,10 +104,12 @@ export default class DashCard extends Component {
 
         const CardVisualization = visualizations.get(series[0].card.display);
         return (
-            <div className={"Card bordered rounded flex flex-column " + cx({ "Card--recent": dashcard.isAdded })}>
+            <div className={"Card bordered rounded flex flex-column " + cx({ "Card--recent": dashcard.isAdded, "Card--slow": isSlow === "usually-slow" })}>
                 <Visualization
                     className="flex-full"
                     error={errorMessage}
+                    isSlow={isSlow}
+                    expectedDuration={expectedDuration}
                     series={series}
                     isDashboard={true}
                     isEditing={isEditing}

frontend/src/dashboard/components/DashboardGrid.jsx

@@ -210,6 +210,7 @@ export default class DashboardGrid extends Component {
                         <DashCard
                             dashcard={dc}
                             cardData={this.props.cardData}
+                            cardDurations={this.props.cardDurations}
                             fetchCardData={this.props.fetchCardData}
                             markNewCardSeen={this.props.markNewCardSeen}
                             isEditing={isEditing}
@@ -246,6 +247,7 @@ export default class DashboardGrid extends Component {
                         <DashCard
                             dashcard={dc}
                             cardData={this.props.cardData}
+                            cardDurations={this.props.cardDurations}
                             fetchCardData={this.props.fetchCardData}
                             markNewCardSeen={this.props.markNewCardSeen}
                             isEditing={isEditing}

frontend/src/dashboard/reducers.js

@@ -8,6 +8,7 @@ import {
     SET_EDITING_DASHBOARD,
     FETCH_DASHBOARD,
     FETCH_CARD_DATA,
+    FETCH_CARD_DURATION,
     SET_DASHBOARD_ATTRIBUTES,
     SET_DASHCARD_ATTRIBUTES,
     SET_DASHCARD_VISUALIZATION_SETTING,
@@ -86,6 +87,10 @@ export const cardData = handleActions({
     [FETCH_CARD_DATA]: { next: (state, { payload: { id, result }}) => ({ ...state, [id]: result }) }
 }, {});
 
+export const cardDurations = handleActions({
+    [FETCH_CARD_DURATION]: { next: (state, { payload: { id, result }}) => ({ ...state, [id]: result }) }
+}, {});
+
 const databases = handleActions({
     [FETCH_DATABASE_METADATA]: { next: (state, { payload }) => ({ ...state, [payload.id]: payload }) }
 }, {});

frontend/src/dashboard/selectors.js

@@ -8,6 +8,7 @@ const cardsSelector             = state => state.cards;
 const dashboardsSelector        = state => state.dashboards;
 const dashcardsSelector         = state => state.dashcards;
 const cardDataSelector          = state => state.cardData;
+const cardDurationsSelector     = state => state.cardDurations;
 const cardIdListSelector        = state => state.cardList;
 const revisionsSelector         = state => state.revisions;
 
@@ -45,6 +46,6 @@ const cardListSelector = createSelector(
 );
 
 export const dashboardSelectors = createSelector(
-    [isEditingSelector, isDirtySelector, selectedDashboardSelector, dashboardCompleteSelector, cardListSelector, revisionsSelector, cardDataSelector, databasesSelector],
-    (isEditing, isDirty, selectedDashboard, dashboard, cards, revisions, cardData, databases) => ({ isEditing, isDirty, selectedDashboard, dashboard, cards, revisions, cardData, databases })
+    [isEditingSelector, isDirtySelector, selectedDashboardSelector, dashboardCompleteSelector, cardListSelector, revisionsSelector, cardDataSelector, cardDurationsSelector, databasesSelector],
+    (isEditing, isDirty, selectedDashboard, dashboard, cards, revisions, cardData, cardDurations, databases) => ({ isEditing, isDirty, selectedDashboard, dashboard, cards, revisions, cardData, cardDurations, databases })
 );

frontend/src/lib/formatting.js

@@ -138,6 +138,16 @@ export function humanize(...args) {
     return inflection.humanize(...args);
 }
 
+export function duration(milliseconds) {
+    if (milliseconds < 60000) {
+        let seconds = Math.round(milliseconds / 1000);
+        return seconds + " " + inflect("second", seconds);
+    } else {
+        let minutes = Math.round(milliseconds / 1000 / 60);
+        return minutes + " " + inflect("minute", minutes);
+    }
+}
+
 // Removes trailing "id" from field names
 export function stripId(name) {
     return name && name.replace(/ id$/i, "");

frontend/src/query_builder/QueryVisualizationObjectDetailTable.jsx

@@ -111,7 +111,7 @@ export default class QueryVisualizationObjectDetailTable extends Component {
         }).map(function(fk) {
 
             var fkCount = (
-                <LoadingSpinner width="25px" height="25px" />
+                <LoadingSpinner size={25} />
             ),
                 fkCountValue = 0,
                 fkClickable = false;

frontend/src/services.js

@@ -593,6 +593,10 @@ CoreServices.factory('Metabase', ['$resource', '$cookies', 'MetabaseCore', funct
                 delete this.then;
                 resolve(this);
             }
+        },
+        dataset_duration: {
+            url: '/api/dataset/duration',
+            method: 'POST'
         }
     });
 }]);

frontend/src/visualizations/components/Visualization.jsx

@@ -6,6 +6,8 @@ import LoadingSpinner from "metabase/components/LoadingSpinner.jsx";
 import Icon from "metabase/components/Icon.jsx";
 import Tooltip from "metabase/components/Tooltip.jsx";
 
+import { duration } from "metabase/lib/formatting";
+
 import visualizations from "metabase/visualizations";
 
 import i from "icepick";
@@ -67,7 +69,7 @@ export default class Visualization extends Component {
     }
 
     render() {
-        const { series, actionButtons, className, isDashboard, width } = this.props;
+        const { series, actionButtons, className, isDashboard, width, isSlow, expectedDuration } = this.props;
         const CardVisualization = visualizations.get(series[0].card.display);
         const small = width < 330;
 
@@ -88,13 +90,20 @@ export default class Visualization extends Component {
             }
         }
 
+        let extra;
+        if (!loading) {
+            extra = actionButtons;
+        } else if (isSlow) {
+            extra = <LoadingSpinner className={isSlow === "usually-slow" ? "text-gold" : "text-slate"} size={18} />
+        }
+
         return (
             <div className={cx(className, "flex flex-column")}>
                 { isDashboard && (loading || error || !CardVisualization.noHeader) ?
                     <div className="p1 flex-no-shrink">
                         <LegendHeader
                             series={series}
-                            actionButtons={actionButtons}
+                            actionButtons={extra}
                         />
                     </div>
                 : null
@@ -112,10 +121,24 @@ export default class Visualization extends Component {
                     </div>
                 : loading ?
                     <div className="flex-full p1 text-centered text-brand flex flex-column layout-centered">
-                        <LoadingSpinner />
-                        <span className="h4 text-bold ml1 text-slate-light">
-                            Loading...
-                        </span>
+                        { isSlow ?
+                            <div className="text-slate">
+                                <div className="h4 text-bold mb1">Still Waiting...</div>
+                                { isSlow === "usually-slow" ?
+                                    <div>
+                                        This usually takes an average of <span style={{whiteSpace: "nowrap"}}>{duration(expectedDuration)}</span>.
+                                        <br />
+                                        (This is a bit long for a dashboard)
+                                    </div>
+                                :
+                                    <div>
+                                        This is usually pretty fast, but seems to be taking awhile right now.
+                                    </div>
+                                }
+                            </div>
+                        :
+                            <LoadingSpinner className="text-slate" />
+                        }
                     </div>
                 :
                     <CardVisualization

src/metabase/api/dataset.clj

 (ns metabase.api.dataset
   "/api/dataset endpoints."
   (:require [clojure.data.csv :as csv]
+            [cheshire.core :as json]
             [compojure.core :refer [GET POST]]
+            [korma.core :as k]
             [metabase.api.common :refer :all]
             [metabase.db :as db]
             (metabase.models [card :refer [Card]]
                              [database :refer [Database]]
-                             [hydrate :refer [hydrate]])
+                             [hydrate :refer [hydrate]]
+                             [query-execution :refer [QueryExecution]])
             [metabase.query-processor :as qp]
             [metabase.util :as u]))
 
@@ -31,6 +34,18 @@
   (let [query (assoc body :constraints dataset-query-api-constraints)]
     (qp/dataset-query query {:executed_by *current-user-id*})))
 
+ (defendpoint POST "/duration"
+   "Get historical query execution duration."
+   [:as {{:keys [database] :as body} :body}]
+   (read-check Database database)
+   ;; add sensible constraints for results limits on our query
+   (let [query (assoc body :constraints dataset-query-api-constraints)]
+     (first (k/select [(k/subselect QueryExecution
+                        (k/fields :running_time)
+                        (k/where {:json_query (json/generate-string query)})
+                        (k/order :started_at :desc)
+                        (k/limit 10)) :_]
+              (k/aggregate (avg :running_time) :average)))))
 
 (defendpoint POST "/csv"
   "Execute an MQL query and download the result data as a CSV file."