Emit INFO logs from all project modules inside the Modal container
Only the comfyui_modal_sync_cloud entrypoint logger had a handler in the container. Flat-imported sibling modules (cloud_durable_invocation, cloud_session_bridge, ...) fell through to the root WARNING handler, so their INFO timing lines were dropped. That hid the durable-result step that accounted for 28-90s per-item delays after execution finished. Inside the container, give every shipped top-level module an INFO handler that prints to the live sys.stdout at emit time (snapshot restores can swap stdout), with propagation off so warnings are not printed twice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
aadfba7c8f
commit
cbdaf4a5c7
@@ -5,14 +5,16 @@ from __future__ import annotations
|
||||
from contextlib import contextmanager
|
||||
import logging
|
||||
import os
|
||||
from pathlib import Path
|
||||
import sys
|
||||
import time
|
||||
from typing import Any, Iterator
|
||||
from typing import Any, Iterable, Iterator
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_TARGET_LOGGER: logging.Logger = logger
|
||||
_HANDLER_NAME = "comfyui-modal-sync-cloud-timestamped"
|
||||
_MODULE_HANDLER_NAME = "comfyui-modal-sync-cloud-modules"
|
||||
|
||||
|
||||
def _build_cloud_log_formatter() -> logging.Formatter:
|
||||
@@ -46,6 +48,69 @@ def configure_cloud_runtime_logging(
|
||||
return target_logger
|
||||
|
||||
|
||||
class _CurrentStdoutHandler(logging.Handler):
|
||||
"""Write each record to whatever ``sys.stdout`` is when the record is emitted.
|
||||
|
||||
Modal memory-snapshot restores can replace ``sys.stdout`` after import, so a
|
||||
stream captured at import time may no longer reach the container log.
|
||||
"""
|
||||
|
||||
def emit(self, record: logging.LogRecord) -> None:
|
||||
"""Format one record and print it to the live stdout stream."""
|
||||
try:
|
||||
print(self.format(record), file=sys.stdout, flush=True)
|
||||
except (OSError, ValueError):
|
||||
self.handleError(record)
|
||||
|
||||
|
||||
def cloud_project_module_names(repo_root: Path) -> tuple[str, ...]:
|
||||
"""Return the flat module names of the project files shipped to the container."""
|
||||
return tuple(
|
||||
sorted(
|
||||
module_path.stem
|
||||
for module_path in repo_root.glob("*.py")
|
||||
if module_path.stem != "__init__"
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def _cloud_module_handler(
|
||||
module_loggers: Iterable[logging.Logger],
|
||||
) -> logging.Handler:
|
||||
"""Return the shared module handler, reusing one already installed."""
|
||||
for module_logger in module_loggers:
|
||||
for existing_handler in module_logger.handlers:
|
||||
if getattr(existing_handler, "name", "") == _MODULE_HANDLER_NAME:
|
||||
return existing_handler
|
||||
handler = _CurrentStdoutHandler(level=logging.INFO)
|
||||
handler.set_name(_MODULE_HANDLER_NAME)
|
||||
handler.setFormatter(_cloud_formatter())
|
||||
return handler
|
||||
|
||||
|
||||
def configure_cloud_module_loggers(module_names: Iterable[str]) -> tuple[str, ...]:
|
||||
"""Emit INFO logs from sibling project modules inside the Modal container.
|
||||
|
||||
Only the entrypoint logger used to get a handler, so INFO records from modules
|
||||
such as ``cloud_durable_invocation`` fell through to Python's WARNING-only
|
||||
fallback and were silently dropped.
|
||||
"""
|
||||
module_loggers = [
|
||||
logging.getLogger(module_name)
|
||||
for module_name in module_names
|
||||
if module_name != _TARGET_LOGGER.name
|
||||
]
|
||||
handler = _cloud_module_handler(module_loggers)
|
||||
for module_logger in module_loggers:
|
||||
module_logger.setLevel(logging.INFO)
|
||||
if handler not in module_logger.handlers:
|
||||
module_logger.addHandler(handler)
|
||||
module_logger.propagate = False
|
||||
configured_names = tuple(module_logger.name for module_logger in module_loggers)
|
||||
logger.debug("Configured cloud module loggers: %s", configured_names)
|
||||
return configured_names
|
||||
|
||||
|
||||
def _is_modal_container_runtime() -> bool:
|
||||
"""Return whether the current process is executing inside a Modal container."""
|
||||
return os.getenv("MODAL_IS_REMOTE") == "1" or bool(os.getenv("MODAL_TASK_ID"))
|
||||
|
||||
@@ -245,6 +245,8 @@ try: # noqa: E402 - support package and flat Modal-container imports.
|
||||
_emit_cloud_info,
|
||||
_is_modal_container_runtime,
|
||||
_timed_phase,
|
||||
cloud_project_module_names,
|
||||
configure_cloud_module_loggers,
|
||||
configure_cloud_runtime_logging,
|
||||
)
|
||||
except ImportError: # pragma: no cover - exercised by flat cloud imports.
|
||||
@@ -258,6 +260,8 @@ except ImportError: # pragma: no cover - exercised by flat cloud imports.
|
||||
_emit_cloud_info,
|
||||
_is_modal_container_runtime,
|
||||
_timed_phase,
|
||||
cloud_project_module_names,
|
||||
configure_cloud_module_loggers,
|
||||
configure_cloud_runtime_logging,
|
||||
)
|
||||
|
||||
@@ -306,6 +310,8 @@ logger = logging.getLogger(__name__)
|
||||
# Cloud app and ComfyUI bootstrap state.
|
||||
_CLOUD_HANDLER_NAME = "comfyui-modal-sync-cloud-timestamped"
|
||||
configure_cloud_runtime_logging(logger, _CLOUD_HANDLER_NAME)
|
||||
if _is_modal_container_runtime():
|
||||
configure_cloud_module_loggers(cloud_project_module_names(_REPO_ROOT))
|
||||
|
||||
# Poisoned-container retirement state.
|
||||
_CONTAINER_TERMINATION_LOCK = threading.Lock()
|
||||
|
||||
@@ -806,3 +806,16 @@ GPUs for `reserve_cooldown_count=1`, and five containers for `instance_count=4`.
|
||||
per Modal environment (mapped > comfy > llm), and the lookup gives the reserve
|
||||
cooldown only to slots in that pool.
|
||||
|
||||
|
||||
## Remote container logs dropped INFO from every module but the entrypoint
|
||||
|
||||
Inside the Modal container the project imports its modules flat, so their loggers
|
||||
are named `cloud_durable_invocation`, `cloud_session_bridge`, and so on. Only
|
||||
`comfyui_modal_sync_cloud` had a handler. The others fell through to the root
|
||||
logger's WARNING-level handler, so their INFO timing lines silently disappeared.
|
||||
In practice this hid a 28s gap between `execute_subgraph_locally` finishing and
|
||||
`remote_engine_execute_payload` returning (the durable-result step).
|
||||
`configure_cloud_module_loggers` now gives every shipped top-level module an INFO
|
||||
handler, in the container only. That handler prints to the *current*
|
||||
`sys.stdout` at emit time, because memory-snapshot restores can replace stdout,
|
||||
which is also why `_emit_cloud_info` prints instead of using a stream handler.
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
"""Tests for Modal container logging of sibling project modules."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
import io
|
||||
import logging
|
||||
from pathlib import Path
|
||||
import sys
|
||||
from typing import Any, Iterator
|
||||
import uuid
|
||||
|
||||
import pytest
|
||||
|
||||
from conftest import PACKAGE_NAME
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def cloud_runtime_logging_module(extension_package: object) -> Any:
|
||||
"""Return the cloud runtime logging helpers."""
|
||||
del extension_package
|
||||
return importlib.import_module(f"{PACKAGE_NAME}.cloud_runtime_logging")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def module_logger_names() -> Iterator[list[str]]:
|
||||
"""Yield unique logger names and remove their handlers afterwards."""
|
||||
names = [f"test_cloud_module_{uuid.uuid4().hex}" for _ in range(2)]
|
||||
yield names
|
||||
for name in names:
|
||||
module_logger = logging.getLogger(name)
|
||||
for handler in list(module_logger.handlers):
|
||||
module_logger.removeHandler(handler)
|
||||
module_logger.setLevel(logging.NOTSET)
|
||||
module_logger.propagate = True
|
||||
|
||||
|
||||
def test_cloud_module_loggers_emit_info_to_current_stdout(
|
||||
cloud_runtime_logging_module: Any,
|
||||
module_logger_names: list[str],
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""INFO from sibling modules must reach the stdout that exists at emit time."""
|
||||
cloud_runtime_logging_module.configure_cloud_module_loggers(module_logger_names)
|
||||
replaced_stdout = io.StringIO()
|
||||
monkeypatch.setattr(sys, "stdout", replaced_stdout)
|
||||
|
||||
logging.getLogger(module_logger_names[0]).info(
|
||||
"Finished remote invocation completion in %.3fs", 1.5
|
||||
)
|
||||
|
||||
output = replaced_stdout.getvalue()
|
||||
assert "Finished remote invocation completion in 1.500s" in output
|
||||
assert f"INFO [{module_logger_names[0]}]" in output
|
||||
|
||||
|
||||
def test_cloud_module_loggers_are_idempotent_and_isolated(
|
||||
cloud_runtime_logging_module: Any,
|
||||
module_logger_names: list[str],
|
||||
) -> None:
|
||||
"""Repeat configuration must not duplicate output or leak to the root logger."""
|
||||
module = cloud_runtime_logging_module
|
||||
first = module.configure_cloud_module_loggers(module_logger_names)
|
||||
second = module.configure_cloud_module_loggers(module_logger_names)
|
||||
|
||||
assert first == second == tuple(module_logger_names)
|
||||
handlers = [logging.getLogger(name).handlers for name in module_logger_names]
|
||||
assert all(len(module_handlers) == 1 for module_handlers in handlers)
|
||||
assert handlers[0][0] is handlers[1][0]
|
||||
for name in module_logger_names:
|
||||
module_logger = logging.getLogger(name)
|
||||
assert module_logger.level == logging.INFO
|
||||
assert module_logger.propagate is False
|
||||
|
||||
|
||||
def test_cloud_module_loggers_skip_the_entrypoint_logger(
|
||||
cloud_runtime_logging_module: Any,
|
||||
) -> None:
|
||||
"""The entrypoint logger keeps its own handler and is never double-configured."""
|
||||
module = cloud_runtime_logging_module
|
||||
entrypoint_name = module._TARGET_LOGGER.name
|
||||
|
||||
configured = module.configure_cloud_module_loggers([entrypoint_name])
|
||||
|
||||
assert configured == ()
|
||||
|
||||
|
||||
def test_cloud_project_module_names_lists_flat_modules(
|
||||
cloud_runtime_logging_module: Any,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Every shipped top-level module except the package marker is configured."""
|
||||
for file_name in ("cloud_durable_invocation.py", "settings.py", "__init__.py"):
|
||||
(tmp_path / file_name).write_text("", encoding="utf-8")
|
||||
(tmp_path / "notes.txt").write_text("", encoding="utf-8")
|
||||
|
||||
names = cloud_runtime_logging_module.cloud_project_module_names(tmp_path)
|
||||
|
||||
assert names == ("cloud_durable_invocation", "settings")
|
||||
Reference in New Issue
Block a user