Skip to content

fix(multipart): harden upload attempt lifecycle and error reporting - #3331

Open
thetapilla wants to merge 4 commits into
OpenListTeam:mainfrom
thetapilla:fix/multipart-hardening
Open

fix(multipart): harden upload attempt lifecycle and error reporting#3331
thetapilla wants to merge 4 commits into
OpenListTeam:mainfrom
thetapilla:fix/multipart-hardening

Conversation

@thetapilla

@thetapilla thetapilla commented Oct 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary / 摘要

加固 #2723 引入的分块上传会话层，修复四个问题：驱动 panic 打挂整个进程、存储的真实错误被 multipart upload window closed 掩盖、被取代的上传尝试与新尝试同时写同一个文件，以及被遗弃的上传尝试一直占着目标文件直到会话过期。每个修复一个 commit。

用户可感知的变化：

实现要点：

  • 流水线兜底 recover：流水线 goroutine 不在 gin Recovery 和任务 worker 的 recover 范围内，是唯一没有兜底的上传路径；现在 panic 转为该次尝试 failed_retriable，连同堆栈记录日志
  • 取代时先交接：新的 init 取代同路径、同大小的活动会话时，等旧尝试退出（最多 10 秒）再启动新尝试。否则在原地写入的驱动上，旧尝试的取消清理可能删掉新尝试正在写的文件，或与之交错写入
  • 报出真实错误：尝试的裁决出来后，在途分片返回 upload failed (<state>): <cause>，措辞与 /complete 一致。
  • 空闲释放：记录流水线读端"在等下一个分片、且没有任何分片字节到达"的时长，GC 时释放超过 10 分钟的尝试。仍在发送的慢客户端、正忙于写存储的驱动都不算空闲；尝试真正退出后会话才转为 failed_retriable，补传不会和它重叠

兼容性说明：不新增设置项和 API 字段。空闲释放新增 receiving → failed_retriable 的转换，按文档实现了补传协议的客户端无需改动。对于取消时仍按路径删除目标的驱动（#3305 合入前的 Local），空闲释放把"被遗弃的上传删掉后来者同名文件"的时间窗从 30 分钟缩短到 10 分钟；删除本身由 #3305（Local）和 #3330（SMB、Chunk）修复。

  • This PR has breaking changes.
    / 此 PR 包含破坏性变更。
  • This PR changes public API, config, storage format, or migration behavior.
    / 此 PR 修改了公开 API、配置、存储格式或迁移行为。
  • This PR requires corresponding changes in related repositories.
    / 此 PR 需要关联仓库同步修改。

Related repository PRs / 关联仓库 PR:

Related Issues / 关联 Issue

Relates to #3178
Relates to #3201
Relates to #3309

说明：#3309 中流式、表单上传同样失败，根因在天翼云盘客户端驱动，本 PR 只解决错误被掩盖的问题；#3201 的 115 驱动由 #3312 修复，本 PR 保证此类驱动 panic 不再打挂进程。

Testing / 测试

测试平台：macOS arm64，Go 1.27.3。

  • go test ./...
  • Manual test / 手动测试:

自动化测试：

  • go test -race -count=1 ./internal/multipart/ ./server/handles/
  • 仓库外的黑盒用例，分别在 main 和本分支上运行：main 上驱动 panic 使测试进程崩溃、取代时新尝试在旧尝试退出前就开写、在途分片只返回 window closed；本分支三项均通过

浏览器实测：

  • 本地只读目录、MinIO 中途宕机、无法覆盖文件的 Samba 三种真实存储错误，上传列表都显示存储返回的原因
  • MinIO 恢复后点"重试"，失败的会话从 chunk 0 补传并完成，内容一致
  • 上传中途换成同名、同大小、内容不同的文件：旧尝试退出后新尝试才开写，新文件完好

