Skip to content

fix(session): 持久化跨 HTTP 请求的 used 状态 - #4485

Open
loading2332 wants to merge 1 commit into
volcengine:mainfrom
loading2332:fix/session-used-persistence
Open

fix(session): 持久化跨 HTTP 请求的 used 状态#4485
loading2332 wants to merge 1 commit into
volcengine:mainfrom
loading2332:fix/session-used-persistence

Conversation

@loading2332

@loading2332 loading2332 commented Aug 29, 2026

Copy link
Copy Markdown

Description

修复 POST /api/v1/sessions/{session_id}/used 的状态只在单次 HTTP 请求内存在的问题。

改动前,每次 REST 请求都会经 SessionService.get() 重建 Session,但 Session.used() 只修改当前 Python 对象中的 _usage_records_stats。因此后续 used 请求无法累计,独立的 commit 请求也会得到空的 usage_snapshot,文档承诺的资源 active_count 更新不会发生。

改动后,尚未被 commit 消费的 usage 会作为 Session 根状态持久化;used 和 commit 复用现有 Session PathLock 进行权威读改写。commit Phase 1 从同一锁保护的快照生成 usage_uris,成功持久化提交边界后清空 pending usage,防止后续 commit 重复计数。

主要入口与 Owner:

  • REST:openviking/server/routers/sessions.py:record_used
  • Service:SessionService.get() / SessionService.commit_async()
  • Memory / Session:Session.used_async() / Session.commit_async()

这是已通过服务层真实对象生命周期复现的问题,不是仅根据代码推断。

Human Involvement

  • A human participated in the implementation or review loop
  • This PR was generated entirely by AI agents without human participation in the loop

Related Issue

Fixes #4484

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • 新增 Session.used_async():在 Session 目录 PathLock 下读取最新 .meta.json、合并并持久化 pending usage;同步 used() 保持兼容并委托给异步实现。
  • Session 加载与 commit Phase 1 会恢复锁内最新 usage;成功提交边界后清空 pending usage。
  • pending usage 仅在内部持久化序列化时输出,不进入默认 SessionMeta.to_dict(),避免通过 GET session 暴露技能输入/输出。
  • 强化既有跨请求和 active_count 测试:覆盖两个重建 Session 的累计,以及一个 Session 记录、另一个由 SessionService.commit_async() 提交的真实链路。

兼容性:旧 session 不含 pending_usage_records 时按空列表加载,无需迁移;公开 REST/SDK 语义不变,只恢复文档已声明的行为。

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing focused unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

实际执行:

uv run --no-sync pytest -q --no-cov tests/unit/session/test_event_tag_concurrency.py
# 3 passed

uv run --no-sync ruff check openviking/session/session.py openviking/server/routers/sessions.py tests/unit/session/test_event_tag_concurrency.py tests/api_test/sessions/slow/test_session_concurrency.py tests/session/test_session_commit.py
# All checks passed!

uv run --no-sync python -m compileall -q openviking/session/session.py openviking/server/routers/sessions.py
# passed

git diff --check
# passed

未在本机运行 live API / VectorDB 集成用例:当前 Windows 源码环境未打包兼容的 x86 PersistStore,fixture 初始化时报 No compatible x86 engine backend was packaged in this wheel。对应既有 API 与 Session 集成测试已更新,交由 Linux CI 执行。

mypy 按改动路径执行时会继续分析完整导入图,并命中仓库现有类型基线错误;使用 --follow-imports=skip 后仍报告 session.py 原有的 3 处错误(2690、2747、4206),本次新增/修改行无新增类型错误。

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

文档未修改:中英文 API 文档已经声明 used() 数据会在 commit() 时更新 active_count;本 PR 恢复该既有契约,没有新增公开行为。

Screenshots (if applicable)

不适用。

Additional Notes

PR 聚焦 Session usage 生命周期,不包含无关清理或重构。内部持久化字段只保存到现有 .meta.json,并沿用其既有存储、加密和 PathLock 边界。

@linhongyu510 linhongyu510 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is still a loss window after the new pending usage is persisted. In Phase 1 the PR sets pending_usage_records = [] and saves .meta.json, then calls _write_phase1_ready_marker(). If that marker write fails, the exception handler writes .failed.json and re-raises, but never restores the persisted usage. The queued worker checks .failed.json before readiness and returns a failed task without _run_memory_extraction(), so those URIs neither update active_count nor remain available to the next commit.

I reproduced this on exact head 63fc2b68 with one persisted context usage, successful phase1 marker/archive/queue/tracker stubs, and _write_phase1_ready_marker() raising. After commit_async() fails, reloading .meta.json yields pending_usage_records == []. This contradicts the PRs stated invariant that failed commits do not consume usage.

A focused regression belongs next to test_used_accumulates_across_recreated_session_instances: inject a ready-marker failure, then assert the persisted pending URI still exists. As a minimum explicit-error fix, restoring usage_snapshot under the still-held Session PathLock before returning the failure makes that test pass; with that compensation, the full concurrency test file is 4 passed, and Ruff/format pass.

That compensation alone is not crash-safe, though: a process can exit after the empty meta write and before the ready marker. Please make consumption recoverable across that boundary, for example by associating stable usage IDs/a consumption transaction with the Phase-1 marker and reconciling it on recovery (or otherwise providing an atomic state transition). A scheme that simply writes ready first has the inverse crash window and can replay the same usage in a later commit. The required invariant is that each captured usage is either still pending or durably owned by one recoverable commit, never neither or both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

[Bug]: sessions used 状态未跨 HTTP 请求持久化,commit 丢失 usage

2 participants