Skip to content

Commit d6a1147

Browse files
committed
feat: use dynamic scheduling for presence status expiration
1 parent 69a783c commit d6a1147

4 files changed

Lines changed: 55 additions & 10 deletions

File tree

ee/packages/presence/src/Presence.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ registerModel('IUsersModel', {
1212
findOneById: findUserMock,
1313
updatePresenceAndStatus: updatePresenceMock,
1414
findExpiredStatuses: jest.fn(),
15+
findNextStatusExpiration: jest.fn().mockResolvedValue(null),
1516
} as any);
1617

1718
registerModel('IUsersSessionsModel', {

ee/packages/presence/src/Presence.ts

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { PresenceReaper } from './lib/PresenceReaper';
99
import { type ClaimUpdate, processPresence } from './lib/presenceEngine';
1010

1111
const MAX_CONNECTIONS = 200;
12+
const EXPIRATION_JOB_NAME = 'presence-status-expiration';
1213

1314
export class Presence extends ServiceClass implements IPresence {
1415
protected name = 'presence';
@@ -99,10 +100,7 @@ export class Presence extends ServiceClass implements IPresence {
99100
// ignore
100101
}
101102

102-
// TODO: Agenda default lockLifetime is 10 min. This job executes in ms and runs every 1 min.
103-
// If an instance crashes mid-execution, expired statuses stay locked for up to 10 min.
104-
// Reduce lockLifetime to 60s once @rocket.chat/agenda exposes the option in its typed API.
105-
await cronJobs.add('presence-status-expiration', '* * * * *', () => this.processExpiredStatuses());
103+
await this.handleExpirationJob();
106104
}
107105

108106
private async processExpiredStatuses(batchSize = 100): Promise<void> {
@@ -112,14 +110,34 @@ export class Presence extends ServiceClass implements IPresence {
112110
return;
113111
}
114112

115-
const results = await Promise.allSettled(expiredUsers.map(({ _id }) => this.endActiveState(_id)));
113+
// Use updateUserPresence directly instead of updatePresenceAndReschedule to avoid rescheduling per user;
114+
// the caller (setupNextExpiration) handles rescheduling after the full batch is processed.
115+
const results = await Promise.allSettled(expiredUsers.map(({ _id }) => this.updateUserPresence(_id, { type: 'endActive' })));
116116
const successful = results.filter((result) => result.status === 'fulfilled').length;
117117

118118
if (expiredUsers.length === batchSize && successful > 0) {
119119
return this.processExpiredStatuses();
120120
}
121121
}
122122

123+
private async setupNextExpiration(): Promise<void> {
124+
if (await cronJobs.has(EXPIRATION_JOB_NAME)) {
125+
await cronJobs.remove(EXPIRATION_JOB_NAME);
126+
}
127+
128+
const next = await Users.findNextStatusExpiration();
129+
if (!next?.statusExpiresAt) {
130+
return;
131+
}
132+
133+
await cronJobs.addAtTimestamp(EXPIRATION_JOB_NAME, next.statusExpiresAt, () => this.handleExpirationJob());
134+
}
135+
136+
private async handleExpirationJob(): Promise<void> {
137+
await this.processExpiredStatuses();
138+
await this.setupNextExpiration();
139+
}
140+
123141
private async handleReaperUpdates(userIds: string[]): Promise<void> {
124142
const results = await Promise.allSettled(userIds.map((uid) => this.updateUserPresence(uid)));
125143
const fulfilled = results.filter((result) => result.status === 'fulfilled');
@@ -139,6 +157,9 @@ export class Presence extends ServiceClass implements IPresence {
139157

140158
override async stopped(): Promise<void> {
141159
this.reaper.stop();
160+
if (await cronJobs.has(EXPIRATION_JOB_NAME)) {
161+
await cronJobs.remove(EXPIRATION_JOB_NAME);
162+
}
142163
if (!this.lostConTimeout) {
143164
return;
144165
}
@@ -251,15 +272,25 @@ export class Presence extends ServiceClass implements IPresence {
251272
return affectedUsers.map(({ _id }) => _id);
252273
}
253274

275+
/**
276+
* Updates presence and reschedules the expiration job.
277+
* All public methods should use this instead of calling updateUserPresence directly.
278+
*/
279+
private async updatePresenceAndReschedule(uid: string, claimUpdate: ClaimUpdate): Promise<boolean> {
280+
const result = await this.updateUserPresence(uid, claimUpdate);
281+
await this.setupNextExpiration();
282+
return result;
283+
}
284+
254285
/**
255286
* @deprecated Use setActiveState, endActiveState, or clearActiveState instead.
256287
*/
257288
async setStatus(userId: string, statusDefault: UserStatus, statusText?: string): Promise<boolean> {
258289
if (statusDefault === UserStatus.ONLINE && !statusText) {
259-
return this.updateUserPresence(userId, { type: 'clearActive' });
290+
return this.updatePresenceAndReschedule(userId, { type: 'clearActive' });
260291
}
261292

262-
return this.updateUserPresence(userId, {
293+
return this.updatePresenceAndReschedule(userId, {
263294
type: 'setActive',
264295
newState: {
265296
statusDefault,
@@ -276,22 +307,22 @@ export class Presence extends ServiceClass implements IPresence {
276307
userId: string,
277308
newState: Pick<IUser, 'statusDefault' | 'statusSource' | 'statusText' | 'statusExpiresAt'>,
278309
): Promise<void> {
279-
await this.updateUserPresence(userId, { type: 'setActive', newState });
310+
await this.updatePresenceAndReschedule(userId, { type: 'setActive', newState });
280311
}
281312

282313
/**
283314
* Ends the current active claim. Restores previous if valid, otherwise
284315
* falls back to system-managed.
285316
*/
286317
async endActiveState(userId: string): Promise<void> {
287-
await this.updateUserPresence(userId, { type: 'endActive' });
318+
await this.updatePresenceAndReschedule(userId, { type: 'endActive' });
288319
}
289320

290321
/**
291322
* Removes all presence claims and resets to "Online" with no text.
292323
*/
293324
async clearActiveState(userId: string): Promise<void> {
294-
await this.updateUserPresence(userId, { type: 'clearActive' });
325+
await this.updatePresenceAndReschedule(userId, { type: 'clearActive' });
295326
}
296327

297328
async setConnectionStatus(uid: string, status: UserStatus, session: string): Promise<boolean> {
@@ -302,6 +333,10 @@ export class Presence extends ServiceClass implements IPresence {
302333
return !!result.modifiedCount;
303334
}
304335

336+
/**
337+
* Low-level presence update. Does not reschedule the expiration job.
338+
* Prefer {@link updatePresenceAndReschedule} for public-facing methods.
339+
*/
305340
private async updateUserPresence(uid: string, claimUpdate?: ClaimUpdate): Promise<boolean> {
306341
const user = await Users.findOneById<
307342
Pick<

packages/model-typings/src/models/IUsersModel.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,8 @@ export interface IUsersModel extends IBaseModel<IUser> {
174174

175175
findExpiredStatuses(limit: number): FindCursor<Pick<IUser, '_id'>>;
176176

177+
findNextStatusExpiration(): Promise<Pick<IUser, '_id' | 'statusExpiresAt'> | null>;
178+
177179
updatePresenceAndStatus(userId: IUser['_id'], values: Record<string, unknown>, clear?: string[]): Promise<IUser | null>;
178180

179181
updateStatusByAppId(appId: string, status: UserStatus): Promise<UpdateResult | Document>;

packages/models/src/models/Users.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,6 +1104,13 @@ export class UsersRaw extends BaseRaw<IUser, DefaultFields<IUser>> implements IU
11041104
);
11051105
}
11061106

1107+
findNextStatusExpiration() {
1108+
return this.findOne<Pick<IUser, '_id' | 'statusExpiresAt'>>(
1109+
{ statusExpiresAt: { $gte: new Date() } },
1110+
{ projection: { _id: 1, statusExpiresAt: 1 }, sort: { statusExpiresAt: 1 } },
1111+
);
1112+
}
1113+
11071114
updatePresenceAndStatus(userId: IUser['_id'], values: Record<string, unknown>, clear?: string[]) {
11081115
const $unset = clear?.length ? Object.fromEntries(clear.map((field) => [field, '' as const])) : undefined;
11091116

0 commit comments

Comments
 (0)