langgenius/dify
1
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-07-22 20:45:26 -04:00
Code Issues 775 Packages Projects Releases 10 Wiki Activity

[PR #30937] fix: add Redis lock to prevent concurrent document update race condition #33022

New Issue
Open
opened 2026-02-21 20:52:31 -05:00 by yindo · 0 comments
yindo commented 2026-02-21 20:52:31 -05:00
Owner
Copy Link
Copy Source

Original Pull Request: https://github.com/langgenius/dify/pull/30937

State: open
Merged: No

Description

This PR fixes a race condition in the document update flow that could cause data inconsistency when multiple requests try to update the same document concurrently.

Fixes #30936

Problem

Multiple concurrent requests successfully update the same document, causing:

  1. Multiple threads check document.display_status != "available" before acquiring any lock
  2. All threads pass the check if the document is available
  3. Multiple threads then proceed to update the same document simultaneously
  4. This causes:
    • Redundant document processing
    • Multiple async indexing tasks triggered
    • Potential data inconsistency

Solution

Added Redis distributed lock with double-check locking pattern:

lock_name = f"update_document_lock_document_id_{document.id}"
try:
    with redis_client.lock(lock_name, timeout=600):
        # Double-check 
        document = DocumentService.get_document(dataset.id, document_data.original_document_id)
        if document is None:
            raise NotFound("Document not found")
        if document.display_status != "available":
            raise ValueError("Document is not available")

        # Now safe to proceed with update
        # ... update operations ...

        return document
except LockNotOwnedError:
    raise ValueError(f"Unable to acquire lock for document {document.id}. Please try again later.")

Key improvements:

  1. Acquire lock before modifying document state
  2. Re-fetch and re-validate document state inside the lock
  3. Proper error handling for lock acquisition failures
  4. Follows existing project patterns (similar to add_document_lock_dataset_id_ at line 1654)

Changes

Modified Files

  • api/services/dataset_service.py (lines 2174-2315)
    • Added Redis distributed lock around document update operations
    • Implemented double-check locking to prevent race conditions
    • Added proper exception handling for LockNotOwnedError

Verification:

  • Concurrent requests now processed sequentially
  • Only one request modifies document state at a time
  • No duplicate indexing tasks triggered
  • Lock timeout prevents deadlocks (600s)
  • Backward compatible with existing functionality

Design Decisions

  1. Lock granularity: Per-document locking (using document.id) rather than dataset-level

    • Allows concurrent updates to different documents in the same dataset
    • Minimizes lock contention

  2. Lock timeout: 600 seconds

    • Consistent with other locks in the codebase (add_document_lock, add_segment_lock)
    • Sufficient for document update operations

  3. Double-check pattern: Re-fetch document state inside lock

    • Ensures we always work with the latest document state

Breaking Changes

None. This change is backward compatible and only adds internal lock protection.

Migration Guide

No migration needed. The fix is transparent to API consumers.

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran make lint and make type-check (backend) and cd web && npx lint-staged (frontend) to appease the lint gods
**Original Pull Request:** https://github.com/langgenius/dify/pull/30937 **State:** open **Merged:** No --- ## Description This PR fixes a race condition in the document update flow that could cause data inconsistency when multiple requests try to update the same document concurrently. Fixes #30936 ## Problem Multiple concurrent requests successfully update the same document, causing: 1. Multiple threads check `document.display_status != "available"` **before** acquiring any lock 2. All threads pass the check if the document is available 3. Multiple threads then proceed to update the same document simultaneously 4. This causes: - Redundant document processing - Multiple async indexing tasks triggered - Potential data inconsistency ## Solution Added Redis distributed lock with **double-check locking pattern**: ```python lock_name = f"update_document_lock_document_id_{document.id}" try: with redis_client.lock(lock_name, timeout=600): # Double-check document = DocumentService.get_document(dataset.id, document_data.original_document_id) if document is None: raise NotFound("Document not found") if document.display_status != "available": raise ValueError("Document is not available") # Now safe to proceed with update # ... update operations ... return document except LockNotOwnedError: raise ValueError(f"Unable to acquire lock for document {document.id}. Please try again later.") ``` **Key improvements:** 1. Acquire lock **before** modifying document state 2. Re-fetch and re-validate document state **inside** the lock 3. Proper error handling for lock acquisition failures 4. Follows existing project patterns (similar to `add_document_lock_dataset_id_` at line 1654) ## Changes ### Modified Files - `api/services/dataset_service.py` (lines 2174-2315) - Added Redis distributed lock around document update operations - Implemented double-check locking to prevent race conditions - Added proper exception handling for `LockNotOwnedError` **Verification:** - Concurrent requests now processed sequentially - Only one request modifies document state at a time - No duplicate indexing tasks triggered - Lock timeout prevents deadlocks (600s) - Backward compatible with existing functionality ### Design Decisions 1. **Lock granularity:** Per-document locking (using `document.id`) rather than dataset-level - Allows concurrent updates to different documents in the same dataset - Minimizes lock contention 2. **Lock timeout:** 600 seconds - Consistent with other locks in the codebase (`add_document_lock`, `add_segment_lock`) - Sufficient for document update operations 3. **Double-check pattern:** Re-fetch document state inside lock - Ensures we always work with the latest document state ## Breaking Changes None. This change is backward compatible and only adds internal lock protection. ## Migration Guide No migration needed. The fix is transparent to API consumers. ## Checklist - [ ] This change requires a documentation update, included: [Dify Document](https://github.com/langgenius/dify-docs) - [x] I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!) - [ ] I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change. - [x] I've updated the documentation accordingly. - [x] I ran `make lint` and `make type-check` (backend) and `cd web && npx lint-staged` (frontend) to appease the lint gods
yindo added the pull-request label 2026-02-21 20:52:31 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Knowledge Pipeline
Proposal
WIP
cloud
dependencies
feat:webapp
good first issue
outdated
performance
pull-request
Mirrored from GitHub Pull Request
 python:uv
refactor
size:L
stale
status: accepting prs
status: blocked
status: in discussion
stt & tts
vulnerablity
web
wont-fix
🌊 feat:workflow
🌚 invalid
🐞 bug
👻 feat:rag
💪 enhancement
📚 documentation
🔨 feat:tools
🙋‍♂️ question
🙏 help wanted
🤔 cant-reproduce
🤖 feat:agent
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: langgenius/dify#33022
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?