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

Bug: message_cycle_manager N+1 query issues caused by commit 084dcd1a50 #21125

New Issue
Closed
opened 2026-02-21 20:10:53 -05:00 by yindo · 2 comments
yindo commented 2026-02-21 20:10:53 -05:00
Owner
Copy Link
Copy Source

Originally created by @qiqizjl on GitHub (Dec 17, 2025).

Originally assigned to: @fatelei on GitHub.

📋 Self Checks

  • I have read the Contributing Guide and Language policy.
  • This is only for bug report, if you would like to ask a question, please head to Discussions.
  • I have searched for existing issues, including closed ones.
  • I confirm that I am using English to submit this report, otherwise it will be closed.
  • 【中文用户 & Non English User】请使用英语提交，否则会被关闭 ：）
  • Please do not modify this template :) and fill in all the required fields.

🌎 Environment

Dify version:
1.11.1

Cloud or Self Hosted:
Self Hosted (Source)

🐛 Bug Description

Commit 084dcd1a50 (#22988) introduced an inefficient query in message_to_stream_response() that causes:

  1. N+1 database queries: A query is executed for every LLM chunk during streaming
  2. Dead logic: The query condition is wrong and never returns MESSAGE_FILE type

Problematic Code

# api/core/app/task_pipeline/message_cycle_manager.py:226-227
def message_to_stream_response(...) -> MessageStreamResponse:
    with Session(db.engine) as session:
        message_file = session.scalar(select(MessageFile).where(MessageFile.id == message_id))
    event_type = StreamEvent.MESSAGE_FILE if message_file else StreamEvent.MESSAGE
    return MessageStreamResponse(..., event=event_type)

❌ Issues Identified

1. N+1 Query Problem

  • During LLM streaming, message_to_stream_response() is called per chunk
  • Each call executes: SELECT FROM message_files WHERE id = ?
  • For 100 chunks = 100 wasted queries

2. Dead Logic (Always Fails)

  • message_id parameter = Message.id (UUID)
  • Query condition = MessageFile.id == Message.id
  • But MessageFile.id is auto-generated, never equals Message.id
  • Result: Always returns StreamEvent.MESSAGE anyway

3. "Fixing" It Creates Worse Bug

If we change to MessageFile.message_id == message_id:

  • Message with files → all LLM chunks return event: "message_file"
  • Text chunks incorrectly labeled as files!

🎯 Expected Behavior

  • No unnecessary database queries during streaming
  • No dead/unreachable code

🔄 Proposed Solution

Revert commit 084dcd1a50 to restore original behavior:

def message_to_stream_response(
    self, answer: str, message_id: str, from_variable_selector: list[str] | None = None
) -> MessageStreamResponse:
    return MessageStreamResponse(
        task_id=self._application_generate_entity.task_id,
        id=message_id,
        answer=answer,
        from_variable_selector=from_variable_selector,
    )

Remove: The query, event type check, and event parameter.

📊 Impact

Before commit 084dcd1a50:

  • ✅ 0 extra queries
  • ✅ Clean, working code

After commit (current):

  • ❌ +N queries during streaming
  • ❌ Logic that never works as intended

📎 Commit Info

  • Bad commit: 084dcd1a50 - "using message_file event instead of message event" (#22988)
  • First appeared: Version 1.7.2
  • File: api/core/app/task_pipeline/message_cycle_manager.py:226-235
Originally created by @qiqizjl on GitHub (Dec 17, 2025). Originally assigned to: @fatelei on GitHub. ## 📋 Self Checks - [x] I have read the [Contributing Guide](https://github.com/langgenius/dify/blob/main/CONTRIBUTING.md) and [Language policy](https://github.com/langgenius/dify/issues/1542). - [x] This is only for bug report, if you would like to ask a question, please head to [Discussions](https://github.com/langgenius/dify/discussions/categories/general). - [x] I have searched for existing issues, including closed ones. - [x] I confirm that I am using English to submit this report, otherwise it will be closed. - [x] 【中文用户 & Non English User】请使用英语提交，否则会被关闭 ：） - [x] Please do not modify this template :) and fill in all the required fields. --- ## 🌎 Environment **Dify version:** `1.11.1` **Cloud or Self Hosted:** Self Hosted (Source) --- ## 🐛 Bug Description Commit `084dcd1a50` (#22988) introduced an inefficient query in `message_to_stream_response()` that causes: 1. **N+1 database queries**: A query is executed for every LLM chunk during streaming 2. **Dead logic**: The query condition is wrong and never returns `MESSAGE_FILE` type ### Problematic Code ```python # api/core/app/task_pipeline/message_cycle_manager.py:226-227 def message_to_stream_response(...) -> MessageStreamResponse: with Session(db.engine) as session: message_file = session.scalar(select(MessageFile).where(MessageFile.id == message_id)) event_type = StreamEvent.MESSAGE_FILE if message_file else StreamEvent.MESSAGE return MessageStreamResponse(..., event=event_type) ``` --- ## ❌ Issues Identified ### 1. N+1 Query Problem - During LLM streaming, `message_to_stream_response()` is called **per chunk** - Each call executes: `SELECT FROM message_files WHERE id = ?` - For 100 chunks = 100 wasted queries ### 2. Dead Logic (Always Fails) - `message_id` parameter = `Message.id` (UUID) - Query condition = `MessageFile.id == Message.id` - But `MessageFile.id` is auto-generated, **never equals `Message.id`** - **Result**: Always returns `StreamEvent.MESSAGE` anyway ### 3. "Fixing" It Creates Worse Bug If we change to `MessageFile.message_id == message_id`: - Message with files → all LLM chunks return `event: "message_file"` - **Text chunks incorrectly labeled as files!** --- ## 🎯 Expected Behavior - No unnecessary database queries during streaming - No dead/unreachable code --- ## 🔄 Proposed Solution **Revert commit `084dcd1a50`** to restore original behavior: ```python def message_to_stream_response( self, answer: str, message_id: str, from_variable_selector: list[str] | None = None ) -> MessageStreamResponse: return MessageStreamResponse( task_id=self._application_generate_entity.task_id, id=message_id, answer=answer, from_variable_selector=from_variable_selector, ) ``` **Remove:** The query, event type check, and event parameter. --- ## 📊 Impact **Before commit `084dcd1a50`:** - ✅ 0 extra queries - ✅ Clean, working code **After commit (current):** - ❌ +N queries during streaming - ❌ Logic that never works as intended --- ## 📎 Commit Info - **Bad commit**: `084dcd1a50` - "using message_file event instead of message event" (#22988) - **First appeared**: Version `1.7.2` - **File**: `api/core/app/task_pipeline/message_cycle_manager.py:226-235`
yindo added the 🐞 bug label 2026-02-21 20:10:53 -05:00
yindo closed this issue 2026-02-21 20:10:53 -05:00
yindo commented 2026-02-21 20:10:58 -05:00
Author
Owner
Copy Link
Copy Source

@dosubot[bot] commented on GitHub (Dec 17, 2025):

You are correct about the N+1 query and dead logic issue introduced by commit 084dcd1a50. The current implementation of message_to_stream_response in api/core/app/task_pipeline/message_cycle_manager.py still performs a database query for every streamed chunk, checking MessageFile.id == message_id, which never matches due to the way IDs are generated. This results in unnecessary queries and always returns StreamEvent.MESSAGE, making the event logic dead code. No relevant pull requests or fixes have been made for this issue, and the code remains unchanged from the problematic commit. Reverting the commit, as you suggested, would restore efficient and correct behavior by removing the unnecessary query and event logic. You can review the current code and confirm these findings here.

To reply, just mention @dosu.

How did I do? Good | Irrelevant | Incorrect | Verbose | Hallucination | Report 🐛 | Other

Chat with Dosu Open in Cursor Join Discord Share on X

@dosubot[bot] commented on GitHub (Dec 17, 2025): <!-- Answer --> You are correct about the N+1 query and dead logic issue introduced by commit 084dcd1a50. The current implementation of message_to_stream_response in api/core/app/task_pipeline/message_cycle_manager.py still performs a database query for every streamed chunk, checking MessageFile.id == message_id, which never matches due to the way IDs are generated. This results in unnecessary queries and always returns StreamEvent.MESSAGE, making the event logic dead code. No relevant pull requests or fixes have been made for this issue, and the code remains unchanged from the problematic commit. Reverting the commit, as you suggested, would restore efficient and correct behavior by removing the unnecessary query and event logic. You can review the current code and confirm these findings [here](https://github.com/langgenius/dify/blob/df2f1eb028f9e7c7083e96b2c90ee28cb7f53a6d/api/core/app/task_pipeline/message_cycle_manager.py#L216-L235). <!-- Dosu Comment Footer --> *To reply, just mention [@dosu](https://go.dosu.dev/dosubot).* --- <sup>How did I do? [Good](https://app.dosu.dev/response-feedback/818d7bf9-58b7-47c2-818c-bd5a4608dfe7?feedback_type=great_response) | [Irrelevant](https://app.dosu.dev/response-feedback/818d7bf9-58b7-47c2-818c-bd5a4608dfe7?feedback_type=irrelevant_answer) | [Incorrect](https://app.dosu.dev/response-feedback/818d7bf9-58b7-47c2-818c-bd5a4608dfe7?feedback_type=incorrect_sources) | [Verbose](https://app.dosu.dev/response-feedback/818d7bf9-58b7-47c2-818c-bd5a4608dfe7?feedback_type=too_verbose) | [Hallucination](https://app.dosu.dev/response-feedback/818d7bf9-58b7-47c2-818c-bd5a4608dfe7?feedback_type=hallucination) | [Report 🐛](https://app.dosu.dev/response-feedback/818d7bf9-58b7-47c2-818c-bd5a4608dfe7?feedback_type=bug_report) | [Other](https://app.dosu.dev/response-feedback/818d7bf9-58b7-47c2-818c-bd5a4608dfe7?feedback_type=other)</sup> [![Chat with Dosu](https://dosu.dev/dosu-chat-badge.svg)](https://app.dosu.dev/6088a4f4-073a-43f3-b3d4-4f06fb0c05a0/ask?utm_source=github)&nbsp;[![Open in Cursor](https://dosu.dev/dosu-in-cursor.svg)](https://cursor.com/link/prompt?text=You%20are%20correct%20about%20the%20N%2B1%20query%20and%20dead%20logic%20issue%20introduced%20by%20commit%20084dcd1a50.%20The%20current%20implementation%20of%20message_to_stream_response%20in%20api/core/app/task_pipeline/message_cycle_manager.py%20still%20performs%20a%20database%20query%20for%20every%20streamed%20chunk%2C%20checking%20MessageFile.id%20%3D%3D%20message_id%2C%20which%20never%20matches%20due%20to%20the%20way%20IDs%20are%20generated.%20This%20results%20in%20unnecessary%20queries%20and%20always%20returns%20StreamEvent.MESSAGE%2C%20making%20the%20event%20logic%20dead%20code.%20No%20relevant%20pull%20requests%20or%20fixes%20have%20been%20made%20for%20this%20issue%2C%20and%20the%20code%20remains%20unchanged%20from%20the%20problematic%20commit.%20Reverting%20the%20commit%2C%20as%20you%20suggested%2C%20would%20restore%20efficient%20and%20correct%20behavior%20by%20removing%20the%20unnecessary%20query%20and%20event%20logic.%20You%20can%20review%20the%20current%20code%20and%20confirm%20these%20findings%20%5Bhere%5D%28https%3A//github.com/langgenius/dify/blob/df2f1eb028f9e7c7083e96b2c90ee28cb7f53a6d/api/core/app/task_pipeline/message_cycle_manager.py%23L216-L235%29.)&nbsp;[![Join Discord](https://img.shields.io/badge/join-5865F2?logo=discord&logoColor=white&label=)](https://go.dosu.dev/discord-bot)&nbsp;[![Share on X](https://img.shields.io/badge/X-share-black)](https://twitter.com/intent/tweet?text=%40dosu_ai%20helped%20me%20solve%20this%20issue!&url=https%3A//github.com/langgenius/dify/issues/29816)
yindo commented 2026-02-21 20:11:02 -05:00
Author
Owner
Copy Link
Copy Source

@qiqizjl commented on GitHub (Dec 17, 2025):

If we try to fix the code by changing the query to look up message_id instead, it creates an even worse bug:

The issue is that message_to_stream_response() is called per chunk during streaming. If a message has files, the query would match for the entire message and every LLM chunk would incorrectly return event: message_file. Text chunks would be misinterpreted as file events.

So the choices are:

  • Keep current code: N+1 queries + dead logic that never sets MESSAGE_FILE
  • Fix the query: All chunks become MESSAGE_FILE (worse bug!)
  • Revert commit 084dcd1a50: Clean, efficient code

Reverting is the right solution.

@jubinsoni - what is your take on this? The original PR #22988 may have had good intent but creates problems without an easy fix.

@qiqizjl commented on GitHub (Dec 17, 2025): If we try to fix the code by changing the query to look up message_id instead, it creates an even worse bug: The issue is that message_to_stream_response() is called per chunk during streaming. If a message has files, the query would match for the entire message and every LLM chunk would incorrectly return event: message_file. Text chunks would be misinterpreted as file events. So the choices are: - Keep current code: N+1 queries + dead logic that never sets MESSAGE_FILE - Fix the query: All chunks become MESSAGE_FILE (worse bug!) - Revert commit 084dcd1a50: Clean, efficient code Reverting is the right solution. @jubinsoni - what is your take on this? The original PR #22988 may have had good intent but creates problems without an easy fix.
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 🐞 bug
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#21125
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?