Skip to content

Personas memories fix#1917

Merged
beastoin merged 7 commits into
mainfrom
personas-memories-fix
Mar 17, 2025
Merged

Personas memories fix#1917
beastoin merged 7 commits into
mainfrom
personas-memories-fix

Conversation

@mdmohsin7
Copy link
Copy Markdown
Member

@mdmohsin7 mdmohsin7 commented Feb 27, 2025

  • 30s timeout for X api calls
  • add try catch for sync_update_persona_prompt
  • update the persona prompts async in a separate thread without disturbing the process_memory

@mdmohsin7 mdmohsin7 marked this pull request as ready for review February 28, 2025 13:18
@beastoin
Copy link
Copy Markdown
Collaborator

beastoin commented Mar 1, 2025

1/ from my experience, this code block is the cause of websocket connection drop (/listen API). do you experience the same ? https://github.com/BasedHardware/omi/pull/1917/files#diff-e518096e8d0948a411d3035bacafc371d1a8600659b7b7f6ea667f73b2231dc4L195-R203

@mdmohsin7

@mdmohsin7
Copy link
Copy Markdown
Member Author

1/ I did not had issues when I was testing locally (with 30s timeout). I'll test it a bit more and maybe will move it to make it independent so no other function should be dependent on it

@beastoin

@mdmohsin7 mdmohsin7 marked this pull request as draft March 4, 2025 12:10
@mdmohsin7 mdmohsin7 marked this pull request as ready for review March 7, 2025 11:16
@beastoin
Copy link
Copy Markdown
Collaborator

beastoin commented Mar 9, 2025

1/ please do a small research. how does the thread impact to the python application ?

@mdmohsin7

@beastoin
Copy link
Copy Markdown
Collaborator

@nquang29 babe, could you help Mohsin a bit with researching, why this code of block drop the WS connection sometime ?

@mdmohsin7
Copy link
Copy Markdown
Member Author

1/ sure will do, just occupied with somewhat more support tickets haha. And Quang has already been a lot helpful to me, and some more insights from him will only help me more

@nquang29
Copy link
Copy Markdown
Collaborator

i think this issue come from join(), calling .join() in the main thread blocks execution until all threads finish.
If the WS is running on the same thread, it might time out or get interrupted while waiting for threads to complete.
I found docs about join() here: https://stackoverflow.com/questions/15085348/what-is-the-use-of-join-in-threading

@beastoin
Copy link
Copy Markdown
Collaborator

nice. can you verify it ? i mean reproducing the issue and solving it. @nquang29 @mdmohsin7

@mdmohsin7
Copy link
Copy Markdown
Member Author

The old code that was there initially indeed blocks the main thread. But the code introduced in this PR does not. Because we are starting the personas updation in a separate thread without joining it to main thread

threading.Thread(target=_update_personas_async, args=(uid,)).start()

This won't block the main thread. This separate thread will be blocked until all the sub threads in it are complete

Below print statements for reference:
Screenshot 2025-03-17 at 12 57 49 AM

Print statements in code:
Screenshot 2025-03-17 at 12 59 08 AM

@beastoin
Copy link
Copy Markdown
Collaborator

ok cool. lgtm @mdmohsin7 🚀

@beastoin beastoin merged commit 5ba6937 into main Mar 17, 2025
@beastoin beastoin deleted the personas-memories-fix branch March 17, 2025 05:00
Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
- [x] 30s timeout for X api calls
- [x] add try catch for sync_update_persona_prompt
- [x] update the persona prompts async in a separate thread without
disturbing the process_memory
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants