From 283ab0459fe25cc26606ccae86bdd99e1c6765b9 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 14 Feb 2018 08:32:54 -0500 Subject: [PATCH] Improve performance bugs and misleading summary info in history list. Prefetch shared user information as counts instead of objects and do so in initial query to eliminate an extra 15 SQL queries per page and load less data related to sharing. Replace columns that would cause HDA information to be joined into the query (history size and HDA state counts) with a spinner that will be fetched in subsequent queries on the client end. There can hundreds of thousands of datasets per history - this information shouldn't be summarized to get the initial page to render - it can be fetched one history at a time once the page is rendered. This commit also adds a new column "Items" that corresponds to the next HID - and that I think is a good summary of the "history size" before the dataset state information is loaded and even gives additional information because that count includes collections. This also renders state information for deleted and hidden datasets that was previously missing and could cause confusion. That said I don't like the dataset summaries - I'd rather just have the item count and then job state summaries. --- .../scripts/mvc/history/history-list.js | 51 ++++++++++++++++++- client/galaxy/style/less/base.less | 6 +++ lib/galaxy/model/mapping.py | 4 ++ lib/galaxy/web/framework/helpers/grids.py | 16 +++++- .../webapps/galaxy/controllers/history.py | 44 +++++++--------- test/selenium_tests/test_saved_histories.py | 2 +- 6 files changed, 93 insertions(+), 30 deletions(-) diff --git a/client/galaxy/scripts/mvc/history/history-list.js b/client/galaxy/scripts/mvc/history/history-list.js index 98f0ad17f6f..5fda9c9be7d 100644 --- a/client/galaxy/scripts/mvc/history/history-list.js +++ b/client/galaxy/scripts/mvc/history/history-list.js @@ -1,11 +1,60 @@ import _l from "utils/localization"; -/** This class renders the grid list. */ +import AjaxQueue from "utils/ajax-queue"; import Utils from "utils/utils"; +/** This class renders the grid list. */ import GridView from "mvc/grid/grid-view"; import HistoryModel from "mvc/history/history-model"; import historyCopyDialog from "mvc/history/copy-dialog"; var HistoryGridView = GridView.extend({ + initialize: function(grid_config) { + this.ajaxQueue = new AjaxQueue.AjaxQueue(); + GridView.prototype.initialize.call(this, grid_config); + }, + + init_grid_elements: function() { + const ajaxQueue = this.ajaxQueue; + ajaxQueue.stop(); + GridView.prototype.init_grid_elements.call(this); + const fetchDetails = $.makeArray( + this.$el.find(".delayed-value-datasets_by_state").map((i, el) => { + return () => { + const historyId = $(el).data("history-id"); + const url = `${ + Galaxy.root + }api/histories/${historyId}?keys=nice_size,contents_active,contents_states`; + const options = {}; + options.url = url; + options.type = "GET"; + options.success = req => { + const contentsStates = req["contents_states"]; + let stateHtml = ""; + for (let state of ["ok", "running", "queued", "new", "error"]) { + const stateCount = contentsStates[state]; + if (stateCount) { + stateHtml += `
${stateCount}
`; + } + } + const contentsActive = req["contents_active"]; + const deleted = contentsActive["deleted"]; + if (deleted) { + stateHtml += `
${deleted}
`; + } + const hidden = contentsActive["hidden"]; + if (hidden) { + stateHtml += `
${hidden}
`; + } + $(`.delayed-value-datasets_by_state[data-history-id='${historyId}']`).html(stateHtml); + $(`.delayed-value-disk_size[data-history-id='${historyId}']`).html(req["nice_size"]); + }; + var xhr = jQuery.ajax(options); + return xhr; + }; + }) + ); + fetchDetails.forEach(fn => ajaxQueue.add(fn)); + ajaxQueue.start(); + }, _showCopyDialog: function(id) { var history = new HistoryModel.History({ id: id }); history diff --git a/client/galaxy/style/less/base.less b/client/galaxy/style/less/base.less index 9f8db6d3181..84267dac07a 100644 --- a/client/galaxy/style/less/base.less +++ b/client/galaxy/style/less/base.less @@ -1181,6 +1181,12 @@ ul.manage-table-actions li { background: @state-deleted-bg; } +.state-color-hidden { + border-color: @state-default-border; + border-style: dotted; + background: @state-default-bg; +} + .state-fg-new { color: #FFB030; } diff --git a/lib/galaxy/model/mapping.py b/lib/galaxy/model/mapping.py index 37eaa887e4d..2d97eacd985 100644 --- a/lib/galaxy/model/mapping.py +++ b/lib/galaxy/model/mapping.py @@ -1574,6 +1574,10 @@ mapper(model.History, model.History.table, properties=dict( average_rating=column_property( select([func.avg(model.HistoryRatingAssociation.table.c.rating)]).where(model.HistoryRatingAssociation.table.c.history_id == model.History.table.c.id), deferred=True + ), + users_shared_with_count=column_property( + select([func.count(model.HistoryUserShareAssociation.table.c.id)]).where(model.History.table.c.id == model.HistoryUserShareAssociation.table.c.history_id), + deferred=True ) )) diff --git a/lib/galaxy/web/framework/helpers/grids.py b/lib/galaxy/web/framework/helpers/grids.py index 2c6da1988c6..9c5ce3d4da3 100644 --- a/lib/galaxy/web/framework/helpers/grids.py +++ b/lib/galaxy/web/framework/helpers/grids.py @@ -822,13 +822,17 @@ class StateColumn(GridColumn): class SharingStatusColumn(GridColumn): """ Grid column to indicate sharing status. """ + def __init__(self, *args, **kwargs): + self.use_shared_with_count = kwargs.pop("use_shared_with_count", False) + super(SharingStatusColumn, self).__init__(*args, **kwargs) + def get_value(self, trans, grid, item): # Delete items cannot be shared. if item.deleted: return "" # Build a list of sharing for this item. sharing_statuses = [] - if item.users_shared_with: + if self._is_shared(item): sharing_statuses.append("Shared") if item.importable: sharing_statuses.append("Accessible") @@ -836,8 +840,16 @@ class SharingStatusColumn(GridColumn): sharing_statuses.append("Published") return ", ".join(sharing_statuses) + def _is_shared(self, item): + if self.use_shared_with_count: + # optimization to skip join for users_shared_with and loading in that data. + return item.users_shared_with_count > 0 + + return item.users_shared_with + def get_link(self, trans, grid, item): - if not item.deleted and (item.users_shared_with or item.importable or item.published): + is_shared = self._is_shared(item) + if not item.deleted and (is_shared or item.importable or item.published): return dict(operation="share or publish", id=item.id) return None diff --git a/lib/galaxy/webapps/galaxy/controllers/history.py b/lib/galaxy/webapps/galaxy/controllers/history.py index 8e34e557f2c..239b01458e0 100644 --- a/lib/galaxy/webapps/galaxy/controllers/history.py +++ b/lib/galaxy/webapps/galaxy/controllers/history.py @@ -4,7 +4,7 @@ import sets from markupsafe import escape from six import string_types from six.moves.urllib.parse import unquote_plus -from sqlalchemy import and_, false, func, null, true +from sqlalchemy import and_, false, null, true from sqlalchemy.orm import eagerload, eagerload_all, undefer import galaxy.util @@ -16,7 +16,7 @@ from galaxy.model.item_attrs import ( UsesAnnotations, UsesItemRatings ) -from galaxy.util import listify, nice_size, Params, parse_int, sanitize_text +from galaxy.util import listify, Params, parse_int, sanitize_text from galaxy.util.odict import odict from galaxy.util.sanitize_html import sanitize_html from galaxy.web import url_for @@ -44,28 +44,13 @@ class NameColumn(grids.TextColumn): class HistoryListGrid(grids.Grid): # Custom column types - class DatasetsByStateColumn(grids.GridColumn): + class DelayedValueColumn(grids.GridColumn): def get_value(self, trans, grid, history): - # States to show in column. - states_to_show = ('ok', 'running', 'queued', 'new', 'error') + return '
' % (self.key, trans.security.encode_id(history.id)) - # Get dataset counts for each state in a state-count dictionary. - state_counts = dict((state, count) for state, count in - trans.sa_session.query(model.Dataset.state, func.count(model.Dataset.state)) - .join(model.HistoryDatasetAssociation) - .group_by(model.Dataset.state) - .filter(model.HistoryDatasetAssociation.history_id == history.id, - model.HistoryDatasetAssociation.visible == true(), - model.HistoryDatasetAssociation.deleted == false(), - model.Dataset.state.in_(states_to_show))) - - # Create HTML. - rval = '' - for state in states_to_show: - count = state_counts.get(state) - if count: - rval += '
%s
' % (state, count) - return rval + class ItemCountColumn(grids.GridColumn): + def get_value(self, trans, grid, history): + return str(history.hid_counter - 1) class HistoryListNameColumn(NameColumn): def get_link(self, trans, grid, history): @@ -91,17 +76,24 @@ class HistoryListGrid(grids.Grid): query = query.order_by(self.model_class.table.c.purged.desc(), self.model_class.table.c.update_time.desc()) return query + def build_initial_query(self, trans, **kwargs): + # Override to preload sharing information used when fetching data for grid. + query = super(HistoryListGrid, self).build_initial_query(trans, **kwargs) + query = query.options(undefer("users_shared_with_count")) + return query + # Grid definition title = "Saved Histories" model_class = model.History default_sort_key = "-update_time" columns = [ HistoryListNameColumn("Name", key="name", attach_popup=True, filterable="advanced"), - DatasetsByStateColumn("Datasets", key="datasets_by_state", sortable=False, nowrap=True), + ItemCountColumn("Items", key="item_count", sortable=False), + DelayedValueColumn("Datasets", key="datasets_by_state", sortable=False, nowrap=True), grids.IndividualTagsColumn("Tags", key="tags", model_tag_association_class=model.HistoryTagAssociation, filterable="advanced", grid_name="HistoryListGrid"), - grids.SharingStatusColumn("Sharing", key="sharing", filterable="advanced", sortable=False), - grids.GridColumn("Size on Disk", key="disk_size", format=nice_size, sortable=False), + grids.SharingStatusColumn("Sharing", key="sharing", filterable="advanced", sortable=False, use_shared_with_count=True), + DelayedValueColumn("Size on Disk", key="disk_size", sortable=False), grids.GridColumn("Created", key="create_time", format=time_ago), grids.GridColumn("Last Updated", key="update_time", format=time_ago), DeletedColumn("Status", key="deleted", filterable="advanced") @@ -109,7 +101,7 @@ class HistoryListGrid(grids.Grid): columns.append( grids.MulticolFilterColumn( "search history names and tags", - cols_to_filter=[columns[0], columns[2]], + cols_to_filter=[columns[0], columns[3]], key="free-text-search", visible=False, filterable="standard") ) operations = [ diff --git a/test/selenium_tests/test_saved_histories.py b/test/selenium_tests/test_saved_histories.py index c96fc2a2a7f..80a8d75c409 100644 --- a/test/selenium_tests/test_saved_histories.py +++ b/test/selenium_tests/test_saved_histories.py @@ -329,7 +329,7 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): for row in grid.find_elements_by_tag_name('tr'): td = row.find_elements_by_tag_name('td') if td[1].text == history_name: - tags_cell = td[3] + tags_cell = td[4] break if tags_cell is None: