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

fix!: Update statuses for stuck Omnichannel rooms via migration #32131

Closed
wants to merge 4 commits into from

Conversation

KevLehman
Copy link
Contributor

@KevLehman KevLehman commented Apr 4, 2024

https://rocketchat.atlassian.net/browse/CORE-238

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Migration will do 3 steps:

  • Remove the inquiries of closed rooms
  • Update the status of inquiries that are queued, but the related room is already served
  • Remove inquiries that point to a room that no longer exists

Since at the time a room is closed the inquiry is removed, we'll assume rooms that are already closed have all the data they require set, so we're only removing the inquiry to fix the status and unblock processing.

Queue will do the same steps for each of the inquiries it touches. Generally speaking, unless someone updates directly to this version or has been using manual selection all the time, this migration will do nothing.

[Merge when 7.0 is happening]

@KevLehman KevLehman requested a review from a team as a code owner April 4, 2024 15:49
Copy link
Contributor

dionisio-bot bot commented Apr 4, 2024

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is targeting the wrong base branch. It should target 7.0.0, but it targets 6.9.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

Copy link

changeset-bot bot commented Apr 4, 2024

🦋 Changeset detected

Latest commit: 2197520

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 32 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/api-client Patch
@rocket.chat/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/models Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/instance-status Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@KevLehman KevLehman marked this pull request as draft April 4, 2024 15:50
Copy link

codecov bot commented Apr 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.85%. Comparing base (3cbbecd) to head (2197520).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #32131   +/-   ##
========================================
  Coverage    55.84%   55.85%           
========================================
  Files         2432     2432           
  Lines        53480    53480           
  Branches     10993    10993           
========================================
+ Hits         29868    29871    +3     
+ Misses       20973    20971    -2     
+ Partials      2639     2638    -1     
Flag Coverage Δ
e2e 55.19% <ø> (+0.01%) ⬆️
unit 72.73% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@KevLehman KevLehman added this to the 7.0 milestone Apr 4, 2024
@KevLehman KevLehman marked this pull request as ready for review April 4, 2024 17:37
@KevLehman KevLehman marked this pull request as draft April 4, 2024 19:22
// 2. Remove subscriptions associated with them

// Closed rooms with open inquiries
const closedRooms = await LivechatRooms.find({ closedAt: { $exists: true } }, { projection: { _id: 1 } }).toArray();
Copy link
Member

Choose a reason for hiding this comment

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

This query takes a long time for cases with a lot of omnichannel conversations. Example with 1 million.


// Closed rooms with open inquiries
const closedRooms = await LivechatRooms.find({ closedAt: { $exists: true } }, { projection: { _id: 1 } }).toArray();
const { deletedCount } = await LivechatInquiry.deleteMany({ rid: { $in: closedRooms.map((room) => room._id) } });
Copy link
Member

Choose a reason for hiding this comment

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

With same case above this could fail because of the size of the array being returned

console.log(`[Migration] Removed ${deletedCount} inquiries from closed rooms`);

// Queued inquiries pointing to a room thats served
const openAndServedRooms = await LivechatRooms.find({ servedBy: { $exists: true } }, { projection: { _id: 1 } }).toArray();
Copy link
Member

Choose a reason for hiding this comment

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

Again will take a long time. While servedBy does exist as index too with a lot of records this still takes a long time when has a lot of records

Copy link
Member

@geekgonecrazy geekgonecrazy left a comment

Choose a reason for hiding this comment

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

This will take a really long time to get responses from mongo on these queries for large omnichannel use cases

@KevLehman
Copy link
Contributor Author

Yep, that's a known thing, as i said over there, that's why this is still a draft 👀

@KevLehman
Copy link
Contributor Author

Closing as no longer needed

@KevLehman KevLehman closed this Jun 5, 2024
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.

None yet

2 participants