From c68fc22171144fd44fc153641c6a3180ed70082f Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Mon, 15 Jun 2015 12:04:14 -0400 Subject: [PATCH 1/5] Initial restriction of metadata size to ~5MB in-memory size per element --- lib/galaxy/model/custom_types.py | 53 ++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/lib/galaxy/model/custom_types.py b/lib/galaxy/model/custom_types.py index 96a8ea084cd..39aef1b3e0c 100644 --- a/lib/galaxy/model/custom_types.py +++ b/lib/galaxy/model/custom_types.py @@ -4,6 +4,10 @@ import json import logging import uuid +from sys import getsizeof +from itertools import chain +from collections import deque + from galaxy import eggs eggs.require("SQLAlchemy") import sqlalchemy @@ -18,6 +22,8 @@ log = logging.getLogger( __name__ ) json_encoder = json.JSONEncoder( sort_keys=True ) json_decoder = json.JSONDecoder( ) +MAX_METADATA_SIZE = 5000000 # 5MB in memory max. No required metadata should be larger than this. + def _sniffnfix_pg9_hex(value): """ @@ -217,11 +223,58 @@ metadata_pickler = AliasPickleModule( { } ) +def total_size(o, handlers={}, verbose=False): + """ Returns the approximate memory footprint an object and all of its contents. + + Automatically finds the contents of the following builtin containers and + their subclasses: tuple, list, deque, dict, set and frozenset. + To search other containers, add handlers to iterate over their contents: + + handlers = {SomeContainerClass: iter, + OtherContainerClass: OtherContainerClass.get_elements} + + Recipe from: https://code.activestate.com/recipes/577504-compute-memory-footprint-of-an-object-and-its-cont/ + """ + dict_handler = lambda d: chain.from_iterable(d.items()) + all_handlers = { tuple: iter, + list: iter, + deque: iter, + dict: dict_handler, + set: iter, + frozenset: iter } + all_handlers.update(handlers) # user handlers take precedence + seen = set() # track which object id's have already been seen + default_size = getsizeof(0) # estimate sizeof object without __sizeof__ + + def sizeof(o): + if id(o) in seen: # do not double count the same object + return 0 + seen.add(id(o)) + s = getsizeof(o, default_size) + + for typ, handler in all_handlers.items(): + if isinstance(o, typ): + s += sum(map(sizeof, handler(o))) + break + return s + + return sizeof(o) + + class MetadataType( JSONType ): """ Backward compatible metadata type. Can read pickles or JSON, but always writes in JSON. """ + + def process_bind_param(self, value, dialect): + if value is not None: + for k, v in value.items(): + if total_size(v) > MAX_METADATA_SIZE: + del value[k] + value = json_encoder.encode(value) + return value + def process_result_value( self, value, dialect ): if value is None: return None From 1d5d322a15d34c5c8f8f9a509d6532ef7f06909d Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Mon, 15 Jun 2015 13:11:18 -0400 Subject: [PATCH 2/5] Make metadata limit configurable, off by default. --- config/galaxy.ini.sample | 5 +++++ lib/galaxy/config.py | 1 + lib/galaxy/model/custom_types.py | 10 +++++----- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/config/galaxy.ini.sample b/config/galaxy.ini.sample index 10c3f633a7e..aa319b22ba1 100644 --- a/config/galaxy.ini.sample +++ b/config/galaxy.ini.sample @@ -973,6 +973,11 @@ use_interactive = True # option to retry externally, or set metadata manually (when possible). #retry_metadata_internally = True +# Very large metadata values can cause Galaxy crashes. This will allow +# limiting the maximum metadata size Galaxy will attempt to save with a +# dataset. 0 to disable this feature. 5000000 is a reasonable size. +#max_metadata_value_size = 0 + # If (for example) you run on a cluster and your datasets (by default, # database/files/) are mounted read-only, this option will override tool output # paths to write outputs to the working directory instead, and the job manager diff --git a/lib/galaxy/config.py b/lib/galaxy/config.py index 006167a5303..dcf0d1c6a64 100644 --- a/lib/galaxy/config.py +++ b/lib/galaxy/config.py @@ -148,6 +148,7 @@ class Configuration( object ): self.tool_secret = kwargs.get( "tool_secret", "" ) self.id_secret = kwargs.get( "id_secret", "USING THE DEFAULT IS NOT SECURE!" ) self.retry_metadata_internally = string_as_bool( kwargs.get( "retry_metadata_internally", "True" ) ) + self.max_metadata_value_size = int( kwargs.get( "max_metadata_value_size", 0 ) ) self.use_remote_user = string_as_bool( kwargs.get( "use_remote_user", "False" ) ) self.normalize_remote_user_email = string_as_bool( kwargs.get( "normalize_remote_user_email", "False" ) ) self.remote_user_maildomain = kwargs.get( "remote_user_maildomain", None ) diff --git a/lib/galaxy/model/custom_types.py b/lib/galaxy/model/custom_types.py index 39aef1b3e0c..308b40201ad 100644 --- a/lib/galaxy/model/custom_types.py +++ b/lib/galaxy/model/custom_types.py @@ -12,6 +12,7 @@ from galaxy import eggs eggs.require("SQLAlchemy") import sqlalchemy +from galaxy import app from galaxy.util.aliaspickler import AliasPickleModule from sqlalchemy.types import CHAR, LargeBinary, String, TypeDecorator from sqlalchemy.ext.mutable import Mutable @@ -22,8 +23,6 @@ log = logging.getLogger( __name__ ) json_encoder = json.JSONEncoder( sort_keys=True ) json_decoder = json.JSONDecoder( ) -MAX_METADATA_SIZE = 5000000 # 5MB in memory max. No required metadata should be larger than this. - def _sniffnfix_pg9_hex(value): """ @@ -269,9 +268,10 @@ class MetadataType( JSONType ): def process_bind_param(self, value, dialect): if value is not None: - for k, v in value.items(): - if total_size(v) > MAX_METADATA_SIZE: - del value[k] + if app.app and app.app.config.max_metadata_value_size: + for k, v in value.items(): + if total_size(v) > app.app.config.max_metadata_value_size: + del value[k] value = json_encoder.encode(value) return value From 4f975da4b6a2343e3b5e1c75f5388e5a51ed71b3 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Mon, 15 Jun 2015 15:22:49 -0400 Subject: [PATCH 3/5] Add log indicating when metadata setting is being aborted --- lib/galaxy/model/custom_types.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/model/custom_types.py b/lib/galaxy/model/custom_types.py index 308b40201ad..5387a582d90 100644 --- a/lib/galaxy/model/custom_types.py +++ b/lib/galaxy/model/custom_types.py @@ -270,8 +270,10 @@ class MetadataType( JSONType ): if value is not None: if app.app and app.app.config.max_metadata_value_size: for k, v in value.items(): - if total_size(v) > app.app.config.max_metadata_value_size: + sz = total_size(v) + if sz > app.app.config.max_metadata_value_size: del value[k] + log.error('Refusing to bind metadata key %s due to size (%s)' % (k, sz)) value = json_encoder.encode(value) return value From 6423454857bbc4b958ec1966b184cc4133edeb94 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 16 Jun 2015 10:23:46 -0400 Subject: [PATCH 4/5] Update sample text. --- config/galaxy.ini.sample | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/config/galaxy.ini.sample b/config/galaxy.ini.sample index aa319b22ba1..a9276c73f74 100644 --- a/config/galaxy.ini.sample +++ b/config/galaxy.ini.sample @@ -974,8 +974,9 @@ use_interactive = True #retry_metadata_internally = True # Very large metadata values can cause Galaxy crashes. This will allow -# limiting the maximum metadata size Galaxy will attempt to save with a -# dataset. 0 to disable this feature. 5000000 is a reasonable size. +# limiting the maximum metadata key size (in bytes used in memory, not the end +# result database value size) Galaxy will attempt to save with a dataset. 0 to +# disable this feature. 5000000 seems to be a reasonable size. #max_metadata_value_size = 0 # If (for example) you run on a cluster and your datasets (by default, From ea0d5a9c8605903cf9a658f11ea6bfd61e44bddc Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 16 Jun 2015 14:18:11 -0400 Subject: [PATCH 5/5] Fix bug with runtime post job actions. Runtime post job actions are post job actions inserted when the workflow is invoked instead of being part of the workflow object in the database. The bug noticed by @kellrott was that these actions were being appended to the original workflow post job actions instead of being transient things just attached to the jobs themselves. This fixes that problem and adds a test to try to prevent regressions. --- lib/galaxy/workflow/modules.py | 9 ++++++--- test/api/test_workflows.py | 7 +++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 1c9bb8c49ae..91c201e81b4 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -863,10 +863,13 @@ class ToolModule( WorkflowModule ): # Create new PJA associations with the created job, to be run on completion. # PJA Parameter Replacement (only applies to immediate actions-- rename specifically, for now) # Pass along replacement dict with the execution of the PJA so we don't have to modify the object. - post_job_actions = step.post_job_actions + + # Combine workflow and runtime post job actions into the effective post + # job actions for this execution. + effective_post_job_actions = step.post_job_actions[:] for key, value in self.runtime_post_job_actions.iteritems(): - post_job_actions.append( self.__to_pja( key, value, step ) ) - for pja in post_job_actions: + effective_post_job_actions.append( self.__to_pja( key, value, None ) ) + for pja in effective_post_job_actions: if pja.action_type in ActionBox.immediate_actions: ActionBox.execute( self.trans.app, self.trans.sa_session, pja, job, replacement_dict ) else: diff --git a/test/api/test_workflows.py b/test/api/test_workflows.py index 2e057545ec7..ac5a8c43c63 100644 --- a/test/api/test_workflows.py +++ b/test/api/test_workflows.py @@ -867,6 +867,13 @@ test_data: content = self.dataset_populator.get_history_dataset_details( history_id, wait=True, assert_ok=True ) assert content[ "name" ] == "foo was replaced", content[ "name" ] + # Test for regression of previous behavior where runtime post job actions + # would be added to the original workflow post job actions. + workflow_id = workflow_request["workflow_id"] + downloaded_workflow = self._download_workflow( workflow_id ) + pjas = downloaded_workflow[ "steps" ][ "2" ][ "post_job_actions" ].values() + assert len( pjas ) == 0, len( pjas ) + @skip_without_tool( "cat1" ) def test_run_with_delayed_runtime_pja( self ): workflow_id = self._upload_yaml_workflow("""