From b87d57aac7c15c0455e0936c0325a1a0b326407e Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 12 Feb 2018 21:57:02 +0100 Subject: [PATCH 1/7] Make galaxy compatible with pysam 0.14 --- lib/galaxy/datatypes/binary.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/datatypes/binary.py b/lib/galaxy/datatypes/binary.py index f15543bd4de..35c9cd3576c 100644 --- a/lib/galaxy/datatypes/binary.py +++ b/lib/galaxy/datatypes/binary.py @@ -12,6 +12,7 @@ import sys import tarfile import tempfile import zipfile +from collections import OrderedDict from json import dumps import h5py @@ -234,7 +235,7 @@ class BamNative(Binary): # TODO: Reference names, lengths, read_groups and headers can become very large, truncate when necessary dataset.metadata.reference_names = list(bam_file.references) dataset.metadata.reference_lengths = list(bam_file.lengths) - dataset.metadata.bam_header = bam_file.header + dataset.metadata.bam_header = OrderedDict((k, v) for k, v in bam_file.header.items()) dataset.metadata.read_groups = [read_group['ID'] for read_group in dataset.metadata.bam_header.get('RG', []) if 'ID' in read_group] dataset.metadata.sort_order = bam_file.header.get('HD', {}).get('SO', None) dataset.metadata.bam_version = bam_file.header.get('HD', {}).get('VN', None) From 349b25b74ae4de51ab567a0215054d854af9a934 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 13 Feb 2018 11:49:10 +0100 Subject: [PATCH 2/7] Minimize duplication of BAM set_meta code --- lib/galaxy/datatypes/binary.py | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/lib/galaxy/datatypes/binary.py b/lib/galaxy/datatypes/binary.py index 35c9cd3576c..10a053d28eb 100644 --- a/lib/galaxy/datatypes/binary.py +++ b/lib/galaxy/datatypes/binary.py @@ -237,8 +237,8 @@ class BamNative(Binary): dataset.metadata.reference_lengths = list(bam_file.lengths) dataset.metadata.bam_header = OrderedDict((k, v) for k, v in bam_file.header.items()) dataset.metadata.read_groups = [read_group['ID'] for read_group in dataset.metadata.bam_header.get('RG', []) if 'ID' in read_group] - dataset.metadata.sort_order = bam_file.header.get('HD', {}).get('SO', None) - dataset.metadata.bam_version = bam_file.header.get('HD', {}).get('VN', None) + dataset.metadata.sort_order = dataset.metadata.bam_header.get('HD', {}).get('SO', None) + dataset.metadata.bam_version = dataset.metadata.bam_header.get('HD', {}).get('VN', None) except Exception: # Per Dan, don't log here because doing so will cause datasets that # fail metadata to end in the error state @@ -384,25 +384,12 @@ class Bam(BamNative): def set_meta(self, dataset, overwrite=True, **kwd): # These metadata values are not accessible by users, always overwrite + super(Bam, self).set_meta(dataset=dataset, overwrite=overwrite, **kwd) index_file = dataset.metadata.bam_index if not index_file: index_file = dataset.metadata.spec['bam_index'].param.new_file(dataset=dataset) pysam.index(dataset.file_name, index_file.file_name) dataset.metadata.bam_index = index_file - # Now use pysam with BAI index to determine additional metadata - try: - bam_file = pysam.AlignmentFile(dataset.file_name, mode='rb', index_filename=index_file.file_name) - # TODO: Reference names, lengths, read_groups and headers can become very large, truncate when necessary - dataset.metadata.reference_names = list(bam_file.references) - dataset.metadata.reference_lengths = list(bam_file.lengths) - dataset.metadata.bam_header = bam_file.header - dataset.metadata.read_groups = [read_group['ID'] for read_group in dataset.metadata.bam_header.get('RG', []) if 'ID' in read_group] - dataset.metadata.sort_order = bam_file.header.get('HD', {}).get('SO', None) - dataset.metadata.bam_version = bam_file.header.get('HD', {}).get('VN', None) - except Exception: - # Per Dan, don't log here because doing so will cause datasets that - # fail metadata to end in the error state - pass def sniff(self, file_name): return super(Bam, self).sniff(file_name) and not self.dataset_content_needs_grooming(file_name) From 3b8a2f9edc93c45e5153896035f321bd23c7ec1a Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 13 Feb 2018 15:22:51 +0100 Subject: [PATCH 3/7] Pin pysam to 0.14 --- lib/galaxy/dependencies/pinned-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/dependencies/pinned-requirements.txt b/lib/galaxy/dependencies/pinned-requirements.txt index adddb8247b9..dd1ccc5f58a 100644 --- a/lib/galaxy/dependencies/pinned-requirements.txt +++ b/lib/galaxy/dependencies/pinned-requirements.txt @@ -11,7 +11,7 @@ mercurial==3.7.3; python_version < '3.0' pycrypto==2.6.1 uWSGI==2.0.15 # Flexible BAM index naming is new to core pysam -pysam>=0.13 +pysam==0.14 # Install python_lzo if you want to support indexed access to lzo-compressed # locally cached maf files via bx-python From e73731f5528a08735a6b65bf419a6f7986c1c771 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 12 Feb 2018 22:05:46 -0500 Subject: [PATCH 4/7] Optimize database interaction for user workflow list. Don't LEFT OUTER JOIN workflow against its steps to produce step count - this causes a lot of duplicated workflow data and fetches step data not needed to just produce a step count. Tweak pre-fetching for workflows shared with user also. --- lib/galaxy/model/mapping.py | 8 +++++++- lib/galaxy/webapps/galaxy/api/workflows.py | 12 +++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/model/mapping.py b/lib/galaxy/model/mapping.py index f2ffed6188b..2fd9c7ab9ee 100644 --- a/lib/galaxy/model/mapping.py +++ b/lib/galaxy/model/mapping.py @@ -14,6 +14,7 @@ from sqlalchemy import ( desc, false, ForeignKey, + func, Integer, MetaData, not_, @@ -2115,7 +2116,12 @@ mapper(model.Workflow, model.Workflow.table, properties=dict( primaryjoin=((model.Workflow.table.c.id == model.WorkflowStep.table.c.workflow_id)), order_by=asc(model.WorkflowStep.table.c.order_index), cascade="all, delete-orphan", - lazy=False) + lazy=False), + step_count=column_property( + select([func.count(model.WorkflowStep.table.c.id)]).where(model.Workflow.table.c.id == model.WorkflowStep.table.c.workflow_id), + deferred=True + ) + )) mapper(model.WorkflowStep, model.WorkflowStep.table, properties=dict( diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 6e6233173ce..c43d46354bf 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -7,6 +7,7 @@ import logging from six.moves.urllib.parse import unquote_plus from sqlalchemy import desc, false, or_, true +from sqlalchemy.orm import eagerload from galaxy import ( exceptions, @@ -125,7 +126,8 @@ class WorkflowsAPIController(BaseAPIController, UsesStoredWorkflowMixin, UsesAnn user = trans.get_user() if show_published: filter1 = or_(filter1, (trans.app.model.StoredWorkflow.published == true())) - for wf in trans.sa_session.query(trans.app.model.StoredWorkflow).filter( + for wf in trans.sa_session.query(trans.app.model.StoredWorkflow).options( + eagerload("latest_workflow").undefer("step_count").lazyload("steps")).filter( filter1, trans.app.model.StoredWorkflow.table.c.deleted == false()).order_by( desc(trans.app.model.StoredWorkflow.table.c.update_time)).all(): @@ -133,15 +135,15 @@ class WorkflowsAPIController(BaseAPIController, UsesStoredWorkflowMixin, UsesAnn encoded_id = trans.security.encode_id(wf.id) item['url'] = url_for('workflow', id=encoded_id) item['owner'] = wf.user.username - item['number_of_steps'] = len(wf.latest_workflow.steps) + item['number_of_steps'] = wf.latest_workflow.step_count item['show_in_tool_panel'] = False for x in user.stored_workflow_menu_entries: if x.stored_workflow_id == wf.id: item['show_in_tool_panel'] = True break rval.append(item) - for wf_sa in trans.sa_session.query(trans.app.model.StoredWorkflowUserShareAssociation).filter_by( - user=trans.user).join('stored_workflow').filter( + for wf_sa in trans.sa_session.query(trans.app.model.StoredWorkflowUserShareAssociation).options( + eagerload("stored_workflow").joinedload("latest_workflow").undefer("step_count").lazyload("steps")).filter_by(user=trans.user).filter( trans.app.model.StoredWorkflow.deleted == false()).order_by( desc(trans.app.model.StoredWorkflow.update_time)).all(): item = wf_sa.stored_workflow.to_dict(value_mapper={'id': trans.security.encode_id}) @@ -149,7 +151,7 @@ class WorkflowsAPIController(BaseAPIController, UsesStoredWorkflowMixin, UsesAnn item['url'] = url_for('workflow', id=encoded_id) item['slug'] = wf_sa.stored_workflow.slug item['owner'] = wf_sa.stored_workflow.user.username - item['number_of_steps'] = len(wf_sa.stored_workflow.latest_workflow.steps) + item['number_of_steps'] = wf_sa.stored_workflow.latest_workflow.step_count item['show_in_tool_panel'] = False for x in user.stored_workflow_menu_entries: if x.stored_workflow_id == wf_sa.id: From b9ddec71a4632942534048069f7d0d474b2c8982 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Tue, 13 Feb 2018 14:16:45 -0500 Subject: [PATCH 5/7] disable loading of webhooks in TS and reports apps --- client/galaxy/scripts/onload.js | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/client/galaxy/scripts/onload.js b/client/galaxy/scripts/onload.js index 232fc87c0ab..e58f49db2e7 100644 --- a/client/galaxy/scripts/onload.js +++ b/client/galaxy/scripts/onload.js @@ -179,18 +179,20 @@ $(document).ready(() => { function onloadWebhooks() { if (Galaxy.root !== undefined) { - // Load all webhooks with the type 'onload' - Webhooks.load({ - type: "onload", - callback: function(webhooks) { - webhooks.each(model => { - var webhook = model.toJSON(); - if (webhook.activate && webhook.script) { - Utils.appendScriptStyle(webhook); - } - }); - } - }); + if (Galaxy.config.enable_webhooks) { + // Load all webhooks with the type 'onload' + Webhooks.load({ + type: "onload", + callback: function(webhooks) { + webhooks.each(model => { + var webhook = model.toJSON(); + if (webhook.activate && webhook.script) { + Utils.appendScriptStyle(webhook); + } + }); + } + }); + } } else { setTimeout(onloadWebhooks, 100); } From 80a41f32744d2d76b74973663cd6d751daf8e8a5 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 12 Feb 2018 16:43:27 -0500 Subject: [PATCH 6/7] Optimize public grid database interactions. For all four grid types (histories, viz, workflows, and pages), this computes rating average on the server and prefetches any annotations. This eliminates at least two extra queries for response element in the grid. See comment in the history controller for why I am fairly confident this is a good idea for annotations but tags are less obvious - these grids still go back to the postgres server multiple times per rendered item to render tags. I'm confident we should either subqueryload, joinedload, or upgrade to sqlalchemy 2.2 and selectinload (http://docs.sqlalchemy.org/en/latest/orm/loading_relationships.html#sqlalchemy.orm.selectinload) the tags as well - but I'm not sure which without being able to hack on a usegalaxy.org. This also brings in less of the user model (only username instead of all of it) to reduce over-the-wire transmission of unneeded data from postgres to Galaxy. For workflows we were LEFT OUTER JOIN-ing on the steps of the latest workflow - so we were bringing back a lot of extra rows for data that was completely unused. In light of this, it makes perfect sense to me why published workflows were the slowest of these and I suspect they will all be equally performant after this change (modulo the number of rows in the tables and the number of rows rendered). --- lib/galaxy/model/mapping.py | 25 ++++++++++++++++--- lib/galaxy/web/framework/helpers/grids.py | 7 +++++- .../webapps/galaxy/controllers/history.py | 18 ++++++++++--- lib/galaxy/webapps/galaxy/controllers/page.py | 5 ++-- .../galaxy/controllers/visualization.py | 5 ++-- .../webapps/galaxy/controllers/workflow.py | 8 +++--- 6 files changed, 53 insertions(+), 15 deletions(-) diff --git a/lib/galaxy/model/mapping.py b/lib/galaxy/model/mapping.py index f2ffed6188b..d3531ea8fa8 100644 --- a/lib/galaxy/model/mapping.py +++ b/lib/galaxy/model/mapping.py @@ -14,6 +14,7 @@ from sqlalchemy import ( desc, false, ForeignKey, + func, Integer, MetaData, not_, @@ -1569,7 +1570,11 @@ mapper(model.History, model.History.table, properties=dict( backref="histories"), ratings=relation(model.HistoryRatingAssociation, order_by=model.HistoryRatingAssociation.table.c.id, - backref="histories") + backref="histories"), + 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 + ) )) # Set up proxy so that @@ -2178,7 +2183,11 @@ mapper(model.StoredWorkflow, model.StoredWorkflow.table, properties=dict( backref="stored_workflows"), ratings=relation(model.StoredWorkflowRatingAssociation, order_by=model.StoredWorkflowRatingAssociation.table.c.id, - backref="stored_workflows") + backref="stored_workflows"), + average_rating=column_property( + select([func.avg(model.StoredWorkflowRatingAssociation.table.c.rating)]).where(model.StoredWorkflowRatingAssociation.table.c.stored_workflow_id == model.StoredWorkflow.table.c.id), + deferred=True + ) )) # Set up proxy so that @@ -2310,7 +2319,11 @@ mapper(model.Page, model.Page.table, properties=dict( backref="pages"), ratings=relation(model.PageRatingAssociation, order_by=model.PageRatingAssociation.table.c.id, - backref="pages") + backref="pages"), + average_rating=column_property( + select([func.avg(model.PageRatingAssociation.table.c.rating)]).where(model.PageRatingAssociation.table.c.page_id == model.Page.table.c.id), + deferred=True + ) )) # Set up proxy so that @@ -2342,7 +2355,11 @@ mapper(model.Visualization, model.Visualization.table, properties=dict( backref="visualizations"), ratings=relation(model.VisualizationRatingAssociation, order_by=model.VisualizationRatingAssociation.table.c.id, - backref="visualizations") + backref="visualizations"), + average_rating=column_property( + select([func.avg(model.VisualizationRatingAssociation.table.c.rating)]).where(model.VisualizationRatingAssociation.table.c.visualization_id == model.Visualization.table.c.id), + deferred=True + ) )) # Set up proxy so that diff --git a/lib/galaxy/web/framework/helpers/grids.py b/lib/galaxy/web/framework/helpers/grids.py index 69354b891d8..2c6da1988c6 100644 --- a/lib/galaxy/web/framework/helpers/grids.py +++ b/lib/galaxy/web/framework/helpers/grids.py @@ -573,7 +573,12 @@ class CommunityRatingColumn(GridColumn, UsesItemRatings): """ Column that displays community ratings for an item. """ def get_value(self, trans, grid, item): - ave_item_rating, num_ratings = self.get_ave_item_rating_data(trans.sa_session, item, webapp_model=trans.model) + if not hasattr(item, "average_rating"): + # No prefetched column property, generate it on the fly. + ave_item_rating, num_ratings = self.get_ave_item_rating_data(trans.sa_session, item, webapp_model=trans.model) + else: + ave_item_rating = item.average_rating + num_ratings = 2 # just used for pluralization return trans.fill_template("tool_shed_rating.mako", ave_item_rating=ave_item_rating, num_ratings=num_ratings, diff --git a/lib/galaxy/webapps/galaxy/controllers/history.py b/lib/galaxy/webapps/galaxy/controllers/history.py index 64772493454..8e34e557f2c 100644 --- a/lib/galaxy/webapps/galaxy/controllers/history.py +++ b/lib/galaxy/webapps/galaxy/controllers/history.py @@ -5,7 +5,7 @@ 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.orm import eagerload, eagerload_all +from sqlalchemy.orm import eagerload, eagerload_all, undefer import galaxy.util from galaxy import exceptions @@ -209,8 +209,20 @@ class HistoryAllPublishedGrid(grids.Grid): operations = [] def build_initial_query(self, trans, **kwargs): - # Join so that searching history.user makes sense. - return trans.sa_session.query(self.model_class).join(model.User.table) + # TODO: Tags are still loaded one at a time, consider doing this all at once: + # - eagerload would keep everything in one query but would explode the number of rows and potentially + # result in unneeded info transferred over the wire. + # - subqueryload("tags").subqueryload("tag") would probably be better under postgres but I'd + # like some performance data against a big database first - might cause problems? + + # - Pull down only username from associated User table since that is all that is used + # (can be used during search). Need join in addition to the eagerload since it is used in + # the .count() query which doesn't respect the eagerload options (could eliminate this with #5523). + # - Undefer average_rating column to prevent loading individual ratings per-history. + # - Eager load annotations - this causes a left join which might be inefficient if there were + # potentially many items per history (like if joining HDAs for instance) but there should only + # be at most one so this is fine. + return trans.sa_session.query(self.model_class).join("user").options(eagerload("user").load_only("username"), eagerload("annotations"), undefer("average_rating")) def apply_query_filter(self, trans, query, **kwargs): # A public history is published, has a slug, and is not deleted. diff --git a/lib/galaxy/webapps/galaxy/controllers/page.py b/lib/galaxy/webapps/galaxy/controllers/page.py index 8f9ff09a41d..2cff6220813 100644 --- a/lib/galaxy/webapps/galaxy/controllers/page.py +++ b/lib/galaxy/webapps/galaxy/controllers/page.py @@ -2,6 +2,7 @@ from json import loads from markupsafe import escape from sqlalchemy import and_, desc, false, true +from sqlalchemy.orm import eagerload, undefer from galaxy import managers, model, util, web from galaxy.model.item_attrs import UsesItemRatings @@ -82,8 +83,8 @@ class PageAllPublishedGrid(grids.Grid): ) def build_initial_query(self, trans, **kwargs): - # Join so that searching history.user makes sense. - return trans.sa_session.query(self.model_class).join(model.User.table) + # See optimization description comments and TODO for tags in matching public histories query. + return trans.sa_session.query(self.model_class).join("user").options(eagerload("user").load_only("username"), eagerload("annotations"), undefer("average_rating")) def apply_query_filter(self, trans, query, **kwargs): return query.filter(self.model_class.deleted == false()).filter(self.model_class.published == true()) diff --git a/lib/galaxy/webapps/galaxy/controllers/visualization.py b/lib/galaxy/webapps/galaxy/controllers/visualization.py index 4e0aa904e2d..0232494646e 100644 --- a/lib/galaxy/webapps/galaxy/controllers/visualization.py +++ b/lib/galaxy/webapps/galaxy/controllers/visualization.py @@ -12,6 +12,7 @@ from paste.httpexceptions import ( ) from six import string_types from sqlalchemy import and_, desc, false, or_, true +from sqlalchemy.orm import eagerload, undefer from galaxy import managers, model, util, web from galaxy.datatypes.interval import Bed @@ -212,8 +213,8 @@ class VisualizationAllPublishedGrid(grids.Grid): ) def build_initial_query(self, trans, **kwargs): - # Join so that searching history.user makes sense. - return trans.sa_session.query(self.model_class).join(model.User.table) + # See optimization description comments and TODO for tags in matching public histories query. + return trans.sa_session.query(self.model_class).join("user").options(eagerload("user").load_only("username"), eagerload("annotations"), undefer("average_rating")) def apply_query_filter(self, trans, query, **kwargs): return query.filter(self.model_class.deleted == false()).filter(self.model_class.published == true()) diff --git a/lib/galaxy/webapps/galaxy/controllers/workflow.py b/lib/galaxy/webapps/galaxy/controllers/workflow.py index e5b9ba020f6..9becce4c39f 100644 --- a/lib/galaxy/webapps/galaxy/controllers/workflow.py +++ b/lib/galaxy/webapps/galaxy/controllers/workflow.py @@ -10,7 +10,7 @@ import requests from markupsafe import escape from six.moves.http_client import HTTPConnection from sqlalchemy import and_ -from sqlalchemy.orm import joinedload +from sqlalchemy.orm import eagerload, joinedload, lazyload, undefer from sqlalchemy.sql import expression from galaxy import ( @@ -138,8 +138,10 @@ class StoredWorkflowAllPublishedGrid(grids.Grid): ] def build_initial_query(self, trans, **kwargs): - # Join so that searching stored_workflow.user makes sense. - return trans.sa_session.query(self.model_class).join(model.User.table) + # See optimization description comments and TODO for tags in matching public histories query. + # In addition to that - be sure to lazyload the latest_workflow - it isn't needed and it causes all + # of its steps to be eagerly loaded. + return trans.sa_session.query(self.model_class).join("user").options(lazyload("latest_workflow"), eagerload("user").load_only("username"), eagerload("annotations"), undefer("average_rating")) def apply_query_filter(self, trans, query, **kwargs): # A public workflow is published, has a slug, and is not deleted. From 5ff6be93ed074485a60e567309247e9dbf02db09 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 14 Feb 2018 14:10:18 +0100 Subject: [PATCH 7/7] Use profile="18.01" for BamNative converters This is because samtools writes to stderr when sorting/merging large files. Part of https://github.com/galaxyproject/galaxy/issues/5496#issuecomment-365458089. --- lib/galaxy/datatypes/converters/bam_native_to_bam_converter.xml | 2 +- lib/galaxy/datatypes/converters/sam_to_bam_native.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/datatypes/converters/bam_native_to_bam_converter.xml b/lib/galaxy/datatypes/converters/bam_native_to_bam_converter.xml index 8ac1dd7b633..ea5bf341bb5 100644 --- a/lib/galaxy/datatypes/converters/bam_native_to_bam_converter.xml +++ b/lib/galaxy/datatypes/converters/bam_native_to_bam_converter.xml @@ -1,4 +1,4 @@ -