Show facets that timed out using new InterruptedError

If the user requests some _facet= options that do not successfully execute in
the configured facet_time_limit_ms, we now show a warning message like this:

    These facets timed out: rowid, Title

To build this I had to clean up our SQLite interrupted logic. We now raise a
custom InterruptedError exception when SQLite terminates due to exceeding a
time limit.

In implementing this I found and fixed a logic error where invalid SQL was
being generated in some cases for our faceting calculations but the resulting
sqlite3.OperationalError had been incorrectly captured and treated as a
timeout.

Refs #255
Closes #269
This commit is contained in:
Simon Willison 2018-05-17 23:07:45 -07:00
commit 08f4b7658f
No known key found for this signature in database
GPG key ID: 17E2DEA2588B7F52
4 changed files with 34 additions and 16 deletions

View file

@ -100,6 +100,10 @@
</p> </p>
{% endif %} {% endif %}
{% if facets_timed_out %}
<p class="facets-timed-out">These facets timed out: {{ ", ".join(facets_timed_out) }}</p>
{% endif %}
{% if facet_results %} {% if facet_results %}
<div class="facet-results"> <div class="facet-results">
{% for facet_info in sorted_facet_results %} {% for facet_info in sorted_facet_results %}

View file

@ -32,6 +32,10 @@ reserved_words = set((
).split()) ).split())
class InterruptedError(Exception):
pass
def urlsafe_components(token): def urlsafe_components(token):
"Splits token on commas and URL decodes each component" "Splits token on commas and URL decodes each component"
return [ return [

View file

@ -13,6 +13,7 @@ from sanic.views import HTTPMethodView
from datasette import __version__ from datasette import __version__
from datasette.utils import ( from datasette.utils import (
CustomJSONEncoder, CustomJSONEncoder,
InterruptedError,
InvalidSql, InvalidSql,
path_from_row_pks, path_from_row_pks,
path_with_added_args, path_with_added_args,
@ -170,7 +171,9 @@ class BaseView(RenderMixin):
else: else:
rows = cursor.fetchall() rows = cursor.fetchall()
truncated = False truncated = False
except Exception as e: except sqlite3.OperationalError as e:
if e.args == ('interrupted',):
raise InterruptedError(e)
print( print(
"ERROR: conn={}, sql = {}, params = {}: {}".format( "ERROR: conn={}, sql = {}, params = {}: {}".format(
conn, repr(sql), params, e conn, repr(sql), params, e
@ -216,6 +219,8 @@ class BaseView(RenderMixin):
else: else:
data, extra_template_data, templates = response_or_template_contexts data, extra_template_data, templates = response_or_template_contexts
except InterruptedError as e:
raise DatasetteError(str(e), title="SQL Interrupted", status=400)
except (sqlite3.OperationalError, InvalidSql) as e: except (sqlite3.OperationalError, InvalidSql) as e:
raise DatasetteError(str(e), title="Invalid SQL", status=400) raise DatasetteError(str(e), title="Invalid SQL", status=400)

View file

@ -7,6 +7,7 @@ from sanic.request import RequestParameters
from datasette.utils import ( from datasette.utils import (
Filters, Filters,
InterruptedError,
compound_keys_after_sql, compound_keys_after_sql,
escape_sqlite, escape_sqlite,
filters_should_redirect, filters_should_redirect,
@ -16,7 +17,7 @@ from datasette.utils import (
path_with_removed_args, path_with_removed_args,
path_with_replaced_args, path_with_replaced_args,
to_css_class, to_css_class,
urlsafe_components urlsafe_components,
) )
from .base import BaseView, DatasetteError, ureg from .base import BaseView, DatasetteError, ureg
@ -75,8 +76,7 @@ class RowTableShared(BaseView):
results = await self.execute( results = await self.execute(
database, sql, list(set(values)) database, sql, list(set(values))
) )
except sqlite3.OperationalError: except InterruptedError:
# Probably hit the timelimit
pass pass
else: else:
for id, value in results: for id, value in results:
@ -135,8 +135,7 @@ class RowTableShared(BaseView):
results = await self.execute( results = await self.execute(
database, sql, list(set(ids_to_lookup)) database, sql, list(set(ids_to_lookup))
) )
except sqlite3.OperationalError: except InterruptedError:
# Probably hit the timelimit
pass pass
else: else:
for id, value in results: for id, value in results:
@ -409,6 +408,10 @@ class TableView(RowTableShared):
"where {} ".format(" and ".join(where_clauses)) "where {} ".format(" and ".join(where_clauses))
) if where_clauses else "", ) if where_clauses else "",
) )
# Store current params and where_clauses for later:
from_sql_params = dict(**params)
from_sql_where_clauses = where_clauses[:]
count_sql = "select count(*) {}".format(from_sql) count_sql = "select count(*) {}".format(from_sql)
_next = special_args.get("_next") _next = special_args.get("_next")
@ -544,6 +547,7 @@ class TableView(RowTableShared):
except KeyError: except KeyError:
pass pass
facet_results = {} facet_results = {}
facets_timed_out = []
for column in facets: for column in facets:
facet_sql = """ facet_sql = """
select {col} as value, count(*) as count select {col} as value, count(*) as count
@ -552,7 +556,7 @@ class TableView(RowTableShared):
""".format( """.format(
col=escape_sqlite(column), col=escape_sqlite(column),
from_sql=from_sql, from_sql=from_sql,
and_or_where='and' if where_clauses else 'where', and_or_where='and' if where_clause else 'where',
limit=facet_size+1, limit=facet_size+1,
) )
try: try:
@ -595,9 +599,8 @@ class TableView(RowTableShared):
), ),
"selected": selected, "selected": selected,
}) })
except sqlite3.OperationalError: except InterruptedError:
# Hit time limit facets_timed_out.append(column)
pass
columns = [r[0] for r in description] columns = [r[0] for r in description]
rows = list(rows) rows = list(rows)
@ -638,10 +641,11 @@ class TableView(RowTableShared):
filtered_table_rows_count = None filtered_table_rows_count = None
if count_sql: if count_sql:
try: try:
count_rows = list(await self.execute(name, count_sql, params)) count_rows = list(await self.execute(
name, count_sql, from_sql_params
))
filtered_table_rows_count = count_rows[0][0] filtered_table_rows_count = count_rows[0][0]
except sqlite3.OperationalError: except InterruptedError:
# Almost certainly hit the timeout
pass pass
# Detect suggested facets # Detect suggested facets
@ -656,13 +660,13 @@ class TableView(RowTableShared):
'''.format( '''.format(
column=escape_sqlite(facet_column), column=escape_sqlite(facet_column),
from_sql=from_sql, from_sql=from_sql,
and_or_where='and' if where_clauses else 'where', and_or_where='and' if from_sql_where_clauses else 'where',
limit=facet_size+1 limit=facet_size+1
) )
distinct_values = None distinct_values = None
try: try:
distinct_values = await self.execute( distinct_values = await self.execute(
name, suggested_facet_sql, params, name, suggested_facet_sql, from_sql_params,
truncate=False, truncate=False,
custom_time_limit=self.ds.limits["facet_suggest_time_limit_ms"], custom_time_limit=self.ds.limits["facet_suggest_time_limit_ms"],
) )
@ -679,7 +683,7 @@ class TableView(RowTableShared):
request, {'_facet': facet_column} request, {'_facet': facet_column}
), ),
}) })
except sqlite3.OperationalError: except InterruptedError:
pass pass
# human_description_en combines filters AND search, if provided # human_description_en combines filters AND search, if provided
@ -717,6 +721,7 @@ class TableView(RowTableShared):
"display_columns": display_columns, "display_columns": display_columns,
"filter_columns": filter_columns, "filter_columns": filter_columns,
"display_rows": display_rows, "display_rows": display_rows,
"facets_timed_out": facets_timed_out,
"sorted_facet_results": sorted( "sorted_facet_results": sorted(
facet_results.values(), facet_results.values(),
key=lambda f: (len(f["results"]), f["name"]), key=lambda f: (len(f["results"]), f["name"]),