Skip to content

Commit 3251af5

Browse files
NiklasEiNiklasEi
authored
Merge pull request #1005 from StefanTNG/feat-create-retry-option-for-failed-schedule-pings
feat: allow failed schedule pings to be retried before throwing an error
2 parents 50eae62 + 3302468 commit 3251af5

5 files changed

Lines changed: 112 additions & 13 deletions

File tree

‎README.md‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,10 @@ You can instantiate a momo job using the `MomoJobBuilder` class. It provides the
118118
You can instantiate a mongo schedule using the `MongoScheduleBuilder` class. It provides the following setter
119119
methods:
120120

121-
| setter | parameter | mandatory | default value | description |
122-
|----------------|------------------------|-----------|---------------|-----------------------------------------|
123-
| withJob | `job: MomoJob` | yes | | Adds a job to the schedule. |
124-
| withConnection | `options: MomoOptions` | yes | | The connection options of the schedule. |
121+
| setter | parameter | mandatory | default value | description |
122+
|-----------------|------------------------------------------------------------------------------------|-----------|---------------|-------------------------------------------|
123+
| withJob | `job: MomoJob` | yes | | Adds a job to the schedule. |
124+
| withConnection | `options: MomoOptions` | yes | | The connection options of the schedule. |
125125

126126
The schedule offers the following methods to create and run jobs:
127127

@@ -151,12 +151,12 @@ database.
151151

152152
#### MomoConnectionOptions
153153

154-
| property | type | mandatory | default | description |
155-
|--------------------|----------------------|-----------|-----------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
156-
| url | `string` | yes | | The connection string of your database. |
157-
| scheduleName | `string` | yes | | Only one schedule per name can be active at a time. If multiple instances of your application define a schedule with the same name, only one at a time will actually run jobs. |
158-
| collectionsPrefix | `string` | no | no prefix | A prefix for all collections created by Momo. |
159-
| pingIntervalMs | number | no | `60_000` | The keep alive ping interval of the schedule, in milliseconds. After twice the amount of time has elapsed without a ping of your Momo instance, other instances may take over. You might want to reduce this if you have jobs running on short intervals. |
154+
| property | type | mandatory | default | description |
155+
|--------------------|---------------------|-----------|-----------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
156+
| url | `string` | yes | | The connection string of your database. |
157+
| scheduleName | `string` | yes | | Only one schedule per name can be active at a time. If multiple instances of your application define a schedule with the same name, only one at a time will actually run jobs. |
158+
| collectionsPrefix | `string` | no | no prefix | A prefix for all collections created by Momo. |
159+
| pingIntervalMs | `number` | no | `60_000` | The keep alive ping interval of the schedule, in milliseconds. After twice the amount of time has elapsed without a ping of your Momo instance, other instances may take over. You might want to reduce this if you have jobs running on short intervals. |
160160
| mongoClientOptions | `MongoClientOptions` | no | | Options for the connection to the MongoDB client as specified by the MongoDB API. Useful for providing configuration options that are not available via the connection string (url). |
161161

162162
### Reacting to events

‎src/schedule/MongoSchedule.ts‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { v4 as uuid } from 'uuid';
33
import { Connection, MomoConnectionOptions } from '../Connection';
44
import { Schedule } from './Schedule';
55
import { SchedulePing } from './SchedulePing';
6+
import { maxNodeTimeoutDelay } from '../job/Job';
67

78
export interface MomoOptions extends MomoConnectionOptions {
89
/**
@@ -17,6 +18,16 @@ export interface MomoOptions extends MomoConnectionOptions {
1718
* Only one schedule per name can be active at a time.
1819
*/
1920
scheduleName: string;
21+
22+
/**
23+
* Configure how often schedule pings should be retried (default is no retries).
24+
*/
25+
pingRetryOptions?: PingRetryOptions;
26+
}
27+
28+
export interface PingRetryOptions {
29+
maxPingAttempts: number;
30+
retryIntervalMs: number;
2031
}
2132

2233
export class MongoSchedule extends Schedule {
@@ -27,7 +38,21 @@ export class MongoSchedule extends Schedule {
2738
protected readonly scheduleId: string,
2839
protected readonly connection: Connection,
2940
pingIntervalMs: number,
41+
maxPingAttempts: number,
42+
retryIntervalMs: number,
3043
) {
44+
if (!Number.isFinite(pingIntervalMs) || pingIntervalMs < 1 || pingIntervalMs > maxNodeTimeoutDelay) {
45+
throw new Error(`Error: pingIntervalMs must be a positive number less than ${maxNodeTimeoutDelay}`);
46+
}
47+
48+
if (!Number.isFinite(maxPingAttempts) || maxPingAttempts < 1) {
49+
throw new Error('Error: maxPingAttempts must be a positive number');
50+
}
51+
52+
if (!Number.isFinite(retryIntervalMs) || retryIntervalMs < 1 || retryIntervalMs > maxNodeTimeoutDelay) {
53+
throw new Error(`Error: retryIntervalMs must be a positive number less than ${maxNodeTimeoutDelay}`);
54+
}
55+
3156
const schedulesRepository = connection.getSchedulesRepository();
3257
const jobRepository = connection.getJobRepository();
3358

@@ -42,6 +67,8 @@ export class MongoSchedule extends Schedule {
4267
this.logger,
4368
pingIntervalMs,
4469
this.startAllJobs.bind(this),
70+
maxPingAttempts,
71+
retryIntervalMs,
4572
);
4673
}
4774

@@ -53,12 +80,14 @@ export class MongoSchedule extends Schedule {
5380
public static async connect({
5481
pingIntervalMs = 60_000,
5582
scheduleName,
83+
pingRetryOptions,
5684
...connectionOptions
5785
}: MomoOptions): Promise<MongoSchedule> {
5886
const scheduleId = uuid();
5987
const connection = await Connection.create(connectionOptions, pingIntervalMs, scheduleId, scheduleName);
88+
const { maxPingAttempts = 1, retryIntervalMs = 500 } = pingRetryOptions ?? {};
6089

61-
return new MongoSchedule(scheduleId, connection, pingIntervalMs);
90+
return new MongoSchedule(scheduleId, connection, pingIntervalMs, maxPingAttempts, retryIntervalMs);
6291
}
6392

6493
/**

‎src/schedule/SchedulePing.ts‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ export class SchedulePing {
1818
private readonly logger: Logger,
1919
private readonly interval: number,
2020
private readonly startAllJobs: () => Promise<void>,
21+
private readonly maxPingAttempts: number = 1,
22+
private readonly retryIntervalMs: number = 500,
2123
) {}
2224

2325
async start(): Promise<void> {
@@ -26,11 +28,29 @@ export class SchedulePing {
2628
}
2729
const errorMessage = 'Pinging or cleaning the Schedules repository failed';
2830
try {
29-
await this.checkActiveSchedule();
31+
await this.checkActiveScheduleWithRetries(errorMessage);
3032
} catch (e) {
3133
this.logger.error(errorMessage, MomoErrorType.internal, this.schedulesRepository.getLogData(), e);
3234
}
33-
this.handle = setSafeInterval(this.checkActiveSchedule.bind(this), this.interval, this.logger, errorMessage);
35+
this.handle = setSafeInterval(
36+
this.checkActiveScheduleWithRetries.bind(this, errorMessage),
37+
this.interval,
38+
this.logger,
39+
errorMessage,
40+
);
41+
}
42+
43+
private async checkActiveScheduleWithRetries(errorMessage: string): Promise<void> {
44+
for (let attempt = 0; attempt < this.maxPingAttempts; attempt++) {
45+
try {
46+
return await this.checkActiveSchedule();
47+
} catch (error) {
48+
if (attempt >= this.maxPingAttempts - 1) throw error;
49+
50+
this.logger.debug(`${errorMessage} after ${attempt} attempt. Retrying in ${this.retryIntervalMs} ms.`);
51+
await new Promise((r) => setTimeout(r, this.retryIntervalMs));
52+
}
53+
}
3454
}
3555

3656
private async checkActiveSchedule(): Promise<void> {

‎test/schedule/MongoScheduleBuilder.integration.spec.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,25 @@ describe('MongoScheduleBuilder', () => {
8080
expect(jobList[2]?.name).toEqual(job3.name);
8181
expect(jobList[2]?.schedule).toEqual(job3.schedule);
8282
});
83+
84+
it('can be built with schedule ping retries', async () => {
85+
const connectSpy = jest.spyOn(MongoSchedule, 'connect');
86+
87+
const pingRetryOptions = {
88+
maxPingAttempts: 2,
89+
retryIntervalMs: 500,
90+
};
91+
92+
mongoSchedule = await new MongoScheduleBuilder()
93+
.withConnection({ ...connectionOptions, pingRetryOptions })
94+
.build();
95+
96+
expect(connectSpy).toHaveBeenCalledWith({
97+
scheduleName,
98+
pingRetryOptions,
99+
url: mongo.getUri(),
100+
});
101+
});
83102
});
84103

85104
it('throws an error when built with no connection', async () => {

‎test/schedule/SchedulePing.spec.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,37 @@ describe('SchedulePing', () => {
5858
);
5959
});
6060

61+
it('retries failed pings if configured', async () => {
62+
const schedulePingWithRetries = new SchedulePing(
63+
instance(schedulesRepository),
64+
{ debug: jest.fn(), error },
65+
10,
66+
startAllJobs,
67+
3,
68+
1,
69+
);
70+
71+
try {
72+
when(schedulesRepository.getLogData()).thenReturn(logData);
73+
const message = 'I am an error that should lead to a retry';
74+
when(schedulesRepository.setActiveSchedule()).thenReject({
75+
message,
76+
} as Error);
77+
78+
await schedulePingWithRetries.start();
79+
80+
verify(schedulesRepository.setActiveSchedule()).thrice();
81+
expect(error).toHaveBeenCalledWith(
82+
'Pinging or cleaning the Schedules repository failed',
83+
'an internal error occurred',
84+
logData,
85+
{ message },
86+
);
87+
} finally {
88+
await schedulePingWithRetries.stop();
89+
}
90+
});
91+
6192
it('does not start any jobs for inactive schedule', async () => {
6293
await schedulePing.start();
6394

0 commit comments

Comments
 (0)