From 3cfa2e09f09584c085798d58423447921efcc062 Mon Sep 17 00:00:00 2001 From: Keith Suderman Date: Wed, 10 Jun 2026 15:56:51 -0400 Subject: [PATCH 1/2] Fix convert_memory_to_mib decimal MB/GB conversions (were off by 1000x) --- .../jobs/runners/util/gcp_batch/helpers.py | 20 ++++++++++++++----- test/unit/app/jobs/test_gcp_batch_runner.py | 13 ++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/jobs/runners/util/gcp_batch/helpers.py b/lib/galaxy/jobs/runners/util/gcp_batch/helpers.py index ee67b99d80d..09ec9a553ed 100644 --- a/lib/galaxy/jobs/runners/util/gcp_batch/helpers.py +++ b/lib/galaxy/jobs/runners/util/gcp_batch/helpers.py @@ -12,6 +12,16 @@ DEFAULT_NFS_MOUNT_PATH = "/mnt/nfs" DEFAULT_NFS_PATH = "/" DEFAULT_MEMORY_MIB = 2048 DEFAULT_CPU_MILLI = 1000 + +# Memory-unit conversion factors to MiB (1 MiB = 1024 * 1024 bytes). Decimal units +# (KB/MB/GB) scale by powers of 1000; binary units (KiB/GiB) by powers of 1024. +# Precomputed so the constant arithmetic isn't re-evaluated on every conversion. +BYTES_PER_MIB = 1024 * 1024 +KIB_TO_MIB = 1 / 1024 +GIB_TO_MIB = 1024 +KB_TO_MIB = 1000 / BYTES_PER_MIB +MB_TO_MIB = 1000 * 1000 / BYTES_PER_MIB +GB_TO_MIB = 1000 * 1000 * 1000 / BYTES_PER_MIB DEFAULT_CVMFS_DOCKER_VOLUME = ( '-v "/cvmfs/data.galaxyproject.org:/cvmfs/data.galaxyproject.org:ro" ' '-v "/cvmfs/cloud.galaxyproject.org:/cvmfs/cloud.galaxyproject.org:ro"' @@ -166,15 +176,15 @@ def convert_memory_to_mib(memory_str): if unit in ["", "mib", "mi"]: return int(value) elif unit in ["gib", "gi"]: - return int(value * 1024) # GiB to MiB + return int(value * GIB_TO_MIB) # GiB to MiB elif unit in ["mb", "m"]: - return int(value * 1000 / 1024) # MB to MiB (decimal to binary) + return int(value * MB_TO_MIB) # MB to MiB (decimal to binary) elif unit in ["gb", "g"]: - return int(value * 1000 * 1000 / 1024 / 1024) # GB to MiB + return int(value * GB_TO_MIB) # GB to MiB (decimal to binary) elif unit in ["kib", "ki"]: - return int(value / 1024) # KiB to MiB + return int(value * KIB_TO_MIB) # KiB to MiB elif unit in ["kb", "k"]: - return int(value * 1000 / 1024 / 1024) # KB to MiB + return int(value * KB_TO_MIB) # KB to MiB (decimal to binary) else: log.warning("Unknown memory unit: %s, treating as MiB", unit) return int(value) diff --git a/test/unit/app/jobs/test_gcp_batch_runner.py b/test/unit/app/jobs/test_gcp_batch_runner.py index 22e65225e0d..57c05b2c9ac 100644 --- a/test/unit/app/jobs/test_gcp_batch_runner.py +++ b/test/unit/app/jobs/test_gcp_batch_runner.py @@ -81,6 +81,19 @@ class TestConvertMemoryToMib: ("1.5Gi", 1536), # decimal GiB ("256Mi", 256), # small MiB value ("4Gi", 4096), # larger GiB value + # Decimal units (KB/MB/GB use powers of 1000, converted to MiB) + ("1gb", 953), # 1 GB -> MiB (10^9 / 1024^2) + ("4gb", 3814), # larger GB value + ("2g", 1907), # GB short form + ("1000mb", 953), # 1000 MB == 1 GB == 953 MiB (consistency check) + ("1024mb", 976), # MB -> MiB + ("1000m", 953), # MB short form + ("1048576kb", 1000), # KB -> MiB + # Binary KiB + ("1024kib", 1), # KiB -> MiB (value / 1024) + ("2048ki", 2), # KiB short form + # Unknown unit falls back to treating the value as MiB + ("5xyz", 5), ], ) def test_convert_memory_to_mib(self, input_value, expected): From 26081fb75dd442e084911942e8ee43e525472359 Mon Sep 17 00:00:00 2001 From: Keith Suderman Date: Wed, 10 Jun 2026 16:11:43 -0400 Subject: [PATCH 2/2] Honor polling_interval in GCP Batch runner via monitor_sleep_time Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/galaxy/jobs/runners/gcp_batch.py | 12 +++++++ test/unit/app/jobs/test_gcp_batch_runner.py | 37 +++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/lib/galaxy/jobs/runners/gcp_batch.py b/lib/galaxy/jobs/runners/gcp_batch.py index 3f916cc1f69..f3f792f97f2 100644 --- a/lib/galaxy/jobs/runners/gcp_batch.py +++ b/lib/galaxy/jobs/runners/gcp_batch.py @@ -136,6 +136,18 @@ class GoogleCloudBatchJobRunner(AsynchronousJobRunner): log.info("Google Cloud Batch client initialized successfully") + @property + def monitor_sleep_time(self): + """Throttle the monitor loop so it polls the GCP Batch API no more often + than the configured ``polling_interval``. + + The base ``AsynchronousJobRunner.monitor()`` loop sleeps this long between + sweeps; each runner has its own monitor thread, so this only affects the + GCP Batch runner. Item access (not ``.get()``) is used so the spec default + of 30 resolves through ``ParamsWithSpecs.__missing__``. + """ + return max(self.app.config.job_runner_monitor_sleep, int(self.runner_params["polling_interval"])) + def queue_job(self, job_wrapper): """Queue a job for execution on Google Cloud Batch.""" log.debug("Starting queue_job for Galaxy job %s", job_wrapper.get_id_tag()) diff --git a/test/unit/app/jobs/test_gcp_batch_runner.py b/test/unit/app/jobs/test_gcp_batch_runner.py index 57c05b2c9ac..6d9b019ac3d 100644 --- a/test/unit/app/jobs/test_gcp_batch_runner.py +++ b/test/unit/app/jobs/test_gcp_batch_runner.py @@ -1,7 +1,15 @@ """Unit tests for Google Cloud Batch job runner utility methods.""" +from types import SimpleNamespace +from typing import ( + Any, + cast, +) + import pytest +from galaxy.jobs.runners import RunnerParams +from galaxy.jobs.runners.gcp_batch import GoogleCloudBatchJobRunner from galaxy.jobs.runners.util.gcp_batch import ( convert_cpu_to_milli, convert_duration_to_seconds, @@ -320,3 +328,32 @@ class TestResolveMaxRunDuration: resource_params={"walltime": ""}, ) assert result == "7200s" + + +def _sleep_runner(runner_params, job_runner_monitor_sleep=1.0): + """A runner instance with __init__ skipped, for exercising monitor_sleep_time.""" + runner = object.__new__(GoogleCloudBatchJobRunner) + runner.runner_params = runner_params + config = SimpleNamespace(job_runner_monitor_sleep=job_runner_monitor_sleep) + runner.app = cast(Any, SimpleNamespace(config=config)) + return runner + + +class TestMonitorSleepTime: + """The runner throttles its monitor loop to the configured polling_interval so + the base loop does not poll the GCP Batch API more often than necessary.""" + + def test_uses_polling_interval_over_default_monitor_sleep(self): + runner = _sleep_runner({"polling_interval": 30}, job_runner_monitor_sleep=1.0) + assert runner.monitor_sleep_time == 30 + + def test_respects_larger_monitor_sleep(self): + # max(global, polling_interval): a larger global sleep wins. + runner = _sleep_runner({"polling_interval": 30}, job_runner_monitor_sleep=60) + assert runner.monitor_sleep_time == 60 + + def test_interval_default_resolves_via_spec(self): + # polling_interval unset -> must fall back to the spec default (30) through + # ParamsWithSpecs.__missing__; .get() would yield None and raise in max(). + runner = _sleep_runner(RunnerParams(specs={"polling_interval": dict(map=int, default=30)}, params={})) + assert runner.monitor_sleep_time == 30