Skip to content
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

Server-sent events fired 1000s times per conversation #936

Closed
alex-mcgovern opened this issue Feb 5, 2025 · 3 comments · Fixed by #1091
Closed

Server-sent events fired 1000s times per conversation #936

alex-mcgovern opened this issue Feb 5, 2025 · 3 comments · Fixed by #1091
Assignees
Labels

Comments

@alex-mcgovern
Copy link
Contributor

Describe the issue

Found it hard to export the logs from the server sent events, but here is a video recording of the network tab. This is for a single chat message that produced a handful of alerts.

Screen.Recording.2025-02-05.at.2.21.19.PM.mov

Steps to Reproduce

  • Open a connection with /api/v1/alerts_notification
  • chat with an LLM
  • notice how many events are sent over the stream

Operating System

MacOS (Arm)

IDE and Version

Visual Studio Code version: 1.96.4

Extension and Version

Identifier github.copilot-chat Version 0.23.2

Provider

GitHub Copilot

Model

GPT 4o mini

Codegate version

CodeGate version: v0.1.16-5abdad6-dev

Logs

No response

Additional Context

No response

@lukehinds
Copy link
Contributor

@aponcedeleonch might have some insight

@aponcedeleonch
Copy link
Contributor

aponcedeleonch commented Feb 6, 2025

Hypothesis: What I think is going on is that we switched from doing simple inserts to doing upserts for Copilot because of how we receive messages. Before, we weren't storing everything (hence the switch), but now a single chat request can involve multiple upserts. Each time we do an upsert, we're sending an SSE. The alerts and everything else should stay the same, and they’re not actually being updated in the database—but we're still sending a bunch of SSEs.

This needs to be confirmed but if that's the case this is a Copilot specific problem. I think the table that needs upserting is outputs table. Right now, when upserting we upsert all 3 tables (prompts, alerts, outputs). If the hypothesis is confirmed we could change the logic to only upsert the outputs table.

@yrobla
Copy link
Contributor

yrobla commented Feb 18, 2025

We changed this logic because with the caching, we were not recording the complete output of the message but just the initial fragment. Then the conversations in the UI didn't make sense because only the first chunk of the conversation was seen. I actually saw the problem in prompts table as well, that's why i modified.
But is true that now with multiple requests, there are multiple calls to DB. That has 2 problems :

  • heavy load on the db
  • multiple SSE calls

For the SSE calls we could implement some kind of debounce, but for the DB calls we should find another solution

@yrobla yrobla self-assigned this Feb 18, 2025
yrobla added a commit that referenced this issue Feb 18, 2025
For copilot chat, we are actually receiving multiple streams, and
we were recording entries and alerts for each one, repeating those.
So detect if we are in the last chunk and propagate it to the
pipeline, so we can record it only in this case, avoiding dupes.
In the case of other providers, we only receive one request, so
always force saving it

Closes: #936
@yrobla yrobla closed this as completed in b23effd Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants