Skip to content

fix: stabilize logging and RabbitMQ shutdown lifecycle#2117

Merged
bittergreen merged 4 commits into
MemTensor:dev-v2.0.24from
bittergreen:fix-dev-v2.0.24
Jul 16, 2026
Merged

fix: stabilize logging and RabbitMQ shutdown lifecycle#2117
bittergreen merged 4 commits into
MemTensor:dev-v2.0.24from
bittergreen:fix-dev-v2.0.24

Conversation

@bittergreen

Copy link
Copy Markdown
Collaborator

Description

fix: configure logging once per process
fix: close scheduler resources on API shutdown

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Unit Test - tests/api/test_lifecycle.py, tests/test_log.py

Checklist

  • I have performed a self-review of my own code | 我已自行检查了自己的代码
  • I have commented my code in hard-to-understand areas | 我已在难以理解的地方对代码进行了注释
  • I have added tests that prove my fix is effective or that my feature works | 我已添加测试以证明我的修复有效或功能正常
  • I have created related documentation issue/PR in MemOS-Docs (if applicable) | 我已在 MemOS-Docs 中创建了相关的文档 issue/PR(如果适用)
  • I have linked the issue to this PR (if applicable) | 我已将 issue 链接到此 PR(如果适用)
  • I have mentioned the person who will review this PR | 我已提及将审查此 PR 的人

Reviewer Checklist

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Made sure Checks passed
  • Tests have been provided

@Memtensor-AI Memtensor-AI added area:api 云服务 / FastAPI / OpenAPI / MCP status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Jul 16, 2026
@bittergreen
bittergreen requested a review from wustzdy July 16, 2026 12:58
@Memtensor-AI

Memtensor-AI commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

🤖 Open Code Review

Target: PR #2117
Task: 36fc19a11812f091
Base: dev-v2.0.24
Head: fix-dev-v2.0.24

🔍 OpenCodeReview found 2 issue(s) in this PR.


1. tests/api/test_lifecycle.py (L34-L38)

This test has no assertion — it only verifies that no exception is raised, which is an implicit and fragile contract. If shutdown_components silently swallowed an error or mutated state unexpectedly, this test would still pass.

Consider adding an explicit assertion to make the intent clear and the test meaningful, e.g. asserting that no method was called (since MinimalScheduler has none to call), or at minimum add a comment explaining the "no exception raised" contract.

💡 Suggested Change

Before:

def test_shutdown_components_skips_missing_methods():
    class MinimalScheduler:
        pass

    shutdown_components({"mem_scheduler": MinimalScheduler()})

After:

def test_shutdown_components_skips_missing_methods():
    class MinimalScheduler:
        pass

    # Should complete without raising; no callable methods means no calls are made
    shutdown_components({"mem_scheduler": MinimalScheduler()})  # assert no exception raised

2. src/memos/api/server_api_ext.py (L65-L68)

Blocking I/O inside an async lifespan handler will stall the event loop.

shutdown_components is a synchronous function that performs potentially blocking network/IO operations (stop() and rabbitmq_close() on mem_scheduler). Calling it directly after the yield in an async def lifespan runs it on the event loop thread, blocking all other async tasks until it completes.

Suggestion: run the synchronous shutdown in a thread executor to avoid stalling the event loop:

import asyncio

@asynccontextmanager
async def lifespan(_app: FastAPI) -> AsyncIterator[None]:
    yield
    loop = asyncio.get_event_loop()
    await loop.run_in_executor(None, shutdown_components, server_router_module.components)

Alternatively, make shutdown_components (and the underlying methods) async-native.

Generated by cloud-assistant via Open Code Review.

Avoid re-running dictConfig on every get_logger call, since repeated configuration can close and replace active handlers while background threads are emitting records. Keep logging configuration process-local and reconfigure only after a PID change.
Use FastAPI lifespan cleanup to stop scheduler and RabbitMQ resources before logging handlers are torn down during worker or pod shutdown.
@bittergreen bittergreen reopened this Jul 16, 2026
@bittergreen
bittergreen requested a review from ZongYue99 July 16, 2026 13:13
@Memtensor-AI

Copy link
Copy Markdown
Collaborator

✅ Automated Test Results: PASSED

All tests passed (7/7 executed). memos_python_core/changed-repo-python: 7/7. Duration: 5s [advisory, non-gating] AI-generated tests on branch test/auto-gen-36fc19a11812f091-20260716211740: 103/106 passed, 1 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix-dev-v2.0.24

@bittergreen
bittergreen merged commit 61c1322 into MemTensor:dev-v2.0.24 Jul 16, 2026
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:api 云服务 / FastAPI / OpenAPI / MCP status:in-progress Someone or AI is working on it | 人工或 AI 正在处理

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants