You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@superset.apache.org by GitBox <gi...@apache.org> on 2018/04/23 05:38:10 UTC
[GitHub] mistercrunch closed pull request #3109: Long running queries are
set to timeout (#2926)
mistercrunch closed pull request #3109: Long running queries are set to timeout (#2926)
URL: https://github.com/apache/incubator-superset/pull/3109
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/superset/assets/javascripts/SqlLab/actions.js b/superset/assets/javascripts/SqlLab/actions.js
index 4203a75742..6073ddc23a 100644
--- a/superset/assets/javascripts/SqlLab/actions.js
+++ b/superset/assets/javascripts/SqlLab/actions.js
@@ -27,6 +27,7 @@ export const ADD_ALERT = 'ADD_ALERT';
export const REMOVE_ALERT = 'REMOVE_ALERT';
export const REFRESH_QUERIES = 'REFRESH_QUERIES';
export const RUN_QUERY = 'RUN_QUERY';
+export const QUERY_TIMEOUT = 'QUERY_TIMEOUT';
export const START_QUERY = 'START_QUERY';
export const STOP_QUERY = 'STOP_QUERY';
export const REQUEST_QUERY_RESULTS = 'REQUEST_QUERY_RESULTS';
@@ -115,6 +116,10 @@ export function fetchQueryResults(query) {
};
}
+export function queryTimeout(query) {
+ return { type: QUERY_TIMEOUT, query };
+}
+
export function runQuery(query) {
return function (dispatch) {
dispatch(startQuery(query));
@@ -184,6 +189,26 @@ export function postStopQuery(query) {
};
}
+export function timeOutQuery(query) {
+ return function (dispatch) {
+ const timeoutQueryUrl = '/superset/timeout_query/';
+ const timeoutQueryRequestData = { client_id: query.id };
+ dispatch(queryTimeout(query));
+ $.ajax({
+ type: 'POST',
+ dataType: 'json',
+ url: timeoutQueryUrl,
+ data: timeoutQueryRequestData,
+ success() {
+ notify.success('Query was timedout.');
+ },
+ error() {
+ notify.error('Failed at timeing out query.');
+ },
+ });
+ };
+}
+
export function setDatabases(databases) {
return { type: SET_DATABASES, databases };
}
diff --git a/superset/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx b/superset/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx
index 892b7caf52..198ce81fe1 100644
--- a/superset/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx
+++ b/superset/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx
@@ -2,6 +2,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
+import { now, addXHours } from '../../modules/dates';
import * as Actions from '../actions';
const $ = require('jquery');
@@ -16,11 +17,18 @@ class QueryAutoRefresh extends React.PureComponent {
componentWillUnmount() {
this.stopTimer();
}
+ setPotentialTimeout(query) {
+ // if a running query is over 24 hrs ago, set the state timeout
+ if (query.state === 'running' && addXHours(query.startDttm, 24) < now()) {
+ this.props.actions.timeOutQuery(query);
+ }
+ }
shouldCheckForQueries() {
// if there are started or running queries, this method should return true
const { queries } = this.props;
const queryKeys = Object.keys(queries);
const queriesAsArray = queryKeys.map(key => queries[key]);
+ queriesAsArray.map(q => this.setPotentialTimeout(q));
return queriesAsArray.some(q =>
['running', 'started', 'pending', 'fetching'].indexOf(q.state) >= 0);
}
diff --git a/superset/assets/javascripts/SqlLab/components/QuerySearch.jsx b/superset/assets/javascripts/SqlLab/components/QuerySearch.jsx
index 44daa3920b..2be84dada9 100644
--- a/superset/assets/javascripts/SqlLab/components/QuerySearch.jsx
+++ b/superset/assets/javascripts/SqlLab/components/QuerySearch.jsx
@@ -4,7 +4,7 @@ import { Button } from 'react-bootstrap';
import Select from 'react-select';
import QueryTable from './QueryTable';
import { now, epochTimeXHoursAgo,
- epochTimeXDaysAgo, epochTimeXYearsAgo } from '../../modules/dates';
+ epochTimeXDaysAgo, epochTimeXYearsAgo, addXHours } from '../../modules/dates';
import { STATUS_OPTIONS, TIME_OPTIONS } from '../constants';
import AsyncSelect from '../../components/AsyncSelect';
@@ -64,6 +64,18 @@ class QuerySearch extends React.PureComponent {
return null;
}
}
+ setPotentialTimeout() {
+ const url = this.insertParams('/superset/search_queries', ['status=running']);
+ $.getJSON(url, (data, status) => {
+ if (status === 'success') {
+ data.forEach(function (query) {
+ if (addXHours(query.startDttm, 24) < now()) {
+ this.props.actions.timeOutQuery(query);
+ }
+ });
+ }
+ });
+ }
changeFrom(user) {
const val = (user) ? user.value : null;
this.setState({ from: val });
@@ -108,6 +120,7 @@ class QuerySearch extends React.PureComponent {
return options;
}
refreshQueries() {
+ this.setPotentialTimeout();
this.setState({ queriesLoading: true });
const params = [
this.state.userId ? `user_id=${this.state.userId}` : '',
diff --git a/superset/assets/javascripts/SqlLab/constants.js b/superset/assets/javascripts/SqlLab/constants.js
index 2a9327509e..a254d7377a 100644
--- a/superset/assets/javascripts/SqlLab/constants.js
+++ b/superset/assets/javascripts/SqlLab/constants.js
@@ -2,6 +2,7 @@ export const STATE_BSSTYLE_MAP = {
failed: 'danger',
pending: 'info',
fetching: 'info',
+ timed_out: 'warning',
running: 'warning',
stopped: 'danger',
success: 'success',
@@ -11,6 +12,7 @@ export const STATUS_OPTIONS = [
'success',
'failed',
'running',
+ 'timed_out',
];
export const TIME_OPTIONS = [
diff --git a/superset/assets/javascripts/SqlLab/reducers.js b/superset/assets/javascripts/SqlLab/reducers.js
index 7bef4546c5..6dd86f6c49 100644
--- a/superset/assets/javascripts/SqlLab/reducers.js
+++ b/superset/assets/javascripts/SqlLab/reducers.js
@@ -179,6 +179,9 @@ export const sqlLabReducer = function (state, action) {
};
return alterInObject(state, 'queries', action.query, alts);
},
+ [actions.QUERY_TIMEOUT]() {
+ return alterInObject(state, 'queries', action.query, { state: 'timed_out' });
+ },
[actions.QUERY_FAILED]() {
if (action.query.state === 'stopped') {
return state;
diff --git a/superset/assets/javascripts/modules/dates.js b/superset/assets/javascripts/modules/dates.js
index 2c2815e8a1..95bbf64f6c 100644
--- a/superset/assets/javascripts/modules/dates.js
+++ b/superset/assets/javascripts/modules/dates.js
@@ -78,6 +78,14 @@ export const now = function () {
return moment().utc().valueOf();
};
+export const addXHours = function (date, h) {
+ // add X hours to given utc date
+ return moment(date)
+ .add(h, 'hours')
+ .utc()
+ .valueOf();
+};
+
export const epochTimeXHoursAgo = function (h) {
return moment()
.subtract(h, 'hours')
diff --git a/superset/assets/spec/javascripts/modules/dates_spec.js b/superset/assets/spec/javascripts/modules/dates_spec.js
index b073921a43..105d20098f 100644
--- a/superset/assets/spec/javascripts/modules/dates_spec.js
+++ b/superset/assets/spec/javascripts/modules/dates_spec.js
@@ -5,6 +5,7 @@ import {
formatDate,
fDuration,
now,
+ addXHours,
epochTimeXHoursAgo,
epochTimeXDaysAgo,
epochTimeXYearsAgo,
@@ -47,6 +48,24 @@ describe('now', () => {
});
});
+describe('addXHours', () => {
+ it('is a function', () => {
+ assert.isFunction(addXHours);
+ });
+
+ it('returns a number', () => {
+ expect(addXHours(0, 0)).to.be.a('number');
+ });
+
+ it('returns the expected output', () => {
+ expect(addXHours(1496293608897, 24)).to.equal(1496380008897);
+ });
+
+ it('returns the expected output', () => {
+ expect(addXHours(1496293608897, 0)).to.equal(1496293608897);
+ });
+});
+
describe('epochTimeXHoursAgo', () => {
it('is a function', () => {
assert.isFunction(epochTimeXHoursAgo);
diff --git a/superset/assets/spec/javascripts/sqllab/QuerySearch_spec.jsx b/superset/assets/spec/javascripts/sqllab/QuerySearch_spec.jsx
index 3892745966..df63e1a3bf 100644
--- a/superset/assets/spec/javascripts/sqllab/QuerySearch_spec.jsx
+++ b/superset/assets/spec/javascripts/sqllab/QuerySearch_spec.jsx
@@ -65,4 +65,12 @@ describe('QuerySearch', () => {
/* eslint-disable no-unused-expressions */
expect(search.called).to.equal(true);
});
+
+ it('check for query timeouts prior to query refresh', () => {
+ const search = sinon.spy(QuerySearch.prototype, 'setPotentialTimeout');
+ wrapper = shallow(<QuerySearch {...mockedProps} />);
+ wrapper.find(Button).simulate('click');
+ /* eslint-disable no-unused-expressions */
+ expect(search.called).to.equal(true);
+ });
});
diff --git a/superset/assets/spec/javascripts/sqllab/actions_spec.js b/superset/assets/spec/javascripts/sqllab/actions_spec.js
index 3a957b8f28..da3f1b6e96 100644
--- a/superset/assets/spec/javascripts/sqllab/actions_spec.js
+++ b/superset/assets/spec/javascripts/sqllab/actions_spec.js
@@ -127,4 +127,33 @@ describe('async actions', () => {
expect(ajaxStub.getCall(0).args[0].data).to.deep.equal(data);
});
});
+
+ describe('timeoutQuery', () => {
+ const makeRequest = () => {
+ const request = actions.timeOutQuery(query);
+ request(dispatch);
+ };
+
+ it('makes the ajax request', () => {
+ makeRequest();
+ expect(ajaxStub.calledOnce).to.be.true;
+ });
+
+ it('calls timeOutQuery', () => {
+ makeRequest();
+ expect(dispatch.args[0][0].type).to.equal(actions.QUERY_TIMEOUT);
+ });
+
+ it('calls the correct url', () => {
+ const url = '/superset/timeout_query/';
+ makeRequest();
+ expect(ajaxStub.getCall(0).args[0].url).to.equal(url);
+ });
+
+ it('sends the correct data', () => {
+ const data = { client_id: query.id };
+ makeRequest();
+ expect(ajaxStub.getCall(0).args[0].data).to.deep.equal(data);
+ });
+ });
});
diff --git a/superset/sql_lab.py b/superset/sql_lab.py
index 4b0bd863bc..8328a420a8 100644
--- a/superset/sql_lab.py
+++ b/superset/sql_lab.py
@@ -196,7 +196,8 @@ def handle_error(msg):
conn.commit()
conn.close()
- if query.status == utils.QueryStatus.STOPPED:
+ if query.status == utils.QueryStatus.STOPPED or query.status == \
+ utils.QueryStatus.TIMED_OUT:
return json.dumps({
'query_id': query.id,
'status': query.status,
diff --git a/superset/views/core.py b/superset/views/core.py
index 8a5de069ef..9e264818f1 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -1957,6 +1957,22 @@ def stop_query(self):
pass
return self.json_response('OK')
+ @has_access_api
+ @expose("/timeout_query/", methods=['POST'])
+ @log_this
+ def timeout_query(self):
+ client_id = request.form.get('client_id')
+ try:
+ query = (
+ db.session.query(Query)
+ .filter_by(client_id=client_id).one()
+ )
+ query.status = utils.QueryStatus.TIMED_OUT
+ db.session.commit()
+ except Exception as e:
+ pass
+ return self.json_response('OK')
+
@has_access_api
@expose("/sql_json/", methods=['POST', 'GET'])
@log_this
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services