fix: add job-level lock to prevent redundant cross-instance reads in …fix: Add job-level lock to prevent redundant cross-instance reads in Presence service - #40987
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (3)📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-05-06T12:21:44.083ZApplied to files:
🔇 Additional comments (1)
WalkthroughThe ChangesDistributed lock for lost-connection cleanup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…nto fix/presence-cross-instance-reads-lock
|
Thanks @ZunedKhan07! Closing this one — the problem it targets (deduping cross-instance presence work in MS mode) is already being solved in #41153, which moves status expiration onto @rocket.chat/cron. Agenda's Mongo job lock guarantees a single instance runs each job, which is the "job-level lock" this was reaching for, and it also clears the TODO #40973 was generated from. Appreciate the contribution! 🙏 |
|
Thanks for the update and clarification, Ricardo! Glad to know the issue is
being handled in #41153. Looking forward to contributing more to
Rocket.Chat.
…On Mon, 20 Jul, 2026, 7:55 pm Ricardo Garim, ***@***.***> wrote:
*ricardogarim* left a comment (RocketChat/Rocket.Chat#40987)
<#40987 (comment)>
Thanks @ZunedKhan07 <https://github.com/ZunedKhan07>! Closing this one —
the problem it targets (deduping cross-instance presence work in MS mode)
is already being solved in #41153
<#41153>, which moves
status expiration onto @rocket.chat/cron. Agenda's Mongo job lock
guarantees a single instance runs each job, which is the "job-level lock"
this was reaching for, and it also clears the TODO #40973
<#40973> was generated
from. Appreciate the contribution! 🙏
—
Reply to this email directly, view it on GitHub
<#40987?email_source=notifications&email_token=BOSIAX3QRR7WSRNYV2NZ2U35FYTT3A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBSGMZTIMZQG432M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5023343077>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BOSIAX7KOHAEJFLXX3P22O35FYTT3AVCNFSNUABEKJSXA33TNF2G64TZHMZTKOBWGY3DSNB3JFZXG5LFHM2DMOBTGQ3TONZZGOQXMAQ>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/BOSIAX74Q33RAW4JH7OSWRT5FYTT3A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBSGMZTIMZQG432M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/BOSIAX6OJOT6YHIS3G7EK235FYTT3A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBSGMZTIMZQG432M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Closes #40973
Summary
In Microservices (MS) mode, every instance of the
presenceservice was independently executing theremoveLostConnectionsoperation inside thestarted()lifecycle timeout loop. This caused redundant cross-instance reads and unnecessary heavy database/broker operations across multiple running instances.Changes
removeLostConnectionslifecycle timeout logic inside a distributed job-level lock usingthis.api?.lock().try-finallyblock to guarantee the safe release of the lock after execution.Summary by CodeRabbit