Checklist / 检查清单

  • I have read CONTRIBUTING.
    / 我已阅读 CONTRIBUTING。
  • I confirm this contribution follows the repository license, contribution policy, and code of conduct.
    / 我确认此贡献符合仓库许可证、贡献规范和行为准则。
  • I have formatted the changed code with gofmt, go fmt, or prettier where applicable.
    / 我已按适用情况使用 gofmt、go fmt 或 prettier 格式化变更代码。
  • I have requested review from relevant maintainers or code owners where applicable.
    / 我已在适用情况下请求相关维护者或代码所有者审查。

AI Disclosure / AI 使用声明

  • This PR includes AI-assisted content.
    / 此 PR 包含 AI 辅助内容。

Tools used / 使用工具:

  • ChatGPT
  • Codex
  • GitHub Copilot
  • Claude
  • Gemini
  • Other (please specify) / 其他（请注明）:

Usage scope / 使用范围:

  • Code generation / 代码生成

  • Refactoring / 重构

  • Documentation / 文档

  • Tests / 测试

  • Translation / 翻译

  • Review assistance / 审查辅助

  • I have reviewed and validated all AI-assisted content included in this PR.
    / 我已审核并验证此 PR 中的所有 AI 辅助内容。

  • I have ensured that all AI-assisted commits include Co-Authored-By attribution.
    / 我已确保所有 AI 辅助提交都包含 Co-Authored-By 归属信息。

  • I can reproduce all AI-assisted content included in this PR without any AI tools.
    / 我可以在没有任何 AI 工具的情况下重现此 PR 中包含的所有 AI 辅助内容。

thetapilla and others added 4 commits October 30, 2026 14:44
@thetapilla @claude
fix(multipart): recover from driver panics in the upload pipeline
1954894 
- Run the storage upload behind a recover so a panicking driver fails the
  attempt (failed_retriable) instead of crashing the whole process. The
  pipeline goroutine sits outside gin's Recovery and the task workers'
  recover, so it was the one upload path without a safety net.
- Log the recovered panic with its stack trace.
- Add TestDriverPanicFailsTheAttempt.

Co-Authored-By: Claude <noreply@anthropic.com>
@thetapilla @claude
fix(multipart): wait for a superseded attempt before starting its rep…
affb4b3 
…lacement

- When a new init supersedes a live session of the same path and size, wait
  (bounded by 10s) for the old pipeline attempt to return before starting
  the new one. Drivers that write the destination in place could otherwise
  have the old attempt's cancellation cleanup remove or interleave with the
  file the new attempt is writing.
- Add TestSupersedeWaitsForTheOldAttempt.

Co-Authored-By: Claude <noreply@anthropic.com>
@thetapilla @claude
fix(multipart): report the storage error instead of a closed window
bf39ff8 
- When the storage fails while a chunk is in flight, the chunk used to
  answer "multipart upload window closed" and the real cause only survived
  in data.error. Once the attempt's verdict is in, report it as
  "upload failed (<state>): <cause>", worded like /complete.
- Return null data for error responses of unknown sessions (404/403)
  instead of an all-zero snapshot, matching the published API document.
- Add TestChunkReportsStorageError and TestSnapshotDataOmitsUnknownSession.

Co-Authored-By: Claude <noreply@anthropic.com>
@thetapilla @claude
fix(multipart): release upload attempts left idle by the client
e6ec5fd 
- Track how long the pipeline's reader has been starved: blocked in Read
  waiting for a chunk while no chunk byte arrives. A slow client that keeps
  sending is not starved, and neither is a driver busy with the storage
  (backpressure), because the reader is not waiting then.
- During GC, release an attempt starved for AttemptIdleTimeout (10 minutes).
  It stops holding the destination for the whole 30-minute session TTL,
  while the session keeps its metadata and resumes through the existing
  refill protocol (resend from chunk 0).
- The session turns failed_retriable only once the released attempt has
  returned, so a refill never overlaps it.
- Add TestWindowStarvedFor, TestIdleAttemptIsReleased,
  TestSlowClientIsNotReleased and TestBackpressureIsNotIdle.

Co-Authored-By: Claude <noreply@anthropic.com>
@thetapilla thetapilla mentioned this pull request Oct 30, 2026
Open
14 of 25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@thetapilla