mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-08-15 11:23:39 +00:00
fix: coordinate upload aliases at commit
This commit is contained in:
parent
783920f75a
commit
96ba5dccd0
8 changed files with 136 additions and 9 deletions
|
|
@ -1461,6 +1461,7 @@ Multi-file upload with automatic document conversion:
|
|||
- Reuses one conversion worker per request when called from an active event loop
|
||||
- Files stored in thread-isolated directories under the resolving user's bucket (`users/{user_id}/threads/{thread_id}/user-data/uploads`). For IM channels the owner is threaded explicitly via the `user_id=` kwarg (see IM Channels → Owner-scoped file storage); HTTP/embedded callers resolve it from `get_effective_user_id()`
|
||||
- Every ingress stages a complete payload and atomically publishes it without replacing an existing entry. Collisions across requests, processes, HTTP, embedded, and IM adapters use `name.ext`, `name_1.ext`, `name_2.ext`; storage that cannot provide atomic no-replace publication fails explicitly.
|
||||
- Exact-name generation leases use a portable NFC-plus-casefold coordination key, so case and Unicode-normalization aliases cannot bypass an active generation on case-insensitive filesystems. The original filename remains the published name. Final lease release is the commit point: cancellation newly arriving during release is delayed and swallowed so a committed upload is returned as success rather than an indeterminate cancelled result.
|
||||
- Filenames containing NUL, `<`, `>`, or reserved model-context boundary markers are rejected before staging so accepted filenames and exact virtual paths remain lossless in model-visible upload context. Legacy files discovered on disk are still neutralized when listed.
|
||||
- Gateway HTTP uploads use same-directory `.upload-*.part` staging files. Each active stage holds a cross-process liveness lock under `.upload-conversions/.locks/stages/`; startup cleanup skips held stages and sweeps only crash-orphaned files. Cancellation during staging creation drains the worker and aborts the returned stage before propagating. Staging files are hidden from upload listings, agent upload context, and sandbox listing/search tools.
|
||||
- Generated Markdown is owned by `user-data/.upload-conversions/<actual-primary-filename>.md` and is omitted from primary upload listings. Deletion removes only the selected primary and that exact generated asset; it never guesses or deletes a legacy/user-owned `uploads/<stem>.md` sibling.
|
||||
|
|
|
|||
|
|
@ -228,6 +228,13 @@ async def _run_file_io_cancellation_safe(function, *args):
|
|||
return result
|
||||
|
||||
|
||||
async def _run_file_io_commit(function, *args):
|
||||
"""Finish a commit operation and ignore cancellation that arrives during it."""
|
||||
task = asyncio.create_task(run_file_io(function, *args))
|
||||
await wait_for_task_completion(task)
|
||||
return task.result()
|
||||
|
||||
|
||||
async def _publish_staged_upload_cancellation_safe(staged: StagedUpload, filename: str) -> PublishedUpload:
|
||||
publish_task = asyncio.create_task(run_upload_lease_io(publish_staged_upload_leased, staged, filename))
|
||||
try:
|
||||
|
|
@ -495,7 +502,7 @@ async def upload_files(
|
|||
)
|
||||
raise
|
||||
finally:
|
||||
await _run_file_io_cancellation_safe(_release_publications, publications)
|
||||
await _run_file_io_commit(_release_publications, publications)
|
||||
|
||||
|
||||
@router.get("/limits", response_model=UploadLimits)
|
||||
|
|
|
|||
|
|
@ -636,7 +636,7 @@ Content-Type: multipart/form-data
|
|||
- Excel (`.xls`, `.xlsx`)
|
||||
- Word (`.doc`, `.docx`)
|
||||
|
||||
All upload entry points publish complete payloads without replacing an existing name. Concurrent collisions are returned as `document.pdf`, `document_1.pdf`, `document_2.pdf`, and so on. A published filename remains leased through conversion, permission adjustment, sandbox synchronization, and response construction; deletion of that exact filename waits for the active lifecycle, while other filenames remain independent. Mounted providers make the exact published paths sandbox-readable; non-mounted providers receive exact private copies for Gateway, embedded-client, and IM-channel ingresses. If a non-mounted sandbox update later fails or the request is cancelled, DeerFlow removes every exact remote path attempted by that request before rolling back its host generations. Gateway cancellation also drains and aborts an in-flight staging creation. Basenames matching the internal `.upload-*.part` staging pattern, containing NUL, `<`, or `>`, or containing the reserved model-context boundary markers are rejected before staging so every accepted model-visible filename and path can be rendered losslessly. Embedded multi-file calls are request-atomic: a later failure rolls back every earlier host and remote generation in that call.
|
||||
All upload entry points publish complete payloads without replacing an existing name. Concurrent collisions are returned as `document.pdf`, `document_1.pdf`, `document_2.pdf`, and so on. A published filename remains leased through conversion, permission adjustment, sandbox synchronization, and response construction; portable case and Unicode-normalization aliases share the same coordination key so deletion cannot bypass that lifecycle on case-insensitive filesystems. Mounted providers make the exact published paths sandbox-readable; non-mounted providers receive exact private copies for Gateway, embedded-client, and IM-channel ingresses. If a non-mounted sandbox update later fails or the request is cancelled, DeerFlow removes every exact remote path attempted by that request before rolling back its host generations. Gateway cancellation also drains and aborts an in-flight staging creation. Final lease release is the commit point: cancellation newly arriving during release is delayed and the already-built successful response is returned. Basenames matching the internal `.upload-*.part` staging pattern, containing NUL, `<`, or `>`, or containing the reserved model-context boundary markers are rejected before staging so every accepted model-visible filename and path can be rendered losslessly. Embedded multi-file calls are request-atomic: a later failure rolls back every earlier host and remote generation in that call.
|
||||
|
||||
Generated Markdown is stored outside the primary namespace and is not returned by the list endpoint. Normal conversion names are `<actual-primary-filename>.md`; if that component would exceed 255 UTF-8 bytes, the response contains a deterministic UTF-8-safe prefix plus the full SHA-256 digest and `.md`. Clients must consume the returned `markdown_*` fields rather than derive the path. Mounted AIO sandboxes use a read-only conversion mount, while Local structured file APIs reject writes through a read-only path mapping; Local host bash is outside that boundary. Non-mounted providers receive a private synchronized copy rather than the authoritative host namespace. Deleting `document.pdf` also deletes only its exact generated conversion; an independent `uploads/document.md` is preserved.
|
||||
|
||||
|
|
|
|||
|
|
@ -54,7 +54,7 @@ POST /api/threads/{thread_id}/uploads
|
|||
|
||||
所有上传入口都先完整写入同目录暂存文件,再以“不替换已有条目”的原子操作发布。同名碰撞依次命名为 `document.pdf`、`document_1.pdf`、`document_2.pdf`;响应中的 `filename` 和各路径字段始终使用实际发布名。系统内部保留 `.upload-*.part` 作为暂存命名空间;使用该模式的 basename、包含 NUL、`<` 或 `>`、或包含保留模型上下文边界标记的文件名会在创建暂存文件前被拒绝,以保证所有已接受的文件名和 Agent 可见路径都能无损呈现。若请求在 staging 创建尚未返回时被取消,Gateway 会等待创建结束并精确 abort 该临时文件。
|
||||
|
||||
实际发布名会在转换、权限调整、沙箱同步和响应构造期间持有同名租约。删除该名称会等待当前生命周期完成;其他文件名仍可并发处理。跨进程协调使用 `.upload-conversions/.locks/` 下稳定保留的摘要锁文件,该目录属于内部实现,不应由 Agent 或部署脚本修改或清理。
|
||||
实际发布名会在转换、权限调整、沙箱同步和响应构造期间持有同名租约。大小写及 Unicode 规范化等可移植文件系统别名共用同一协调键,避免在 APFS/Windows 上用别名绕过 generation lease。删除该名称会等待当前生命周期完成;其他文件名仍可并发处理。跨进程协调使用 `.upload-conversions/.locks/` 下稳定保留的摘要锁文件,该目录属于内部实现,不应由 Agent 或部署脚本修改或清理。最终 lease release 是明确提交点:如果新的取消恰在 release 期间到达,系统会先完成 release 并返回已构造的成功结果,而不会把已提交文件报告成取消。
|
||||
|
||||
### 2. 查询上传限制
|
||||
```
|
||||
|
|
|
|||
|
|
@ -96,15 +96,19 @@ async def publish_upload_bytes_leased_async(
|
|||
|
||||
|
||||
async def release_published_upload_async(publication: PublishedUpload) -> None:
|
||||
"""Release a publication off-thread before propagating cancellation."""
|
||||
"""Commit by releasing a publication, delaying and swallowing new cancellation.
|
||||
|
||||
Callers use this only after response metadata has been constructed or after
|
||||
rollback has completed. Once release starts, the transaction's outcome is
|
||||
fixed, so a newly arriving cancellation must not turn committed files into
|
||||
an indeterminate cancelled result.
|
||||
"""
|
||||
release_task = asyncio.create_task(
|
||||
asyncio.to_thread(publication.release),
|
||||
name=f"release-upload:{publication.path.name}",
|
||||
)
|
||||
cancelled = await wait_for_task_completion(release_task)
|
||||
await wait_for_task_completion(release_task)
|
||||
release_task.result()
|
||||
if cancelled:
|
||||
raise asyncio.CancelledError
|
||||
|
||||
|
||||
async def rollback_published_upload_async(publication: PublishedUpload) -> None:
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import os
|
|||
import stat
|
||||
import threading
|
||||
import time
|
||||
import unicodedata
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
from typing import BinaryIO
|
||||
|
|
@ -35,6 +36,11 @@ class _ThreadLockEntry:
|
|||
_THREAD_LOCKS: dict[tuple[int, int, str], _ThreadLockEntry] = {}
|
||||
|
||||
|
||||
def _portable_name_coordination_key(filename: str) -> str:
|
||||
"""Collapse portable filesystem case and Unicode aliases for lease locking."""
|
||||
return unicodedata.normalize("NFC", filename).casefold()
|
||||
|
||||
|
||||
def _acquire_thread_lock(uploads_dir: Path, filename: str) -> tuple[tuple[int, int, str], _ThreadLockEntry]:
|
||||
directory_stat = os.lstat(uploads_dir)
|
||||
if stat.S_ISLNK(directory_stat.st_mode) or not stat.S_ISDIR(directory_stat.st_mode):
|
||||
|
|
@ -267,8 +273,9 @@ class UploadNameLease:
|
|||
raise UnsafeUploadPathError("Upload lease filename is too long")
|
||||
|
||||
uploads_dir = Path(uploads_dir)
|
||||
digest = hashlib.sha256(filename.encode("utf-8")).hexdigest()
|
||||
thread_lock_key, thread_lock_entry = _acquire_thread_lock(uploads_dir, filename)
|
||||
coordination_key = _portable_name_coordination_key(filename)
|
||||
digest = hashlib.sha256(coordination_key.encode("utf-8")).hexdigest()
|
||||
thread_lock_key, thread_lock_entry = _acquire_thread_lock(uploads_dir, coordination_key)
|
||||
lock_file: BinaryIO | None = None
|
||||
try:
|
||||
lock_path = ensure_upload_lock_dir(uploads_dir) / f"{digest}.lock"
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
"""Tests for deerflow.uploads.manager — shared upload management logic."""
|
||||
|
||||
import asyncio
|
||||
import errno
|
||||
import multiprocessing
|
||||
import os
|
||||
|
|
@ -12,6 +13,7 @@ from unittest.mock import patch
|
|||
|
||||
import pytest
|
||||
|
||||
from deerflow.uploads.async_helpers import release_published_upload_async
|
||||
from deerflow.uploads.layout import (
|
||||
artifact_url_for_virtual_path,
|
||||
conversion_path_for_upload,
|
||||
|
|
@ -125,6 +127,58 @@ class TestNormalizeFilename:
|
|||
normalize_filename("bad\0name.pdf")
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("first_name", "alias_name"),
|
||||
[
|
||||
("Report.pdf", "report.pdf"),
|
||||
("caf\u00e9.pdf", "cafe\u0301.pdf"),
|
||||
],
|
||||
)
|
||||
def test_portable_filesystem_aliases_share_one_generation_lease(tmp_path, first_name, alias_name):
|
||||
first = UploadNameLease.acquire(tmp_path, first_name)
|
||||
alias_acquired = threading.Event()
|
||||
|
||||
def acquire_alias():
|
||||
lease = UploadNameLease.acquire(tmp_path, alias_name)
|
||||
alias_acquired.set()
|
||||
return lease
|
||||
|
||||
with ThreadPoolExecutor(max_workers=1) as pool:
|
||||
future = pool.submit(acquire_alias)
|
||||
try:
|
||||
assert not alias_acquired.wait(0.1)
|
||||
finally:
|
||||
first.release()
|
||||
alias = future.result(timeout=5)
|
||||
alias.release()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_release_commit_delays_and_swallows_new_cancellation(tmp_path, monkeypatch):
|
||||
publication = publish_upload_bytes_leased(tmp_path, "report.pdf", b"payload")
|
||||
release_started = threading.Event()
|
||||
allow_release = threading.Event()
|
||||
real_release = UploadNameLease.release
|
||||
|
||||
def paused_release(lease):
|
||||
release_started.set()
|
||||
assert allow_release.wait(5)
|
||||
real_release(lease)
|
||||
|
||||
monkeypatch.setattr(UploadNameLease, "release", paused_release)
|
||||
task = asyncio.create_task(release_published_upload_async(publication))
|
||||
assert await asyncio.to_thread(release_started.wait, 5)
|
||||
try:
|
||||
task.cancel()
|
||||
await asyncio.sleep(0.05)
|
||||
assert not task.done()
|
||||
finally:
|
||||
allow_release.set()
|
||||
|
||||
await task
|
||||
assert not publication.is_active
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# claim_unique_filename
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -298,6 +298,60 @@ async def test_cancellation_during_staging_creation_drains_and_aborts(tmp_path):
|
|||
assert list(tmp_path.glob(".upload-*.part")) == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cancellation_during_final_release_returns_committed_success(tmp_path):
|
||||
thread_uploads_dir = tmp_path / "uploads"
|
||||
thread_uploads_dir.mkdir(parents=True)
|
||||
provider = MagicMock()
|
||||
provider.uses_thread_data_mounts = False
|
||||
provider.acquire_async = AsyncMock(return_value="remote-1")
|
||||
remote_files: dict[str, bytes] = {}
|
||||
|
||||
class Sandbox:
|
||||
def update_file(self, virtual_path: str, data: bytes) -> None:
|
||||
remote_files[virtual_path] = data
|
||||
|
||||
def remove_file(self, virtual_path: str) -> None:
|
||||
remote_files.pop(virtual_path, None)
|
||||
|
||||
provider.get.return_value = Sandbox()
|
||||
release_started = threading.Event()
|
||||
allow_release = threading.Event()
|
||||
real_release = uploads._release_publications
|
||||
|
||||
def paused_release(publications):
|
||||
release_started.set()
|
||||
assert allow_release.wait(5)
|
||||
real_release(publications)
|
||||
|
||||
with (
|
||||
patch.object(uploads, "ensure_uploads_dir", return_value=thread_uploads_dir),
|
||||
patch.object(uploads, "get_sandbox_provider", return_value=provider),
|
||||
patch.object(uploads, "_release_publications", side_effect=paused_release),
|
||||
):
|
||||
task = asyncio.create_task(
|
||||
call_unwrapped(
|
||||
uploads.upload_files,
|
||||
"thread-remote",
|
||||
request=MagicMock(),
|
||||
files=[UploadFile(filename="notes.txt", file=BytesIO(b"payload"))],
|
||||
config=SimpleNamespace(),
|
||||
)
|
||||
)
|
||||
assert await asyncio.to_thread(release_started.wait, 5)
|
||||
try:
|
||||
task.cancel()
|
||||
await asyncio.sleep(0.05)
|
||||
assert not task.done()
|
||||
finally:
|
||||
allow_release.set()
|
||||
result = await task
|
||||
|
||||
assert result.success is True
|
||||
assert (thread_uploads_dir / "notes.txt").read_bytes() == b"payload"
|
||||
assert remote_files == {"/mnt/user-data/uploads/notes.txt": b"payload"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cancellation_after_remote_sync_still_removes_the_completed_copy(tmp_path):
|
||||
thread_uploads_dir = tmp_path / "uploads"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue