From aa73b52a201d8ca8bf03e3b0237459a4cec27887 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 Dec 2021 11:53:48 +0100 Subject: [PATCH 1/5] Use yield_per option to limit amount of ORM objects loaded into memory I've applied it only to non-legacy code where we don't need the entire list up-front. This might help with > I have an issue with the job handlers from a few days. Randomly one of them gets trapped in a death loop. It starts to monitor jobs, the memory ramps up until the systemd limit of 12GB and then is killed by sytemd report by @gmauro on usegalaxy.eu --- lib/galaxy/jobs/handler.py | 4 ++-- lib/galaxy/model/__init__.py | 1 + lib/galaxy/webapps/galaxy/api/jobs.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/jobs/handler.py b/lib/galaxy/jobs/handler.py index 90d32966b68..d1492150f8f 100644 --- a/lib/galaxy/jobs/handler.py +++ b/lib/galaxy/jobs/handler.py @@ -237,11 +237,11 @@ class JobHandlerQueue(Monitors): .outerjoin(model.User) \ .filter(model.Job.state.in_(in_list) & (model.Job.handler == self.app.config.server_name) - & or_((model.Job.user_id == null()), (model.User.active == true()))).all() + & or_((model.Job.user_id == null()), (model.User.active == true()))).yield_per(model.YIELD_PER_ROWS) else: jobs_at_startup = self.sa_session.query(model.Job).enable_eagerloads(False) \ .filter(model.Job.state.in_(in_list) - & (model.Job.handler == self.app.config.server_name)).all() + & (model.Job.handler == self.app.config.server_name)).yield_per(model.YIELD_PER_ROWS) for job in jobs_at_startup: if not self.app.toolbox.has_tool(job.tool_id, job.tool_version, exact=True): diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 072cb37f05e..e2d1544cb93 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -135,6 +135,7 @@ JOB_METRIC_PRECISION = 26 JOB_METRIC_SCALE = 7 # Tags that get automatically propagated from inputs to outputs when running jobs. AUTO_PROPAGATED_TAGS = ["name"] +YIELD_PER_ROWS = 100 if TYPE_CHECKING: diff --git a/lib/galaxy/webapps/galaxy/api/jobs.py b/lib/galaxy/webapps/galaxy/api/jobs.py index e856a16450b..a63f2efcdae 100644 --- a/lib/galaxy/webapps/galaxy/api/jobs.py +++ b/lib/galaxy/webapps/galaxy/api/jobs.py @@ -204,7 +204,7 @@ class JobController(BaseGalaxyAPIController, UsesVisualizationMixin): query = query.limit(limit) out = [] - for job in query.all(): + for job in query.yield_per(model.YIELD_PER_ROWS): job_dict = job.to_dict(view, system_details=is_admin) j = self.encode_all_ids(trans, job_dict, True) if view == 'admin_job_list': From 9e52fb6ac7a45a06839e4efd878f1020e96fc82f Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 Dec 2021 12:08:07 +0100 Subject: [PATCH 2/5] Disable eagerload of job pja relationship --- lib/galaxy/model/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index e2d1544cb93..ad671eaf389 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -916,7 +916,7 @@ class Job(Base, JobLike, UsesCreateAndUpdateTime, Dictifiable, RepresentById): back_populates='job', lazy=True) output_dataset_collections = relationship('JobToImplicitOutputDatasetCollectionAssociation', back_populates='job', lazy=True) - post_job_actions = relationship('PostJobActionAssociation', back_populates='job', lazy=False) + post_job_actions = relationship('PostJobActionAssociation', back_populates='job', lazy=True) input_library_datasets = relationship('JobToInputLibraryDatasetAssociation', back_populates='job') output_library_datasets = relationship('JobToOutputLibraryDatasetAssociation', From b7cb55091b3deb0a6f5899f21c5d365dc5bf8f9b Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 20 Dec 2021 11:23:21 +0100 Subject: [PATCH 3/5] Fix dataset details button when using prefix If Galaxy is served at a prefix we can't push the prefixed URL into Galaxy.router. This fix probably also fixes a few more issues with path prefixes when using the beta history, considering the legacyNavigationMixin incorrectly (I think) added the prefix before pushing a route into the router. Fixes https://github.com/galaxyproject/galaxy/issues/13089. --- .../History/ContentItem/Dataset/DatasetMenu.vue | 8 +++++--- client/src/components/plugins/legacyNavigation.js | 2 +- client/src/mvc/dataset/dataset-li.js | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/client/src/components/History/ContentItem/Dataset/DatasetMenu.vue b/client/src/components/History/ContentItem/Dataset/DatasetMenu.vue index ae3c4d58c7f..28fb3d24f45 100644 --- a/client/src/components/History/ContentItem/Dataset/DatasetMenu.vue +++ b/client/src/components/History/ContentItem/Dataset/DatasetMenu.vue @@ -261,8 +261,9 @@ export default { // wierd iframe navigation visualize() { + const showDetailsUrl = `/datasets/${this.dataset.id}/details`; const redirectParams = { - path: this.dataset.getUrl("show_params"), + path: showDetailsUrl, title: "Dataset details", tryIframe: false, }; @@ -274,13 +275,14 @@ export default { }, showDetails() { + const showDetailsUrl = `/datasets/${this.dataset.id}/details`; const redirectParams = { - path: this.dataset.getUrl("show_params"), + path: showDetailsUrl, title: "Dataset details", tryIframe: false, }; if (!this.iframeAdd(redirectParams)) { - this.backboneRoute(this.dataset.getUrl("show_params")); + this.backboneRoute(showDetailsUrl); } }, diff --git a/client/src/components/plugins/legacyNavigation.js b/client/src/components/plugins/legacyNavigation.js index c401ee7acfa..a0c64f920c6 100644 --- a/client/src/components/plugins/legacyNavigation.js +++ b/client/src/components/plugins/legacyNavigation.js @@ -51,7 +51,7 @@ export const legacyNavigationMixin = { // galaxy router, wrapper for backbone router backboneRoute(path, ...args) { try { - getGalaxyInstance().router.push(prependPath(path), ...args); + getGalaxyInstance().router.push(path, ...args); } catch (err) { console.warn("Failed galaxy route change", err, ...arguments); throw err; diff --git a/client/src/mvc/dataset/dataset-li.js b/client/src/mvc/dataset/dataset-li.js index ba0e646aa3c..5a9d8573eab 100644 --- a/client/src/mvc/dataset/dataset-li.js +++ b/client/src/mvc/dataset/dataset-li.js @@ -280,15 +280,16 @@ export var DatasetListItemView = _super.extend( faIcon: "fa-info-circle", onclick: (ev) => { const Galaxy = getGalaxyInstance(); + const showDetailsUrl = `/datasets/${this.model.get("id")}/details`; if (Galaxy.frame && Galaxy.frame.active) { ev.preventDefault(); Galaxy.frame.add({ - url: this.model.urls.show_params, + url: showDetailsUrl, title: `Dataset Details of ${this.model.get("name")}`, }); } else if (Galaxy.router) { ev.preventDefault(); - Galaxy.router.push(this.model.urls.show_params); + Galaxy.router.push(showDetailsUrl); Galaxy.trigger("activate-hda", this.model.get("id")); } }, From 634eb24aab26697b065100f0e92439d6fd370b3e Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 20 Dec 2021 11:54:19 +0100 Subject: [PATCH 4/5] Fix center panel reload when clicking help button --- client/src/mvc/dataset/dataset-li-edit.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/src/mvc/dataset/dataset-li-edit.js b/client/src/mvc/dataset/dataset-li-edit.js index 85247b65d7a..12d67112252 100644 --- a/client/src/mvc/dataset/dataset-li-edit.js +++ b/client/src/mvc/dataset/dataset-li-edit.js @@ -177,7 +177,8 @@ var DatasetListItemEdit = _super.extend( classes: "icon-btn", href: "#", faIcon: "fa-question", - onclick: function () { + onclick: function (ev) { + ev.preventDefault(); if (self.$el.find(".toolhelp").length > 0) { self.$el.find(".toolhelp").toggle(); } else { From 11c5a1923ee944a4cb558660a0d2e624e4bb9b39 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 20 Dec 2021 17:39:04 +0100 Subject: [PATCH 5/5] Limit possible size of info field, read only 100 MB of stdio --- lib/galaxy/metadata/set_metadata.py | 77 ++++++++++++++--------------- 1 file changed, 37 insertions(+), 40 deletions(-) diff --git a/lib/galaxy/metadata/set_metadata.py b/lib/galaxy/metadata/set_metadata.py index 2eb2efd1969..87ad6c7fe89 100644 --- a/lib/galaxy/metadata/set_metadata.py +++ b/lib/galaxy/metadata/set_metadata.py @@ -66,6 +66,9 @@ logging.basicConfig() log = logging.getLogger(__name__) +MAX_STDIO_READ_BYTES = 100 * 10 ** 6 # 100 MB + + def set_validated_state(dataset_instance): datatype_validation = validate(dataset_instance) @@ -162,46 +165,40 @@ def set_metadata_portable(): outputs_directory = tool_job_working_directory # TODO: constants... - if os.path.exists(os.path.join(outputs_directory, "tool_stdout")): - with open(os.path.join(outputs_directory, "tool_stdout"), "rb") as f: - tool_stdout = f.read() - - with open(os.path.join(outputs_directory, "tool_stderr"), "rb") as f: - tool_stderr = f.read() - elif os.path.exists(os.path.join(tool_job_working_directory, "stdout")): - with open(os.path.join(tool_job_working_directory, "stdout"), "rb") as f: - tool_stdout = f.read() - - with open(os.path.join(tool_job_working_directory, "stderr"), "rb") as f: - tool_stderr = f.read() - elif os.path.exists(os.path.join(outputs_directory, "stdout")): - # Puslar style output directory? Was this ever used - did this ever work? - with open(os.path.join(outputs_directory, "stdout"), "rb") as f: - tool_stdout = f.read() - - with open(os.path.join(outputs_directory, "stderr"), "rb") as f: - tool_stderr = f.read() - elif os.path.exists(os.path.join(tool_job_working_directory, 'task_0')): - # We have a task splitting job - tool_stdout = b'' - tool_stderr = b'' - paths = Path(tool_job_working_directory).glob('task_*') - for path in paths: - with open(path / 'outputs' / 'tool_stdout', 'rb') as f: - task_stdout = f.read() - if task_stdout: - tool_stdout = b"%s[%s stdout]\n%s\n" % (tool_stdout, path.name.encode(), task_stdout) - with open(path / 'outputs' / 'tool_stderr', 'rb') as f: - task_stderr = f.read() - if task_stderr: - tool_stderr = b"%s[%s stdout]\n%s\n" % (tool_stderr, path.name.encode(), task_stderr) + locations = [ + (outputs_directory, 'tool_'), + (tool_job_working_directory, ''), + (outputs_directory, ''), # # Pulsar style output directory? Was this ever used - did this ever work? + ] + for directory, prefix in locations: + if os.path.exists(os.path.join(directory, f"{prefix}stdout")): + with open(os.path.join(directory, f"{prefix}stdout"), 'rb') as f: + tool_stdout = f.read(MAX_STDIO_READ_BYTES) + with open(os.path.join(directory, f"{prefix}stderr"), 'rb') as f: + tool_stderr = f.read(MAX_STDIO_READ_BYTES) + break else: - wdc = os.listdir(tool_job_working_directory) - odc = os.listdir(outputs_directory) - error_desc = "Failed to find tool_stdout or tool_stderr for this job, cannot collect metadata" - error_extra = f"Working dir contents [{wdc}], output directory contents [{odc}]" - log.warn(f"{error_desc}. {error_extra}") - raise Exception(error_desc) + if os.path.exists(os.path.join(tool_job_working_directory, 'task_0')): + # We have a task splitting job + tool_stdout = b'' + tool_stderr = b'' + paths = Path(tool_job_working_directory).glob('task_*') + for path in paths: + with open(path / 'outputs' / 'tool_stdout', 'rb') as f: + task_stdout = f.read(MAX_STDIO_READ_BYTES) + if task_stdout: + tool_stdout = b"%s[%s stdout]\n%s\n" % (tool_stdout, path.name.encode(), task_stdout) + with open(path / 'outputs' / 'tool_stderr', 'rb') as f: + task_stderr = f.read(MAX_STDIO_READ_BYTES) + if task_stderr: + tool_stderr = b"%s[%s stdout]\n%s\n" % (tool_stderr, path.name.encode(), task_stderr) + else: + wdc = os.listdir(tool_job_working_directory) + odc = os.listdir(outputs_directory) + error_desc = "Failed to find tool_stdout or tool_stderr for this job, cannot collect metadata" + error_extra = f"Working dir contents [{wdc}], output directory contents [{odc}]" + log.warn(f"{error_desc}. {error_extra}") + raise Exception(error_desc) job_id_tag = metadata_params["job_id_tag"] @@ -217,7 +214,7 @@ def set_metadata_portable(): version_string_path = os.path.join('outputs', COMMAND_VERSION_FILENAME) version_string = collect_shrinked_content_from_path(version_string_path) - expression_context = ExpressionContext(dict(stdout=tool_stdout, stderr=tool_stderr)) + expression_context = ExpressionContext(dict(stdout=tool_stdout[:255], stderr=tool_stderr[:255])) # Load outputs. export_store = store.DirectoryModelExportStore('metadata/outputs_populated', serialize_dataset_objects=True, for_edit=True, strip_metadata_files=False, serialize_jobs=False)