From 0ab4a055fef139c22aad09abae72dc057bc7f3e9 Mon Sep 17 00:00:00 2001 From: JosepSampe Date: Fri, 9 Oct 2026 12:38:22 +0200 Subject: [PATCH 1/3] Fix worker and monitoring issues --- CHANGELOG.md | 8 + lithops/concurrent/futures.py | 2 + lithops/config.py | 48 ++ lithops/executors.py | 85 +++- lithops/future.py | 11 + lithops/invokers.py | 81 +++- lithops/job/partitioner.py | 11 +- lithops/job/serialize.py | 26 +- .../monitoring/backends/storage/storage.py | 43 +- lithops/monitoring/job_monitor.py | 30 +- lithops/monitoring/monitor.py | 244 ++++++++-- lithops/retries.py | 52 +- .../storage/backends/localhost/localhost.py | 70 +-- lithops/storage/cloud_proxy.py | 74 +-- lithops/tests/test_concurrent_futures.py | 11 + lithops/tests/test_config.py | 41 ++ lithops/tests/test_executors.py | 141 +++++- lithops/tests/test_future.py | 17 + lithops/tests/test_invokers.py | 103 +++- lithops/tests/test_job.py | 32 ++ lithops/tests/test_map.py | 41 ++ lithops/tests/test_monitor.py | 447 ++++++++++++++++++ lithops/tests/test_retries.py | 134 ++++++ lithops/tests/test_storage_layer.py | 137 +++++- lithops/tests/test_utils.py | 143 ++++++ lithops/tests/test_wait.py | 173 ++++++- lithops/tests/test_worker.py | 68 ++- lithops/utils.py | 197 +++++--- lithops/wait.py | 125 ++++- lithops/worker/handler.py | 39 +- lithops/worker/jobrunner.py | 78 +-- 31 files changed, 2388 insertions(+), 324 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 162131267..780648bed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## [v3.8.1.dev0] +### Fixed +- [Core] Chaining, cleanup, invocation errors, generator inputs, empty `fs`, `wait()` timeouts (threads, SIGALRM, `ANY_COMPLETED`), argument binding and `RetryingFunctionExecutor` retries and percentage waits; object partitioning is now rejected by the retrying executor, and a storage or monitoring service at a loopback address is refused for serverless workers (a warning for standalone). +- [Worker] Partitioned object streams (lost, duplicated or cut rows, rows longer than 128 KB, multi-byte `obj_newline`, `readline()`, iteration), module discovery of nested functions, a parameter named `future`, and `extra_env` restoring the previous environment. +- [Localhost] Absolute interpreter paths are accepted as runtime; storage listing is a literal prefix match including dotfiles, and open-ended byte ranges are supported. +- [Storage] `CloudFileProxy` path handling (`isfile`, `listdir`, `exists`, leading `/`), double `close()`, and `r+` mode writing back. +- [Monitoring] Worker tokens of jobs dropped by `wait()`, replaced monitors and restarted invokers are handed back once, and a held `__end__` is no longer replaced by an `__init__`. +- [Concurrent] `wait()` and `as_completed()` accept generators. + ## [v3.8.0] ### Added diff --git a/lithops/concurrent/futures.py b/lithops/concurrent/futures.py index 679d68d98..82ac728ac 100644 --- a/lithops/concurrent/futures.py +++ b/lithops/concurrent/futures.py @@ -899,11 +899,13 @@ def _sync_all(fs): def wait(fs, timeout=None, return_when=ALL_COMPLETED): """Wait for futures to complete. Same contract as concurrent.futures.wait.""" + fs = list(fs) _sync_all(fs) return _cf_wait(fs, timeout=timeout, return_when=return_when) def as_completed(fs, timeout=None): """Yield futures as they complete. Same contract as concurrent.futures.as_completed.""" + fs = list(fs) _sync_all(fs) return _cf_as_completed(fs, timeout=timeout) diff --git a/lithops/config.py b/lithops/config.py index 9168ac407..aafe62501 100644 --- a/lithops/config.py +++ b/lithops/config.py @@ -19,7 +19,9 @@ import copy import json import importlib +import ipaddress import logging +from urllib.parse import urlsplit from lithops import constants as c from lithops.version import __version__ @@ -263,10 +265,56 @@ def default_config( _load_monitoring_backend_config(config_data) _load_telemetry_backend_config(config_data) + _check_loopback_addresses(config_data) return config_data +# The keys of a storage or monitoring section that hold the address workers connect to +_ADDRESS_KEYS = ('host', 'endpoint', 'amqp_url') + + +def _is_loopback(address): + """Whether a host, host:port or URL names this machine""" + try: + return ipaddress.ip_address(address).is_loopback + except ValueError: + pass + host = urlsplit(address if '://' in address else '//' + address).hostname + if host == 'localhost': + return True + try: + return ipaddress.ip_address(host).is_loopback + except ValueError: + return False + + +def _check_loopback_addresses(config_data): + """ + Refuses a storage or monitoring service at a loopback address for + serverless workers, which run in containers of their own where that + address is the worker itself. Standalone workers only get a warning: an + SSH tunnel can bring the service to the loopback address of the VM + """ + lithops_cfg = config_data['lithops'] + mode = lithops_cfg.get('mode') + if mode == c.LOCALHOST: + return + for section in (lithops_cfg.get('storage'), lithops_cfg.get('monitoring')): + for key in _ADDRESS_KEYS: + address = (config_data.get(section) or {}).get(key) + if not isinstance(address, str) or not _is_loopback(address): + continue + msg = ( + f"{section}.{key} '{address}' is a loopback address, which the " + f"{lithops_cfg['backend']} workers cannot reach. Set it to an " + "address they can reach" + ) + if mode == c.SERVERLESS: + raise Exception(msg) + logger.warning(msg) + + def _load_monitoring_backend_config(config_data): """Lets the config module of the monitoring backend fill in its own defaults""" from lithops.monitoring.backends import ( diff --git a/lithops/executors.py b/lithops/executors.py index 73a0393f9..12decb187 100644 --- a/lithops/executors.py +++ b/lithops/executors.py @@ -23,7 +23,7 @@ import tempfile import subprocess as sp from typing import Any, Dict, List, Optional, Tuple, Union -from collections.abc import Callable +from collections.abc import Callable, Iterator from datetime import datetime from lithops import constants @@ -103,13 +103,27 @@ def _missing_plotting_extra(method_name: str) -> ModuleNotFoundError: ) -def _output_settled(future) -> bool: +def _consumers_settled(future, settled_jobs) -> bool: + """ + Whether every chained job that reads the result of this call has ended. + _consumers holds the futures of each of those jobs, a list all the + producers share, so settled_jobs keeps the answer per list + """ + for consumers in getattr(future, '_consumers', ()): + if id(consumers) not in settled_jobs: + settled_jobs[id(consumers)] = all(c.ready or c.success or c.done for c in consumers) + if not settled_jobs[id(consumers)]: + return False + return True + + +def _output_settled(future, settled_jobs) -> bool: """ Whether nothing is left to read from storage for this call: its result was downloaded or it produced none, it failed, or it handed back - futures of its own + futures of its own, and no chained call still has to read it """ - return future.done or future.futures + return bool(future.done or future.futures) and _consumers_settled(future, settled_jobs) def _group_futures_by_job( @@ -335,13 +349,16 @@ def _monitor_of(self, futures): return None @staticmethod - def _disable_iterdata_output(iterdata): + def _disable_iterdata_output(iterdata, consumers): """ Marks the futures used as input as consumed, so that get_result() - returns the output of this job only + returns the output of this job only. The consumers keep their data + in storage until they have read it. All producers share one copy """ + consumers = list(consumers) for fut in _chained_futures(iterdata) or (): fut._produce_output = False + fut._consumers.append(consumers) def _invoke(self, job): """ @@ -429,6 +446,10 @@ def _submit_map( """ Common path of call_async(), map() and the map stage of map_reduce() """ + # A generator or any other one-shot iterator is read once here: the + # job is built from it in several passes, and it cannot be pickled + if isinstance(iterdata, Iterator): + iterdata = list(iterdata) job_id = self._create_job_id(job_prefix) job, futures = self._run_map_job( job_id=job_id, @@ -445,7 +466,7 @@ def _submit_map( obj_chunk_number=obj_chunk_number, obj_newline=obj_newline ) - self._disable_iterdata_output(iterdata) + self._disable_iterdata_output(iterdata, futures) return job_id, job, futures def _cleanup_jobs(self, futures, exception=None, force=False): @@ -698,7 +719,8 @@ def map_reduce( ) if spawn_reducer != ALWAYS: - self.wait(map_futures, return_when=spawn_reducer) + # The reducer, not invoked yet, is what reads the map outputs + self.wait(map_futures, return_when=spawn_reducer, clean_jobs=False) logger.debug( f'{log_prefix(self.executor_id, map_job_id)} - {spawn_reducer}% of map ' f'activations done. Spawning reduce stage' @@ -717,8 +739,10 @@ def map_reduce( exclude_modules=exclude_modules ) + reducers = list(reduce_futures) for future in map_futures: future._set_mapreduce() + future._consumers.append(reducers) return create_futures_list(map_futures + reduce_futures, self) @@ -769,7 +793,10 @@ def wait( list of futures that have completed and `fs_notdone` is a list of futures that have not completed. """ - futures = self._as_future_list(fs or self.futures) + # No fs means every future of this executor; an empty list, none + futures = self._as_future_list(self.futures if fs is None else fs) + if not futures: + return create_futures_list([], self), create_futures_list([], self) do_clean = self.data_cleaner if clean_jobs is None else clean_jobs try: @@ -784,7 +811,7 @@ def wait( threadpool_size=threadpool_size, wait_dur_sec=wait_dur_sec, show_progressbar=show_progressbar, - futures_from_executor_wait=not fs, + futures_from_executor_wait=fs is None, ) self._release_finished_from_monitor(futures) @@ -797,10 +824,7 @@ def wait( else: # Only the jobs waited on end here. Another job of this # executor may still have calls queued for a free worker - self.invoker.discard_pending( - {f.job_key for f in futures}, - {f.job_id for f in futures}, - ) + self.invoker.discard_pending({f.job_key for f in futures}) self.job_monitor.remove(futures) for future in futures: future._set_exception() @@ -839,7 +863,7 @@ def get_result( :return: The result of the future/s """ pending_to_read = ( - len(self._as_future_list(fs)) if fs + len(self._as_future_list(fs)) if fs is not None else sum(1 for f in self.futures if not f._read and not f.futures) ) @@ -863,21 +887,22 @@ def get_result( for future in fs_done: if future.futures or not future._produce_output: continue - if not fs and future._read: + if fs is None and future._read: continue result.append(future.result( throw_except=throw_except, internal_storage=self.internal_storage )) - if not fs: + if fs is None: future._read = True logger.debug( f'{log_prefix(self.executor_id)} - Finished getting results' ) - if self.data_cleaner: - self._cleanup_jobs(self._as_future_list(fs or self.futures)) + to_clean = self._as_future_list(self.futures if fs is None else fs) + if self.data_cleaner and to_clean: + self._cleanup_jobs(to_clean) if len(result) == 1 and self.last_call != 'map': return result[0] @@ -991,21 +1016,39 @@ def clean( 'storage_config': storage_config }) - futures = self._as_future_list(fs or self.futures) + futures = self._as_future_list(self.futures if fs is None else fs) + settled_jobs = {} if force or on_exit: # On exit nothing will read the leftover results, so a job # that still has one unread call would otherwise stay forever present_jobs = { create_job_key(f.executor_id, f.job_id) for f in futures } + if not on_exit: + # A chained job still running reads them, though + present_jobs -= { + create_job_key(f.executor_id, f.job_id) + for f in list(self.futures) + list(futures) + if not _consumers_settled(f, settled_jobs) + } else: + # The calls a chained job reads are cleaned along with it: a + # clean of theirs skipped them while it ran + chained_jobs = {create_job_key(f.executor_id, f.job_id) for f in futures} + futures = list(futures) + [ + f for f in self.futures + if any( + create_job_key(c[0].executor_id, c[0].job_id) in chained_jobs + for c in getattr(f, '_consumers', ()) if c + ) + ] # A job's data is one prefix, so it goes only once no call of # the job, including the ones not passed here, still has a # result to read from it unread_jobs = { create_job_key(f.executor_id, f.job_id) for f in list(self.futures) + list(futures) - if not _output_settled(f) + if not _output_settled(f, settled_jobs) } present_jobs = { create_job_key(f.executor_id, f.job_id) diff --git a/lithops/future.py b/lithops/future.py index a564a152e..9a3f6e425 100644 --- a/lithops/future.py +++ b/lithops/future.py @@ -93,6 +93,8 @@ def __init__(self, call_id, job, job_metadata, storage_config): self._storage_config = storage_config self._produce_output = True + self._mapreduce_map = False + self._consumers = [] self._read = False self._state = self.State.New self._exception = Exception() @@ -108,6 +110,14 @@ def __init__(self, call_id, job, job_metadata, storage_config): self.stats.update(_stats_from_prefixed_keys(job_metadata)) self._storage_path = get_storage_path(self._storage_config) + def __getstate__(self): + # The chained jobs that read this call are tracked by the client + # alone. Pickled into the input of a later job, they would carry + # every one of their calls and its result along + state = self.__dict__.copy() + state['_consumers'] = [] + return state + def _id_prefix(self) -> str: """ Identity of the job this call belongs to, for the log messages @@ -191,6 +201,7 @@ def _set_futures(self, call_status): def _set_mapreduce(self): """Set the future as mapreduce map""" + self._mapreduce_map = True self._read = True self._produce_output = False if self.success: diff --git a/lithops/invokers.py b/lithops/invokers.py index 6732cf107..e29aa6e3d 100644 --- a/lithops/invokers.py +++ b/lithops/invokers.py @@ -26,6 +26,7 @@ from concurrent.futures import ThreadPoolExecutor from lithops.future import ResponseFuture +from lithops.monitoring.monitor import error_status from lithops.config import extract_storage_config from lithops.version import __version__ from lithops.utils import ( @@ -158,7 +159,8 @@ def __init__( self.runtime_name = self.runtime_info['runtime_name'] self.max_workers = self.runtime_info['max_workers'] - verify_runtime_name(self.runtime_name) + # The localhost mode may run a local interpreter given by its path + verify_runtime_name(self.runtime_name, allow_local_path=self.mode == 'localhost') logger.debug( f'{log_prefix(self.executor_id)} - Invoker initialized. Max workers: {self.max_workers}' @@ -293,6 +295,9 @@ def _run_job(self, job): f'{job.worker_processes} - Chunksize: {job.chunksize}' ) + # Built before invoking, so that a call that fails to be invoked has a + # future to report the error to + futures = self._build_futures(job) try: self._invoke_job(job) except (KeyboardInterrupt, Exception): @@ -301,7 +306,7 @@ def _run_job(self, job): log_file = os.path.join(LOGS_DIR, job.job_key + '.log') logger.info(f'{prefix} - View execution logs at {log_file}') - return self._build_futures(job) + return futures def stop(self, wait: bool = False): """ @@ -309,7 +314,7 @@ def stop(self, wait: bool = False): """ pass - def discard_pending(self, job_keys, job_ids=None): + def discard_pending(self, job_keys): """ Drops the calls of the given jobs not invoked yet. Only an invoker that queues calls has any @@ -508,9 +513,21 @@ def _invoke_task(self, job, call_ids_range): for call_id in call_ids ] - activation_id, resp_time = _timed_invoke( - self.compute_handler, payload - ) + try: + activation_id, resp_time = _timed_invoke( + self.compute_handler, payload + ) + except Exception as e: + # Nobody waits for this call: a raised error would only end up in + # the log of the thread pool, with the calls left invoked for ever + # and the worker slot of the chunk lost + logger.exception( + f'{log_prefix(job.executor_id, job.job_id)} - Invocation of ' + f'calls {", ".join(call_ids)} failed' + ) + self._fail_calls(job, call_ids, e) + self.job_monitor.token_bucket_q.put('#') + return if not activation_id: time.sleep(random.randint(0, 5)) @@ -523,6 +540,21 @@ def _invoke_task(self, job, call_ids_range): f'invoked ({resp_time}s) - Activation ID: {activation_id}' ) + def _fail_calls(self, job, call_ids, error): + """ + Finishes the calls of a chunk the backend could not invoke with the + error it raised, so that waiting for them ends. The monitor is told + no worker runs them + """ + # Wrapped in an error that pickles, whatever the backend raised + failure = RuntimeError(f'Invocation failed: {error!r}') + for call_id in call_ids: + job.futures[int(call_id)]._set_ready(error_status( + job.executor_id, job.job_id, call_id, + exc_info=(RuntimeError, failure, None), + )) + self.job_monitor.discard_calls({job.job_id: len(call_ids)}) + def _invoke_job_remote(self, job): """ Logic for invoking a job using a remote function @@ -533,7 +565,8 @@ def _invoke_job_remote(self, job): 'runtime_name': job.runtime_name, 'runtime_memory': job.runtime_memory, 'remote_invoker': True, - 'job': job.__dict__, + # The futures stay here: the remote invoker builds its own + 'job': {k: v for k, v in vars(job).items() if k != 'futures'}, } activation_id, resp_time = _timed_invoke( self.compute_handler, payload @@ -554,25 +587,29 @@ def _empty_token_bucket(self): except queue.Empty: return - def discard_pending(self, job_keys, job_ids=None): + def discard_pending(self, job_keys): """ Drops the calls of the given jobs that are still waiting for a worker, leaving the queued calls of every other job in place. - The monitor also stops tracking those jobs, so their workers will - not hand tokens back. When no other job is still queued, the count - of running workers is forgotten; otherwise a later map() of the - same executor stays capped by workers that never free + When no other job is still queued, the count of running workers is + forgotten, and every job invoked so far with it; otherwise a later + map() of the same executor stays capped by workers that never free """ kept = [] + discarded = {} while True: try: item = self.pending_calls_q.get(block=False) except queue.Empty: break - job, _ = item + job, call_ids_range = item if job is None or job.job_key not in job_keys: kept.append(item) + else: + discarded[job.job_id] = ( + discarded.get(job.job_id, 0) + len(call_ids_range) + ) other_jobs = False for item in kept: job, _ = item @@ -580,11 +617,22 @@ def discard_pending(self, job_keys, job_ids=None): other_jobs = True self.pending_calls_q.put(item) if other_jobs: + # The workers of these jobs still count, and the dropped calls + # will never free one + if discarded: + self.job_monitor.discard_calls(discarded) return + self._forget_workers() + + def _forget_workers(self): + """ + Stops counting the workers invoked so far. Every job they run is + closed before the bucket is emptied, so that none of them hands a + token back in between + """ + self.job_monitor.close_jobs() self._empty_token_bucket() self.running_workers = 0 - if job_ids and self.job_monitor is not None: - self.job_monitor.close_jobs(job_ids) def _drain_token_bucket(self): """ @@ -644,8 +692,7 @@ def _invoke_job(self, job): # Tokens a monitor handed back after the stop belong to workers # this restart no longer counts, and would each invoke one more # worker than max_workers allows - self._empty_token_bucket() - self.running_workers = 0 + self._forget_workers() self.should_run = True self._start_async_invokers() diff --git a/lithops/job/partitioner.py b/lithops/job/partitioner.py index 9c07693f3..3169d22e5 100644 --- a/lithops/job/partitioner.py +++ b/lithops/job/partitioner.py @@ -137,6 +137,10 @@ def _build_partitions( obj_partitions = [] size = obj_total_partitions = 0 + # Every chunk but the first one starts as many bytes early as the newline + # has, which the chunk before it must have + newline_len = len(obj_newline.encode()) if obj_newline is not None else 0 + obj_chunk_size = max(obj_chunk_size, newline_len) parts = obj_size // obj_chunk_size + (obj_size % obj_chunk_size > 0) logger.debug( @@ -151,15 +155,15 @@ def _build_partitions( elif obj_newline is None: brange = (size, size + obj_chunk_size - 1) elif size + obj_chunk_size < obj_size: - # Records must not be cut in two: a partition starts one byte + # Records must not be cut in two: a partition starts a newline # early to see whether it begins mid record, and overshoots by # CHUNK_THRESHOLD so that it can finish the last record it reads brange = ( - size - 1 if size > 0 else 0, + size - newline_len if size > 0 else 0, size + obj_chunk_size + CHUNK_THRESHOLD ) else: - brange = (size - 1, obj_size - 1) + brange = (size - newline_len, obj_size - 1) obj_chunk_size = obj_size - size obj_total_partitions += 1 @@ -170,6 +174,7 @@ def _build_partitions( partition['obj'].chunk_size = obj_chunk_size partition['obj'].part = obj_total_partitions partition['obj'].newline = obj_newline + partition['obj'].total_size = obj_size obj_partitions.append(partition) size += obj_chunk_size diff --git a/lithops/job/serialize.py b/lithops/job/serialize.py index 58fbd67cd..b38db05f4 100644 --- a/lithops/job/serialize.py +++ b/lithops/job/serialize.py @@ -289,9 +289,13 @@ def _module_inspect(self, obj: Any) -> Set[str]: elif hasattr(value, "__module__"): mods.add(value.__module__) + # The code nested in a function, such as an inner function, reads + # its global names from the module of that function + fn_globals = getattr(fn, '__globals__', {}) for block in codeworklist: for kind, value in ( - self._inner_module_inspect(inst) for inst in Bytecode(block) + self._inner_module_inspect(inst, fn_globals) + for inst in Bytecode(block) ): if kind is None: continue @@ -301,23 +305,27 @@ def _module_inspect(self, obj: Any) -> Set[str]: if hasattr(mod, "__name__") ) elif kind == "code" and id(value) not in seen: + # Only followed once: a recursive function names + # itself, and the worklists would never end seen.add(id(value)) if hasattr(value, "__module__"): mods.add(value.__module__) - - if inspect.isfunction(value): - worklist.append(value) - elif inspect.iscode(value): - codeworklist.append(value) + if inspect.isfunction(value): + worklist.append(value) + elif inspect.iscode(value): + codeworklist.append(value) # Dynamically built functions and code objects can have a # __module__ of None, which names no module to ship return {mod_name.split(".")[0] for mod_name in mods if mod_name} - def _inner_module_inspect(self, inst: Any) -> Tuple[Optional[str], Any]: + def _inner_module_inspect( + self, inst: Any, fn_globals: Dict[str, Any] + ) -> Tuple[Optional[str], Any]: """ Reads the module or the code object that a single bytecode - instruction refers to + instruction refers to. Global names are looked up in fn_globals, the + globals of the function the instruction belongs to """ if inst.opname == "IMPORT_NAME": try: @@ -328,7 +336,7 @@ def _inner_module_inspect(self, inst: Any) -> Tuple[Optional[str], Any]: except Exception: return (None, None) if inst.opname == "LOAD_GLOBAL": - value = globals().get(inst.argval) + value = fn_globals.get(inst.argval) if isinstance(value, (CodeType, FunctionType)): return ("code", value) if isinstance(value, ModuleType): diff --git a/lithops/monitoring/backends/storage/storage.py b/lithops/monitoring/backends/storage/storage.py index 0826cafa5..1877e115c 100644 --- a/lithops/monitoring/backends/storage/storage.py +++ b/lithops/monitoring/backends/storage/storage.py @@ -240,6 +240,7 @@ def _generate_tokens(self, callids_running, callids_done): self.callids_running_processed.update(running_new) self.callids_done_processed.update(attributed) + self._give_up_dropped_jobs() def _release_free_workers(self): """ @@ -250,14 +251,16 @@ def _release_free_workers(self): in consecutive chunks from call 0, so the chunk a call falls in, and the number of calls of the job, say how long that chunk is """ - present_jobs = self.present_jobs + # A job wait() dropped still hands its tokens back: the workers + # are this executor's whether a future waits on them or not + jobs = self.present_jobs | self._dropped_jobs.keys() for worker_id, done_calls in self.callids_done_worker.items(): if worker_id in self.workers_done: continue job_id = self.worker_job.get(worker_id) if job_id is None or job_id in self._token_closed_jobs: continue - if job_id not in present_jobs: + if job_id not in jobs: continue chunksize = self.job_chunksize.get(job_id) if chunksize is None: @@ -267,10 +270,7 @@ def _release_free_workers(self): ) if len(done_calls) < worker_calls: continue - self.workers_done.add(worker_id) - if not self._releases_tokens(): - break - self.token_bucket_q.put('#') + self._hand_back(job_id, worker_id) def _releases_tokens(self): """ @@ -287,11 +287,12 @@ def _release_timed_out_worker(self, call_status): return if call_status['executor_id'] != self.executor_id: return - self.callids_done_worker.setdefault(worker_id, set()).add( - _status_id(call_status) - ) - self.worker_job.setdefault(worker_id, call_status['job_id']) - self._release_free_workers() + with self._apply_lock: + self.callids_done_worker.setdefault(worker_id, set()).add( + _status_id(call_status) + ) + self.worker_job.setdefault(worker_id, call_status['job_id']) + self._release_free_workers() def _poll_and_process_job_status(self): """ @@ -301,19 +302,31 @@ def _poll_and_process_job_status(self): # Nothing tracked: do not list. An empty job_ids used to list the # whole executor prefix, which on S3 is a LIST of every leftover # job key and the function pickle, once per monitoring_interval, - # for as long as the monitor stays up after wait() + # for as long as the monitor stays up after wait(). The jobs wait() + # dropped are listed until their workers have handed their tokens + # back, which no future of theirs is left to wait for job_ids = self.job_ids() - if not job_ids: + owing = self._jobs_owing_tokens() + if not job_ids and not owing: return set() status = self.internal_storage.get_job_status( - self.executor_id, job_ids=job_ids + self.executor_id, job_ids=job_ids | owing ) callids_running, callids_done = status + # Shared with the monitor that may replace this one + with self._apply_lock: + self._generate_tokens(callids_running, callids_done) + + # Only the tracked jobs have futures to tag. The done calls of a + # dropped job are never read, and would otherwise count as new on + # every round + if owing: + callids_running = {c for c in callids_running if c[0][1] in job_ids} + callids_done = {c for c in callids_done if c[1] in job_ids} new_callids_done = ( callids_done - self.callids_done_processed_status ) - self._generate_tokens(callids_running, callids_done) self._tag_future_as_ready(callids_done) self._tag_future_as_running(callids_running) diff --git a/lithops/monitoring/job_monitor.py b/lithops/monitoring/job_monitor.py index a902fbeac..9988e117c 100644 --- a/lithops/monitoring/job_monitor.py +++ b/lithops/monitoring/job_monitor.py @@ -18,6 +18,7 @@ import queue from lithops.monitoring.backends import load_backend_attr, resolve_backend +from lithops.monitoring.monitor import TOKEN_STATE from lithops.telemetry import get_telemetry from lithops.utils import log_prefix @@ -63,6 +64,9 @@ def __init__( self.monitor = None self.job_chunksize = {} self.job_total_calls = {} + # The token bookkeeping every monitor carries on with, so that one + # that replaces another neither loses a token nor hands one back twice + self.token_state = None # Metrics are produced from the statuses the monitor reads, so the # telemetry of the executor is resolved here and handed to every @@ -134,8 +138,19 @@ def _spawn_monitor(self, generate_tokens): # Attached before the caller adds any future, so that no status # can reach the monitor while it is still pointing at the no-op self.monitor.attach_telemetry(self.telemetry) + if self.token_state is None: + self.token_state = { + name: getattr(self.monitor, name) for name in TOKEN_STATE + } + vars(self.monitor).update(self.token_state) if previous is not None: self.monitor.adopt_held_status(previous) + # A thread that outlived the join hands nothing back any more, + # and the jobs it still tracked are owed through this monitor + with self.monitor._apply_lock: + previous._retired = True + if previous.generate_tokens: + self.monitor.drop_jobs(previous.tracked_futures()) def _thread_finished(self): """ @@ -186,15 +201,22 @@ def remove(self, fs): if self.monitor and self.monitor.is_alive(): self.monitor.remove_futures(fs) - def close_jobs(self, job_ids): + def close_jobs(self, job_ids=None): """ - The invoker will no longer wait for these jobs. Later statuses of - theirs must not hand another token back: the capacity they held - was already forgotten + The invoker will no longer wait for these jobs, every job started + so far when None. Later statuses of theirs must not hand another + token back: the capacity they held was already forgotten """ + if job_ids is None: + job_ids = set(self.job_total_calls) if self.monitor is not None: self.monitor.close_jobs(job_ids) + def discard_calls(self, discarded): + """The invoker dropped these calls, counted per job, uninvoked""" + if self.monitor is not None: + self.monitor.discard_calls(discarded) + def stop(self): """ Asks the monitor thread to exit, without waiting for it. diff --git a/lithops/monitoring/monitor.py b/lithops/monitoring/monitor.py index 550750398..e752bdf0d 100644 --- a/lithops/monitoring/monitor.py +++ b/lithops/monitoring/monitor.py @@ -20,6 +20,7 @@ import sys import threading import time +from math import ceil from typing import Any, Dict, Optional from tblib import pickling_support @@ -47,6 +48,11 @@ # there is a backend, and the contract is checked against it BACKENDS_PACKAGE = 'lithops.monitoring.backends' +# The token bookkeeping JobMonitor shares with every monitor it spawns, lock +# included, so that it outlives the monitor that replaces another +TOKEN_STATE = ('_apply_lock', 'workers_done', 'callids_done_worker', '_token_closed_jobs', + '_dropped_jobs', '_credited_calls', '_discarded_calls') + def _status_id(call_status): return ( @@ -64,6 +70,32 @@ def _is_started(fut): return fut.running or _is_finished(fut) +def _holds_end(held_status, status_id): + """Whether the held statuses keep an __end__ for this call""" + entry = held_status.get(status_id) + return entry is not None and entry[1]['type'] == '__end__' + + +def error_status(executor_id, job_id, call_id, activation_id=None, + start_tstamp=None, exc_info=None): + """ + An __end__ status that ends a call with an error, the one being handled + unless ``exc_info`` is given, which the client re-raises + """ + now = time.time() + return { + 'type': '__end__', + 'exception': True, + 'exc_info': str(pickle.dumps(exc_info or sys.exc_info())), + 'executor_id': executor_id, + 'job_id': job_id, + 'call_id': call_id, + 'activation_id': activation_id, + 'worker_start_tstamp': start_tstamp or now, + 'worker_end_tstamp': now, + } + + def is_named_error(exc, *names): """ Tells whether an exception is one of the named cloud SDK errors, without @@ -168,6 +200,9 @@ def __init__(self, executor_id, # early). One lock, re-entrant because applying one status can # reveal the futures whose own statuses were held self._apply_lock = threading.RLock() + # Set once another monitor replaced this one: the workers it finds + # free from then on are left for that one to hand back + self._retired = False self.futures = set() self._futures_by_id = {} self._timeout_query_failures = {} @@ -179,8 +214,12 @@ def __init__(self, executor_id, # Jobs whose capacity the invoker already forgot. A late __end__ # of theirs must not put another token in the bucket self._token_closed_jobs = set() - # Jobs wait() dropped, whose workers still have to free a token - self._dropped_jobs = set() + # Jobs wait() dropped whose workers still have to free a token, and + # when to give up on them. A job is owed tokens for the calls the + # invoker did not discard and no free worker ran yet + self._dropped_jobs = {} + self._credited_calls = {} + self._discarded_calls = {} # vars for MessageMonitor._hold_status self._held_status = {} self._held_lock = threading.Lock() @@ -270,6 +309,12 @@ def add_futures(self, fs): for future in fs ] ) + # A job tracked again is owed nothing until it is dropped again + if self._dropped_jobs: + with self._apply_lock: + for future in fs: + if future.executor_id == self.executor_id: + self._dropped_jobs.pop(future.job_id, None) def remove_futures(self, fs): """ @@ -282,19 +327,52 @@ def remove_futures(self, fs): future_id = _future_id(future) if self._futures_by_id.get(future_id) is future: del self._futures_by_id[future_id] - remaining = {future.job_id for future in self.futures} - self.present_jobs = remaining - self._dropped_jobs.update( - future.job_id for future in fs if future.job_id not in remaining - ) + self.present_jobs = {future.job_id for future in self.futures} + if self.generate_tokens: + self.drop_jobs(fs) + + def drop_jobs(self, fs): + """ + Owes the tokens of the jobs of ``fs`` no future of this monitor + tracks any more. Nested executors reuse its job ids, so only its + own futures count + """ + own = self.executor_id + with self._futures_lock: + tracked = {f.job_id for f in self.futures if f.executor_id == own} + dropped = {f.job_id for f in fs if f.executor_id == own} - tracked + # A worker is gone once past the execution timeout, which the + # runtime timeout caps, plus the margin a status gets to show up. + # Waiting chunksize times as long kept a job listed for days + timeout = max((f.execution_timeout or 0 for f in fs), default=0) + deadline = time.time() + timeout + self.HELD_STATUS_SLACK + with self._apply_lock: + for job_id in dropped - self._token_closed_jobs: + self._dropped_jobs.setdefault(job_id, deadline) + self._settle(job_id) def close_jobs(self, job_ids): """ Marks jobs whose remaining worker tokens the invoker already - wrote off, so a late status does not hand one back again + wrote off, or that owe none, so a late status does not hand one back """ with self._apply_lock: self._token_closed_jobs.update(job_ids) + for job_id in job_ids: + self._dropped_jobs.pop(job_id, None) + self._credited_calls.pop(job_id, None) + self._discarded_calls.pop(job_id, None) + + def discard_calls(self, discarded): + """ + Counts the calls per job the invoker dropped before any worker ran + them, which no worker frees a token for + """ + with self._apply_lock: + for job_id, calls in discarded.items(): + if job_id not in self._token_closed_jobs: + self._discarded_calls[job_id] = self._discarded_calls.get(job_id, 0) + calls + self._settle(job_id) def tracked_futures(self): """ @@ -311,6 +389,74 @@ def job_ids(self): with self._futures_lock: return set(self.present_jobs) + def _releases_tokens(self): + """Whether a worker found free is handed back to the invoker""" + return self.should_run + + def _hand_back(self, job_id, worker_id=None, tokens=1): + """ + Hands back the token of a free worker, or of workers that never + reported back, unless another monitor replaced this one. Called + with the apply lock held + """ + if ( + self._retired + or job_id in self._token_closed_jobs + or not self._releases_tokens() + ): + return False + if worker_id is not None: + self.workers_done.add(worker_id) + calls = len(self.callids_done_worker.get(worker_id, ())) + self._credited_calls[job_id] = self._credited_calls.get(job_id, 0) + calls + for _ in range(tokens): + self.token_bucket_q.put('#') + self._settle(job_id) + return True + + def _owed_calls(self, job_id): + """The calls of a job neither discarded nor run by a free worker""" + return ( + self.job_total_calls.get(job_id, 0) + - self._discarded_calls.get(job_id, 0) + - self._credited_calls.get(job_id, 0) + ) + + def _settle(self, job_id): + """Closes a dropped job, then no longer listed, once it owes no call""" + if job_id not in self._dropped_jobs or job_id not in self.job_total_calls: + return + if self._owed_calls(job_id) <= 0: + self.close_jobs({job_id}) + + def _give_up_dropped_jobs(self): + """ + Closes the dropped jobs past their deadline, handing back a token per + chunk of calls no free worker ran, as a timed-out call does + """ + now = time.time() + with self._apply_lock: + expired = [ + job_id for job_id, deadline in self._dropped_jobs.items() + if now >= deadline + ] + for job_id in expired: + chunksize = self.job_chunksize.get(job_id) or 1 + stuck = ceil(max(self._owed_calls(job_id), 0) / chunksize) + if self._hand_back(job_id, tokens=stuck): + self.close_jobs({job_id}) + logger.debug( + f'{log_prefix(self.executor_id, job_id)} - Gave up waiting ' + f'for {stuck} worker(s) of a dropped job to report back' + ) + + def _jobs_owing_tokens(self): + """A snapshot of the jobs wait() dropped that still owe a token""" + if not self.generate_tokens: + return set() + with self._apply_lock: + return set(self._dropped_jobs) + def future_by_id(self, future_id): """ The tracked future a call status belongs to, or None when it is not @@ -365,7 +511,15 @@ def adopt_held_status(self, other): if not held: return with self._held_lock: - self._held_status = {**held, **self._held_status} + merged = {**held, **self._held_status} + # An __init__ held here does not replace an __end__ the other + # monitor was holding for the same call + for status_id, entry in held.items(): + if entry[1]['type'] != '__end__': + continue + if not _holds_end(self._held_status, status_id): + merged[status_id] = entry + self._held_status = merged self._held_may_match = True def _worker_calls(self, executor_id, job_id, call_id, chunksize): @@ -519,18 +673,10 @@ def _future_timeout_checker(self, futures=None): # Raising and catching the error right away is what fills # sys.exc_info(), so that the client re-raises a real # traceback for a worker that never reported back - pickled_exception = str(pickle.dumps(sys.exc_info())) - call_status = { - 'type': '__end__', - 'exception': True, - 'exc_info': pickled_exception, - 'executor_id': fut.executor_id, - 'job_id': fut.job_id, - 'call_id': fut.call_id, - 'activation_id': fut.activation_id, - 'worker_start_tstamp': start_tstamp, - 'worker_end_tstamp': time.time(), - } + call_status = error_status( + fut.executor_id, fut.job_id, fut.call_id, + fut.activation_id, start_tstamp, + ) self._mark_ready(fut, call_status, OUTCOME_TIMEOUT) self._release_timed_out_worker(call_status) @@ -598,7 +744,9 @@ def _hold_status(self, call_status): the status that tells this monitor the nested futures exist. A message is read once, so dropping it here would leave a future running for ever. Held by call id, so an __end__ supersedes the - __init__ of the same call. + __init__ of the same call, and never the other way round: a service + that does not keep the order, or that redelivers, can hand the + __init__ over after the __end__. A worker that waits on an executor of its own sends every status of it here, most of which never match a future of this one. They are @@ -610,6 +758,10 @@ def _hold_status(self, call_status): status_id = _status_id(held) now = time.time() with self._held_lock: + if held['type'] == '__init__' and _holds_end( + self._held_status, status_id + ): + return # Taken out first, so that the dict stays in order of arrival # and the first key is always the oldest self._held_status.pop(status_id, None) @@ -756,9 +908,7 @@ def _generate_tokens(self, call_status): worker_id not in self.workers_done and len(done_for_worker) >= worker_calls ): - self.workers_done.add(worker_id) - if self.should_run: - self.token_bucket_q.put('#') + self._hand_back(call_status['job_id'], worker_id) def _apply_status_message(self, call_status): """ @@ -778,10 +928,13 @@ def _apply_status_message(self, call_status): return if self._tag_future_as_ready(call_status): self._generate_tokens(call_status) - elif call_status.get('job_id') in self._dropped_jobs: + elif call_status.get('executor_id') == self.executor_id and ( + call_status.get('job_id') in self._dropped_jobs + or call_status.get('job_id') in self._token_closed_jobs + ): # wait() dropped the futures, but the worker is still # this executor's and must free its token, unless the - # invoker already wrote the job off + # job owes none any more self._generate_tokens(call_status) else: self._hold_status(call_status) @@ -848,16 +1001,20 @@ def _storage_sweep(self): return 0 pending = [f for f in self.tracked_futures() if not _is_finished(f)] - if not pending: + owing = self._jobs_owing_tokens() + if not pending and not owing: return 0 _running, callids_done = self.internal_storage.get_job_status( - self.executor_id, job_ids=self.job_ids() + self.executor_id, job_ids=self.job_ids() | owing ) - if not callids_done: - return 0 recovered = 0 + if owing: + recovered += self._sweep_dropped_jobs(owing, callids_done) + if not callids_done: + return recovered + for future in pending: future_id = _future_id(future) if future_id not in callids_done or _is_finished(future): @@ -876,6 +1033,31 @@ def _storage_sweep(self): ) return recovered + def _sweep_dropped_jobs(self, jobs, callids_done): + """ + Hands back the token of a worker of a job wait() dropped whose + __end__ message never arrived, which no future is left to recover. + Only the calls no __end__ was counted for are read + """ + with self._apply_lock: + counted = set().union(*self.callids_done_worker.values()) + + recovered = 0 + for status_id in callids_done: + if status_id[1] not in jobs or status_id in counted: + continue + call_status = self.internal_storage.get_call_status(*status_id) + if not call_status or call_status.get('activation_id') is None: + continue + executor_id, job_id, call_id = status_id + self._generate_tokens(dict( + call_status, executor_id=executor_id, job_id=job_id, call_id=call_id, + )) + recovered += 1 + + self._give_up_dropped_jobs() + return recovered + class PollingMessageMonitor(MessageMonitor): """ diff --git a/lithops/retries.py b/lithops/retries.py index 395b84a4e..332c47f06 100644 --- a/lithops/retries.py +++ b/lithops/retries.py @@ -12,11 +12,13 @@ # limitations under the License. # +from collections.abc import Iterator from typing import Any, Callable, Dict, List, Optional, Tuple, Union from lithops import FunctionExecutor from lithops.future import ResponseFuture from lithops.storage.utils import CloudObject +from lithops.utils import _as_iterdata_list, _chained_futures, is_object_processing_function from lithops.wait import ( ALL_COMPLETED, ALWAYS, @@ -136,7 +138,8 @@ def status( internal_storage=internal_storage, check_only=check_only, ) - self._reraise_if_error() + if throw_except: + self._reraise_if_error() return stat def result(self, throw_except: bool = True, internal_storage: Any = None): @@ -150,7 +153,8 @@ def result(self, throw_except: bool = True, internal_storage: Any = None): res = self.response_future.result( throw_except=throw_except, internal_storage=internal_storage ) - self._reraise_if_error() + if throw_except: + self._reraise_if_error() return res @@ -239,6 +243,14 @@ def map( function activation. """ + # A retry resubmits the input of the call that failed, and a single + # partition of an object cannot be resubmitted on its own + if (obj_chunk_size or obj_chunk_number) and is_object_processing_function(map_function): + raise ValueError( + 'RetryingFunctionExecutor does not support obj_chunk_size or ' + 'obj_chunk_number: a failed partition cannot be retried on its ' + 'own. Use FunctionExecutor to process objects in chunks' + ) retries_to_use = self._retries_to_use(retries) map_kwargs = dict( @@ -254,11 +266,27 @@ def map( exclude_modules=exclude_modules, ) + # A generator can be read only once, and the inputs are kept to be + # resubmitted on a retry + if isinstance(map_iterdata, Iterator): + map_iterdata = list(map_iterdata) + # The future of each call is paired with the input it was made from. + # A chain leaves out the futures that produce no input + inputs = _chained_futures(map_iterdata) + if inputs is None: + inputs = _as_iterdata_list(map_iterdata) + futures_list = self.executor.map( map_function, map_iterdata, **map_kwargs, ) + if len(futures_list) != len(inputs): + raise ValueError( + f'The {len(inputs)} inputs turned into {len(futures_list)} ' + 'calls, which are running now but cannot be retried: a retry ' + 'resubmits the input of the call that failed' + ) return [ RetryingFuture( f, @@ -267,7 +295,7 @@ def map( retries=retries_to_use, **map_kwargs, ) - for i, f in zip(map_iterdata, futures_list) + for i, f in zip(inputs, futures_list) ] def _split_done_and_retried(self, done, pending, lookup): @@ -310,7 +338,11 @@ def _wait_is_over(return_when, retrying_done, retrying_pending): return bool(retrying_done) if return_when == ALL_COMPLETED: return not retrying_pending - return False + # A percentage of the calls, as lithops.wait() takes it + total = len(retrying_done) + len(retrying_pending) + if not total: + return True + return len(retrying_done) * 100 / total >= return_when def wait( self, @@ -338,14 +370,18 @@ def wait( :return: A tuple (done, pending) of lists of RetryingFutures. """ + if not fs: + return [], [] lookup = {f.response_future: f for f in fs} while True: # A retry replaces the response future of a RetryingFuture, so - # the list has to be rebuilt on every round + # the list has to be rebuilt on every round. The inner wait never + # raises: a call that failed would end the whole wait there, and + # the executor would discard every future instead of retrying it done, pending = self.executor.wait( [f.response_future for f in fs], - throw_except=throw_except, + throw_except=False, return_when=return_when, download_results=download_results, timeout=timeout, @@ -361,6 +397,10 @@ def wait( if self._wait_is_over( return_when, retrying_done, retrying_pending ): + if throw_except: + # Only the calls with no retries left are still failed + for retrying_future in retrying_done: + retrying_future._reraise_if_error() return retrying_done, retrying_pending def clean( diff --git a/lithops/storage/backends/localhost/localhost.py b/lithops/storage/backends/localhost/localhost.py index 84e00582a..cef061d86 100644 --- a/lithops/storage/backends/localhost/localhost.py +++ b/lithops/storage/backends/localhost/localhost.py @@ -16,7 +16,6 @@ import os import io -import glob import shutil import logging from lithops.storage.utils import StorageNoSuchKeyError @@ -90,24 +89,37 @@ def get_object(self, bucket_name, key, stream=False, extra_get_args={}): :return: Data of the object :rtype: str/bytes """ - buffer = None + first_byte = last_byte = None + if 'Range' in extra_get_args: + # 'bytes=a-b', 'bytes=a-' (to the end) or 'bytes=-n' (the last n) + byte_range = extra_get_args['Range'].replace('bytes=', '') + first_byte, last_byte = byte_range.split('-') + first_byte = int(first_byte) if first_byte else None + last_byte = int(last_byte) if last_byte else None + + file_path = os.path.join(LITHOPS_TEMP_DIR, bucket_name, key) + # Windows raises PermissionError when opening a directory + if not os.path.isfile(file_path): + raise StorageNoSuchKeyError(os.path.join(LITHOPS_TEMP_DIR, bucket_name), key) try: - file_path = os.path.join(LITHOPS_TEMP_DIR, bucket_name, key) with open(file_path, "rb") as f: - if 'Range' in extra_get_args: - byte_range = extra_get_args['Range'].replace('bytes=', '') - first_byte, last_byte = map(int, byte_range.split('-')) + if first_byte is None and last_byte is not None: + f.seek(max(os.fstat(f.fileno()).st_size - last_byte, 0)) + buffer = io.BytesIO(f.read()) + elif first_byte is not None: f.seek(first_byte) - buffer = io.BytesIO(f.read(last_byte - first_byte + 1)) + size = -1 if last_byte is None else last_byte - first_byte + 1 + buffer = io.BytesIO(f.read(size)) else: buffer = io.BytesIO(f.read()) - if stream: - return buffer - else: - return buffer.read() - except Exception: + except FileNotFoundError: raise StorageNoSuchKeyError(os.path.join(LITHOPS_TEMP_DIR, bucket_name), key) + if stream: + return buffer + else: + return buffer.read() + def upload_file(self, file_name, bucket, key=None, extra_args={}, config=None): """Upload a file @@ -245,21 +257,23 @@ def list_keys(self, bucket_name, prefix=None): """ key_list = [] base_dir = os.path.join(LITHOPS_TEMP_DIR, bucket_name, '') - - if prefix: - if prefix.endswith('/'): - roots = [os.path.join(base_dir, prefix, '**')] - else: - roots = [ - os.path.join(base_dir, prefix + '*'), - os.path.join(base_dir, prefix + '*', '**'), - ] - else: - roots = [os.path.join(base_dir, '**')] - - for root in roots: - for file_name in glob.glob(root, recursive=True): - if os.path.isfile(file_name): - key_list.append(file_name.replace(base_dir, '').replace('\\', '/')) + prefix = prefix or '' + + # A prefix is literal, as in S3, and dotfiles are keys like any other, + # so walk the bucket rather than glob it, and only descend into the + # directories that can hold a key starting with the prefix + for dir_path, dir_names, file_names in os.walk(base_dir): + rel_dir = os.path.relpath(dir_path, base_dir).replace(os.sep, '/') + rel_dir = '' if rel_dir == '.' else rel_dir + '/' + dir_names[:] = [ + name for name in dir_names + if (rel_dir + name + '/').startswith(prefix) or prefix.startswith(rel_dir + name + '/') + ] + for name in file_names: + key = rel_dir + name + if key.startswith(prefix): + key_list.append(key) + + key_list.sort() return key_list diff --git a/lithops/storage/cloud_proxy.py b/lithops/storage/cloud_proxy.py index fec481c8c..df3144293 100644 --- a/lithops/storage/cloud_proxy.py +++ b/lithops/storage/cloud_proxy.py @@ -42,6 +42,18 @@ def remove_lithops_keys(keys: Iterable[str]) -> List[str]: return [key for key in keys if not key.startswith(_LITHOPS_PREFIXES)] +def _to_key(path, as_dir=False): + """ + Turns a path into the key, or key prefix, that matches it: keys have no + leading slash, and a trailing one is added when only the contents of a + directory should match + """ + key = path[1:] if path.startswith('/') else path + if as_dir and key != '' and not key.endswith('/'): + key = key + '/' + return key + + # # Picklable cloud object storage client # @@ -112,12 +124,7 @@ def listdir(self, path='', suffix_dirs=False): flat in storage, so a name is the first segment left after the prefix, and the ones that stand for a directory can be marked with a slash """ - if path == '': - prefix = '' - elif path.startswith('/'): - prefix = path[1:] - else: - prefix = path if path.endswith('/') else path + '/' + prefix = _to_key(path, as_dir=True) names = set() for p in remove_lithops_keys(self._storage.list_bucket_keys(prefix=prefix)): @@ -169,7 +176,7 @@ def walk(self, top, topdown=True, onerror=None, followlinks=False): def remove(self, path): """Deletes the object a path names""" - self._storage.delete_data(path) + self._storage.delete_data(_to_key(path)) def mkdir(self, *args, **kwargs): """Does nothing: storage has no directories to create""" @@ -193,31 +200,17 @@ def __getattr__(self, name): # we only reach here if the attr is not defined return getattr(base_os.path, name) - def _prefix(self, path, as_dir=False): - """ - Turns a path into the key prefix that matches it, with a trailing - slash when only the contents of a directory should match - """ - prefix = path[1:] if path.startswith('/') else path - if as_dir and prefix != '' and not prefix.endswith('/'): - prefix = prefix + '/' - return prefix - def isfile(self, path): - """True when the path names one object and not a prefix of others""" - prefix = self._prefix(path) + """True when the path names an object""" + key = _to_key(path) keys = remove_lithops_keys( - self._storage.list_bucket_keys(prefix=prefix) + self._storage.list_bucket_keys(prefix=key) ) - if len(keys) == 1: - key = keys.pop() - key = key[len(prefix):] - return key == '' - return False + return key != '' and key in keys def isdir(self, path): """True when there is at least one object under the path""" - prefix = self._prefix(path, as_dir=True) + prefix = _to_key(path, as_dir=True) keys = remove_lithops_keys( self._storage.list_bucket_keys(prefix=prefix) ) @@ -225,9 +218,12 @@ def isdir(self, path): def exists(self, path): """True when the path names an object or a directory holding one""" - dirpath = path if path.endswith('/') else path + '/' - for key in self._storage.list_bucket_keys(prefix=path): - if key.startswith(dirpath) or key == path: + if path == '': + return False + key = _to_key(path) + dirpath = _to_key(path, as_dir=True) + for k in self._storage.list_bucket_keys(prefix=key): + if k.startswith(dirpath) or k == key: return True return False @@ -239,6 +235,10 @@ class _DelayedClose: """ def close(self): + # Closing again is a no-op, as for a file, and the buffer stays open + # if the upload fails so that it can be retried + if self.closed: + return self._action(self.getvalue()) super().close() @@ -262,23 +262,31 @@ def __init__(self, action, initial_value=None): def cloud_open(filename, mode='r', cloud_storage=None): """ Opens an object as a file-like buffer. Reading brings the whole object - into memory, and writing uploads it when the buffer is closed + into memory, and writing uploads it when the buffer is closed. With + 'r+' the buffer starts with the object, as a file does, and is uploaded + on close """ storage = cloud_storage or CloudStorage() + key = _to_key(filename) if 'r' in mode: - data = storage.get_data(filename) + data = storage.get_data(key) + if '+' in mode: + action = partial(storage.put_data, key) + if 'b' in mode: + return DelayedBytesBuffer(action, data) + return DelayedStringBuffer(action, data.decode()) if 'b' in mode: # we could get_data(stream=True) but some streams are not seekable return io.BytesIO(data) return io.StringIO(data.decode()) if 'w' in mode: - action = partial(storage.put_data, filename) + action = partial(storage.put_data, key) if 'b' in mode: return DelayedBytesBuffer(action) return DelayedStringBuffer(action) - raise ValueError(f"Unsupported mode '{mode}': only 'r' and 'w' are") + raise ValueError(f"Unsupported mode '{mode}': only 'r', 'w' and their '+' and 'b' forms are") def __getattr__(name): diff --git a/lithops/tests/test_concurrent_futures.py b/lithops/tests/test_concurrent_futures.py index d9b9e05c6..b436900ab 100644 --- a/lithops/tests/test_concurrent_futures.py +++ b/lithops/tests/test_concurrent_futures.py @@ -689,6 +689,17 @@ def test_wait_and_as_completed(self): assert not not_done assert {f.result() for f in as_completed([f1, f2])} == {256, 8} + def test_wait_and_as_completed_take_a_generator(self): + """Reading the futures to sync them used to exhaust it, and leave none""" + inner = FakeInnerExecutor() + inner._call_results = [FakeLithopsFuture(value=1), FakeLithopsFuture(value=2)] + with _adapter(inner) as ex: + fs = [ex.submit(str, 1), ex.submit(str, 2)] + done, not_done = wait(f for f in fs) + assert done == set(fs) + assert not not_done + assert set(as_completed(f for f in fs)) == set(fs) + def test_wait_first_completed(self): pending = FakeLithopsFuture(finished=False) inner = FakeInnerExecutor() diff --git a/lithops/tests/test_config.py b/lithops/tests/test_config.py index b98a06f9d..0c0d3d93f 100644 --- a/lithops/tests/test_config.py +++ b/lithops/tests/test_config.py @@ -21,6 +21,7 @@ from lithops import constants as c from lithops.config import ( + _check_loopback_addresses, _ensure_lithops_section, _resolve_mode_and_backend, _section_with_user_agent, @@ -554,6 +555,46 @@ def test_azure_queue_monitoring_requires_account(self): default_config(config_data=_localhost_input(monitoring='azure_queue')) +class TestLoopbackAddresses: + + @staticmethod + def _config(mode, storage, monitoring, **sections): + backend = 'aws_ec2' if mode == 'standalone' else 'aws_lambda' + lithops_cfg = {'mode': mode, 'backend': backend, 'storage': storage, 'monitoring': monitoring} + return {'lithops': lithops_cfg, **sections} + + @pytest.mark.parametrize('storage, monitoring, sections', [ + ('aws_s3', 'redis', {'redis': {'host': '127.0.0.1'}}), + ('aws_s3', 'redis', {'redis': {'host': '::1'}}), + ('aws_s3', 'rabbitmq', {'rabbitmq': {'amqp_url': 'amqp://user:pass@localhost:5672/vhost'}}), + ('minio', 'storage', {'minio': {'endpoint': 'http://127.0.0.1:9000'}}), + ('redis', 'storage', {'redis': {'host': 'localhost:6379'}}), + ]) + def test_serverless_workers_refuse_a_loopback_address(self, storage, monitoring, sections): + """A serverless worker would reach its own container, so the job cannot work""" + config = self._config('serverless', storage, monitoring, **sections) + with pytest.raises(Exception, match='is a loopback address, which the aws_lambda workers cannot reach'): + _check_loopback_addresses(config) + + def test_standalone_workers_only_warn(self): + """An SSH tunnel can bring the service to the loopback address of the VM""" + config = self._config('standalone', 'aws_s3', 'redis', redis={'host': '127.0.0.1'}) + with patch('lithops.config.logger') as logger: + _check_loopback_addresses(config) + assert 'is a loopback address, which the aws_ec2 workers cannot reach' in logger.warning.call_args[0][0] + + @pytest.mark.parametrize('mode, host', [ + ('localhost', '127.0.0.1'), + ('serverless', '10.0.0.5'), + ('standalone', 'redis.example.com'), + ]) + def test_a_reachable_address_is_accepted(self, mode, host): + config = self._config(mode, 'aws_s3', 'redis', redis={'host': host}) + with patch('lithops.config.logger') as logger: + _check_loopback_addresses(config) + logger.warning.assert_not_called() + + class TestStorageAndExtract: def test_default_storage_config_localhost(self): diff --git a/lithops/tests/test_executors.py b/lithops/tests/test_executors.py index 7787ee02b..93988369f 100644 --- a/lithops/tests/test_executors.py +++ b/lithops/tests/test_executors.py @@ -71,6 +71,8 @@ def __init__(self, **kwargs): self.success = False self.error = False self.futures = None + self.ready = False + self._consumers = [] self._produce_output = True self._read = False self.job_id = 'M000' @@ -133,11 +135,11 @@ def test_as_future_list_wraps_single_future(self): def test_disable_iterdata_output_only_for_futures_list(self): future = FakeFuture(_produce_output=True) - FunctionExecutor._disable_iterdata_output([future]) + FunctionExecutor._disable_iterdata_output([future], []) assert future._produce_output is True wrapped = FakeFuture(_produce_output=True) - FunctionExecutor._disable_iterdata_output(FuturesList([wrapped])) + FunctionExecutor._disable_iterdata_output(FuturesList([wrapped]), []) assert wrapped._produce_output is False def test_create_job_id_increments_and_zero_fills(self): @@ -259,6 +261,70 @@ def test_clean_keeps_a_job_with_results_still_to_read(self): executor.clean(fs=[read, unread], clean_cloudobjects=False) assert create_job_key('abc-0', 'M000') in executor.cleaned_jobs + def test_clean_keeps_a_producer_until_its_chained_consumer_ends(self): + """A chained call reads its producer's result from storage, whoever waited on it""" + consumer = FakeFuture(executor_id='abc-0', job_id='M001', ready=False) + producer = FakeFuture( + executor_id='abc-0', job_id='M000', done=True, + _produce_output=False, _consumers=[[consumer]], + ) + executor = _bare_executor( + cleaned_jobs=set(), executor_id='abc-0', futures=[producer, consumer] + ) + with patch('lithops.executors._dump_cleaner_data') as dump, \ + patch('lithops.executors.sp.Popen'): + executor.clean(fs=[producer], clean_cloudobjects=False) + dump.assert_not_called() + assert executor.cleaned_jobs == set() + + consumer.ready = True + with patch('lithops.executors._dump_cleaner_data'), \ + patch('lithops.executors.sp.Popen'): + executor.clean(fs=[producer], clean_cloudobjects=False) + assert create_job_key('abc-0', 'M000') in executor.cleaned_jobs + + def test_clean_of_a_consumer_cleans_the_producer_it_kept(self): + consumer = FakeFuture(executor_id='abc-0', job_id='M001') + producer = FakeFuture( + executor_id='abc-0', job_id='M000', done=True, + _produce_output=False, _consumers=[[consumer]], + ) + executor = _bare_executor( + cleaned_jobs=set(), executor_id='abc-0', futures=[producer, consumer] + ) + with patch('lithops.executors._dump_cleaner_data'), \ + patch('lithops.executors.sp.Popen'): + executor.clean(fs=[producer], clean_cloudobjects=False) + assert executor.cleaned_jobs == set() + consumer.done = True + executor.clean(fs=[consumer], clean_cloudobjects=False) + assert executor.cleaned_jobs == { + create_job_key('abc-0', 'M000'), create_job_key('abc-0', 'M001') + } + + def test_forced_clean_keeps_a_producer_while_its_consumer_runs(self): + """A wait that failed forces the clean of the jobs it waited on""" + consumer = FakeFuture(executor_id='abc-0', job_id='M001') + producer = FakeFuture( + executor_id='abc-0', job_id='M000', done=True, + _produce_output=False, _consumers=[[consumer]], + ) + executor = _bare_executor( + cleaned_jobs=set(), executor_id='abc-0', futures=[producer, consumer] + ) + with patch('lithops.executors._dump_cleaner_data'), \ + patch('lithops.executors.sp.Popen'): + executor.clean(fs=[producer], clean_cloudobjects=False, force=True) + assert executor.cleaned_jobs == set() + executor.clean(fs=[producer], clean_cloudobjects=False, on_exit=True) + assert executor.cleaned_jobs == {create_job_key('abc-0', 'M000')} + + def test_disable_iterdata_output_records_the_consumers(self): + producer = FakeFuture() + consumers = [FakeFuture()] + FunctionExecutor._disable_iterdata_output(FuturesList([producer]), consumers) + assert producer._consumers == [consumers] + def test_clean_on_exit_deletes_a_job_with_unread_results(self): """ After the process exits nothing will read the leftover result, so @@ -488,9 +554,7 @@ def test_wait_exception_drops_its_queued_calls_and_reraises(self, mock_wait): executor.wait([future], show_progressbar=False) executor.invoker.stop.assert_not_called() - executor.invoker.discard_pending.assert_called_once_with( - {future.job_key}, {future.job_id} - ) + executor.invoker.discard_pending.assert_called_once_with({future.job_key}) executor.job_monitor.remove.assert_called_once() assert future._exception_set is True assert cleanup.call_args.kwargs['force'] is True @@ -658,6 +722,73 @@ def test_map_reduce_always_skips_waiting_for_map(self): assert map_futures[0]._mapreduce is True assert list(result) == map_futures + reduce_futures + def test_map_reduce_keeps_the_map_job_until_the_reducer_ends(self): + executor = _bare_executor( + total_jobs=0, cleaned_jobs=set(), executor_id='abc-0', data_cleaner=True + ) + map_futures = [FakeFuture(executor_id='abc-0', job_id='M000', done=True)] + reduce_futures = [FakeFuture(executor_id='abc-0', job_id='R000')] + executor.futures = map_futures + reduce_futures + with patch.object( + executor, '_submit_map', return_value=('M000', MagicMock(), map_futures) + ), patch('lithops.executors.wait'), patch.object( + executor, '_run_reduce_job', return_value=reduce_futures + ), patch('lithops.executors._dump_cleaner_data'), patch('lithops.executors.sp.Popen'): + executor.map_reduce(lambda x: x, [1], lambda xs: xs, spawn_reducer=ALL_COMPLETED) + assert executor.cleaned_jobs == set() + executor.wait(map_futures, show_progressbar=False) + assert executor.cleaned_jobs == set() + reduce_futures[0].done = True + executor.wait(reduce_futures, show_progressbar=False) + assert executor.cleaned_jobs == { + create_job_key('abc-0', 'M000'), create_job_key('abc-0', 'R000') + } + + +class TestExplicitEmptyFutures: + """An empty fs means no futures at all, only None means every one""" + + @patch('lithops.executors.wait') + def test_wait_on_an_empty_list_waits_on_nothing(self, mock_wait): + executor = _bare_executor(futures=[FakeFuture()]) + done, not_done = executor.wait([], show_progressbar=False) + assert (list(done), list(not_done)) == ([], []) + mock_wait.assert_not_called() + + @patch('lithops.executors.wait') + def test_only_no_fs_waits_on_the_executor_futures(self, mock_wait): + future = FakeFuture() + executor = _bare_executor(futures=[future]) + executor.wait(show_progressbar=False) + assert mock_wait.call_args.kwargs['fs'] == [future] + assert mock_wait.call_args.kwargs['futures_from_executor_wait'] is True + executor.wait([future], show_progressbar=False) + assert mock_wait.call_args.kwargs['futures_from_executor_wait'] is False + + def test_get_result_of_an_empty_list_is_empty(self): + future = FakeFuture(done=True, success=True, _result=7) + executor = _bare_executor( + data_cleaner=True, last_call='map', futures=[future] + ) + with patch.object(executor, '_cleanup_jobs') as cleanup: + assert executor.get_result([]) == [] + cleanup.assert_not_called() + assert future._read is False + with patch.object(executor, 'wait', return_value=([future], [])), \ + patch.object(executor, '_cleanup_jobs'): + assert executor.get_result() == [7] + + def test_clean_of_an_empty_list_cleans_nothing(self): + future = FakeFuture(executor_id='abc-0', job_id='M000', done=True) + executor = _bare_executor( + cleaned_jobs=set(), executor_id='abc-0', futures=[future] + ) + with patch('lithops.executors._dump_cleaner_data') as dump, \ + patch('lithops.executors.sp.Popen'): + executor.clean(fs=[], clean_cloudobjects=False, force=True) + dump.assert_not_called() + assert executor.cleaned_jobs == set() + class TestExecutorLocalhost: """Live localhost checks for the refactored public API.""" diff --git a/lithops/tests/test_future.py b/lithops/tests/test_future.py index a72b8924f..bb5e11d8e 100644 --- a/lithops/tests/test_future.py +++ b/lithops/tests/test_future.py @@ -22,7 +22,9 @@ import pytest import lithops +from lithops.executors import FunctionExecutor from lithops.future import ResponseFuture, _pickle_from_encoded, _stats_from_prefixed_keys +from lithops.utils import FuturesList class HasAmbiguousTruthValue: @@ -203,6 +205,21 @@ def test_set_exception_does_not_change_already_done_state(self): assert future._state == ResponseFuture.State.Done assert future._read is True + def test_pickling_a_producer_leaves_its_consumers_out(self): + """ + A chained job pickles its producers into its input: their consumers, + with the results the client downloaded, used to travel along + """ + producer = _future() + size = len(pickle.dumps(producer)) + consumer = _future(job_id='M001') + consumer._call_output = b'x' * 10**6 + FunctionExecutor._disable_iterdata_output(FuturesList([producer]), [consumer]) + + assert len(pickle.dumps(producer)) < size + 100 + assert pickle.loads(pickle.dumps(producer))._consumers == [] + assert producer._consumers == [[consumer]] + def test_futures_property_tracks_new_futures(self): future = _future() assert future.futures is False diff --git a/lithops/tests/test_invokers.py b/lithops/tests/test_invokers.py index 9e91efea2..64ffd9db2 100644 --- a/lithops/tests/test_invokers.py +++ b/lithops/tests/test_invokers.py @@ -334,6 +334,8 @@ def test_discard_pending_keeps_the_calls_of_other_jobs(self): assert left == [other, other] assert inv.running_workers == 4 inv.job_monitor.close_jobs.assert_not_called() + # The job settles on the calls dropped here, with no grace to wait + inv.job_monitor.discard_calls.assert_called_once_with({'M000': 4}) def test_discard_pending_forgets_workers_when_no_other_job_is_queued(self): """ @@ -347,11 +349,22 @@ def test_discard_pending_forgets_workers_when_no_other_job_is_queued(self): failed = _job(chunksize=1) failed.job_key = 'sess-0/M000' inv._queue_call_ranges(failed, range(2)) - inv.discard_pending({'sess-0/M000'}, {'M000'}) + inv.discard_pending({'sess-0/M000'}) assert inv.running_workers == 0 assert inv.pending_calls_q.empty() assert inv.job_monitor.token_bucket_q.empty() - inv.job_monitor.close_jobs.assert_called_once_with({'M000'}) + # Every job invoked so far, not only the ones waited on + inv.job_monitor.close_jobs.assert_called_once_with() + + def test_discard_pending_closes_the_jobs_before_emptying_the_bucket(self): + """A token handed back before the jobs are closed would outlive the reset""" + inv = self._faas() + inv.running_workers = 4 + inv.job_monitor.close_jobs.side_effect = ( + lambda *args: inv.job_monitor.token_bucket_q.put('#') + ) + inv.discard_pending({'sess-0/M000'}) + assert inv.job_monitor.token_bucket_q.empty() def test_a_restart_forgets_tokens_handed_back_after_the_stop(self): """ @@ -403,6 +416,92 @@ def test_invoke_task_requeues_when_activation_missing(self, monkeypatch): assert list(ids) == [0, 1] assert inv.job_monitor.token_bucket_q.get_nowait() == '#' + def test_invoke_task_fails_the_calls_when_the_invocation_raises(self): + """A raised error must end the wait for the chunk and return its token""" + inv = self._faas() + inv.compute_handler.invoke.side_effect = RuntimeError('provider rejected request') + job = _job() + job.futures = [MagicMock(ready=False, success=False, done=False) for _ in range(2)] + inv._invoke_task(job, [0]) + job.futures[0]._set_ready.assert_called_once() + status = job.futures[0]._set_ready.call_args[0][0] + assert status['exception'] is True + assert status['call_id'] == '00000' + job.futures[1]._set_ready.assert_not_called() + assert inv.pending_calls_q.empty() + assert inv.job_monitor.token_bucket_q.get_nowait() == '#' + # The invoker returned the token, so no worker of the monitor owes it + inv.job_monitor.discard_calls.assert_called_once_with({'M000': 1}) + + def test_an_error_that_does_not_pickle_still_reaches_the_future(self): + class Unpicklable(Exception): + def __reduce__(self): + raise TypeError('cannot pickle') + + inv = self._faas() + inv.compute_handler.invoke.side_effect = Unpicklable('provider rejected request') + job = _job(total_calls=1, metadata={'func_name': 'fn'}) + futures = inv._build_futures(job) + inv._invoke_task(job, [0]) + with pytest.raises(RuntimeError, match='provider rejected request'): + futures[0].status() + + def test_the_remote_invoker_payload_is_json(self): + """The futures built before invoking are not part of it: aws_lambda json.dumps it""" + import json + inv = self._faas() + inv.remote_invoker = True + inv.compute_handler.invoke.return_value = 'act-1' + job = _job() + inv._build_futures(job) + inv._invoke_job(job) + payload = inv.compute_handler.invoke.call_args[0][0] + assert 'futures' not in payload['job'] + json.dumps(payload) + + def test_a_failed_invocation_reaches_the_future_as_its_exception(self): + inv = self._faas() + inv.compute_handler.invoke.side_effect = RuntimeError('provider rejected request') + job = _job(total_calls=1, metadata={'func_name': 'fn'}) + inv.storage_config = {'backend': 'localhost', 'localhost': {'storage_bucket': 'bkt'}} + futures = inv._build_futures(job) + inv._invoke_task(job, [0]) + assert futures[0].ready + with pytest.raises(RuntimeError, match='provider rejected request'): + futures[0].status() + + def test_a_failed_sync_invocation_reaches_the_returned_futures(self, tmp_path, monkeypatch): + """Inside a worker the invoker waits for the invocation, so the futures must exist by then""" + from concurrent.futures import ThreadPoolExecutor + monkeypatch.setattr('lithops.invokers.LOGS_DIR', str(tmp_path)) + inv = self._faas() + inv.sync = True + inv.should_run = True + inv.max_workers = 10 + inv.executor = ThreadPoolExecutor(2) + inv.include_function = False + inv.telemetry = MagicMock() + inv.storage_config = {'backend': 'localhost', 'localhost': {'storage_bucket': 'bkt'}} + inv.compute_handler.invoke.side_effect = RuntimeError('provider rejected request') + futures = inv._run_job(_job(total_calls=1, data_byte_ranges=[(0, 1)])) + inv.executor.shutdown() + assert futures[0].ready + with pytest.raises(RuntimeError, match='provider rejected request'): + futures[0].status() + + def test_a_restart_closes_the_jobs_invoked_before_the_stop(self): + """Their workers no longer count, so the monitor must not hand their tokens back""" + inv = self._faas(max_workers=10) + inv._start_async_invokers = MagicMock() + inv._invoke_direct = MagicMock() + inv.should_run = True + inv._invoke_job(_job(job_id='M000')) + inv.job_monitor.close_jobs.assert_not_called() + + inv.should_run = False + inv._invoke_job(_job(job_id='M001')) + inv.job_monitor.close_jobs.assert_called_once_with() + def test_invoke_job_queues_all_when_at_max_workers(self): inv = self._faas() inv.should_run = True diff --git a/lithops/tests/test_job.py b/lithops/tests/test_job.py index b309cb3ef..d02396aa8 100644 --- a/lithops/tests/test_job.py +++ b/lithops/tests/test_job.py @@ -13,6 +13,7 @@ # import errno +import json import logging import os import pickle @@ -711,6 +712,37 @@ def test_dict_iterdata_with_function(self): mods = ser._module_inspect({'fn': _echo, 'n': 1}) assert _echo.__module__.split('.')[0] in mods + def test_module_used_only_by_a_nested_function_is_found(self): + """ + The global names of an inner function belong to the module of the + outer one, such as the __main__ of a script, not to the serializer's + """ + namespace = {'__name__': '__main__', 'helper_mod_xyz': json} + exec( + 'def outer():\n' + ' def inner():\n' + ' return helper_mod_xyz.dumps(1)\n' + ' return inner()\n', + namespace + ) + ser = SerializeIndependent([['os', True]]) + assert 'json' in ser._module_inspect(namespace['outer']) + + def test_recursive_functions_are_inspected_once(self): + namespace = {'__name__': '__main__', 'helper_mod_xyz': json} + exec( + 'def fact(n):\n' + ' return 1 if n < 2 else n * fact(n - 1)\n' + 'def ping(n):\n' + ' return pong(n - 1) if n else helper_mod_xyz.dumps(n)\n' + 'def pong(n):\n' + ' return ping(n - 1) if n else 0\n', + namespace + ) + ser = SerializeIndependent([['os', True]]) + assert ser._module_inspect(namespace['fact']) == {'__main__'} + assert 'json' in ser._module_inspect(namespace['pong']) + def test_cython_function_name_inspects_globals(self): class cython_function_or_method: __globals__ = {'__file__': '/tmp/foo.pyx'} diff --git a/lithops/tests/test_map.py b/lithops/tests/test_map.py index a5949ff60..f3eec4a40 100644 --- a/lithops/tests/test_map.py +++ b/lithops/tests/test_map.py @@ -18,6 +18,7 @@ import lithops from lithops.tests.functions import ( simple_map_function, + simple_reduce_function, hello_world, lithops_inside_lithops_map_function, lithops_return_futures_map, @@ -52,6 +53,17 @@ def test_range_iterdata(self): result = fexec.get_result() assert result == ['Hello World!'] * 2 + def test_generator_iterdata(self): + """A generator is one input per element, not a single unpicklable input""" + fexec = lithops.FunctionExecutor(config=pytest.lithops_config) + fexec.map(simple_map_function, ((x, 1) for x in range(3))) + assert fexec.get_result() == [1, 2, 3] + + def test_iterator_iterdata_in_map_reduce(self): + fexec = lithops.FunctionExecutor(config=pytest.lithops_config) + fexec.map_reduce(simple_map_function, iter([(1, 1), (2, 2)]), simple_reduce_function) + assert fexec.get_result() == 6 + def test_dict_iterdata(self): fexec = lithops.FunctionExecutor(config=pytest.lithops_config) listDicts_iterdata = [{'x': 2, 'y': 8}, {'x': 2, 'y': 8}] @@ -198,3 +210,32 @@ def test_chaining_a_slice_keeps_unconsumed_outputs(self): first = fexec.map(lambda x: x * x, [1, 2]) fexec.map(lambda x: x * 2, first[:1]) assert fexec.get_result() == [4, 2] + + def test_chaining_after_map_reduce_maps_the_reduce_output_only(self): + """ + map_reduce returns the futures of both stages, but the map stage + output was already consumed by the reducer + """ + with lithops.FunctionExecutor(config=pytest.lithops_config) as fexec: + chained = fexec.map_reduce( + lambda x: x * 2, [1, 2, 3], lambda results: sum(results) + ).map(lambda x: x + 1) + assert len(chained) == 1 + assert chained.get_result() == [13] + + def test_chaining_the_map_stage_of_a_map_reduce_is_refused(self): + """It used to run a job of no calls and return an empty result""" + with lithops.FunctionExecutor(config=pytest.lithops_config) as fexec: + mr = fexec.map_reduce(lambda x: x * 2, [1, 2], lambda results: sum(results)) + fexec.wait(mr) + n_futures = len(fexec.futures) + with pytest.raises(ValueError, match='map stage of a map_reduce'): + fexec.map(lambda x: x + 1, mr[:-1]) + assert len(fexec.futures) == n_futures + + def test_chaining_after_a_call_that_returned_none(self): + """A call that returns None produces no output file, yet is chained""" + with lithops.FunctionExecutor(config=pytest.lithops_config) as fexec: + fs = fexec.map(lambda x: None, [1, 2]) + fexec.wait(fs) + assert fs.map(lambda x: x is None).get_result() == [True, True] diff --git a/lithops/tests/test_monitor.py b/lithops/tests/test_monitor.py index a4a7578d4..ceaa5c6ba 100644 --- a/lithops/tests/test_monitor.py +++ b/lithops/tests/test_monitor.py @@ -308,6 +308,40 @@ def test_start_creates_monitor_and_records_chunksize(self): instance.add_futures.assert_called_once_with(futures) instance.start.assert_called_once() + def test_a_replacing_monitor_carries_on_with_the_tokens_still_owed(self): + storage = MagicMock() + storage.get_storage_config.return_value = {'monitoring_interval': 2} + storage.backend = 'localhost' + job_monitor = JobMonitor('sess-0', storage) + job_monitor.job_chunksize.update({'M000': 1, 'M001': 1}) + job_monitor.job_total_calls.update({'M000': 1, 'M001': 1}) + job_monitor._spawn_monitor(generate_tokens=True) + old = job_monitor.monitor + dropped = FakeFuture('M000', invoked=True, call_id='00000') + tracked = FakeFuture('M001', invoked=True, call_id='00000') + old.add_futures([dropped, tracked]) + old.remove_futures([dropped]) + old.workers_done.add('w0') + old.stop() + + job_monitor._spawn_monitor(generate_tokens=True) + + new = job_monitor.monitor + assert new is not old and old._retired + # The job the old one still tracked is no future's any more either + assert new._jobs_owing_tokens() == {'M000', 'M001'} + assert 'w0' in new.workers_done + + def test_closing_every_job_closes_the_ones_started_so_far(self): + storage = MagicMock() + storage.get_storage_config.return_value = {'monitoring_interval': 2} + storage.backend = 'localhost' + job_monitor = JobMonitor('sess-0', storage) + job_monitor._spawn_monitor(generate_tokens=True) + job_monitor.job_total_calls.update({'M000': 1, 'M001': 1}) + job_monitor.close_jobs() + assert job_monitor.monitor._token_closed_jobs == {'M000', 'M001'} + def test_start_reuses_live_monitor(self): storage = MagicMock() storage.get_storage_config.return_value = {'monitoring_interval': 2} @@ -1036,6 +1070,291 @@ def sleep(seconds): assert sleeps == [] assert len(polls) == 2 + @staticmethod + def _listing(running, done): + """A get_job_status that, like the real one, only lists the jobs asked for""" + def get_job_status(executor_id, job_ids=None): + ids = set(job_ids or ()) + return ( + {call for call in running if call[0][1] in ids}, + {call_id for call_id in done if call_id[1] in ids}, + ) + return get_job_status + + def _two_jobs(self): + """A monitor with a job about to be dropped and another one still waited on""" + monitor = self._storage(chunksize=1) + monitor.job_chunksize['M001'] = 1 + monitor.job_total_calls = {'M000': 1, 'M001': 1} + dropped = FakeFuture('M000', invoked=True, call_id='00000') + other = FakeFuture('M001', invoked=True, call_id='00000') + monitor.add_futures([dropped, other]) + monitor.internal_storage.get_call_status.return_value = None + return monitor, dropped + + def _job_monitor(self, calls): + """A JobMonitor whose storage monitor follows a job of ``calls`` calls""" + storage = MagicMock() + storage.get_storage_config.return_value = {'monitoring_interval': 1} + storage.backend = 'localhost' + storage.get_call_status.return_value = None + job_monitor = JobMonitor('sess-0', storage) + job_monitor.job_chunksize['M000'] = 1 + job_monitor.job_total_calls['M000'] = calls + job_monitor._spawn_monitor(generate_tokens=True) + futures = [ + FakeFuture('M000', invoked=True, call_id=f'{i:05d}') for i in range(calls) + ] + job_monitor.monitor.add_futures(futures) + return job_monitor, futures + + def test_a_dropped_job_still_hands_its_token_back(self): + """ + wait() drops the futures of a job whose wait failed while another job + still has calls queued, so the invoker does not write it off. Its + worker finishes afterwards and must free its token + """ + monitor, dropped = self._two_jobs() + worker = (('sess-0', 'M000', '00000'), 'w1') + storage = monitor.internal_storage + storage.get_job_status.side_effect = self._listing({worker}, set()) + monitor._poll_and_process_job_status() + assert dropped.running is True + + monitor.remove_futures([dropped]) + storage.get_job_status.side_effect = self._listing( + {worker}, {('sess-0', 'M000', '00000')} + ) + new_done = monitor._poll_and_process_job_status() + + assert monitor.token_bucket_q.qsize() == 1 + # Its done calls are not news for the futures still tracked + assert new_done == set() + assert storage.get_job_status.call_args.kwargs['job_ids'] == {'M000', 'M001'} + + def test_a_job_dropped_before_the_listing_saw_it_done_hands_its_token_back(self): + """ + The blind sweep can read a status the listing does not show yet, and + wait() then drops the job before any listing charged the worker + """ + monitor, dropped = self._two_jobs() + monitor._last_blind_sweep = 0 + worker = (('sess-0', 'M000', '00000'), 'w1') + storage = monitor.internal_storage + storage.get_job_status.side_effect = self._listing({worker}, set()) + storage.get_call_status.side_effect = lambda e, j, c: ( + {'type': '__end__', 'activation_id': 'w1'} if j == 'M000' else None + ) + monitor._poll_and_process_job_status() + assert dropped.ready is True + monitor.remove_futures([dropped]) + + storage.get_job_status.side_effect = self._listing( + {worker}, {('sess-0', 'M000', '00000')} + ) + monitor._poll_and_process_job_status() + assert monitor.token_bucket_q.qsize() == 1 + + def test_a_dropped_job_whose_workers_are_free_is_not_listed_again(self): + """Every wait() that returns drops its jobs, which cost a LIST each once more""" + monitor = self._storage(chunksize=1) + monitor.job_total_calls = {'M000': 1} + future = FakeFuture('M000', invoked=True, call_id='00000') + monitor.add_futures([future]) + storage = monitor.internal_storage + storage.get_job_status.side_effect = self._listing( + {(('sess-0', 'M000', '00000'), 'w1')}, {('sess-0', 'M000', '00000')} + ) + storage.get_call_status.return_value = {'type': '__end__', 'activation_id': 'w1'} + monitor._poll_and_process_job_status() + assert future.ready is True + assert monitor.token_bucket_q.qsize() == 1 + + monitor.remove_futures([future]) + monitor._poll_and_process_job_status() + assert storage.get_job_status.call_count == 1 + assert monitor.token_bucket_q.qsize() == 1 + + def test_a_dropped_job_the_invoker_wrote_off_is_not_listed(self): + monitor, dropped = self._two_jobs() + monitor.remove_futures([dropped]) + monitor.close_jobs({'M000'}) + storage = monitor.internal_storage + storage.get_job_status.side_effect = self._listing( + {(('sess-0', 'M000', '00000'), 'w1')}, {('sess-0', 'M000', '00000')} + ) + monitor._poll_and_process_job_status() + assert storage.get_job_status.call_args.kwargs['job_ids'] == {'M001'} + assert monitor.token_bucket_q.empty() + + def test_a_monitor_without_tokens_does_not_list_the_jobs_still_owed(self): + """It can neither settle nor give up on them, and would list them on every poll""" + monitor, dropped = self._two_jobs() + monitor.remove_futures([dropped]) + monitor.generate_tokens = False + monitor.remove_futures(monitor.tracked_futures()) + monitor._poll_and_process_job_status() + monitor.internal_storage.get_job_status.assert_not_called() + + def test_a_dropped_job_waits_for_a_worker_the_listing_does_not_show_yet(self): + """ + A worker still on its way when the job was dropped is not in the + listing yet, and settling the job on the workers seen so far lost + the token of that one + """ + monitor = self._storage(chunksize=1) + monitor.job_total_calls = {'M000': 2} + futures = [ + FakeFuture('M000', invoked=True, call_id=f'{i:05d}') for i in range(2) + ] + monitor.add_futures(futures) + monitor.remove_futures(futures) + first, second = (('sess-0', 'M000', f'{i:05d}') for i in range(2)) + storage = monitor.internal_storage + storage.get_job_status.side_effect = self._listing({(first, 'w1')}, {first}) + monitor._poll_and_process_job_status() + assert monitor.token_bucket_q.qsize() == 1 + assert monitor._jobs_owing_tokens() == {'M000'} + + storage.get_job_status.side_effect = self._listing( + {(first, 'w1'), (second, 'w2')}, {first, second} + ) + monitor._poll_and_process_job_status() + assert monitor.token_bucket_q.qsize() == 2 + assert monitor._jobs_owing_tokens() == set() + + def test_a_dropped_job_settles_at_once_on_the_calls_the_invoker_discarded(self): + """A failed wait() drops the queued calls, which no worker ever frees""" + monitor = self._storage(chunksize=1) + monitor.job_total_calls = {'M000': 3} + futures = [ + FakeFuture('M000', invoked=True, call_id=f'{i:05d}') for i in range(3) + ] + monitor.add_futures(futures) + call = ('sess-0', 'M000', '00000') + monitor.internal_storage.get_call_status.return_value = None + monitor.internal_storage.get_job_status.side_effect = self._listing({(call, 'w1')}, {call}) + monitor._poll_and_process_job_status() + + monitor.discard_calls({'M000': 2}) + monitor.remove_futures(futures) + assert monitor._jobs_owing_tokens() == set() + assert monitor.token_bucket_q.qsize() == 1 + + def test_a_dropped_job_is_given_up_on_once_its_grace_is_over(self, monkeypatch): + """ + A worker that crashed never reports back, and the job stayed listed + for the life of the monitor. Past the execution timeout plus + HELD_STATUS_SLACK its worker hands its token back, as a timed-out + call does, and a late status hands none back again + """ + now = [1000.0] + monkeypatch.setattr('lithops.monitoring.monitor.time.time', lambda: now[0]) + monitor = self._storage(chunksize=1) + monitor.job_total_calls = {'M000': 1} + future = FakeFuture('M000', invoked=True, call_id='00000', execution_timeout=60) + monitor.add_futures([future]) + monitor.remove_futures([future]) + + call = ('sess-0', 'M000', '00000') + storage = monitor.internal_storage + storage.get_job_status.side_effect = self._listing({(call, 'w1')}, set()) + monitor._poll_and_process_job_status() + assert monitor.token_bucket_q.empty() + + now[0] += 60 + monitor.HELD_STATUS_SLACK + monitor._poll_and_process_job_status() + assert monitor.token_bucket_q.qsize() == 1 + assert monitor._jobs_owing_tokens() == set() + + storage.get_job_status.side_effect = self._listing({(call, 'w1')}, {call}) + polls = storage.get_job_status.call_count + monitor._poll_and_process_job_status() + assert storage.get_job_status.call_count == polls + assert monitor.token_bucket_q.qsize() == 1 + + def test_a_replacing_monitor_takes_over_the_tokens_still_owed(self): + """ + A failed wait() can stop the monitor, and the next job starts a new + one. The workers the old one was still waiting for hand their token + back through the new one, and the ones it already freed do not + """ + job_monitor, futures = self._job_monitor(calls=2) + old = job_monitor.monitor + first, second = (('sess-0', 'M000', f'{i:05d}') for i in range(2)) + running = {(first, 'w1'), (second, 'w2')} + storage = old.internal_storage + storage.get_job_status.side_effect = self._listing(running, {first}) + old._poll_and_process_job_status() + assert old.token_bucket_q.qsize() == 1 + old.remove_futures(futures) + old.stop() + + job_monitor._spawn_monitor(generate_tokens=True) + new = job_monitor.monitor + storage.get_job_status.side_effect = self._listing(running, {first, second}) + new._poll_and_process_job_status() + assert new.token_bucket_q.qsize() == 2 + assert new._jobs_owing_tokens() == set() + + def test_a_monitor_that_did_not_stop_in_time_hands_back_nothing_after_the_takeover(self): + """ + The old thread may outlive the join timeout. The worker it then finds + free is handed back by the new monitor only, never by both + """ + job_monitor, futures = self._job_monitor(calls=1) + old = job_monitor.monitor + call = ('sess-0', 'M000', '00000') + running = {(call, 'w1')} + storage = old.internal_storage + storage.get_job_status.side_effect = self._listing(running, set()) + old._poll_and_process_job_status() + old.remove_futures(futures) + + job_monitor._spawn_monitor(generate_tokens=True) + new = job_monitor.monitor + + # The old thread, still alive, now sees the worker finish + storage.get_job_status.side_effect = self._listing(running, {call}) + old._poll_and_process_job_status() + assert old.token_bucket_q.qsize() == 0 + + new._poll_and_process_job_status() + assert new.token_bucket_q.qsize() == 1 + + def test_no_token_bookkeeping_grows_when_tokens_are_off(self): + """Batch, localhost and standalone jobs kept one entry per call for the session""" + monitor = self._storage(generate_tokens=False) + for job in range(50): + futures = [ + FakeFuture(f'M{job:03d}', invoked=True, call_id=f'{i:05d}') for i in range(10) + ] + monitor.add_futures(futures) + monitor.remove_futures(futures) + assert monitor._dropped_jobs == {} + assert monitor._credited_calls == {} + assert monitor._discarded_calls == {} + + def test_a_nested_job_with_the_same_id_does_not_settle_this_one(self): + """A nested executor numbers its jobs from M000 too""" + monitor = self._storage(chunksize=1) + monitor.job_total_calls = {'M000': 1} + nested = FakeFuture('M000', invoked=True, executor_id='sess-1') + call = ('sess-0', 'M000', '00000') + storage = monitor.internal_storage + storage.get_call_status.return_value = None + storage.get_job_status.side_effect = self._listing(set(), set()) + monitor.add_futures([nested]) + monitor.remove_futures([nested]) + monitor._poll_and_process_job_status() + + own = FakeFuture('M000', invoked=True, call_id='00000') + monitor.add_futures([own]) + monitor.remove_futures([own]) + storage.get_job_status.side_effect = self._listing({(call, 'w1')}, {call}) + monitor._poll_and_process_job_status() + assert monitor.token_bucket_q.qsize() == 1 + class TestRabbitmqMonitorTags: @@ -1870,6 +2189,102 @@ def test_a_failing_sweep_does_not_take_the_monitor_down(self): monitor.add_futures([FakeFuture('M000', invoked=True)]) assert monitor._sweep_storage(0) > 0 + def test_the_storage_sweep_hands_back_the_token_of_a_dropped_job(self): + """ + wait() dropped the job and the __end__ message of its worker was + lost: no future is left for the sweep to recover, but the worker + still has to free its token + """ + storage = MagicMock() + tokens = queue.Queue() + monitor = self._monitor(storage=storage, tokens=tokens, chunksize={'M000': 1}) + monitor.job_total_calls = {'M000': 1} + future = FakeFuture('M000', invoked=True, call_id='00000') + monitor.add_futures([future]) + monitor.remove_futures([future]) + + storage.get_job_status.return_value = ( + {(('sess-0', 'M000', '00000'), 'act-1')}, {('sess-0', 'M000', '00000')} + ) + storage.get_call_status.return_value = {'type': '__end__', 'activation_id': 'act-1'} + assert monitor._storage_sweep() == 1 + assert tokens.qsize() == 1 + storage.get_job_status.assert_called_once_with('sess-0', job_ids={'M000'}) + + # The worker is free and every call is counted: nothing left to list + assert monitor._storage_sweep() == 0 + assert storage.get_job_status.call_count == 1 + assert tokens.qsize() == 1 + + def test_a_dropped_job_whose_end_was_counted_is_not_swept(self): + storage = MagicMock() + tokens = queue.Queue() + monitor = self._monitor(storage=storage, tokens=tokens, chunksize={'M000': 1}) + monitor.job_total_calls = {'M000': 1} + future = FakeFuture('M000', invoked=True, call_id='00000') + monitor.add_futures([future]) + payload, _raw = _status(kind='__end__') + monitor._apply_status_message(payload) + monitor.remove_futures([future]) + + assert monitor._storage_sweep() == 0 + storage.get_job_status.assert_not_called() + assert tokens.qsize() == 1 + assert monitor._jobs_owing_tokens() == set() + + def test_the_storage_sweep_gives_up_on_a_dropped_job(self, monkeypatch): + """ + A dropped job whose worker never reports back used to be listed on + every sweep for the life of the monitor + """ + now = [1000.0] + monkeypatch.setattr('lithops.monitoring.monitor.time.time', lambda: now[0]) + storage = MagicMock() + tokens = queue.Queue() + monitor = self._monitor(storage=storage, tokens=tokens, chunksize={'M000': 1}) + monitor.job_total_calls = {'M000': 1} + future = FakeFuture('M000', invoked=True, call_id='00000', execution_timeout=60) + monitor.add_futures([future]) + monitor.remove_futures([future]) + storage.get_job_status.return_value = ( + {(('sess-0', 'M000', '00000'), 'act-1')}, set() + ) + + monitor._storage_sweep() + assert tokens.empty() + now[0] += 60 + monitor.HELD_STATUS_SLACK + monitor._storage_sweep() + assert tokens.qsize() == 1 + + monitor._storage_sweep() + assert storage.get_job_status.call_count == 2 + # A late __end__ of the written-off job hands nothing back + payload, _raw = _status(kind='__end__') + payload['activation_id'] = 'act-2' + monitor._apply_status_message(payload) + assert tokens.qsize() == 1 + assert monitor._held_status == {} + + def test_a_stopped_monitor_does_not_give_up_on_a_dropped_job(self, monkeypatch): + """It hands no token back, so the job stays owed for the monitor that replaces it""" + now = [1000.0] + monkeypatch.setattr('lithops.monitoring.monitor.time.time', lambda: now[0]) + storage = MagicMock() + tokens = queue.Queue() + monitor = self._monitor(storage=storage, tokens=tokens, chunksize={'M000': 1}) + monitor.job_total_calls = {'M000': 1} + future = FakeFuture('M000', invoked=True, call_id='00000', execution_timeout=60) + monitor.add_futures([future]) + monitor.remove_futures([future]) + storage.get_job_status.return_value = ( + {(('sess-0', 'M000', '00000'), 'act-1')}, set() + ) + monitor.stop() + now[0] += 60 + monitor.HELD_STATUS_SLACK + monitor._storage_sweep() + assert tokens.empty() + assert monitor._jobs_owing_tokens() == {'M000'} + class TestBackendContract: """ @@ -3437,6 +3852,38 @@ def test_a_repeated_status_moves_to_the_back_of_the_line( (self.NESTED, 'M000', '00000'), ] + def test_a_later_init_does_not_replace_a_held_end(self): + """ + SQS, Pub/Sub and Azure do not keep the order, and may redeliver: the + __init__ of a call can arrive after its __end__. Held by call id, it + used to throw the __end__ away and leave the future running + """ + tokens = queue.Queue() + monitor = self.FakePoll('sess-0', None, tokens, {'M000': 1}, True, {}) + end, _raw = _status(kind='__end__') + init, _raw = _status(kind='__init__') + monitor._apply_status_message(end) + monitor._apply_status_message(init) + + future = FakeFuture('M000', invoked=True, call_id='00000') + monitor.add_futures([future]) + assert future.ready is True + assert tokens.qsize() == 1 + + def test_an_adopted_end_is_not_replaced_by_an_init(self): + old = self.FakePoll('sess-0', None, queue.Queue(), {}, False, {}) + new = self.FakePoll('sess-0', None, queue.Queue(), {}, False, {}) + old._apply_status_message(self._nested_end()) + init = self._nested_end() + init['type'] = '__init__' + new._apply_status_message(init) + + new.adopt_held_status(old) + + nested = FakeFuture('M000', invoked=True, executor_id=self.NESTED) + new.add_futures([nested]) + assert nested.ready is True + def _localhost_redis(): try: diff --git a/lithops/tests/test_retries.py b/lithops/tests/test_retries.py index 813bc1186..a0b78c94d 100644 --- a/lithops/tests/test_retries.py +++ b/lithops/tests/test_retries.py @@ -1,4 +1,5 @@ import time +from types import SimpleNamespace from unittest.mock import MagicMock import pytest @@ -6,6 +7,7 @@ from lithops import FunctionExecutor from lithops import RetryingFunctionExecutor from lithops.retries import RetryingFuture +from lithops.utils import FuturesList from lithops.wait import ALWAYS, ANY_COMPLETED @@ -317,6 +319,30 @@ def test_wait_any_completed_stops_when_one_succeeds(self): assert still_pending == [pending_f] assert inner.wait.call_count == 1 + def test_wait_a_percentage_returns_once_that_share_is_done(self): + """A percentage return_when used to loop for ever, even with every call done""" + finished = [FakeResponseFuture(error=False, result=i) for i in range(2)] + pending = [FakeResponseFuture() for _ in range(2)] + inner = MagicMock() + inner.config = {} + inner.wait.return_value = (finished, pending) + fs = [RetryingFuture(f, map_function=lambda x: x, input=i, retries=0) + for i, f in enumerate(finished + pending)] + executor = RetryingFunctionExecutor(inner) + done, still_pending = executor.wait(fs, return_when=50) + assert done == fs[:2] + assert still_pending == fs[2:] + assert inner.wait.call_count == 1 + + def test_wait_on_no_futures_returns_at_once(self): + """ANY_COMPLETED used to wait for ever for one of no futures""" + inner = MagicMock() + inner.config = {} + inner.wait.side_effect = [([], [])] + executor = RetryingFunctionExecutor(inner) + assert executor.wait([], return_when=ANY_COMPLETED) == ([], []) + assert executor.wait([]) == ([], []) + def test_wait_exhausted_retries_are_treated_as_done(self): failed = FakeResponseFuture(error=True) inner = MagicMock() @@ -347,3 +373,111 @@ def test_retries_to_use_prefers_explicit_then_config(self): assert executor._retries_to_use(None) == 9 executor.config = {} assert executor._retries_to_use(None) == 0 + + +def test_wait_retries_with_the_default_throw_except(tmp_path): + """The first failure used to end the wait instead of being retried""" + timing_map = {0: [-1]} + + def partial_map_function(x): + return deterministic_failure(tmp_path, timing_map, x) + + fexec = FunctionExecutor(config=pytest.lithops_config) + with RetryingFunctionExecutor(fexec) as executor: + futures = executor.map(partial_map_function, range(2), retries=1) + done, pending = executor.wait(futures) + assert pending == [] + assert sorted(f.result() for f in done) == [0, 1] + check_invocation_counts(tmp_path, timing_map, 2, retries=1) + + +def test_wait_raises_by_default_once_retries_are_exhausted(tmp_path): + timing_map = {0: [-1, -1]} + + def partial_map_function(x): + return deterministic_failure(tmp_path, timing_map, x) + + fexec = FunctionExecutor(config=pytest.lithops_config) + with RetryingFunctionExecutor(fexec) as executor: + futures = executor.map(partial_map_function, range(2), retries=1) + with pytest.raises(RuntimeError, match='Deliberately fail'): + executor.wait(futures) + check_invocation_counts(tmp_path, timing_map, 2, retries=1) + + +def _count_lines(obj): + return obj.data_stream.read().count(b'\n') + + +class TestRetryingThrowExcept: + + def test_result_and_status_honour_throw_except_false(self): + future = RetryingFuture(FakeResponseFuture(error=True, result='partial'), lambda x: x, 1) + assert future.result(throw_except=False) == 'partial' + assert future.status(throw_except=False) == 'ok' + + def test_inner_wait_never_raises_and_the_outer_one_reraises_at_the_end(self): + first = FakeResponseFuture(error=True) + still_failing = FakeResponseFuture(error=True) + inner = MagicMock() + inner.config = {} + inner.wait.side_effect = [([first], []), ([still_failing], [])] + inner.map.return_value = [still_failing] + retrying = RetryingFuture(first, map_function=lambda x: x, input=1, retries=1) + + with pytest.raises(RuntimeError, match='failed'): + RetryingFunctionExecutor(inner).wait([retrying]) + + assert inner.map.call_count == 1 + assert [c.kwargs['throw_except'] for c in inner.wait.call_args_list] == [False, False] + + +class TestRetryingMapInputs: + + @staticmethod + def _inner(n_futures): + inner = MagicMock() + inner.config = {} + inner.map.return_value = [FakeResponseFuture() for _ in range(n_futures)] + return inner + + @pytest.mark.parametrize('chunks', [{'obj_chunk_number': 2}, {'obj_chunk_size': 64}]) + def test_object_chunks_are_refused_before_invoking(self, chunks): + inner = self._inner(4) + with pytest.raises(ValueError, match='obj_chunk'): + RetryingFunctionExecutor(inner).map( + _count_lines, ['localhost://bucket/a.txt', 'localhost://bucket/b.txt'], **chunks + ) + inner.map.assert_not_called() + + def test_an_input_that_holds_several_objects_is_an_error(self): + """A prefix expands into one call per object, which cannot be retried""" + inner = self._inner(3) + with pytest.raises(ValueError, match='cannot be retried'): + RetryingFunctionExecutor(inner).map(_count_lines, ['localhost://bucket/prefix/']) + + def test_single_objects_get_one_future_each(self, tmp_path): + local_file = tmp_path / 'a.txt' + local_file.write_text('x\n') + iterdata = ['localhost://bucket/a.txt', str(local_file)] + inner = self._inner(2) + futures = RetryingFunctionExecutor(inner).map(_count_lines, iterdata) + assert [f.input for f in futures] == iterdata + + def test_more_calls_than_inputs_is_an_error(self): + inner = self._inner(3) + with pytest.raises(ValueError, match='cannot be retried'): + RetryingFunctionExecutor(inner).map(lambda x: x, [1, 2]) + + def test_a_generator_is_paired_with_its_futures(self): + inner = self._inner(3) + futures = RetryingFunctionExecutor(inner).map(lambda x: x, (i for i in range(3))) + assert [f.input for f in futures] == [0, 1, 2] + assert inner.map.call_args.args[1] == [0, 1, 2] + + def test_a_chain_after_map_reduce_pairs_the_reduce_future_only(self): + map_stage = [SimpleNamespace(_mapreduce_map=True) for _ in range(3)] + reducer = SimpleNamespace(_mapreduce_map=False) + inner = self._inner(1) + futures = RetryingFunctionExecutor(inner).map(lambda x: x, FuturesList(map_stage + [reducer])) + assert [f.input for f in futures] == [reducer] diff --git a/lithops/tests/test_storage_layer.py b/lithops/tests/test_storage_layer.py index 57e26d4e0..396862d14 100644 --- a/lithops/tests/test_storage_layer.py +++ b/lithops/tests/test_storage_layer.py @@ -25,6 +25,7 @@ SESSION_ID_ENV, TEMP_PREFIX, ) +from lithops.storage.backends.localhost.localhost import LocalhostStorageBackend from lithops.storage.cloud_proxy import ( CloudFileProxy, CloudStorage, @@ -167,6 +168,73 @@ def sleep(seconds): assert sleeps == [2, 2] +class TestLocalhostStorageKeys: + + @pytest.fixture + def backend(self, tmp_path, monkeypatch): + monkeypatch.setattr( + 'lithops.storage.backends.localhost.localhost.LITHOPS_TEMP_DIR', + str(tmp_path), + ) + return LocalhostStorageBackend({}) + + def test_list_keys_prefix_is_literal(self, backend): + # The prefix went to glob, so 'data[1]' matched 'data1.csv' only + backend.put_object('b', 'data[1].csv', b'x') + backend.put_object('b', 'data1.csv', b'y') + backend.put_object('b', 'data[1]/inner', b'z') + assert sorted(backend.list_keys('b', 'data[1]')) == ['data[1].csv', 'data[1]/inner'] + assert backend.list_keys('b', 'data?') == [] + assert [o['Key'] for o in backend.list_objects('b', 'data[1].')] == ['data[1].csv'] + + def test_list_keys_includes_dotfiles(self, backend): + # glob skips dotfiles, so clean_bucket never deleted them + backend.put_object('b', 'dir/.hidden', b'x') + backend.put_object('b', '.top', b'x') + backend.put_object('b', 'dir/.sub/f', b'x') + backend.put_object('b', 'dir/visible', b'y') + assert sorted(backend.list_keys('b')) == ['.top', 'dir/.hidden', 'dir/.sub/f', 'dir/visible'] + assert sorted(backend.list_keys('b', 'dir/')) == ['dir/.hidden', 'dir/.sub/f', 'dir/visible'] + assert sorted(backend.list_keys('b', 'dir/.')) == ['dir/.hidden', 'dir/.sub/f'] + clean_bucket(backend, 'b', 'dir/', sleep=0) + assert backend.list_keys('b') == ['.top'] + + def test_list_keys_partial_and_missing_prefixes(self, backend): + backend.put_object('b', 'a/b/c1', b'x') + backend.put_object('b', 'a/b/c2', b'x') + backend.put_object('b', 'a/bc', b'x') + assert sorted(backend.list_keys('b', 'a/b')) == ['a/b/c1', 'a/b/c2', 'a/bc'] + assert sorted(backend.list_keys('b', 'a/b/c')) == ['a/b/c1', 'a/b/c2'] + assert backend.list_keys('b', 'a/b/c1') == ['a/b/c1'] + assert backend.list_keys('b', 'zz/') == [] + assert backend.list_keys('b', '/a') == [] + assert backend.list_keys('missing-bucket') == [] + + @pytest.mark.parametrize('byte_range, expected', [ + ('bytes=2-4', b'234'), + ('bytes=5-', b'56789'), + ('bytes=-3', b'789'), + ('bytes=-30', b'0123456789'), + ]) + def test_get_object_ranges(self, backend, byte_range, expected): + # An open-ended range crashed in int('') and was reported as a + # missing key + backend.put_object('b', 'k', b'0123456789') + assert backend.get_object('b', 'k', extra_get_args={'Range': byte_range}) == expected + + def test_get_object_bad_range_is_not_a_missing_key(self, backend): + backend.put_object('b', 'k', b'0123456789') + with pytest.raises(ValueError): + backend.get_object('b', 'k', extra_get_args={'Range': 'bytes=x-'}) + with pytest.raises(StorageNoSuchKeyError): + backend.get_object('b', 'missing', extra_get_args={'Range': 'bytes=5-'}) + with pytest.raises(StorageNoSuchKeyError): + backend.get_object('b', 'k/below-a-file') + backend.put_object('b', 'dir/f', b'x') + with pytest.raises(StorageNoSuchKeyError): + backend.get_object('b', 'dir') + + class TestStorageCloudObjects: def test_put_cloudobject_uses_temp_prefix_and_hex_id(self, monkeypatch): @@ -379,11 +447,72 @@ def test_listdir_and_path_helpers(self): assert proxy.path.exists('dir/a.txt') is True assert proxy.path.exists('missing') is False - def test_exists_keeps_leading_slash_on_list_prefix(self): + def test_exists_strips_the_leading_slash_like_isfile(self): + # isfile and isdir dropped the leading slash and exists did not, so + # '/k' was a file that did not exist + fake = FakeCloudStorage(keys=['k', 'dir/a']) + path = _path(fake) + assert path.isfile('/k') is True + assert path.exists('/k') is True + assert path.exists('/dir') is True + assert path.exists('/di') is False + + def test_the_empty_path_does_not_exist(self): + # As os.path.exists(''); stripped to the key '', it matched any key + path = _path(FakeCloudStorage(keys=['k'])) + assert path.exists('') is False + assert path.isfile('') is False + + def test_isfile_is_an_exact_key_match(self): + # A sibling sharing the prefix made the key count as a directory + fake = FakeCloudStorage(keys=['foo', 'foo.bak', 'bar/x']) + path = _path(fake) + assert path.isfile('foo') is True + assert path.isfile('/foo') is True + assert path.isfile('fo') is False + assert path.isfile('bar') is False + + def test_listdir_of_an_absolute_path_stays_in_that_directory(self): + # '/foo' asked for the prefix 'foo', which also matches 'foobar/' + fake = FakeCloudStorage(keys=['foo/a', 'foobar/b']) + proxy = CloudFileProxy(fake) + assert proxy.listdir('foo') == ['a'] + assert proxy.listdir('/foo') == ['a'] + assert proxy.listdir('/foo/') == ['a'] + + def test_open_and_remove_strip_the_leading_slash(self): + fake = FakeCloudStorage(data={'k': b'v'}) + proxy = CloudFileProxy(fake) + assert proxy.open('/k', 'rb').read() == b'v' + with proxy.open('/out', 'wb') as f: + f.write(b'x') + assert fake.puts[-1] == ('out', b'x') + proxy.remove('/k') + assert fake.deleted == ['k'] + + def test_delayed_buffers_close_twice_uploads_once(self): + # A second close, as after a with block, read the closed buffer and + # raised ValueError fake = FakeCloudStorage() - fake.list_bucket_keys = MagicMock(return_value=[]) - _path(fake).exists('/foo') - fake.list_bucket_keys.assert_called_once_with(prefix='/foo') + with cloud_open('t', 'w', cloud_storage=fake) as f: + f.write('hello') + f.close() + b = cloud_open('b', 'wb', cloud_storage=fake) + b.write(b'hi') + b.close() + b.close() + assert fake.puts == [('t', 'hello'), ('b', b'hi')] + + @pytest.mark.parametrize('mode', ['r+', 'r+b', 'rb+']) + def test_open_read_plus_writes_back_on_close(self, mode): + # 'r+' returned a plain buffer, so the writes were silently dropped + conv = (lambda s: s.encode()) if 'b' in mode else str + fake = FakeCloudStorage(data={'k': b'abc'}) + with cloud_open('k', mode, cloud_storage=fake) as f: + assert f.read(1) == conv('a') + f.seek(0) + f.write(conv('X')) + assert fake.data['k'] == conv('Xbc') def test_listdir_of_the_root_matches_the_slash_form(self): # The empty path is the default argument, and it used to ask for the diff --git a/lithops/tests/test_utils.py b/lithops/tests/test_utils.py index 579a9ae60..4a2b10eb0 100644 --- a/lithops/tests/test_utils.py +++ b/lithops/tests/test_utils.py @@ -224,6 +224,25 @@ def fn(**kwargs): assert verify_args(fn, [{'x': 1}], None) == [{'x': 1}] + def test_dict_may_leave_out_params_with_defaults(self): + def fn(a, b=2, *rest): + return a + b + + assert verify_args(fn, [{'a': 1}], None) == [{'a': 1}] + assert verify_args(fn, [(1,)], None) == [{'a': 1}] + with pytest.raises(ValueError, match='Check the args names'): + verify_args(fn, [{'b': 1}], None) + + def test_single_values_with_dict_extra_args(self): + """The dict names params: its keys are not positional values""" + def fn(x, y): + return x + y + + assert verify_args(fn, [1, 2], {'y': 10}) == [ + {'x': 1, 'y': 10}, + {'x': 2, 'y': 10}, + ] + class TestMiscUtils: @@ -271,6 +290,22 @@ def test_verify_runtime_name(self): with pytest.raises(AssertionError, match='not valid'): verify_runtime_name('bad name') + @pytest.mark.parametrize('path', [ + pytest.param(path, marks=pytest.mark.skipif(os.name == 'nt', reason='Unix path')) + for path in ('/opt/homebrew/opt/python@3.12/bin/python3.12', '/Users/Some User/venv/bin/python') + ] + [ + pytest.param(path, marks=pytest.mark.skipif(os.name != 'nt', reason='Windows path')) + for path in ('C:\\Python312\\python.exe', 'C:\\Program Files\\Python312\\python.exe') + ]) + def test_verify_runtime_name_local_interpreter_path(self, path): + verify_runtime_name(path, allow_local_path=True) + with pytest.raises(AssertionError, match='not valid'): + verify_runtime_name(path) + + def test_verify_runtime_name_local_path_must_be_absolute(self): + with pytest.raises(AssertionError, match='not valid'): + verify_runtime_name('venv/bin/python@3', allow_local_path=True) + def test_timeout_handler_raises(self): with pytest.raises(TimeoutError, match='too slow'): timeout_handler('too slow', None, None) @@ -799,6 +834,81 @@ def read(self, n=None): ) assert part.readline() == b'bbb\n' + def test_iterating_wrapped_streams_stops(self): + body = WrappedStreamingBody(io.BytesIO(b'abc'), 3) + assert list(body) == [b'abc'] + part = WrappedStreamingBodyPartition(io.BytesIO(b'abc\n'), 4, (0, 3)) + assert list(part) == [b'abc\n'] + + @staticmethod + def _partition_streams(path, chunk_size, newline='\n'): + """The streams the workers read the partitions of a local file from""" + from lithops.job.partitioner import create_partitions + from lithops.worker.jobrunner import JobRunner + + parts, _ = create_partitions( + {}, None, [{'obj': str(path)}], chunk_size, None, newline + ) + runner = JobRunner.__new__(JobRunner) + streams = [] + for part in parts: + data = {'obj': part['obj']} + runner._load_object(data) + streams.append(data['obj'].data_stream) + return streams + + @pytest.mark.parametrize('newline', ['\n', '\r\n']) + @pytest.mark.parametrize('trailing_newline', [True, False]) + @pytest.mark.parametrize('how', ['read', 'small_reads', 'readline', 'iter']) + def test_partitions_reassemble_the_object(self, tmp_path, how, trailing_newline, newline): + """ + Every row is returned by exactly one partition, whatever the size of + the reads and of the newline, and the last byte of an object without + a trailing newline is kept + """ + # The rows hold the bytes of a two byte newline on their own too + rows = [b'x' * 30, b'a', b'', b'b\r', b'%04d' % 7, b'\n', b'y' * 25 + b'\r\r'] + data = newline.encode().join(rows * 6) + newline.encode() + if not trailing_newline: + data = data[:-len(newline)] + path = tmp_path / 'data.txt' + path.write_bytes(data) + + def read_all(stream): + pieces = [] + while True: + if how == 'readline': + piece = stream.readline() + elif how == 'small_reads': + piece = stream.read(7) + else: + piece = stream.read() + if not piece: + return b''.join(pieces) + pieces.append(piece) + + # With a chunk size of 1 the second chunk reads from byte 0 too, like + # the first one + for chunk_size in (1, 2, 3, 5, 16, 20, 64): + streams = self._partition_streams(path, chunk_size, newline) + assert len(streams) > 1 + if how == 'iter': + got = b''.join(b''.join(stream) for stream in streams) + else: + got = b''.join(read_all(stream) for stream in streams) + assert got == data + + @pytest.mark.parametrize('trailing_newline', [True, False]) + def test_partitions_keep_rows_longer_than_the_overshoot(self, tmp_path, trailing_newline): + """A row that goes on past the CHUNK_THRESHOLD a chunk reads ahead is not cut""" + from lithops.job.partitioner import CHUNK_THRESHOLD + rows = [b'a' * 10, b'b' * (2 * CHUNK_THRESHOLD + 7), b'c' * 10, b'd' * (CHUNK_THRESHOLD + 1)] + data = b'\n'.join(rows) + (b'\n' if trailing_newline else b'') + path = tmp_path / 'data.txt' + path.write_bytes(data) + streams = self._partition_streams(path, 1000) + assert b''.join(stream.read() for stream in streams) == data + def test_create_handler_zip_removes_partial_zip_on_write_error(self, tmp_path): from lithops.utils import create_handler_zip @@ -830,3 +940,36 @@ def boom(self, *args, **kwargs): with pytest.raises(KeyboardInterrupt): create_handler_zip(str(dest), [str(entry)]) assert not dest.exists() + + +class TestChainedFuturesAfterMapReduce: + + @staticmethod + def _future(mapreduce_map=False, produce_output=True): + from lithops.future import ResponseFuture + future = ResponseFuture.__new__(ResponseFuture) + future._mapreduce_map = mapreduce_map + future._produce_output = produce_output + return future + + @pytest.mark.parametrize('container', [FuturesList, list]) + def test_the_map_stage_of_a_map_reduce_is_not_chained(self, container): + from lithops.utils import _chained_futures + map_stage = [self._future(mapreduce_map=True) for _ in range(3)] + reducer = self._future() + assert _chained_futures(container(map_stage + [reducer])) == [reducer] + + def test_a_future_with_no_output_file_is_still_chained(self): + """A call that returned None has no output file, but is chained""" + from lithops.utils import _chained_futures + returned_none = self._future(produce_output=False) + assert _chained_futures([returned_none]) == [returned_none] + + @pytest.mark.parametrize('container', [FuturesList, list]) + def test_the_map_stage_alone_is_refused(self, container): + """It used to become a job of no calls, and its result an empty list""" + from lithops.utils import _chained_futures + map_stage = [self._future(mapreduce_map=True) for _ in range(2)] + with pytest.raises(ValueError, match='map stage of a map_reduce'): + _chained_futures(container(map_stage)) + assert _chained_futures(FuturesList()) == [] diff --git a/lithops/tests/test_wait.py b/lithops/tests/test_wait.py index 651cb0466..1153953dd 100644 --- a/lithops/tests/test_wait.py +++ b/lithops/tests/test_wait.py @@ -14,6 +14,8 @@ import importlib import signal +import threading +import time from types import SimpleNamespace from unittest.mock import MagicMock, patch @@ -353,14 +355,15 @@ def test_timeout_registers_sigalrm_with_interpolated_message(self): handlers = {} def fake_signal(sig, handler): - handlers[sig] = handler + # The first one is the wait's own; the last gives the old one back + handlers.setdefault(sig, handler) def get_data(fs, exec_data, **kwargs): future.success = True return 1 with patch.object(wait_mod.signal, 'signal', side_effect=fake_signal), \ - patch.object(wait_mod.signal, 'alarm') as alarm, \ + patch.object(wait_mod.signal, 'alarm', return_value=0) as alarm, \ patch.object(wait_mod, '_get_executor_data', side_effect=get_data): wait( [future], @@ -450,3 +453,169 @@ def test_returns_produced_results_and_skips_nested(self): assert get_result( [produced, nested, silent], show_progressbar=False ) == [1] + + +def _local_monitor(): + monitor = MagicMock() + monitor.type = 'storage' + monitor.storage_backend = 'localhost' + monitor.is_alive.return_value = True + internal = MagicMock() + internal.backend = 'localhost' + return monitor, internal + + +def _completing_poll(future): + def get_data(fs, exec_data, **kwargs): + future.success = True + return 1 + return get_data + + +@pytest.fixture +def saved_sigalrm(): + """Gives the test process back its own SIGALRM handler and alarm""" + if not hasattr(signal, 'SIGALRM'): + pytest.skip('SIGALRM is unix-only') + handler = signal.getsignal(signal.SIGALRM) + yield + signal.alarm(0) + signal.signal(signal.SIGALRM, handler) + + +class TestWaitTimeout: + + def test_timeout_off_the_main_thread_raises_timeout_error(self): + """signal.signal() raises ValueError anywhere but the main thread""" + future = FakeFuture() + monitor, internal = _local_monitor() + raised = [] + + def run(): + try: + wait([future], timeout=0.3, show_progressbar=False, + job_monitor=monitor, internal_storage=internal) + except Exception as e: + raised.append(e) + + with patch.object(wait_mod, '_get_executor_data', return_value=0): + thread = threading.Thread(target=run) + start = time.monotonic() + thread.start() + thread.join(10) + + assert len(raised) == 1 + assert isinstance(raised[0], TimeoutError) + assert 'Timeout of 0.3 seconds exceeded' in str(raised[0]) + assert time.monotonic() - start < 5 + + def test_timeout_off_the_main_thread_returns_when_done(self): + future = FakeFuture() + monitor, internal = _local_monitor() + results = [] + + def run(): + results.append(wait([future], timeout=5, show_progressbar=False, + job_monitor=monitor, internal_storage=internal)) + + with patch.object(wait_mod, '_get_executor_data', side_effect=_completing_poll(future)): + thread = threading.Thread(target=run) + thread.start() + thread.join(10) + + assert results == [([future], [])] + + def test_wait_restores_the_callers_alarm_and_handler(self, saved_sigalrm): + def user_handler(signum, frame): + pass + + future = FakeFuture() + monitor, internal = _local_monitor() + signal.signal(signal.SIGALRM, user_handler) + signal.alarm(60) + + with patch.object(wait_mod, '_get_executor_data', side_effect=_completing_poll(future)): + wait([future], timeout=5, show_progressbar=False, + job_monitor=monitor, internal_storage=internal) + + assert signal.getsignal(signal.SIGALRM) is user_handler + assert 55 <= signal.alarm(0) <= 60 + + def test_a_callers_alarm_due_first_is_left_armed(self, saved_sigalrm): + """The wait leaves it alone and enforces its own timeout by polling""" + fired = [] + + def user_handler(signum, frame): + fired.append(signum) + + future = FakeFuture() + monitor, internal = _local_monitor() + signal.signal(signal.SIGALRM, user_handler) + signal.alarm(1) + + with patch.object(wait_mod, '_get_executor_data', return_value=0): + with pytest.raises(TimeoutError, match='Timeout of 1.5 seconds'): + wait([future], timeout=1.5, show_progressbar=False, + job_monitor=monitor, internal_storage=internal) + + assert fired == [signal.SIGALRM] + assert signal.getsignal(signal.SIGALRM) is user_handler + + def test_no_timeout_leaves_the_callers_alarm_alone(self, saved_sigalrm): + future = FakeFuture() + monitor, internal = _local_monitor() + signal.alarm(60) + + with patch.object(wait_mod, '_get_executor_data', side_effect=_completing_poll(future)): + wait([future], show_progressbar=False, + job_monitor=monitor, internal_storage=internal) + + assert signal.alarm(0) > 0 + + +class TestWaitOnExecutorFutures: + """ + FunctionExecutor.wait() with no fs hands over every future of the + executor, including the ones earlier jobs finished + """ + + def _wait_any(self, futures_from_executor_wait, return_when=ANY_COMPLETED): + earlier = FakeFuture(done=True, success=True, job_id='M000') + pending = [FakeFuture(job_id='M001', call_id=f'0000{i}') for i in range(2)] + monitor, internal = _local_monitor() + polls = [] + + def get_data(fs, exec_data, **kwargs): + polls.append(1) + if len(polls) == 2: + pending[0].success = True + return 1 + + with patch.object(wait_mod, '_get_executor_data', side_effect=get_data): + done, not_done = wait( + [earlier] + pending, return_when=return_when, + show_progressbar=False, job_monitor=monitor, + internal_storage=internal, + futures_from_executor_wait=futures_from_executor_wait, + ) + return len(polls), done, not_done, pending + + def test_any_completed_ignores_futures_done_before_the_wait(self): + polls, done, not_done, pending = self._wait_any(True) + assert polls == 2 + assert pending[0] in done + assert not_done == [pending[1]] + + def test_percentage_ignores_futures_done_before_the_wait(self): + # The one done earlier would make it a third of the three already + polls, done, not_done, pending = self._wait_any(True, return_when=25) + assert polls == 2 + assert pending[0] in done + + def test_explicit_futures_count_the_ones_already_done(self): + polls, _, _, _ = self._wait_any(False) + assert polls == 0 + + +def test_get_result_of_no_futures_is_empty(): + assert get_result([], show_progressbar=False) == [] diff --git a/lithops/tests/test_worker.py b/lithops/tests/test_worker.py index 519316ab6..baa285eb6 100644 --- a/lithops/tests/test_worker.py +++ b/lithops/tests/test_worker.py @@ -37,7 +37,7 @@ WORKER_ENV, ) from lithops.storage.utils import CloudObject, CloudObjectLocal, CloudObjectUrl -from lithops.utils import bytes_to_b64str, is_unix_system +from lithops.utils import bytes_to_b64str, is_unix_system, verify_args from lithops.worker import function_handler, function_invoker from lithops.worker.handler import ( ShutdownSentinel, @@ -47,6 +47,7 @@ task_consumer, run_task, ) +from lithops.future import ResponseFuture from lithops.worker.jobrunner import JobRunner, JobStats, _prepare_args from lithops.monitoring import ( CallStatus, @@ -455,6 +456,35 @@ def test_sets_env_creates_dir_and_clears_extra_env( assert task.stats_file == os.path.join(task.task_dir, 'job_stats.txt') assert len(os.environ['__LITHOPS_ACTIVATION_ID']) == 12 + def test_restores_the_variables_extra_env_overrides( + self, tmp_path, monkeypatch + ): + """ + The worker process runs more tasks after this one: a variable it had + before the task must not be lost, even when the task fails + """ + monkeypatch.setattr( + 'lithops.worker.handler.LITHOPS_TEMP_DIR', str(tmp_path) + ) + monkeypatch.setenv('LITHOPS_TEST_PREVIOUS', 'orig') + monkeypatch.delenv('LITHOPS_TEST_NEW', raising=False) + task = _task(extra_env={ + 'LITHOPS_TEST_PREVIOUS': 'override', 'LITHOPS_TEST_NEW': 'new' + }) + seen = {} + + def run(task): + seen['previous'] = os.environ.get('LITHOPS_TEST_PREVIOUS') + seen['new'] = os.environ.get('LITHOPS_TEST_NEW') + raise RuntimeError('task failed') + + with patch('lithops.worker.handler.run_task', run): + with pytest.raises(RuntimeError): + prepare_and_run_task(task) + assert seen == {'previous': 'override', 'new': 'new'} + assert os.environ['LITHOPS_TEST_PREVIOUS'] == 'orig' + assert 'LITHOPS_TEST_NEW' not in os.environ + def test_keeps_existing_activation_id(self, tmp_path, monkeypatch): monkeypatch.setattr( 'lithops.worker.handler.LITHOPS_TEMP_DIR', str(tmp_path) @@ -1286,16 +1316,13 @@ def test_prepare_args_kwargs_only(self): def f(a, b=1): return a + b args, kwargs = _prepare_args(f, {'a': 2, 'b': 3}) - assert args == () - assert kwargs == {'a': 2, 'b': 3} assert f(*args, **kwargs) == 5 - def test_prepare_args_varargs_empty_list_is_kept(self): + def test_prepare_args_empty_varargs(self): def f(*args, **kwargs): return args, kwargs args, kwargs = _prepare_args(f, {'args': [], 'kwargs': {}, 'x': 1}) - assert args == [] - assert kwargs == {'x': 1} + assert f(*args, **kwargs) == ((), {'x': 1}) def test_prepare_args_custom_var_names(self): def f(*xs, **kw): @@ -1306,6 +1333,24 @@ def f(*xs, **kw): assert args == (1, 2) assert kwargs == {'a': 3, 'b': 4} + def test_prepare_args_named_params_before_varargs_stay_positional(self): + def f(a, *rest): + return a, rest + data = verify_args(f, [(1, 2, 3)], None)[0] + args, kwargs = _prepare_args(f, data) + assert f(*args, **kwargs) == (1, (2, 3)) + + def test_prepare_args_positional_only_params(self): + data = verify_args(abs, [(-2,)], None)[0] + args, kwargs = _prepare_args(abs, data) + assert abs(*args, **kwargs) == 2 + + def test_prepare_args_keeps_keyword_params_after_positional_ones(self): + def f(a, b, *rest, c=0): + return a, b, rest, c + args, kwargs = _prepare_args(f, {'a': 1, 'b': 2, 'rest': (3,), 'c': 4}) + assert f(*args, **kwargs) == (1, 2, (3,), 4) + class TestJobRunner: @@ -1442,13 +1487,22 @@ def f(ibm_cos): def test_fill_optional_args_future_chaining(self): jr = self._runner(_echo, {'x': 1}) - future = MagicMock() + future = MagicMock(spec=ResponseFuture) future.result.return_value = 9 data = {'future': future} jr._fill_optional_args(_echo, data) assert data['x'] == 9 assert 'future' not in data + def test_fill_optional_args_param_named_future_is_not_chaining(self): + def f(future): + return future + + jr = self._runner(_echo, {'x': 1}) + data = {'future': 5} + jr._fill_optional_args(f, data) + assert data == {'future': 5} + def test_wait_futures_replaces_first_value(self): jr = self._runner(_echo, {'x': 1}) done = MagicMock(done=True, futures=False) diff --git a/lithops/utils.py b/lithops/utils.py index d85414aae..4eef62939 100644 --- a/lithops/utils.py +++ b/lithops/utils.py @@ -16,6 +16,7 @@ # limitations under the License. # +import io import re import os import sys @@ -498,10 +499,16 @@ def create_handler_zip( os.remove(dst_zip_location) -def verify_runtime_name(runtime_name: str) -> None: - """Asserts that the runtime name can be used as a container image name""" - assert re.match("^[A-Za-z0-9_/.:-]*$", runtime_name), \ - f'Runtime name "{runtime_name}" not valid' +def verify_runtime_name(runtime_name: str, allow_local_path: bool = False) -> None: + """ + Asserts that the runtime name can be used as a container image name. With + ``allow_local_path`` the absolute path to a local interpreter is valid too: + the localhost mode runs it without a shell, so it may hold any character + """ + valid = re.match("^[A-Za-z0-9_/.:-]*$", runtime_name) is not None + if not valid and allow_local_path: + valid = os.path.isabs(runtime_name) + assert valid, f'Runtime name "{runtime_name}" not valid' def timeout_handler(error_msg, signum, frame): @@ -743,7 +750,9 @@ def split_path(path): def format_data(iterdata, extra_args): """ Converts iterdata to a list, appending extra_args to every element. The - element decides how: tuples are concatenated, dicts are merged + element decides how: tuples are concatenated, dicts are merged. A single + value is left alone when extra_args is a dict: only the signature of the + function names its param, so verify_args merges the dict when binding it """ data = _as_iterdata_list(iterdata) if not extra_args: @@ -764,6 +773,8 @@ def format_data(iterdata, extra_args): ) data_i.update(extra_args) new_iterdata.append(data_i) + elif isinstance(extra_args, dict): + new_iterdata.append(data_i) else: new_iterdata.append((data_i, *extra_args)) return new_iterdata @@ -800,12 +811,25 @@ def _chained_futures(iterdata): A slice of a FuturesList, or a list built from one, is a chain too: both lose the FuturesList type, and binding a future to a parameter as if it - were data fails with an error that says nothing about chaining + were data fails with an error that says nothing about chaining. + + The map stage of a map_reduce is left out: its reducer already consumed + its output, and only the reduce stage produces the output of that job """ from lithops.future import ResponseFuture + def producing_output(futures): + produced = [f for f in futures if not getattr(f, '_mapreduce_map', False)] + if futures and not produced: + raise ValueError( + "The map stage of a map_reduce cannot be chained: its output " + "is consumed by the reducer. Chain the whole map_reduce to " + "take the reducer output" + ) + return produced + if isinstance(iterdata, FuturesList): - return list(iterdata) + return producing_output(iterdata) if not isinstance(iterdata, (list, tuple)) or not iterdata: return None @@ -821,7 +845,7 @@ def _chained_futures(iterdata): "Chaining takes the futures of one job as the whole input of " "the next one" ) - return list(iterdata) + return producing_output(iterdata) def verify_args(func, iterdata, extra_args): @@ -854,12 +878,18 @@ def verify_args(func, iterdata, extra_args): p.kind == inspect.Parameter.VAR_KEYWORD for p in func_sig.parameters.values() ) + # A dict element only has to name the params that have no default + required = { + name for name, p in func_sig.parameters.items() + if p.default is inspect.Parameter.empty + and p.kind not in (inspect.Parameter.VAR_POSITIONAL, inspect.Parameter.VAR_KEYWORD) + } new_data = [] for elem in data: if isinstance(elem, dict): - if has_var_keyword or set(func_sig.parameters) <= set(elem): + if has_var_keyword or required <= set(elem): new_data.append(elem) else: raise ValueError( @@ -870,8 +900,10 @@ def verify_args(func, iterdata, extra_args): elif isinstance(elem, tuple): new_data.append(dict(func_sig.bind(*elem).arguments)) else: - # A single value of any other type binds to the first param - new_data.append(dict(func_sig.bind(elem).arguments)) + # A single value of any other type binds to the first param, and + # dict extra_args to the params they name + extra_kwargs = extra_args if isinstance(extra_args, dict) else {} + new_data.append(dict(func_sig.bind(elem, **extra_kwargs).arguments)) return new_data @@ -930,7 +962,14 @@ def __iter__(self): return self def __next__(self): - return self.read(64 * 1024) + # An empty read is the end of the stream, whether it gives bytes or str + try: + chunk = self.read(64 * 1024) + except EOFError: + raise StopIteration + if not chunk: + raise StopIteration + return chunk def __getattr__(self, attr): # Only reached for the attributes this wrapper does not define, so @@ -938,71 +977,103 @@ def __getattr__(self, attr): return getattr(self.sb, attr) +class _PartitionRows(io.RawIOBase): + """The rows of a partition as a raw stream, for io.BufferedReader to buffer""" + def __init__(self, read_rows): + self._read_rows = read_rows + + def readable(self): + return True + + def readinto(self, buf): + piece = self._read_rows(len(buf)) + buf[:len(piece)] = piece + return len(piece) + + class WrappedStreamingBodyPartition(WrappedStreamingBody): """ Wrap boto3's StreamingBody object to provide line integrity of the partitions based on the newline character. """ - def __init__(self, sb, size, byterange, newline='\n'): + def __init__(self, sb, size, byterange, newline='\n', first_chunk=None, more=None): super().__init__(sb, size) + # Opens the object again from a byte on, for a last row longer than + # the range; returns None past the end of the object + self._more = more self.range = byterange self.newline_char = newline.encode() - # Every chunk but the first one reads one byte early, so that read() - # can tell whether the previous chunk ended in the middle of a row - self._plusbytes = 0 if not self.range or self.range[0] == 0 else 1 - self._first_byte = None + # Every chunk but the first one reads the newline length early, so that + # it can tell whether the previous chunk ended in the middle of a row. + # The start of the range cannot always tell: with a chunk size as long + # as the newline, the second chunk reads from byte 0 too + if first_chunk is None: + first_chunk = not self.range or self.range[0] == 0 + self._plusbytes = 0 if first_chunk else len(self.newline_char) + # Set while the cut row at the start of the chunk, which the previous + # chunk returns, is being skipped + self._discarding = not first_chunk + # The last bytes read, in case a newline is split between two reads + self._carry = b'' self._eof = False - self._first_read = True + self._rows = io.BufferedReader(_PartitionRows(self._read_rows)) - def read(self, n=None): - if self._eof: - return b'' - - if not self._first_byte and self._plusbytes == 1: - self._first_byte = self.sb.read(self._plusbytes) - - retval = self.sb.read(n) - last_row_end_pos = len(retval) - self.pos += last_row_end_pos - first_row_start_pos = 0 - - if self._first_read and self._first_byte and \ - self._first_byte != self.newline_char: - # The previous chunk did not end in a newline, so the first row of - # this one is a cut row that the previous chunk already returned - logger.debug('Discarding first partial row') - first_row_start_pos = retval.find(self.newline_char) + 1 - self._first_read = False - - # The last row of a chunk is completed past its own end - if self.pos >= self.size: - current_end_pos = last_row_end_pos - (self.pos - self.size) - last_byte_pos = retval[current_end_pos - 1:].find(self.newline_char) - last_row_end_pos = current_end_pos + last_byte_pos - self._eof = True - - return retval[first_row_start_pos:last_row_end_pos] - - def readline(self): - if self._eof: - return b'' - - if not self._first_byte and self._plusbytes == 1: - self._first_byte = self.sb.read(self._plusbytes) - if self._first_byte != self.newline_char: + def _read_rows(self, n): + """ + Reads the next piece of the rows of this chunk, at most n bytes of the + stream at a time, and b'' once they are over. The chunk owns the rows + that start within its size: the cut row at its start is skipped, and + the last one is read past its size until its newline or the end of the + stream. The previous chunk ends at the same newline this one skips + """ + newline_len = len(self.newline_char) + # The end of the chunk in the stream, which starts _plusbytes early + end = self.size + self._plusbytes + while not self._eof: + data = self.sb.read(n) + if not data: + if self._more is None or self._discarding or self.pos < end: + break + stream = self._more(self.range[0] + self.pos) + if stream is None: + break + self.sb.close() + self.sb = stream + continue + window = self._carry + data + # Position of the first byte of window within the stream + offset = self.pos - len(self._carry) + self.pos += len(data) + self._carry = window[len(window) - newline_len + 1:] + begin = len(window) - len(data) + + if self._discarding: + idx = window.find(self.newline_char) + if idx < 0: + continue logger.debug('Discarding first partial row') - self.sb._raw_stream.readline() - try: - retval = self.sb._raw_stream.readline() - except struct.error: - raise EOFError() - self.pos += len(retval) + self._discarding = False + begin = idx + newline_len + + if self.pos >= end: + # The last row ends at the first newline that ends from the end + # of the chunk on, which is where the next chunk starts + idx = window.find(self.newline_char, max(end - offset - newline_len, begin - newline_len, 0)) + if idx >= 0: + self._eof = True + return window[begin:idx + newline_len] + + if begin < len(window): + return window[begin:] + self._eof = True + return b'' - if self.pos >= self.size: - self._eof = True + def read(self, n=None): + return self._rows.read(n) - return retval + def readline(self): + return self._rows.readline() def docker_login(docker_user, docker_password, docker_server): diff --git a/lithops/wait.py b/lithops/wait.py index d5d22a9da..85c5eef4f 100644 --- a/lithops/wait.py +++ b/lithops/wait.py @@ -16,6 +16,7 @@ import signal import logging +import threading import time import math import concurrent.futures as cf @@ -94,9 +95,31 @@ def _log_wait_start(prefix: str, return_when: Any, pending: int) -> None: ) -def _set_wait_alarm(timeout: float) -> None: +def _timeout_error_msg(timeout: float) -> str: + return ( + f'Timeout of {timeout} seconds exceeded waiting for ' + 'function activations to finish' + ) + + +def _can_use_alarm() -> bool: + """ + SIGALRM only exists on Unix, and Python only lets the main thread set a + signal handler: anywhere else signal.signal() raises ValueError + """ + return ( + is_unix_system() + and threading.current_thread() is threading.main_thread() + ) + + +def _set_wait_alarm(timeout: float) -> Optional[SimpleNamespace]: """ - Arms a SIGALRM that aborts the wait once the timeout is exceeded. + Arms a SIGALRM that aborts the wait once the timeout is exceeded, and + returns what _clear_wait_alarm() needs to give the caller back the + handler and the alarm it had before. Returns None, with the caller's + alarm and handler put back, when that alarm goes off first: the timeout + of the wait is then checked by the polling loop. signal.alarm() takes whole seconds and rounds nothing, so a fractional timeout used to raise TypeError before the wait even started, which is @@ -107,17 +130,51 @@ def _set_wait_alarm(timeout: float) -> None: A timeout that has already passed raises here, since there is no shorter alarm than one second to arm """ - error_msg = ( - f'Timeout of {timeout} seconds exceeded waiting for ' - 'function activations to finish' - ) if timeout <= 0: - raise TimeoutError(error_msg) + raise TimeoutError(_timeout_error_msg(timeout)) seconds = math.ceil(timeout) logger.debug(f'Setting waiting timeout to {timeout} seconds') - signal.signal(signal.SIGALRM, partial(timeout_handler, error_msg)) - signal.alarm(seconds) + previous_handler = signal.signal( + signal.SIGALRM, partial(timeout_handler, _timeout_error_msg(timeout)) + ) + previous_remaining = signal.alarm(seconds) + if not isinstance(previous_remaining, int): + # Only a stand-in for signal.alarm returns anything else + previous_remaining = 0 + armed = SimpleNamespace( + handler=previous_handler, + remaining=previous_remaining, + armed_at=time.monotonic(), + ) + if previous_remaining and previous_remaining <= seconds: + _clear_wait_alarm(armed) + return None + return armed + + +def _clear_wait_alarm(armed: SimpleNamespace) -> None: + """ + Cancels the alarm of the wait and restores the handler and the alarm the + caller had set, minus the time the wait took + """ + signal.alarm(0) + # None stands for a handler not set from Python, which cannot be set back + signal.signal( + signal.SIGALRM, + signal.SIG_DFL if armed.handler is None else armed.handler, + ) + if armed.remaining: + elapsed = time.monotonic() - armed.armed_at + signal.alarm(max(1, math.ceil(armed.remaining - elapsed))) + + +def _check_deadline(deadline: Optional[float], timeout: Optional[float]) -> None: + """ + Raises the same TimeoutError as the alarm once the deadline has passed + """ + if deadline is not None and time.monotonic() >= deadline: + raise TimeoutError(_timeout_error_msg(timeout)) def _create_progressbar(total: int, initial: int): @@ -163,13 +220,25 @@ def _poll_until_done( download_results, sleep_sec, poll_kwargs, + already_complete=(), + deadline=None, + timeout=None, ): """ Polls every executor until return_when% of the futures are done. A round that fetched something is followed immediately by another one, as more - statuses are likely to be waiting already + statuses are likely to be waiting already. + + The futures in already_complete do not count: they were done before the + wait started. A deadline is the timeout of a wait no alarm can abort """ - while not _check_done(fs, return_when, download_results): + def watched(): + if not already_complete: + return fs + return [f for f in fs if id(f) not in already_complete] + + while not _check_done(watched(), return_when, download_results): + _check_deadline(deadline, timeout) # The monitor is a daemon thread that exits on its own once every # future it knows about is done, so it may need waking up for the # futures that showed up afterwards @@ -183,7 +252,10 @@ def _poll_until_done( if new_data: continue - time.sleep(sleep_sec) + if deadline is not None: + time.sleep(max(0, min(sleep_sec, deadline - time.monotonic()))) + else: + time.sleep(sleep_sec) def wait( @@ -243,17 +315,27 @@ def wait( fs_to_wait = math.ceil(return_when * len(not_done_futures) / 100) _log_wait_start(prefix, return_when, len(not_done_futures)) - if is_unix_system() and timeout is not None: - _set_wait_alarm(timeout) - + # Waiting on every future of an executor, the ones some earlier wait saw + # finish count for nothing, or ANY_COMPLETED would return right away + already_complete = ( + {id(f) for f in fs_done} if futures_from_executor_wait else () + ) pbar = ( - _create_progressbar(fs_to_wait, len(fs_done)) + _create_progressbar(fs_to_wait, 0 if futures_from_executor_wait else len(fs_done)) if show_progressbar else None ) started_monitors = [] pool = None + armed_alarm = None + deadline = None try: + if timeout is not None: + if _can_use_alarm(): + armed_alarm = _set_wait_alarm(timeout) + if armed_alarm is None: + deadline = time.monotonic() + timeout + executors_data = _create_executors_data_from_futures( fs, internal_storage ) @@ -280,7 +362,10 @@ def wait( else: _poll_until_done( fs, executors_data, job_monitor, return_when, - download_results, sleep_sec, poll_kwargs + download_results, sleep_sec, poll_kwargs, + already_complete=already_complete, + deadline=deadline, + timeout=timeout, ) except KeyboardInterrupt: @@ -296,8 +381,8 @@ def wait( pool.shutdown(wait=True) for monitor in started_monitors: monitor.stop() - if is_unix_system(): - signal.alarm(0) + if armed_alarm is not None: + _clear_wait_alarm(armed_alarm) if pbar and not pbar.disable: pbar.close() if not is_notebook(): @@ -333,6 +418,8 @@ def get_result( :return: The result of the future/s """ fs = _as_future_list(fs) + if not fs: + return [] prefix = log_prefix(fs[0].executor_id) logger.info( diff --git a/lithops/worker/handler.py b/lithops/worker/handler.py index 661c4b69b..8e6493109 100644 --- a/lithops/worker/handler.py +++ b/lithops/worker/handler.py @@ -324,24 +324,31 @@ def prepare_and_run_task(task: SimpleNamespace) -> None: os.environ[WORKER_ENV] = 'True' os.environ['PYTHONUNBUFFERED'] = 'True' + # The worker process runs more tasks after this one, so the variables the + # task overrides get their previous values back when it finishes + previous_env = {key: os.environ.get(key) for key in task.extra_env} os.environ.update(task.extra_env) - storage_backend = task.config['lithops']['storage'] - bucket = task.config[storage_backend]['storage_bucket'] - task.task_dir = os.path.join( - LITHOPS_TEMP_DIR, bucket, JOBS_PREFIX, task.job_key, task.call_id - ) - task.log_file = os.path.join(task.task_dir, 'execution.log') - task.stats_file = os.path.join(task.task_dir, 'job_stats.txt') - os.makedirs(task.task_dir, exist_ok=True) - - with open(task.log_file, 'a') as log_stream: - task.log_stream = LogStream(log_stream) - with custom_redirection(task.log_stream): - run_task(task) - - for key in task.extra_env: - os.environ.pop(key, None) + try: + storage_backend = task.config['lithops']['storage'] + bucket = task.config[storage_backend]['storage_bucket'] + task.task_dir = os.path.join( + LITHOPS_TEMP_DIR, bucket, JOBS_PREFIX, task.job_key, task.call_id + ) + task.log_file = os.path.join(task.task_dir, 'execution.log') + task.stats_file = os.path.join(task.task_dir, 'job_stats.txt') + os.makedirs(task.task_dir, exist_ok=True) + + with open(task.log_file, 'a') as log_stream: + task.log_stream = LogStream(log_stream) + with custom_redirection(task.log_stream): + run_task(task) + finally: + for key, value in previous_env.items(): + if value is None: + os.environ.pop(key, None) + else: + os.environ[key] = value # Windows has no SIGKILL, and no process there is reported as killed by one diff --git a/lithops/worker/jobrunner.py b/lithops/worker/jobrunner.py index 7a0dcc1ec..93d052b2e 100644 --- a/lithops/worker/jobrunner.py +++ b/lithops/worker/jobrunner.py @@ -41,6 +41,7 @@ from lithops.constants import REDUCE_JOB_ENV from lithops.storage import Storage +from lithops.job.partitioner import CHUNK_THRESHOLD from lithops.wait import wait from lithops.future import ResponseFuture from lithops.utils import ( @@ -166,8 +167,9 @@ def _fill_optional_args( """ func_sig = inspect.signature(function) - if len(data) == 1 and 'future' in data: - # Function chaining feature + if len(data) == 1 and isinstance(data.get('future'), ResponseFuture): + # Function chaining feature. A param of the user function can be + # named future too, so the value tells a chained call apart out = [ data.pop('future').result( internal_storage=self.internal_storage @@ -200,8 +202,12 @@ def _wait_futures(self, data: Dict[str, Any]) -> None: fut_list.clear() data[key] = results - def _open_object_stream(self, obj: Any, extra_get_args: Dict[str, Any]): - """Opens the object to process, wherever it lives""" + def _open_object_stream(self, obj: Any, byte_range: Optional[Tuple[int, int]]): + """Opens the object to process, or the given byte range of it, wherever it lives""" + extra_get_args = {} + if byte_range is not None: + extra_get_args['Range'] = f'bytes={byte_range[0]}-{byte_range[1]}' + if hasattr(obj, 'bucket') and not hasattr(obj, 'path'): logger.info( f'Getting dataset from {obj.backend}://{obj.bucket}/{obj.key}' @@ -224,9 +230,9 @@ def _open_object_stream(self, obj: Any, extra_get_args: Dict[str, Any]): logger.info(f'Getting dataset from {obj.path}') with open(obj.path, "rb") as f: - if obj.data_byte_range is None: + if byte_range is None: return io.BytesIO(f.read()) - first_byte, last_byte = obj.data_byte_range + first_byte, last_byte = byte_range f.seek(first_byte) return io.BytesIO(f.read(last_byte - first_byte + 1)) @@ -236,12 +242,10 @@ def _load_object(self, data: Dict[str, Any]) -> None: down to the chunk that this task is responsible for """ obj = data['obj'] - extra_get_args = {} if obj.data_byte_range is not None: first_byte, last_byte = obj.data_byte_range - extra_get_args['Range'] = f'bytes={first_byte}-{last_byte}' - stream = self._open_object_stream(obj, extra_get_args) + stream = self._open_object_stream(obj, obj.data_byte_range) if obj.data_byte_range is None: obj.data_stream = stream @@ -252,8 +256,12 @@ def _load_object(self, data: Dict[str, Any]) -> None: if obj.newline is None: obj.data_stream = WrappedStreamingBody(stream, obj.chunk_size) else: + # The partitioner numbers the chunks of an object from 1 + part = getattr(obj, 'part', None) obj.data_stream = WrappedStreamingBodyPartition( - stream, obj.chunk_size, obj.data_byte_range, obj.newline + stream, obj.chunk_size, obj.data_byte_range, obj.newline, + first_chunk=None if part is None else part == 1, + more=lambda first: self._more_of_object(obj, first) ) if last_byte - first_byte > obj.chunk_size: last_byte = first_byte + obj.chunk_size - 1 @@ -264,6 +272,17 @@ def _load_object(self, data: Dict[str, Any]) -> None: f'Range: {first_byte}-{last_byte}' ) + def _more_of_object(self, obj: Any, first_byte: int): + """ + The next CHUNK_THRESHOLD bytes of the object from first_byte on, for a + last row longer than the range of its chunk. None past the end + """ + total_size = getattr(obj, 'total_size', None) + if total_size is None or first_byte >= total_size: + return None + last_byte = min(first_byte + CHUNK_THRESHOLD, total_size) - 1 + return self._open_object_stream(obj, (first_byte, last_byte)) + def _write_function_stats( self, start_tstamp: float, end_tstamp: float ) -> None: @@ -417,32 +436,13 @@ def _prepare_args( Converts the data envelope into normal args and kwargs, respecting the actual var-length parameter names of func """ - func_sig = inspect.signature(func) - var_pos_name = None - var_kw_name = None - - for name, param in func_sig.parameters.items(): - if param.kind == inspect.Parameter.VAR_POSITIONAL: - var_pos_name = name - elif param.kind == inspect.Parameter.VAR_KEYWORD: - var_kw_name = name - - payload = dict(data) - - if var_pos_name is not None and var_pos_name in payload: - args = payload.pop(var_pos_name) - if args is None: - args = () - else: - args = () - - if var_kw_name is not None and var_kw_name in payload: - kwargs = payload.pop(var_kw_name) - if kwargs is None: - kwargs = {} - else: - kwargs = {} - - kwargs.update(payload) - - return args, kwargs + # BoundArguments puts every param in its place: positional-only ones and + # the ones before *args by position, the rest by name. Names that are not + # params go to **kwargs + sig = inspect.signature(func) + bound = inspect.BoundArguments( + sig, {name: data[name] for name in sig.parameters if name in data} + ) + kwargs = bound.kwargs + kwargs.update((k, v) for k, v in data.items() if k not in sig.parameters) + return bound.args, kwargs From 07e891f88c5fac5c53968b9511c7d6aaafd7dd48 Mon Sep 17 00:00:00 2001 From: JosepSampe Date: Wed, 14 Oct 2026 16:35:02 +0200 Subject: [PATCH 2/3] Update executors --- CHANGELOG.md | 8 +- docs/source/api_futures.rst | 3 +- lithops/config.py | 25 +++-- lithops/executors.py | 9 +- lithops/job/partitioner.py | 74 +++++++++++-- .../monitoring/backends/storage/storage.py | 15 ++- lithops/monitoring/monitor.py | 24 +++- lithops/retries.py | 47 +++++--- lithops/tests/test_config.py | 31 +++++- lithops/tests/test_invokers.py | 17 +++ lithops/tests/test_monitor.py | 68 +++++++++++- lithops/tests/test_retries.py | 104 ++++++++++++++++-- lithops/tests/test_worker.py | 20 ++++ lithops/worker/jobrunner.py | 23 +++- 14 files changed, 406 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 780648bed..2ccf62695 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,16 @@ ## [v3.8.1.dev0] +### Changed +- [Core] A storage or monitoring service at a loopback address is refused for serverless workers, which cannot reach it; standalone and Singularity workers get a warning. +- [Core] `RetryingFunctionExecutor` lists a bucket, prefix or directory before invoking, so that each object is retried on its own, and rejects `obj_chunk_size` and `obj_chunk_number`. + ### Fixed -- [Core] Chaining, cleanup, invocation errors, generator inputs, empty `fs`, `wait()` timeouts (threads, SIGALRM, `ANY_COMPLETED`), argument binding and `RetryingFunctionExecutor` retries and percentage waits; object partitioning is now rejected by the retrying executor, and a storage or monitoring service at a loopback address is refused for serverless workers (a warning for standalone). +- [Core] Chaining, cleanup, invocation errors, generator inputs, empty `fs`, `wait()` timeouts (threads, SIGALRM, `ANY_COMPLETED`), argument binding and `RetryingFunctionExecutor` retries and percentage waits. - [Worker] Partitioned object streams (lost, duplicated or cut rows, rows longer than 128 KB, multi-byte `obj_newline`, `readline()`, iteration), module discovery of nested functions, a parameter named `future`, and `extra_env` restoring the previous environment. - [Localhost] Absolute interpreter paths are accepted as runtime; storage listing is a literal prefix match including dotfiles, and open-ended byte ranges are supported. - [Storage] `CloudFileProxy` path handling (`isfile`, `listdir`, `exists`, leading `/`), double `close()`, and `r+` mode writing back. -- [Monitoring] Worker tokens of jobs dropped by `wait()`, replaced monitors and restarted invokers are handed back once, and a held `__end__` is no longer replaced by an `__init__`. +- [Monitoring] Worker tokens of jobs dropped by `wait()`, replaced monitors and restarted invokers are handed back once, a held `__end__` is no longer replaced by an `__init__`, and the workers of a job that owes no token are forgotten instead of kept for the life of the executor. - [Concurrent] `wait()` and `as_completed()` accept generators. ## [v3.8.0] diff --git a/docs/source/api_futures.rst b/docs/source/api_futures.rst index e61cf6e15..d207856a1 100644 --- a/docs/source/api_futures.rst +++ b/docs/source/api_futures.rst @@ -17,7 +17,8 @@ Primary Executors * **RetryingFunctionExecutor** (``lithops.RetryingFunctionExecutor()``): A robust wrapper around ``FunctionExecutor`` that transparently handles retries on failed tasks. - It supports all features of ``FunctionExecutor`` with added automatic retry logic, improving fault tolerance and reliability for unstable or transient-failure-prone environments. + It supports the features of ``FunctionExecutor`` with added automatic retry logic, improving fault tolerance and reliability for unstable or transient-failure-prone environments. + A failed call is retried with its own input: a bucket, prefix or directory passed to a function that processes objects is retried one object at a time, and splitting objects into chunks (``obj_chunk_size``, ``obj_chunk_number``) is not supported. Secondary Executors ------------------- diff --git a/lithops/config.py b/lithops/config.py index aafe62501..df35efc64 100644 --- a/lithops/config.py +++ b/lithops/config.py @@ -273,6 +273,10 @@ def default_config( # The keys of a storage or monitoring section that hold the address workers connect to _ADDRESS_KEYS = ('host', 'endpoint', 'amqp_url') +# Serverless backends whose containers share the network of the host they run +# on, so that a loopback address is the service itself when that host is this one +_HOST_NETWORK_BACKENDS = ('singularity',) + def _is_loopback(address): """Whether a host, host:port or URL names this machine""" @@ -293,26 +297,31 @@ def _check_loopback_addresses(config_data): """ Refuses a storage or monitoring service at a loopback address for serverless workers, which run in containers of their own where that - address is the worker itself. Standalone workers only get a warning: an - SSH tunnel can bring the service to the loopback address of the VM + address is the worker itself. The others only get a warning: an SSH + tunnel can bring the service to the loopback address of a standalone VM, + and a Singularity container on this machine shares its network """ lithops_cfg = config_data['lithops'] mode = lithops_cfg.get('mode') if mode == c.LOCALHOST: return + backend = lithops_cfg['backend'] + refused = mode == c.SERVERLESS and backend not in _HOST_NETWORK_BACKENDS for section in (lithops_cfg.get('storage'), lithops_cfg.get('monitoring')): for key in _ADDRESS_KEYS: address = (config_data.get(section) or {}).get(key) if not isinstance(address, str) or not _is_loopback(address): continue - msg = ( + if refused: + raise Exception( + f"{section}.{key} '{address}' is a loopback address, which the " + f"{backend} workers cannot reach. Set it to an address they can reach" + ) + logger.warning( f"{section}.{key} '{address}' is a loopback address, which the " - f"{lithops_cfg['backend']} workers cannot reach. Set it to an " - "address they can reach" + f"{backend} workers only reach if the service runs on, or is tunnelled " + "to, the machine they run on. Set it to an address they can reach otherwise" ) - if mode == c.SERVERLESS: - raise Exception(msg) - logger.warning(msg) def _load_monitoring_backend_config(config_data): diff --git a/lithops/executors.py b/lithops/executors.py index 12decb187..ecdf1d71b 100644 --- a/lithops/executors.py +++ b/lithops/executors.py @@ -771,7 +771,8 @@ def wait( used to control the maximum number of seconds to wait before returning. - :param fs: Futures list. Default None + :param fs: Futures list. Default None, every future of this + executor. An empty list waits on none :param throw_except: Re-raise exception if call raised. Default True :param return_when: Percentage of done futures @@ -853,7 +854,8 @@ def get_result( """ For getting the results from all function activations - :param fs: Futures list. Default None + :param fs: Futures list. Default None, every future of this + executor. An empty list has no results :param throw_except: Reraise exception if call raised. Default True. :param timeout: Timeout for waiting for results. :param threadpool_size: Number of threads to use. Default 64 @@ -983,7 +985,8 @@ def clean( function invocation results. It can also clean cloudobjects. - :param fs: List of futures to clean + :param fs: List of futures to clean. Default None, every future + of this executor. An empty list cleans none :param cs: List of cloudobjects to clean :param clean_cloudobjects: Delete all cloudobjects created with this executor diff --git a/lithops/job/partitioner.py b/lithops/job/partitioner.py index 3169d22e5..118affbeb 100644 --- a/lithops/job/partitioner.py +++ b/lithops/job/partitioner.py @@ -57,6 +57,26 @@ def create_partitions( Splits the objects referenced by the iterdata into partitions, one task each. Only one kind of source is partitioned per call """ + urls, paths, objects = _sort_sources(map_iterdata) + + if urls: + return _split_objects_from_urls( + urls, obj_chunk_size, obj_chunk_number, obj_newline + ) + if paths: + return _split_objects_from_paths( + paths, obj_chunk_size, obj_chunk_number, obj_newline + ) + if objects: + return _split_objects_from_object_storage( + objects, obj_chunk_size, obj_chunk_number, + internal_storage, config, obj_newline + ) + return [], [] + + +def _sort_sources(map_iterdata: List[Entry]) -> Tuple[List[Entry], List[Entry], List[Entry]]: + """Sorts the entries into URLs, local paths and storage objects""" urls = [] paths = [] objects = [] @@ -72,20 +92,50 @@ def create_partitions( else: objects.append(elem) + return urls, paths, objects + + +def split_into_objects( + config: Dict[str, Any], + internal_storage: Any, + map_iterdata: List[Entry] +) -> List[Entry]: + """ + Turns the iterdata into one entry per object, each one a call of its own + once partitioned without chunks. The buckets, prefixes, glob patterns and + directories are listed as create_partitions() lists them, and a single + object is left as it is, with no request. Only one kind of source is + taken, as create_partitions() does + """ + urls, paths, objects = _sort_sources(map_iterdata) + if urls: - return _split_objects_from_urls( - urls, obj_chunk_size, obj_chunk_number, obj_newline - ) + return urls if paths: - return _split_objects_from_paths( - paths, obj_chunk_size, obj_chunk_number, obj_newline - ) - if objects: - return _split_objects_from_object_storage( - objects, obj_chunk_size, obj_chunk_number, - internal_storage, config, obj_newline - ) - return [], [] + return _expand_paths(paths) + if not objects: + return [] + + # Copies: their objects are rewritten as full storage URLs + objects = [dict(elem) for elem in objects] + storage = _resolve_object_storage(objects, internal_storage, config) + + entries = [] + for elem in objects: + sb, bucket, prefix, obj_name = utils.split_object_url(elem['obj']) + globbed = sb in _GLOBBER_BACKENDS and ('*' in prefix or '*' in obj_name) + if obj_name and not globbed: + entries.append(elem) + continue + params = {k: v for k, v in elem.items() if k != 'obj'} + for dobj in _list_objects(storage, sb, bucket, prefix, obj_name): + # A key that ends in a slash is a folder marker, never partitioned + if not dobj['Key'].endswith('/'): + entries.append({'obj': f"{sb}://{bucket}/{dobj['Key']}", **params}) + + if not entries: + raise Exception('No objects found') + return entries def _log_chunk_settings(chunk_size: Optional[int], chunk_number: Optional[int]) -> None: diff --git a/lithops/monitoring/backends/storage/storage.py b/lithops/monitoring/backends/storage/storage.py index 1877e115c..4d268164e 100644 --- a/lithops/monitoring/backends/storage/storage.py +++ b/lithops/monitoring/backends/storage/storage.py @@ -225,6 +225,10 @@ def _generate_tokens(self, callids_running, callids_done): # Marking it processed here drops the token for good attributed = set() for callid_done in done_new: + if callid_done[1] in self._token_closed_jobs: + # Nothing of a closed job counts, nor is kept, any more + attributed.add(callid_done) + continue worker_id = self.callids_running_worker.get(callid_done) if worker_id is None: continue @@ -254,7 +258,9 @@ def _release_free_workers(self): # A job wait() dropped still hands its tokens back: the workers # are this executor's whether a future waits on them or not jobs = self.present_jobs | self._dropped_jobs.keys() - for worker_id, done_calls in self.callids_done_worker.items(): + # A copy: handing the last worker of a job back closes the job, which + # forgets its workers + for worker_id, done_calls in list(self.callids_done_worker.items()): if worker_id in self.workers_done: continue job_id = self.worker_job.get(worker_id) @@ -281,6 +287,11 @@ def _releases_tokens(self): """ return self.should_run or self._final_sweep + def _forget_workers_of(self, job_ids): + super()._forget_workers_of(job_ids) + for worker_id in [w for w, job_id in self.worker_job.items() if job_id in job_ids]: + del self.worker_job[worker_id] + def _release_timed_out_worker(self, call_status): worker_id = call_status.get('activation_id') if not self.generate_tokens or worker_id is None: @@ -288,6 +299,8 @@ def _release_timed_out_worker(self, call_status): if call_status['executor_id'] != self.executor_id: return with self._apply_lock: + if call_status['job_id'] in self._token_closed_jobs: + return self.callids_done_worker.setdefault(worker_id, set()).add( _status_id(call_status) ) diff --git a/lithops/monitoring/monitor.py b/lithops/monitoring/monitor.py index e752bdf0d..111702155 100644 --- a/lithops/monitoring/monitor.py +++ b/lithops/monitoring/monitor.py @@ -362,6 +362,22 @@ def close_jobs(self, job_ids): self._dropped_jobs.pop(job_id, None) self._credited_calls.pop(job_id, None) self._discarded_calls.pop(job_id, None) + self._forget_workers_of(job_ids) + + def _forget_workers_of(self, job_ids): + """ + Drops what is kept about the workers of closed jobs. No status of + theirs counts any more, and every monitor of the executor shares + this bookkeeping, so it would otherwise hold every call the + executor ever ran. Called with the apply lock held + """ + gone = [ + worker_id for worker_id, calls in self.callids_done_worker.items() + if any(status_id[1] in job_ids for status_id in calls) + ] + for worker_id in gone: + del self.callids_done_worker[worker_id] + self.workers_done.discard(worker_id) def discard_calls(self, discarded): """ @@ -423,8 +439,12 @@ def _owed_calls(self, job_id): ) def _settle(self, job_id): - """Closes a dropped job, then no longer listed, once it owes no call""" - if job_id not in self._dropped_jobs or job_id not in self.job_total_calls: + """ + Closes a job once it owes no call, every one discarded or run by a + worker that was handed back. A dropped job is then no longer listed, + and the workers of any job are forgotten + """ + if job_id not in self.job_total_calls: return if self._owed_calls(job_id) <= 0: self.close_jobs({job_id}) diff --git a/lithops/retries.py b/lithops/retries.py index 332c47f06..2990bde91 100644 --- a/lithops/retries.py +++ b/lithops/retries.py @@ -17,8 +17,9 @@ from lithops import FunctionExecutor from lithops.future import ResponseFuture +from lithops.job.partitioner import split_into_objects from lithops.storage.utils import CloudObject -from lithops.utils import _as_iterdata_list, _chained_futures, is_object_processing_function +from lithops.utils import _as_iterdata_list, _chained_futures, is_object_processing_function, verify_args from lithops.wait import ( ALL_COMPLETED, ALWAYS, @@ -216,6 +217,9 @@ def map( :param map_function: The function to map over the data. :param map_iterdata: An iterable of input data (e.g., Python list). + For a function that processes objects, a bucket, a prefix, a + glob pattern or a directory is listed first, so that each of its + objects is an input of its own, retried on its own. :param chunksize: Split map_iterdata in chunks of this size. One worker per chunk. :param extra_args: Additional arguments to pass to each @@ -224,10 +228,10 @@ def map( the function environment. :param runtime_memory: Memory (in MB) to allocate per function activation. - :param obj_chunk_size: For file processing. Split each - object into chunks of this size (in bytes). - :param obj_chunk_number: For file processing. Number of - chunks to split each object into. + :param obj_chunk_size: Not supported: a failed chunk of an + object cannot be retried on its own, so ValueError is raised. + Use FunctionExecutor to process objects in chunks. + :param obj_chunk_number: Not supported, as obj_chunk_size. :param obj_newline: Newline character for line integrity in file partitioning. :param timeout: Max time per function activation @@ -253,6 +257,27 @@ def map( ) retries_to_use = self._retries_to_use(retries) + # A generator can be read only once, and the inputs are kept to be + # resubmitted on a retry + if isinstance(map_iterdata, Iterator): + map_iterdata = list(map_iterdata) + # The future of each call is paired with the input it was made from. + # A chain leaves out the futures that produce no input + inputs = _chained_futures(map_iterdata) + if inputs is None and is_object_processing_function(map_function): + # A bucket, a prefix or a directory runs one call per object in + # it. Each object is made an input of its own before invoking, so + # that a retry resubmits only the one whose call failed + map_iterdata = inputs = split_into_objects( + self.executor.config, + self.executor.internal_storage, + verify_args(map_function, map_iterdata, extra_args), + ) + # verify_args merged them into every input already + extra_args = None + if inputs is None: + inputs = _as_iterdata_list(map_iterdata) + map_kwargs = dict( chunksize=chunksize, extra_args=extra_args, @@ -266,22 +291,14 @@ def map( exclude_modules=exclude_modules, ) - # A generator can be read only once, and the inputs are kept to be - # resubmitted on a retry - if isinstance(map_iterdata, Iterator): - map_iterdata = list(map_iterdata) - # The future of each call is paired with the input it was made from. - # A chain leaves out the futures that produce no input - inputs = _chained_futures(map_iterdata) - if inputs is None: - inputs = _as_iterdata_list(map_iterdata) - futures_list = self.executor.map( map_function, map_iterdata, **map_kwargs, ) if len(futures_list) != len(inputs): + # Not expected since the objects are split above. Said rather + # than pairing a future with the input of another call raise ValueError( f'The {len(inputs)} inputs turned into {len(futures_list)} ' 'calls, which are running now but cannot be retried: a retry ' diff --git a/lithops/tests/test_config.py b/lithops/tests/test_config.py index 0c0d3d93f..c2aabdc8c 100644 --- a/lithops/tests/test_config.py +++ b/lithops/tests/test_config.py @@ -558,11 +558,12 @@ def test_azure_queue_monitoring_requires_account(self): class TestLoopbackAddresses: @staticmethod - def _config(mode, storage, monitoring, **sections): - backend = 'aws_ec2' if mode == 'standalone' else 'aws_lambda' + def _config(mode, storage, monitoring, backend=None, **sections): + backend = backend or ('aws_ec2' if mode == 'standalone' else 'aws_lambda') lithops_cfg = {'mode': mode, 'backend': backend, 'storage': storage, 'monitoring': monitoring} return {'lithops': lithops_cfg, **sections} + @pytest.mark.parametrize('storage, monitoring, sections', [ ('aws_s3', 'redis', {'redis': {'host': '127.0.0.1'}}), ('aws_s3', 'redis', {'redis': {'host': '::1'}}), @@ -581,7 +582,31 @@ def test_standalone_workers_only_warn(self): config = self._config('standalone', 'aws_s3', 'redis', redis={'host': '127.0.0.1'}) with patch('lithops.config.logger') as logger: _check_loopback_addresses(config) - assert 'is a loopback address, which the aws_ec2 workers cannot reach' in logger.warning.call_args[0][0] + assert 'is a loopback address, which the aws_ec2 workers only reach if' in logger.warning.call_args[0][0] + + def test_singularity_workers_only_warn(self): + """Its containers share the network of the host, which can be this one""" + config = self._config( + 'serverless', 'aws_s3', 'rabbitmq', backend='singularity', + rabbitmq={'amqp_url': 'amqp://guest@localhost:5672'}, + ) + with patch('lithops.config.logger') as logger: + _check_loopback_addresses(config) + assert 'which the singularity workers only reach if' in logger.warning.call_args[0][0] + + def test_default_config_checks_the_addresses(self): + # Backends whose modules import no SDK beyond the tests extra + def config(redis_host): + return { + 'lithops': {'backend': 'openwhisk', 'storage': 'redis'}, + 'openwhisk': {'endpoint': 'https://openwhisk.example.com', 'namespace': 'ns', 'api_key': 'key'}, + 'redis': {'host': redis_host}, + } + + with pytest.raises(Exception, match='is a loopback address, which the openwhisk workers cannot reach'): + default_config(config_data=config('127.0.0.1')) + cfg = default_config(config_data=config('redis.example.com')) + assert cfg['lithops']['mode'] == c.SERVERLESS @pytest.mark.parametrize('mode, host', [ ('localhost', '127.0.0.1'), diff --git a/lithops/tests/test_invokers.py b/lithops/tests/test_invokers.py index 64ffd9db2..615cb0cb3 100644 --- a/lithops/tests/test_invokers.py +++ b/lithops/tests/test_invokers.py @@ -281,6 +281,23 @@ def test_run_job_starts_monitor(self, tmp_path, monkeypatch): futures = inv.run_job(job) inv.job_monitor.start.assert_called_once_with(futures) + @pytest.mark.parametrize('mode', [LOCALHOST, SERVERLESS]) + def test_only_the_localhost_mode_takes_an_interpreter_path_as_runtime(self, mode): + """The path of a local interpreter can hold any character, such as an @""" + interpreter = os.path.join(os.path.abspath(os.sep), 'opt', 'python@3.12', 'bin', 'python3') + handler = MagicMock() + handler.get_runtime_info.return_value = {'runtime_name': interpreter, 'max_workers': 1} + config = { + 'lithops': {'mode': mode, 'backend': 'localhost', 'storage': 'localhost', 'telemetry': False}, + 'localhost': {'storage_bucket': 'test-bucket'}, + } + if mode == LOCALHOST: + inv = BatchInvoker(config, 'sess-0', MagicMock(), handler, MagicMock()) + assert inv.runtime_name == interpreter + else: + with pytest.raises(AssertionError, match='not valid'): + BatchInvoker(config, 'sess-0', MagicMock(), handler, MagicMock()) + class TestFaaSInvokerHelpers: diff --git a/lithops/tests/test_monitor.py b/lithops/tests/test_monitor.py index ceaa5c6ba..bf04160ec 100644 --- a/lithops/tests/test_monitor.py +++ b/lithops/tests/test_monitor.py @@ -1335,6 +1335,35 @@ def test_no_token_bookkeeping_grows_when_tokens_are_off(self): assert monitor._credited_calls == {} assert monitor._discarded_calls == {} + def test_the_workers_of_a_job_that_owes_nothing_are_forgotten(self): + """ + Every monitor of an executor shares this bookkeeping, so it would + otherwise hold every call the executor ever ran + """ + job_monitor, futures = self._job_monitor(calls=2) + old = job_monitor.monitor + calls = {('sess-0', 'M000', f'{i:05d}') for i in range(2)} + running = {(call, f'w{call[2]}') for call in calls} + storage = old.internal_storage + storage.get_job_status.side_effect = self._listing(running, calls) + old._poll_and_process_job_status() + assert old.token_bucket_q.qsize() == 2 + assert old.callids_done_worker == {} + assert old.workers_done == set() + assert old.worker_job == {} + assert 'M000' in old._token_closed_jobs + + # A monitor that lists the job again neither hands a worker back + # twice nor keeps anything of it + old.stop() + job_monitor._spawn_monitor(generate_tokens=True) + new = job_monitor.monitor + new.add_futures(futures) + new._poll_and_process_job_status() + assert new.token_bucket_q.qsize() == 2 + assert new.callids_done_worker == {} + assert new.worker_job == {} + def test_a_nested_job_with_the_same_id_does_not_settle_this_one(self): """A nested executor numbers its jobs from M000 too""" monitor = self._storage(chunksize=1) @@ -2216,7 +2245,7 @@ def test_the_storage_sweep_hands_back_the_token_of_a_dropped_job(self): assert storage.get_job_status.call_count == 1 assert tokens.qsize() == 1 - def test_a_dropped_job_whose_end_was_counted_is_not_swept(self): + def test_a_dropped_job_whose_end_was_counted_is_not_listed(self): storage = MagicMock() tokens = queue.Queue() monitor = self._monitor(storage=storage, tokens=tokens, chunksize={'M000': 1}) @@ -2232,6 +2261,43 @@ def test_a_dropped_job_whose_end_was_counted_is_not_swept(self): assert tokens.qsize() == 1 assert monitor._jobs_owing_tokens() == set() + def test_the_sweep_reads_only_the_calls_no_end_was_counted_for(self): + """The status of a call whose __end__ was counted is not read back""" + storage = MagicMock() + tokens = queue.Queue() + monitor = self._monitor(storage=storage, tokens=tokens, chunksize={'M000': 1}) + monitor.job_total_calls = {'M000': 2} + futures = [FakeFuture('M000', invoked=True, call_id=f'{i:05d}') for i in range(2)] + monitor.add_futures(futures) + counted, _raw = _status(call_id='00000', kind='__end__') + monitor._apply_status_message(counted) + monitor.remove_futures(futures) + + lost = ('sess-0', 'M000', '00001') + storage.get_job_status.return_value = (set(), {('sess-0', 'M000', '00000'), lost}) + storage.get_call_status.return_value = {'type': '__end__', 'activation_id': 'act-2'} + assert monitor._storage_sweep() == 1 + storage.get_call_status.assert_called_once_with(*lost) + assert tokens.qsize() == 2 + assert monitor._jobs_owing_tokens() == set() + + def test_a_redelivered_end_of_a_job_that_owes_nothing_is_dropped(self): + """Its job is closed, so nothing of it is counted, kept or held again""" + tokens = queue.Queue() + monitor = self._monitor(tokens=tokens, chunksize={'M000': 1}) + monitor.job_total_calls = {'M000': 1} + monitor.add_futures([FakeFuture('M000', invoked=True, call_id='00000')]) + end, _raw = _status(kind='__end__') + monitor._apply_status_message(end) + assert tokens.qsize() == 1 + assert monitor.callids_done_worker == {} + assert monitor.workers_done == set() + + monitor._apply_status_message(dict(end)) + assert tokens.qsize() == 1 + assert monitor.callids_done_worker == {} + assert monitor._held_status == {} + def test_the_storage_sweep_gives_up_on_a_dropped_job(self, monkeypatch): """ A dropped job whose worker never reports back used to be listed on diff --git a/lithops/tests/test_retries.py b/lithops/tests/test_retries.py index a0b78c94d..eeceb998b 100644 --- a/lithops/tests/test_retries.py +++ b/lithops/tests/test_retries.py @@ -1,4 +1,6 @@ +import os import time +import uuid from types import SimpleNamespace from unittest.mock import MagicMock @@ -409,6 +411,36 @@ def _count_lines(obj): return obj.data_stream.read().count(b'\n') +def _count_lines_from(obj, skip): + return obj.data_stream.read().count(b'\n') - skip + + +def test_a_directory_is_retried_one_file_at_a_time(tmp_path): + """Each file is a call of its own, and a retry runs only the one that failed""" + data = tmp_path / 'data' + data.mkdir() + for name, lines in (('a.txt', 1), ('b.txt', 2), ('c.txt', 3)): + (data / name).write_text('x\n' * lines) + calls = tmp_path / 'calls' + calls.mkdir() + + def count_lines(obj): + name = os.path.basename(obj.path) + first = not any(f.startswith(name) for f in os.listdir(calls)) + (calls / f'{name}.{uuid.uuid4().hex}').touch() + if name == 'b.txt' and first: + raise RuntimeError('Deliberately fail on the first read of b.txt') + return obj.data_stream.read().count(b'\n') + + fexec = FunctionExecutor(config=pytest.lithops_config) + with RetryingFunctionExecutor(fexec) as executor: + futures = executor.map(count_lines, [str(data)], retries=1) + done, pending = executor.wait(futures) + assert pending == [] + assert sorted(f.result() for f in done) == [1, 2, 3] + assert sorted(f.rsplit('.', 1)[0] for f in os.listdir(calls)) == ['a.txt', 'b.txt', 'b.txt', 'c.txt'] + + class TestRetryingThrowExcept: def test_result_and_status_honour_throw_except_false(self): @@ -435,10 +467,13 @@ def test_inner_wait_never_raises_and_the_outer_one_reraises_at_the_end(self): class TestRetryingMapInputs: @staticmethod - def _inner(n_futures): + def _inner(n_futures, objects=()): + """An executor whose storage, of the localhost backend, lists ``objects``""" inner = MagicMock() inner.config = {} inner.map.return_value = [FakeResponseFuture() for _ in range(n_futures)] + inner.internal_storage = SimpleNamespace(backend='localhost', storage=MagicMock()) + inner.internal_storage.storage.list_objects.return_value = list(objects) return inner @pytest.mark.parametrize('chunks', [{'obj_chunk_number': 2}, {'obj_chunk_size': 64}]) @@ -450,19 +485,66 @@ def test_object_chunks_are_refused_before_invoking(self, chunks): ) inner.map.assert_not_called() - def test_an_input_that_holds_several_objects_is_an_error(self): - """A prefix expands into one call per object, which cannot be retried""" - inner = self._inner(3) - with pytest.raises(ValueError, match='cannot be retried'): - RetryingFunctionExecutor(inner).map(_count_lines, ['localhost://bucket/prefix/']) + def test_a_prefix_is_split_into_one_input_per_object(self): + """ + It used to raise once its calls were running, since a retry of one + of them would have resubmitted the whole prefix + """ + objects = [ + {'Key': 'prefix/a.txt', 'Size': 2}, + {'Key': 'prefix/', 'Size': 0}, + {'Key': 'prefix/b.txt', 'Size': 4}, + ] + inner = self._inner(2, objects) + futures = RetryingFunctionExecutor(inner).map( + _count_lines_from, ['localhost://bucket/prefix/'], extra_args={'skip': 1} + ) + expected = [ + {'obj': 'localhost://bucket/prefix/a.txt', 'skip': 1}, + {'obj': 'localhost://bucket/prefix/b.txt', 'skip': 1}, + ] + assert [f.input for f in futures] == expected + assert inner.map.call_args.args[1] == expected + inner.internal_storage.storage.list_objects.assert_called_once_with('bucket', 'prefix/', None) + # Merged into every input already, so neither the map nor a retry adds them again + assert inner.map.call_args.kwargs['extra_args'] is None + assert all(f.map_kwargs['extra_args'] is None for f in futures) + + def test_a_directory_is_split_into_one_input_per_file(self, tmp_path): + for name in ('a.txt', 'b.txt'): + (tmp_path / name).write_text('x\n') + futures = RetryingFunctionExecutor(self._inner(2)).map(_count_lines, [str(tmp_path)]) + assert sorted(f.input['obj'] for f in futures) == [ + str(tmp_path / 'a.txt'), str(tmp_path / 'b.txt') + ] - def test_single_objects_get_one_future_each(self, tmp_path): + def test_single_objects_are_not_listed(self, tmp_path): local_file = tmp_path / 'a.txt' local_file.write_text('x\n') - iterdata = ['localhost://bucket/a.txt', str(local_file)] - inner = self._inner(2) - futures = RetryingFunctionExecutor(inner).map(_count_lines, iterdata) - assert [f.input for f in futures] == iterdata + inner = self._inner(1) + futures = RetryingFunctionExecutor(inner).map(_count_lines, ['localhost://bucket/a.txt']) + assert [f.input for f in futures] == [{'obj': 'localhost://bucket/a.txt'}] + inner.internal_storage.storage.list_objects.assert_not_called() + inner.internal_storage.storage.head_object.assert_not_called() + futures = RetryingFunctionExecutor(self._inner(1)).map(_count_lines, [str(local_file)]) + assert [f.input for f in futures] == [{'obj': str(local_file)}] + + def test_a_glob_is_listed_only_where_the_backend_globs(self): + inner = self._inner(1, [{'Key': 'data/a.csv', 'Size': 1}]) + inner.internal_storage.backend = 'aws_s3' + futures = RetryingFunctionExecutor(inner).map(_count_lines, ['aws_s3://bucket/data/*.csv']) + assert [f.input for f in futures] == [{'obj': 'aws_s3://bucket/data/a.csv'}] + + inner = self._inner(1) + futures = RetryingFunctionExecutor(inner).map(_count_lines, ['localhost://bucket/a*.txt']) + assert [f.input for f in futures] == [{'obj': 'localhost://bucket/a*.txt'}] + inner.internal_storage.storage.list_objects.assert_not_called() + + def test_a_prefix_with_no_objects_is_refused_before_invoking(self): + inner = self._inner(0) + with pytest.raises(Exception, match='No objects found'): + RetryingFunctionExecutor(inner).map(_count_lines, ['localhost://bucket/empty/']) + inner.map.assert_not_called() def test_more_calls_than_inputs_is_an_error(self): inner = self._inner(3) diff --git a/lithops/tests/test_worker.py b/lithops/tests/test_worker.py index baa285eb6..1a058ea27 100644 --- a/lithops/tests/test_worker.py +++ b/lithops/tests/test_worker.py @@ -1351,6 +1351,26 @@ def f(a, b, *rest, c=0): args, kwargs = _prepare_args(f, {'a': 1, 'b': 2, 'rest': (3,), 'c': 4}) assert f(*args, **kwargs) == (1, 2, (3,), 4) + def test_prepare_args_none_varargs_are_empty(self): + def f(*args, **kwargs): + return args, kwargs + args, kwargs = _prepare_args(f, {'args': None, 'kwargs': None, 'x': 1}) + assert f(*args, **kwargs) == ((), {'x': 1}) + + def test_prepare_args_a_param_left_out_before_varargs_takes_its_default(self): + """verify_args lets a dict leave out a param that has a default""" + def f(a, b=2, *rest, c=0): + return a, b, rest, c + args, kwargs = _prepare_args(f, {'a': 1, 'rest': (3, 4)}) + assert f(*args, **kwargs) == (1, 2, (3, 4), 0) + args, kwargs = _prepare_args(f, {'a': 1, 'c': 5}) + assert f(*args, **kwargs) == (1, 2, (), 5) + + def g(a=1, b=2, /): + return a, b + args, kwargs = _prepare_args(g, {'b': 5}) + assert g(*args, **kwargs) == (1, 5) + class TestJobRunner: diff --git a/lithops/worker/jobrunner.py b/lithops/worker/jobrunner.py index 93d052b2e..0f9c2cdea 100644 --- a/lithops/worker/jobrunner.py +++ b/lithops/worker/jobrunner.py @@ -440,9 +440,26 @@ def _prepare_args( # the ones before *args by position, the rest by name. Names that are not # params go to **kwargs sig = inspect.signature(func) - bound = inspect.BoundArguments( - sig, {name: data[name] for name in sig.parameters if name in data} - ) + params = list(sig.parameters.values()) + arguments = {} + for i, param in enumerate(params): + if param.name in data: + value = data[param.name] + # An empty *args or **kwargs may come as None + if value is None and param.kind == param.VAR_POSITIONAL: + value = () + elif value is None and param.kind == param.VAR_KEYWORD: + value = {} + arguments[param.name] = value + elif param.default is not param.empty and any( + later.name in data and later.kind in (later.POSITIONAL_ONLY, later.VAR_POSITIONAL) + for later in params[i + 1:] + ): + # The data may leave out a param that has a default. Before one + # that only goes by position it takes that default, or the + # positional ones after it would be passed by name + arguments[param.name] = param.default + bound = inspect.BoundArguments(sig, arguments) kwargs = bound.kwargs kwargs.update((k, v) for k, v in data.items() if k not in sig.parameters) return bound.args, kwargs From d917a33d39fd8158e59a52182115416b20b7d6ff Mon Sep 17 00:00:00 2001 From: JosepSampe Date: Wed, 14 Oct 2026 17:32:20 +0200 Subject: [PATCH 3/3] Update partitioner --- lithops/job/partitioner.py | 83 ++++++++------------------- lithops/retries.py | 20 ++++--- lithops/tests/test_job.py | 27 +++++++++ lithops/tests/test_retries.py | 104 ++++++++++++++++++++++++---------- 4 files changed, 137 insertions(+), 97 deletions(-) diff --git a/lithops/job/partitioner.py b/lithops/job/partitioner.py index 118affbeb..27510fe99 100644 --- a/lithops/job/partitioner.py +++ b/lithops/job/partitioner.py @@ -57,26 +57,16 @@ def create_partitions( Splits the objects referenced by the iterdata into partitions, one task each. Only one kind of source is partitioned per call """ - urls, paths, objects = _sort_sources(map_iterdata) + # Partitions made by an earlier call are taken as they are: the retrying + # executor makes them before invoking, so that each call has an input of + # its own to resubmit, and partitioning them again would cost a request + # per object to learn the sizes they already carry + if map_iterdata and all(_is_partition(elem['obj']) for elem in map_iterdata): + parts_per_object = [ + elem['obj'].total_parts for elem in map_iterdata if elem['obj'].part == 1 + ] + return map_iterdata, parts_per_object - if urls: - return _split_objects_from_urls( - urls, obj_chunk_size, obj_chunk_number, obj_newline - ) - if paths: - return _split_objects_from_paths( - paths, obj_chunk_size, obj_chunk_number, obj_newline - ) - if objects: - return _split_objects_from_object_storage( - objects, obj_chunk_size, obj_chunk_number, - internal_storage, config, obj_newline - ) - return [], [] - - -def _sort_sources(map_iterdata: List[Entry]) -> Tuple[List[Entry], List[Entry], List[Entry]]: - """Sorts the entries into URLs, local paths and storage objects""" urls = [] paths = [] objects = [] @@ -92,50 +82,25 @@ def _sort_sources(map_iterdata: List[Entry]) -> Tuple[List[Entry], List[Entry], else: objects.append(elem) - return urls, paths, objects - - -def split_into_objects( - config: Dict[str, Any], - internal_storage: Any, - map_iterdata: List[Entry] -) -> List[Entry]: - """ - Turns the iterdata into one entry per object, each one a call of its own - once partitioned without chunks. The buckets, prefixes, glob patterns and - directories are listed as create_partitions() lists them, and a single - object is left as it is, with no request. Only one kind of source is - taken, as create_partitions() does - """ - urls, paths, objects = _sort_sources(map_iterdata) - if urls: - return urls + return _split_objects_from_urls( + urls, obj_chunk_size, obj_chunk_number, obj_newline + ) if paths: - return _expand_paths(paths) - if not objects: - return [] - - # Copies: their objects are rewritten as full storage URLs - objects = [dict(elem) for elem in objects] - storage = _resolve_object_storage(objects, internal_storage, config) + return _split_objects_from_paths( + paths, obj_chunk_size, obj_chunk_number, obj_newline + ) + if objects: + return _split_objects_from_object_storage( + objects, obj_chunk_size, obj_chunk_number, + internal_storage, config, obj_newline + ) + return [], [] - entries = [] - for elem in objects: - sb, bucket, prefix, obj_name = utils.split_object_url(elem['obj']) - globbed = sb in _GLOBBER_BACKENDS and ('*' in prefix or '*' in obj_name) - if obj_name and not globbed: - entries.append(elem) - continue - params = {k: v for k, v in elem.items() if k != 'obj'} - for dobj in _list_objects(storage, sb, bucket, prefix, obj_name): - # A key that ends in a slash is a folder marker, never partitioned - if not dobj['Key'].endswith('/'): - entries.append({'obj': f"{sb}://{bucket}/{dobj['Key']}", **params}) - if not entries: - raise Exception('No objects found') - return entries +def _is_partition(obj: Any) -> bool: + """Whether an object is one of the partitions _build_partitions() makes""" + return hasattr(obj, 'data_byte_range') and hasattr(obj, 'total_parts') def _log_chunk_settings(chunk_size: Optional[int], chunk_number: Optional[int]) -> None: diff --git a/lithops/retries.py b/lithops/retries.py index 2990bde91..9e961ce58 100644 --- a/lithops/retries.py +++ b/lithops/retries.py @@ -17,7 +17,7 @@ from lithops import FunctionExecutor from lithops.future import ResponseFuture -from lithops.job.partitioner import split_into_objects +from lithops.job.partitioner import create_partitions from lithops.storage.utils import CloudObject from lithops.utils import _as_iterdata_list, _chained_futures, is_object_processing_function, verify_args from lithops.wait import ( @@ -219,7 +219,9 @@ def map( :param map_iterdata: An iterable of input data (e.g., Python list). For a function that processes objects, a bucket, a prefix, a glob pattern or a directory is listed first, so that each of its - objects is an input of its own, retried on its own. + objects is an input of its own, retried on its own. It costs the + same requests as map(), and an empty object is no input, as it + runs no call. :param chunksize: Split map_iterdata in chunks of this size. One worker per chunk. :param extra_args: Additional arguments to pass to each @@ -266,13 +268,17 @@ def map( inputs = _chained_futures(map_iterdata) if inputs is None and is_object_processing_function(map_function): # A bucket, a prefix or a directory runs one call per object in - # it. Each object is made an input of its own before invoking, so - # that a retry resubmits only the one whose call failed - map_iterdata = inputs = split_into_objects( + # it, and an empty object none. Partitioned here, with the + # requests map() would make, each call gets an input of its own, + # so that a retry resubmits only the object whose call failed. + # map() takes the partitions as they are + map_iterdata, _ = create_partitions( self.executor.config, self.executor.internal_storage, verify_args(map_function, map_iterdata, extra_args), + None, None, obj_newline, ) + inputs = map_iterdata # verify_args merged them into every input already extra_args = None if inputs is None: @@ -297,8 +303,8 @@ def map( **map_kwargs, ) if len(futures_list) != len(inputs): - # Not expected since the objects are split above. Said rather - # than pairing a future with the input of another call + # Not expected, since the objects are partitioned above. Said + # rather than pairing a future with the input of another call raise ValueError( f'The {len(inputs)} inputs turned into {len(futures_list)} ' 'calls, which are running now but cannot be retried: a retry ' diff --git a/lithops/tests/test_job.py b/lithops/tests/test_job.py index d02396aa8..441aea45f 100644 --- a/lithops/tests/test_job.py +++ b/lithops/tests/test_job.py @@ -844,6 +844,33 @@ def test_empty_iterdata_returns_empty_partitions(self): [], ) + def test_partitions_are_taken_back_as_they_are(self): + """ + The retrying executor partitions before invoking. Partitioned again, + each of its partitions cost a request to learn its size + """ + storage = _storage() + storage.storage.list_objects.return_value = [ + {'Key': 'p/a.txt', 'Size': 2}, {'Key': 'p/b.txt', 'Size': 300}, + ] + parts, ppo = create_partitions( + {}, storage, [{'obj': 'localhost://bucket/p/', 'n': 1}], 100, None, '\n' + ) + assert ppo == [1, 3] + + again = _storage() + assert create_partitions({}, again, parts, 100, None, '\n') == (parts, [1, 3]) + again.storage.list_objects.assert_not_called() + again.storage.head_object.assert_not_called() + + # A CloudObject of the user's own is no partition, and is sized + again.storage.head_object.return_value = {'content-length': '2'} + parts, _ = create_partitions( + {}, again, [{'obj': CloudObject('localhost', 'bucket', 'k')}], None, None, '\n' + ) + again.storage.head_object.assert_called_once_with('bucket', 'k') + assert parts[0]['obj'].total_size == 2 + @pytest.mark.parametrize('path', [ r'C:\\data\\a.txt', 'C:/data/a.txt', r'\\\\server\\share\\a.txt', ]) diff --git a/lithops/tests/test_retries.py b/lithops/tests/test_retries.py index eeceb998b..8d9d257d3 100644 --- a/lithops/tests/test_retries.py +++ b/lithops/tests/test_retries.py @@ -8,6 +8,7 @@ from lithops import FunctionExecutor from lithops import RetryingFunctionExecutor +from lithops import Storage from lithops.retries import RetryingFuture from lithops.utils import FuturesList from lithops.wait import ALWAYS, ANY_COMPLETED @@ -416,10 +417,10 @@ def _count_lines_from(obj, skip): def test_a_directory_is_retried_one_file_at_a_time(tmp_path): - """Each file is a call of its own, and a retry runs only the one that failed""" + """Each file is a call of its own, and a retry runs only the one that failed. An empty one runs none""" data = tmp_path / 'data' data.mkdir() - for name, lines in (('a.txt', 1), ('b.txt', 2), ('c.txt', 3)): + for name, lines in (('a.txt', 1), ('b.txt', 2), ('c.txt', 3), ('empty.txt', 0)): (data / name).write_text('x\n' * lines) calls = tmp_path / 'calls' calls.mkdir() @@ -441,6 +442,40 @@ def count_lines(obj): assert sorted(f.rsplit('.', 1)[0] for f in os.listdir(calls)) == ['a.txt', 'b.txt', 'b.txt', 'c.txt'] + +def test_a_prefix_costs_the_requests_of_map(monkeypatch): + """ + It is listed once and its objects are not sized again: map() used to + send a head request per object, one after the other + """ + from lithops.storage.backends.localhost.localhost import LocalhostStorageBackend + + storage = Storage(config=pytest.lithops_config) + prefix = f'retries-{uuid.uuid4().hex}/' + for name, data in (('a.txt', b'x\n'), ('b.txt', b'x\nx\n'), ('empty.txt', b'')): + storage.put_object(storage.bucket, prefix + name, data) + + requests = [] + for method in ('list_objects', 'head_object'): + original = getattr(LocalhostStorageBackend, method) + + def counting(self, bucket, key=None, *args, _method=method, _original=original, **kwargs): + if key and key.startswith(prefix): + requests.append(_method) + return _original(self, bucket, key, *args, **kwargs) + monkeypatch.setattr(LocalhostStorageBackend, method, counting) + + try: + fexec = FunctionExecutor(config=pytest.lithops_config) + with RetryingFunctionExecutor(fexec) as executor: + futures = executor.map(_count_lines, [f'localhost://{storage.bucket}/{prefix}']) + done, pending = executor.wait(futures) + assert pending == [] + assert sorted(f.result() for f in done) == [1, 2] + assert requests == ['list_objects'] + finally: + storage.delete_objects(storage.bucket, storage.list_keys(storage.bucket, prefix)) + class TestRetryingThrowExcept: def test_result_and_status_honour_throw_except_false(self): @@ -485,6 +520,16 @@ def test_object_chunks_are_refused_before_invoking(self, chunks): ) inner.map.assert_not_called() + @staticmethod + def _names(futures): + """The object the input of each future names, and its other params""" + def name(obj): + return getattr(obj, 'path', None) or f'{obj.backend}://{obj.bucket}/{obj.key}' + return [ + (name(f.input['obj']), {k: v for k, v in f.input.items() if k != 'obj'}) + for f in futures + ] + def test_a_prefix_is_split_into_one_input_per_object(self): """ It used to raise once its calls were running, since a retry of one @@ -493,52 +538,49 @@ def test_a_prefix_is_split_into_one_input_per_object(self): objects = [ {'Key': 'prefix/a.txt', 'Size': 2}, {'Key': 'prefix/', 'Size': 0}, + {'Key': 'prefix/empty.txt', 'Size': 0}, {'Key': 'prefix/b.txt', 'Size': 4}, ] inner = self._inner(2, objects) futures = RetryingFunctionExecutor(inner).map( _count_lines_from, ['localhost://bucket/prefix/'], extra_args={'skip': 1} ) - expected = [ - {'obj': 'localhost://bucket/prefix/a.txt', 'skip': 1}, - {'obj': 'localhost://bucket/prefix/b.txt', 'skip': 1}, + # A folder marker and an empty object run no call, as with map() + assert self._names(futures) == [ + ('localhost://bucket/prefix/a.txt', {'skip': 1}), + ('localhost://bucket/prefix/b.txt', {'skip': 1}), ] - assert [f.input for f in futures] == expected - assert inner.map.call_args.args[1] == expected - inner.internal_storage.storage.list_objects.assert_called_once_with('bucket', 'prefix/', None) + assert inner.map.call_args.args[1] == [f.input for f in futures] + storage = inner.internal_storage.storage + storage.list_objects.assert_called_once_with('bucket', 'prefix/', None) + storage.head_object.assert_not_called() # Merged into every input already, so neither the map nor a retry adds them again assert inner.map.call_args.kwargs['extra_args'] is None assert all(f.map_kwargs['extra_args'] is None for f in futures) def test_a_directory_is_split_into_one_input_per_file(self, tmp_path): - for name in ('a.txt', 'b.txt'): - (tmp_path / name).write_text('x\n') + """An empty file runs no call, so it is no input either""" + for name, data in (('a.txt', 'x\n'), ('empty.txt', ''), ('b.txt', 'x\n')): + (tmp_path / name).write_text(data) futures = RetryingFunctionExecutor(self._inner(2)).map(_count_lines, [str(tmp_path)]) - assert sorted(f.input['obj'] for f in futures) == [ + assert sorted(name for name, _ in self._names(futures)) == [ str(tmp_path / 'a.txt'), str(tmp_path / 'b.txt') ] + futures = RetryingFunctionExecutor(self._inner(1)).map(_count_lines, [str(tmp_path / 'a.txt')]) + assert self._names(futures) == [(str(tmp_path / 'a.txt'), {})] - def test_single_objects_are_not_listed(self, tmp_path): - local_file = tmp_path / 'a.txt' - local_file.write_text('x\n') - inner = self._inner(1) - futures = RetryingFunctionExecutor(inner).map(_count_lines, ['localhost://bucket/a.txt']) - assert [f.input for f in futures] == [{'obj': 'localhost://bucket/a.txt'}] - inner.internal_storage.storage.list_objects.assert_not_called() - inner.internal_storage.storage.head_object.assert_not_called() - futures = RetryingFunctionExecutor(self._inner(1)).map(_count_lines, [str(local_file)]) - assert [f.input for f in futures] == [{'obj': str(local_file)}] - - def test_a_glob_is_listed_only_where_the_backend_globs(self): - inner = self._inner(1, [{'Key': 'data/a.csv', 'Size': 1}]) - inner.internal_storage.backend = 'aws_s3' - futures = RetryingFunctionExecutor(inner).map(_count_lines, ['aws_s3://bucket/data/*.csv']) - assert [f.input for f in futures] == [{'obj': 'aws_s3://bucket/data/a.csv'}] - + def test_single_objects_are_sized_once_and_an_empty_one_is_no_input(self): inner = self._inner(1) - futures = RetryingFunctionExecutor(inner).map(_count_lines, ['localhost://bucket/a*.txt']) - assert [f.input for f in futures] == [{'obj': 'localhost://bucket/a*.txt'}] - inner.internal_storage.storage.list_objects.assert_not_called() + storage = inner.internal_storage.storage + storage.head_object.side_effect = lambda bucket, key: { + 'content-length': '0' if key == 'empty.txt' else '2' + } + futures = RetryingFunctionExecutor(inner).map( + _count_lines, ['localhost://bucket/a.txt', 'localhost://bucket/empty.txt'] + ) + assert self._names(futures) == [('localhost://bucket/a.txt', {})] + assert storage.head_object.call_count == 2 + storage.list_objects.assert_not_called() def test_a_prefix_with_no_objects_is_refused_before_invoking(self): inner = self._inner(0)