mirror of
https://github.com/GSA/notifications-api.git
synced 2025-12-20 23:41:17 -05:00
Optimise queries run for creating pagination links
We have been running in to the problem in https://github.com/pallets/flask-sqlalchemy/issues/518 where our page loads very slow when viewing a single page of notifications for a service in the admin app. Tracing this back and using SQL explain analyze I can see that getting the notifications takes about a second but the second query to count how many notifications there are (to work out if there is a next page of pagination) can take up to 100 seconds. As suggested in that issue, we do the pagination ourselves. Our pagination doesn't need us to know exactly how many notifications there are, just whether there are any on the next page and that can be done without running the slow query to count how many notifications in total by using `count_pages=False`.
This commit is contained in:
@@ -241,7 +241,8 @@ def get_notifications_for_service(
|
|||||||
include_from_test_key=False,
|
include_from_test_key=False,
|
||||||
older_than=None,
|
older_than=None,
|
||||||
client_reference=None,
|
client_reference=None,
|
||||||
include_one_off=True
|
include_one_off=True,
|
||||||
|
error_out=True
|
||||||
):
|
):
|
||||||
if page_size is None:
|
if page_size is None:
|
||||||
page_size = current_app.config['PAGE_SIZE']
|
page_size = current_app.config['PAGE_SIZE']
|
||||||
@@ -280,7 +281,8 @@ def get_notifications_for_service(
|
|||||||
return query.order_by(desc(Notification.created_at)).paginate(
|
return query.order_by(desc(Notification.created_at)).paginate(
|
||||||
page=page,
|
page=page,
|
||||||
per_page=page_size,
|
per_page=page_size,
|
||||||
count=count_pages
|
count=count_pages,
|
||||||
|
error_out=error_out,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -420,6 +420,8 @@ def get_all_notifications_for_service(service_id):
|
|||||||
include_from_test_key = data.get('include_from_test_key', False)
|
include_from_test_key = data.get('include_from_test_key', False)
|
||||||
include_one_off = data.get('include_one_off', True)
|
include_one_off = data.get('include_one_off', True)
|
||||||
|
|
||||||
|
# count_pages is not being used for whether to count the number of pages, but instead as a flag
|
||||||
|
# for whether to show pagination links
|
||||||
count_pages = data.get('count_pages', True)
|
count_pages = data.get('count_pages', True)
|
||||||
|
|
||||||
pagination = notifications_dao.get_notifications_for_service(
|
pagination = notifications_dao.get_notifications_for_service(
|
||||||
@@ -427,7 +429,7 @@ def get_all_notifications_for_service(service_id):
|
|||||||
filter_dict=data,
|
filter_dict=data,
|
||||||
page=page,
|
page=page,
|
||||||
page_size=page_size,
|
page_size=page_size,
|
||||||
count_pages=count_pages,
|
count_pages=False,
|
||||||
limit_days=limit_days,
|
limit_days=limit_days,
|
||||||
include_jobs=include_jobs,
|
include_jobs=include_jobs,
|
||||||
include_from_test_key=include_from_test_key,
|
include_from_test_key=include_from_test_key,
|
||||||
@@ -442,25 +444,44 @@ def get_all_notifications_for_service(service_id):
|
|||||||
else:
|
else:
|
||||||
notifications = notification_with_template_schema.dump(pagination.items, many=True).data
|
notifications = notification_with_template_schema.dump(pagination.items, many=True).data
|
||||||
|
|
||||||
|
# We try and get the next page of results to work out if we need provide a pagination link to the next page
|
||||||
|
# in our response if it exists. Note, this could be done instead by changing `count_pages` in the previous
|
||||||
|
# call to be True which will enable us to use Flask-Sqlalchemy to tell if there is a next page of results but
|
||||||
|
# this way is much more performant for services with many results (unlike Flask SqlAlchemy, this approach
|
||||||
|
# doesn't do an additional query to count all the results of which there could be millions but instead only
|
||||||
|
# asks for a single extra page of results).
|
||||||
|
next_page_of_pagination = notifications_dao.get_notifications_for_service(
|
||||||
|
service_id,
|
||||||
|
filter_dict=data,
|
||||||
|
page=page + 1,
|
||||||
|
page_size=page_size,
|
||||||
|
count_pages=False,
|
||||||
|
limit_days=limit_days,
|
||||||
|
include_jobs=include_jobs,
|
||||||
|
include_from_test_key=include_from_test_key,
|
||||||
|
include_one_off=include_one_off,
|
||||||
|
error_out=False # False so that if there are no results, it doesn't end in aborting with a 404
|
||||||
|
)
|
||||||
|
|
||||||
def get_prev_next_pagination_links(pagination, endpoint, **kwargs):
|
def get_prev_next_pagination_links(current_page, next_page_exists, endpoint, **kwargs):
|
||||||
if 'page' in kwargs:
|
if 'page' in kwargs:
|
||||||
kwargs.pop('page', None)
|
kwargs.pop('page', None)
|
||||||
links = {}
|
links = {}
|
||||||
if pagination.has_prev:
|
if page > 1:
|
||||||
links['prev'] = url_for(endpoint, page=pagination.prev_num, **kwargs)
|
links['prev'] = url_for(endpoint, page=page - 1, **kwargs)
|
||||||
if pagination.has_next:
|
if next_page_exists:
|
||||||
links['next'] = url_for(endpoint, page=pagination.next_num, **kwargs)
|
links['next'] = url_for(endpoint, page=page + 1, **kwargs)
|
||||||
return links
|
return links
|
||||||
|
|
||||||
return jsonify(
|
return jsonify(
|
||||||
notifications=notifications,
|
notifications=notifications,
|
||||||
page_size=page_size,
|
page_size=page_size,
|
||||||
links=get_prev_next_pagination_links(
|
links=get_prev_next_pagination_links(
|
||||||
pagination,
|
page,
|
||||||
|
len(next_page_of_pagination.items),
|
||||||
'.get_all_notifications_for_service',
|
'.get_all_notifications_for_service',
|
||||||
**kwargs
|
**kwargs
|
||||||
)
|
) if count_pages else {}
|
||||||
), 200
|
), 200
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user