From 76fb6ec6b4c952ca447e01d70694526d718d385e Mon Sep 17 00:00:00 2001 From: Ken Simpson Date: Sun, 19 Apr 2026 22:21:44 -0700 Subject: [PATCH] tune: reduce default node cache size cap --- README.md | 2 +- docs/LEARNINGS.md | 1 + settings.py | 4 ++-- tests/test_settings.py | 15 +++++++++++++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0a89dfa..8a8c699 100644 --- a/README.md +++ b/README.md @@ -348,7 +348,7 @@ If a remote-marked node depends on a model filename that cannot be resolved to a - `COMFY_MODAL_APP_NAME` and `COMFY_MODAL_VOLUME_NAME`: Override Modal app and volume naming. - `COMFY_MODAL_INTERRUPT_DICT_NAME`: Override the shared Modal `Dict` used for remote cancellation flags. Default: `-interrupts`. - `COMFY_MODAL_NODE_CACHE_DICT_NAME`: Override the shared Modal `Dict` used for persisted transport-safe node outputs. Default: `-node-cache`. -- `COMFY_MODAL_NODE_CACHE_MAX_BYTES`: Maximum raw output size eligible for persisted node caching. Default: `52428800` (50 MiB). Set to `0` to disable the persisted node-output cache. +- `COMFY_MODAL_NODE_CACHE_MAX_BYTES`: Maximum raw output size eligible for persisted node caching. Default: `5242880` (5 MiB). Set to `0` to disable the persisted node-output cache. - `COMFY_MODAL_TERMINATE_CONTAINER_ON_ERROR`: When true, a remote execution crash makes the worker exit its own container after returning the error. Default: `true`. - `COMFY_MODAL_AUTO_DEPLOY`: Automatically deploy the Modal app on first remote invocation when lookup fails. Default: `true`. - `COMFY_MODAL_ALLOW_EPHEMERAL_FALLBACK`: Re-enable slow `app.run()` fallback in remote mode. Default: `false`. diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index b7aa88a..deb4d35 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -134,3 +134,4 @@ - Static mapped sub-runs cannot reuse the same "suppress status chatter" rule as per-item mapped lanes for numeric node progress. Those static phases usually do not have a lane id at all, so the local relay has to keep lane-less `node_progress` events for execute/boundary nodes or upstream samplers like `KSampler` will run remotely without ever showing a bar in the UI. - Once those lane-less static progress events are forwarded, they also need their own explicit clear path. Suppressed static streams do not forward the terminal `execution_success` status event into the frontend, so relying on phase completion to clear ordinary progress leaves the last `100%` sampler bar stuck onscreen. - Distributed node-cache restore and persist should not be treated as unrelated phases. If one prompt run restores `NC_` into the live `PromptExecutor` cache and then persists the same node/key again unconditionally, Modal Dict churns on pure cache hits for no benefit; the persist pass should recognize restored hits and skip rewriting the same key. +- A distributed node-cache size cap that is generous enough for medium images can still be too expensive in practice. In these logs, repeatedly serializing and writing ~25 MiB cache entries dominated the post-execution phase, so the default cap should bias toward smaller transport-safe artifacts unless there is a clear reason to keep large intermediates. diff --git a/settings.py b/settings.py index c6b709d..89537fa 100644 --- a/settings.py +++ b/settings.py @@ -32,7 +32,7 @@ class ModalSyncSettings: custom_nodes_dir: Path | None interrupt_dict_name: str = "comfy-modal-sync-interrupts" node_output_cache_dict_name: str = "comfy-modal-sync-node-cache" - node_output_cache_max_bytes: int = 50 * 1024 * 1024 + node_output_cache_max_bytes: int = 5 * 1024 * 1024 terminate_container_on_error: bool = True modal_gpu: str = "A100" scaledown_window_seconds: int = 600 @@ -179,7 +179,7 @@ def get_settings() -> ModalSyncSettings: ), node_output_cache_max_bytes=_read_int_env( "COMFY_MODAL_NODE_CACHE_MAX_BYTES", - 50 * 1024 * 1024, + 5 * 1024 * 1024, ), terminate_container_on_error=_read_bool_env("COMFY_MODAL_TERMINATE_CONTAINER_ON_ERROR") is not False, diff --git a/tests/test_settings.py b/tests/test_settings.py index 122a788..719e777 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -138,6 +138,21 @@ def test_settings_defaults_node_cache_dict_name_from_app_name( assert settings.node_output_cache_dict_name == "my-modal-app-node-cache" +def test_settings_defaults_node_cache_max_bytes_to_five_mib( + settings_module: Any, + monkeypatch: Any, +) -> None: + """The distributed node-cache size cap should default to 5 MiB.""" + monkeypatch.delenv("COMFY_MODAL_NODE_CACHE_MAX_BYTES", raising=False) + settings_module.get_settings.cache_clear() + try: + settings = settings_module.get_settings() + finally: + settings_module.get_settings.cache_clear() + + assert settings.node_output_cache_max_bytes == 5 * 1024 * 1024 + + def test_settings_reads_node_cache_overrides( settings_module: Any, monkeypatch: Any,