Skip to content

fix(knowledge): prevent abnormal deletion of local SQL files during knowledge base removal #8151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

GeorgeDong32
Copy link
Collaborator

What this PR does

Before this PR:

删除知识库时,对应的本地数据库可能会因为来不及释放文件占用而导致删除失败

After this PR:

删除前释放RagApplication占用,添加删除异常重试机制实现文件的正确删除

Fixes #7592

Why we need it and why it was done in this way

The following tradeoffs were made:

The following alternatives were considered:

Links to places where the discussion took place:

Breaking changes

If this PR introduces breaking changes, please describe the changes and the impact on users.

Special notes for your reviewer

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

Release note


@GeorgeDong32 GeorgeDong32 requested a review from Copilot July 14, 2025 12:44
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Provides robust cleanup when removing a knowledge base by closing database connections and retrying file deletions to avoid “file busy” errors.

  • Track and reuse RAGApplication and LibSqlDb instances per knowledge base
  • Close database client before removal and pause to ensure file handles are released
  • Implement a retry loop with backoff for deleting the local storage directory
Comments suppressed due to low confidence (2)

src/main/services/KnowledgeService.ts:104

  • [nitpick] Comments are in Chinese; consider translating to English to maintain consistency with the rest of the codebase.
  // 存储数据库实例,用于后续关闭

src/main/services/KnowledgeService.ts:205

  • The new retry logic isn’t covered by existing tests. Add unit or integration tests to simulate busy-file scenarios and verify correct retry behavior.
      for (let i = 0; i < maxRetries; i++) {

@GeorgeDong32 GeorgeDong32 marked this pull request as ready for review July 14, 2025 12:45
@GeorgeDong32 GeorgeDong32 requested a review from beyondkmp July 14, 2025 12:49
@GeorgeDong32 GeorgeDong32 added this to the v1.5.0 milestone Jul 14, 2025
@GeorgeDong32 GeorgeDong32 added the needs-review the feature/bug/pr needs a product/development reviewing label Jul 14, 2025
@alephpiece
Copy link
Collaborator

网络搜索那边也用了 delete,我发现有些情况下会一直重试:

  • 开启网络搜索 RAG,先发送一条用户消息 A
  • 然后修改网络搜索 RAG 的模型
  • 找到用户消息 A,重新发送
image

是我的用法有问题吗?我给网络搜索 RAG 建立了临时知识库,在配置变化的时候会先 delete,然后重新创建

@alephpiece
Copy link
Collaborator

// 如果已存在且配置未变,直接复用
if (state.searchBase && this.isConfigMatched(state.searchBase, config)) {
return state.searchBase
}
// 清理旧的知识库
if (state.searchBase) {
await window.api.knowledgeBase.delete(state.searchBase.id)
}

@GeorgeDong32
Copy link
Collaborator Author

网络搜索那边也用了 delete,我发现有些情况下会一直重试:

这个我还真没注意,我明天研究一下

@0xfullex 0xfullex modified the milestones: v1.5.0, v1.5.1 Jul 16, 2025
@GeorgeDong32 GeorgeDong32 added Pending On hold due to priority/low, unresolved issues, suboptimal solution or needs more research and removed needs-review the feature/bug/pr needs a product/development reviewing labels Jul 16, 2025
@GeorgeDong32 GeorgeDong32 marked this pull request as draft July 16, 2025 11:40
@YinsenHo
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending On hold due to priority/low, unresolved issues, suboptimal solution or needs more research
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[功能]: 知识库文件清理不干净
4 participants